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.
This commit is contained in:
+48
-10
@@ -11,13 +11,29 @@ use syscall::{
|
||||
};
|
||||
|
||||
enum Handle {
|
||||
Shm(Rc<str>),
|
||||
Shm {
|
||||
path: Rc<str>,
|
||||
readable: bool,
|
||||
writable: bool,
|
||||
},
|
||||
SchemeRoot,
|
||||
}
|
||||
impl Handle {
|
||||
fn as_shm(&self) -> Result<&Rc<str>, Error> {
|
||||
match self {
|
||||
Self::Shm(path) => Ok(path),
|
||||
Self::Shm { path, .. } => Ok(path),
|
||||
Self::SchemeRoot => Err(Error::new(EBADF)),
|
||||
}
|
||||
}
|
||||
fn shm_readable(&self) -> Result<bool, Error> {
|
||||
match self {
|
||||
Self::Shm { readable, .. } => Ok(*readable),
|
||||
Self::SchemeRoot => Err(Error::new(EBADF)),
|
||||
}
|
||||
}
|
||||
fn shm_writable(&self) -> Result<bool, Error> {
|
||||
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<usize> {
|
||||
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<usize> {
|
||||
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<usize> {
|
||||
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<usize> {
|
||||
if offset >= self.len {
|
||||
// FIXME read as zeros
|
||||
if self.base.is_null() || offset >= self.len {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user