Skip to content

Commit

Permalink
Fix misaligned memory access bug (affects non-64 bit ARM)
Browse files Browse the repository at this point in the history
  • Loading branch information
yellowhatter committed Jul 21, 2024
1 parent 4189c5c commit 4c8575d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 19 deletions.
22 changes: 9 additions & 13 deletions commons/zenoh-shm/src/posix_shm/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
// ZettaScale Zenoh Team, <[email protected]>
//

use std::{
fmt::{Debug, Display},
mem::size_of,
};
use std::fmt::{Debug, Display};

use rand::Rng;
use shared_memory::{Shmem, ShmemConf, ShmemError};
Expand Down Expand Up @@ -63,15 +60,14 @@ where
// If creation fails because segment already exists for this id,
// the creation attempt will be repeated with another id
match ShmemConf::new()
.size(alloc_size + size_of::<usize>())
.size(alloc_size)
.os_id(Self::os_id(id.clone(), id_prefix))
.create()
{
Ok(shmem) => {
tracing::debug!(
"Created SHM segment, size: {alloc_size}, prefix: {id_prefix}, id: {id}"
);
unsafe { *(shmem.as_ptr() as *mut usize) = alloc_size };
return Ok(Segment { shmem, id });
}
Err(ShmemError::LinkExists) => {}
Expand All @@ -94,10 +90,6 @@ where
)
})?;

if shmem.len() <= size_of::<usize>() {
bail!("SHM segment too small")
}

tracing::debug!("Opened SHM segment, prefix: {id_prefix}, id: {id}");

Ok(Self { shmem, id })
Expand All @@ -110,17 +102,21 @@ where
}

pub fn as_ptr(&self) -> *mut u8 {
unsafe { self.shmem.as_ptr().add(size_of::<usize>()) }
self.shmem.as_ptr()
}

/// Returns the length of this [`Segment<ID>`].
/// NOTE: one some platforms (at least windows) the returned len will be the actual length of an shm segment
/// (a required len rounded up to the nearest multiply of page size), on other (at least linux and macos) this
/// returns a value requested upon segment creation
pub fn len(&self) -> usize {
unsafe { *(self.shmem.as_ptr() as *mut usize) }
self.shmem.len()
}

// TODO: dead code warning occurs because of `tested_crate_module!()` macro when feature `test` is not enabled. Better to fix that
#[allow(dead_code)]
pub fn is_empty(&self) -> bool {
unsafe { *(self.shmem.as_ptr() as *mut usize) == 0 }
self.len() == 0
}

pub fn id(&self) -> ID {
Expand Down
17 changes: 11 additions & 6 deletions io/zenoh-transport/src/unicast/establishment/ext/shm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ const AUTH_SEGMENT_PREFIX: &str = "auth";
pub(crate) type AuthSegmentID = u32;
pub(crate) type AuthChallenge = u64;

const LEN_INDEX: usize = 0;
const CHALLENGE_INDEX: usize = 1;
const ID_START_INDEX: usize = 2;

#[derive(Debug)]
pub struct AuthSegment {
array: ArrayInSHM<AuthSegmentID, AuthChallenge, usize>,
Expand All @@ -44,13 +48,14 @@ pub struct AuthSegment {
impl AuthSegment {
pub fn create(challenge: AuthChallenge, shm_protocols: &[ProtocolID]) -> ZResult<Self> {
let array = ArrayInSHM::<AuthSegmentID, AuthChallenge, usize>::create(
1 + shm_protocols.len(),
ID_START_INDEX + shm_protocols.len(),
AUTH_SEGMENT_PREFIX,
)?;
unsafe {
(*array.elem_mut(0)) = challenge;
for elem in 1..array.elem_count() {
(*array.elem_mut(elem)) = shm_protocols[elem - 1] as u64;
(*array.elem_mut(LEN_INDEX)) = shm_protocols.len() as AuthChallenge;
(*array.elem_mut(CHALLENGE_INDEX)) = challenge;
for elem in ID_START_INDEX..array.elem_count() {
(*array.elem_mut(elem)) = shm_protocols[elem - ID_START_INDEX] as u64;
}
};
Ok(Self { array })
Expand All @@ -62,12 +67,12 @@ impl AuthSegment {
}

pub fn challenge(&self) -> AuthChallenge {
unsafe { *self.array.elem(0) }
unsafe { *self.array.elem(CHALLENGE_INDEX) }
}

pub fn protocols(&self) -> Vec<ProtocolID> {
let mut result = vec![];
for elem in 1..self.array.elem_count() {
for elem in ID_START_INDEX..self.array.elem_count() {
result.push(unsafe { *self.array.elem(elem) as u32 });
}
result
Expand Down

0 comments on commit 4c8575d

Please sign in to comment.