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 {
|
enum Handle {
|
||||||
Shm(Rc<str>),
|
Shm {
|
||||||
|
path: Rc<str>,
|
||||||
|
readable: bool,
|
||||||
|
writable: bool,
|
||||||
|
},
|
||||||
SchemeRoot,
|
SchemeRoot,
|
||||||
}
|
}
|
||||||
impl Handle {
|
impl Handle {
|
||||||
fn as_shm(&self) -> Result<&Rc<str>, Error> {
|
fn as_shm(&self) -> Result<&Rc<str>, Error> {
|
||||||
match self {
|
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)),
|
Self::SchemeRoot => Err(Error::new(EBADF)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -48,7 +64,6 @@ impl SchemeSync for ShmScheme {
|
|||||||
fn scheme_root(&mut self) -> Result<usize> {
|
fn scheme_root(&mut self) -> Result<usize> {
|
||||||
Ok(self.handles.insert(Handle::SchemeRoot))
|
Ok(self.handles.insert(Handle::SchemeRoot))
|
||||||
}
|
}
|
||||||
//FIXME: Handle O_RDONLY/O_WRONLY/O_RDWR
|
|
||||||
fn openat(
|
fn openat(
|
||||||
&mut self,
|
&mut self,
|
||||||
dirfd: usize,
|
dirfd: usize,
|
||||||
@@ -82,7 +97,11 @@ impl SchemeSync for ShmScheme {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
entry.refs += 1;
|
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 {
|
Ok(OpenResult::ThisScheme {
|
||||||
number: id,
|
number: id,
|
||||||
@@ -96,7 +115,7 @@ impl SchemeSync for ShmScheme {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
fn on_close(&mut self, id: usize) {
|
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;
|
return;
|
||||||
};
|
};
|
||||||
let mut entry = match self.maps.entry(path) {
|
let mut entry = match self.maps.entry(path) {
|
||||||
@@ -190,7 +209,11 @@ impl SchemeSync for ShmScheme {
|
|||||||
_fcntl_flags: u32,
|
_fcntl_flags: u32,
|
||||||
_ctx: &CallerCtx,
|
_ctx: &CallerCtx,
|
||||||
) -> Result<usize> {
|
) -> 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
|
self.maps
|
||||||
.get_mut(path)
|
.get_mut(path)
|
||||||
.expect("handle pointing to nothing")
|
.expect("handle pointing to nothing")
|
||||||
@@ -205,7 +228,11 @@ impl SchemeSync for ShmScheme {
|
|||||||
_fcntl_flags: u32,
|
_fcntl_flags: u32,
|
||||||
_ctx: &CallerCtx,
|
_ctx: &CallerCtx,
|
||||||
) -> Result<usize> {
|
) -> 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
|
self.maps
|
||||||
.get_mut(path)
|
.get_mut(path)
|
||||||
.expect("handle pointing to nothing")
|
.expect("handle pointing to nothing")
|
||||||
@@ -229,7 +256,7 @@ impl MmapGuard {
|
|||||||
|
|
||||||
fn grow_to(&mut self, new_len: usize) -> Result<()> {
|
fn grow_to(&mut self, new_len: usize) -> Result<()> {
|
||||||
if new_len <= self.total_capacity() {
|
if new_len <= self.total_capacity() {
|
||||||
// FIXME clear bytes after new_len
|
self.zero_range(self.len, new_len);
|
||||||
self.len = new_len;
|
self.len = new_len;
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
@@ -238,6 +265,8 @@ impl MmapGuard {
|
|||||||
let page_count = needed.div_ceil(PAGE_SIZE);
|
let page_count = needed.div_ceil(PAGE_SIZE);
|
||||||
let alloc_size = page_count * PAGE_SIZE;
|
let alloc_size = page_count * PAGE_SIZE;
|
||||||
|
|
||||||
|
let old_len = self.len;
|
||||||
|
|
||||||
let new_base = unsafe {
|
let new_base = unsafe {
|
||||||
if self.base.is_null() {
|
if self.base.is_null() {
|
||||||
syscall::fmap(
|
syscall::fmap(
|
||||||
@@ -263,9 +292,19 @@ impl MmapGuard {
|
|||||||
|
|
||||||
self.base = new_base as *mut ();
|
self.base = new_base as *mut ();
|
||||||
self.len = new_len;
|
self.len = new_len;
|
||||||
|
self.zero_range(old_len, new_len);
|
||||||
Ok(())
|
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 {
|
fn total_capacity(&self) -> usize {
|
||||||
self.len.next_multiple_of(PAGE_SIZE)
|
self.len.next_multiple_of(PAGE_SIZE)
|
||||||
}
|
}
|
||||||
@@ -289,8 +328,7 @@ impl MmapGuard {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn read(&self, offset: usize, buf: &mut [u8]) -> Result<usize> {
|
pub fn read(&self, offset: usize, buf: &mut [u8]) -> Result<usize> {
|
||||||
if offset >= self.len {
|
if self.base.is_null() || offset >= self.len {
|
||||||
// FIXME read as zeros
|
|
||||||
return Ok(0);
|
return Ok(0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user