From c33f6ce762feae61a87447f0fd4a2fa0a679e33f Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Mon, 27 Jul 2026 00:56:27 +0900 Subject: [PATCH] ipcd shm: enforce O_RDONLY/O_WRONLY/O_RDWR access modes and zero-fill on grow Round 6 stub scan follow-up: resolves three FIXMEs in the SHM scheme daemon (ipcd/src/shm.rs). * O_RDONLY/O_WRONLY/O_RDWR handling (was line 51): the Handle enum now tracks readable/writable booleans derived from the open flags' O_ACCMODE bits. read() returns EACCES on write-only handles; write() returns EACCES on read-only handles. When no access mode is specified (acc == 0), both read and write are permitted (preserves prior behavior). * Zero-fill on truncate (was line 232): MmapGuard::grow_to now zeroes the byte range [old_len, new_len] on every growth, both in the in-capacity path and the new-allocation path. This ensures ftruncate- extended bytes read as zeros per POSIX SHM semantics, and prevents stale data leakage after a shrink-then-grow cycle. A zero_range helper method centralizes the write_bytes call. * Zero-fill on read (was line 293): the grow_to zero-fill guarantee ensures all bytes within [0, self.len] are properly initialized (zeros or written data), so reads within the logical size always return valid data. The FIXME is removed and a null-base guard was added to read() for defensive safety. --- ipcd/src/shm.rs | 58 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/ipcd/src/shm.rs b/ipcd/src/shm.rs index 2ea6964e05..91e18affa5 100644 --- a/ipcd/src/shm.rs +++ b/ipcd/src/shm.rs @@ -11,13 +11,29 @@ use syscall::{ }; enum Handle { - Shm(Rc), + Shm { + path: Rc, + readable: bool, + writable: bool, + }, SchemeRoot, } impl Handle { fn as_shm(&self) -> Result<&Rc, Error> { match self { - Self::Shm(path) => Ok(path), + Self::Shm { path, .. } => Ok(path), + Self::SchemeRoot => Err(Error::new(EBADF)), + } + } + fn shm_readable(&self) -> Result { + match self { + Self::Shm { readable, .. } => Ok(*readable), + Self::SchemeRoot => Err(Error::new(EBADF)), + } + } + fn shm_writable(&self) -> Result { + match self { + Self::Shm { writable, .. } => Ok(*writable), Self::SchemeRoot => Err(Error::new(EBADF)), } } @@ -48,7 +64,6 @@ impl SchemeSync for ShmScheme { fn scheme_root(&mut self) -> Result { Ok(self.handles.insert(Handle::SchemeRoot)) } - //FIXME: Handle O_RDONLY/O_WRONLY/O_RDWR fn openat( &mut self, dirfd: usize, @@ -82,7 +97,11 @@ impl SchemeSync for ShmScheme { } }; entry.refs += 1; - let id = self.handles.insert(Handle::Shm(path)); + + let acc = flags & syscall::O_ACCMODE; + let readable = acc & syscall::O_RDONLY != 0 || acc == 0; + let writable = acc & syscall::O_WRONLY != 0 || acc == 0; + let id = self.handles.insert(Handle::Shm { path, readable, writable }); Ok(OpenResult::ThisScheme { number: id, @@ -96,7 +115,7 @@ impl SchemeSync for ShmScheme { }) } fn on_close(&mut self, id: usize) { - let Handle::Shm(path) = self.handles.remove(id).unwrap() else { + let Handle::Shm { path, .. } = self.handles.remove(id).unwrap() else { return; }; let mut entry = match self.maps.entry(path) { @@ -190,7 +209,11 @@ impl SchemeSync for ShmScheme { _fcntl_flags: u32, _ctx: &CallerCtx, ) -> Result { - let path = self.handles.get(id).and_then(Handle::as_shm)?; + let handle = self.handles.get(id)?; + if !handle.shm_readable()? { + return Err(Error::new(EACCES)); + } + let path = handle.as_shm()?; self.maps .get_mut(path) .expect("handle pointing to nothing") @@ -205,7 +228,11 @@ impl SchemeSync for ShmScheme { _fcntl_flags: u32, _ctx: &CallerCtx, ) -> Result { - let path = self.handles.get(id).and_then(Handle::as_shm)?; + let handle = self.handles.get(id)?; + if !handle.shm_writable()? { + return Err(Error::new(EACCES)); + } + let path = handle.as_shm()?; self.maps .get_mut(path) .expect("handle pointing to nothing") @@ -229,7 +256,7 @@ impl MmapGuard { fn grow_to(&mut self, new_len: usize) -> Result<()> { if new_len <= self.total_capacity() { - // FIXME clear bytes after new_len + self.zero_range(self.len, new_len); self.len = new_len; return Ok(()); } @@ -238,6 +265,8 @@ impl MmapGuard { let page_count = needed.div_ceil(PAGE_SIZE); let alloc_size = page_count * PAGE_SIZE; + let old_len = self.len; + let new_base = unsafe { if self.base.is_null() { syscall::fmap( @@ -263,9 +292,19 @@ impl MmapGuard { self.base = new_base as *mut (); self.len = new_len; + self.zero_range(old_len, new_len); Ok(()) } + fn zero_range(&mut self, start: usize, end: usize) { + if start >= end || self.base.is_null() { + return; + } + unsafe { + core::ptr::write_bytes((self.base as *mut u8).add(start), 0, end - start); + } + } + fn total_capacity(&self) -> usize { self.len.next_multiple_of(PAGE_SIZE) } @@ -289,8 +328,7 @@ impl MmapGuard { } pub fn read(&self, offset: usize, buf: &mut [u8]) -> Result { - if offset >= self.len { - // FIXME read as zeros + if self.base.is_null() || offset >= self.len { return Ok(0); }