From 4c8575d571b28892bbf6ed3f7490ed6487cb6e2a Mon Sep 17 00:00:00 2001 From: yellowhatter Date: Sun, 21 Jul 2024 17:57:01 +0300 Subject: [PATCH] Fix misaligned memory access bug (affects non-64 bit ARM) --- commons/zenoh-shm/src/posix_shm/segment.rs | 22 ++++++++----------- .../src/unicast/establishment/ext/shm.rs | 17 +++++++++----- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/commons/zenoh-shm/src/posix_shm/segment.rs b/commons/zenoh-shm/src/posix_shm/segment.rs index 5458ab3e3e..657976ece1 100644 --- a/commons/zenoh-shm/src/posix_shm/segment.rs +++ b/commons/zenoh-shm/src/posix_shm/segment.rs @@ -12,10 +12,7 @@ // ZettaScale Zenoh Team, // -use std::{ - fmt::{Debug, Display}, - mem::size_of, -}; +use std::fmt::{Debug, Display}; use rand::Rng; use shared_memory::{Shmem, ShmemConf, ShmemError}; @@ -63,7 +60,7 @@ 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::()) + .size(alloc_size) .os_id(Self::os_id(id.clone(), id_prefix)) .create() { @@ -71,7 +68,6 @@ where 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) => {} @@ -94,10 +90,6 @@ where ) })?; - if shmem.len() <= size_of::() { - bail!("SHM segment too small") - } - tracing::debug!("Opened SHM segment, prefix: {id_prefix}, id: {id}"); Ok(Self { shmem, id }) @@ -110,17 +102,21 @@ where } pub fn as_ptr(&self) -> *mut u8 { - unsafe { self.shmem.as_ptr().add(size_of::()) } + self.shmem.as_ptr() } + /// Returns the length of this [`Segment`]. + /// 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 { diff --git a/io/zenoh-transport/src/unicast/establishment/ext/shm.rs b/io/zenoh-transport/src/unicast/establishment/ext/shm.rs index e2068af94a..025aaaef44 100644 --- a/io/zenoh-transport/src/unicast/establishment/ext/shm.rs +++ b/io/zenoh-transport/src/unicast/establishment/ext/shm.rs @@ -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, @@ -44,13 +48,14 @@ pub struct AuthSegment { impl AuthSegment { pub fn create(challenge: AuthChallenge, shm_protocols: &[ProtocolID]) -> ZResult { let array = ArrayInSHM::::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 }) @@ -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 { 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