From 7c0c84b0eeb85cd693d5c51b1b68f2f0fd1a9940 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Tue, 11 Aug 2026 11:18:18 +0800 Subject: [PATCH] refactor(fspy-shm): open backing files through sigsafe Co-authored-by: GPT-5 Codex --- Cargo.lock | 1 + crates/fspy_shm/Cargo.toml | 3 +++ crates/fspy_shm/src/unix.rs | 32 +++++++++++++++++++++++---- crates/sigsafe/src/c_str.rs | 19 ++++++++++++++++ crates/sigsafe/src/fs/linux.rs | 40 ++++++++++++++++++++++++++++++++-- crates/sigsafe/src/fs/mac.rs | 7 ++++-- crates/sigsafe/src/fs/mod.rs | 18 ++++++++++++++- crates/sigsafe/src/lib.rs | 3 ++- 8 files changed, 113 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f448972d..93ca36e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1417,6 +1417,7 @@ version = "0.0.0" dependencies = [ "ctor", "memmap2", + "sigsafe", "subprocess_test", "uuid", "windows-sys 0.61.2", diff --git a/crates/fspy_shm/Cargo.toml b/crates/fspy_shm/Cargo.toml index bc0caf38..a9533cd4 100644 --- a/crates/fspy_shm/Cargo.toml +++ b/crates/fspy_shm/Cargo.toml @@ -10,6 +10,9 @@ rust-version.workspace = true [dependencies] memmap2 = { workspace = true } +[target.'cfg(unix)'.dependencies] +sigsafe = { workspace = true } + [target.'cfg(target_os = "windows")'.dependencies] windows-sys = { workspace = true, features = [ "Win32_Foundation", diff --git a/crates/fspy_shm/src/unix.rs b/crates/fspy_shm/src/unix.rs index 95950265..8b9829b2 100644 --- a/crates/fspy_shm/src/unix.rs +++ b/crates/fspy_shm/src/unix.rs @@ -4,7 +4,7 @@ use std::{ ffi::OsStr, fs::{self, File, OpenOptions}, io, - os::unix::fs::OpenOptionsExt as _, + os::unix::{ffi::OsStrExt as _, fs::OpenOptionsExt as _, io::FromRawFd as _}, path::PathBuf, }; @@ -82,9 +82,7 @@ pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { /// Returns an error if the shared memory is unavailable, which is the common /// case once its keeper has been dropped. pub fn open(path: &OsStr) -> io::Result { - // Rust opens are `O_CLOEXEC`, so a traced process never leaks this - // descriptor. - let file = OpenOptions::new().read(true).write(true).open(path)?; + let file = open_file(path)?; // If another process shrinks the file before `map`, mapping fails. If it // resizes afterwards, nothing here touches the mapped pages. A concurrent // resize cannot make a mapping access invalid memory. @@ -96,6 +94,32 @@ pub fn open(path: &OsStr) -> io::Result { Ok(ShmHandle { file, size }) } +fn open_file(path: &OsStr) -> io::Result { + let path_bytes = path.as_bytes(); + let len_with_nul = path_bytes.len().checked_add(1).ok_or(io::ErrorKind::InvalidInput)?; + let mut path_buf = [0_u8; sigsafe::fs::PATH_MAX]; + let path = path_buf + .get_mut(..len_with_nul) + .ok_or_else(|| io::Error::from_raw_os_error(sigsafe::Errno::NAMETOOLONG.raw_os_error()))?; + path[..path_bytes.len()].copy_from_slice(path_bytes); + let path = sigsafe::CStr::from_bytes_with_nul(path).map_err(|_| io::ErrorKind::InvalidInput)?; + let fd = sigsafe::fs::openat( + sigsafe::CWD, + path, + sigsafe::fs::OFlags::RDWR | sigsafe::fs::OFlags::CLOEXEC, + sigsafe::fs::Mode::empty(), + ) + .map_err(errno_to_io)?; + let fd = sigsafe::IntoRawFd::into_raw_fd(fd); + // SAFETY: ownership of the descriptor returned by `openat` transfers to + // this `File` without closing or duplicating it. + Ok(unsafe { File::from_raw_fd(fd) }) +} + +fn errno_to_io(errno: sigsafe::Errno) -> io::Error { + io::Error::from_raw_os_error(errno.raw_os_error()) +} + impl Drop for ShmKeeper { fn drop(&mut self) { let _ = fs::remove_file(&self.path); diff --git a/crates/sigsafe/src/c_str.rs b/crates/sigsafe/src/c_str.rs index c246a047..b48f2974 100644 --- a/crates/sigsafe/src/c_str.rs +++ b/crates/sigsafe/src/c_str.rs @@ -130,6 +130,18 @@ impl<'a> CStr<'a, Thin> { } impl<'a> CStr<'a, Fat> { + /// Creates a length-retaining C string after validating its bytes. + /// + /// # Errors + /// + /// Returns an error unless `bytes` contains exactly one NUL byte at the + /// end. + pub fn from_bytes_with_nul(bytes: &'a [u8]) -> Result { + core::ffi::CStr::from_bytes_with_nul(bytes)?; + // SAFETY: validation above established exactly one trailing NUL. + Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) }) + } + /// Creates a length-retaining C string from bytes without validation. /// /// # Safety @@ -191,6 +203,13 @@ mod tests { assert_eq!(counted.as_bytes_with_nul(), fat.as_bytes_with_nul()); } + #[test] + fn checked_fat_constructor_validates_the_terminator() { + assert_eq!(CStr::::from_bytes_with_nul(b"abc\0").unwrap().as_bytes(), b"abc"); + assert!(CStr::::from_bytes_with_nul(b"abc").is_err()); + assert!(CStr::::from_bytes_with_nul(b"a\0bc\0").is_err()); + } + #[test] fn thin_view_accepts_a_checked_non_null_pointer() { let ptr = NonNull::new(c"abc".as_ptr().cast_mut()).unwrap(); diff --git a/crates/sigsafe/src/fs/linux.rs b/crates/sigsafe/src/fs/linux.rs index c9a4689c..1edfc3e7 100644 --- a/crates/sigsafe/src/fs/linux.rs +++ b/crates/sigsafe/src/fs/linux.rs @@ -1,10 +1,46 @@ use core::{mem::MaybeUninit, slice}; -use crate::{AsRawFd as _, BorrowedFd, CStr, Errno, Fat, Result, Thin}; +use rustix::fd::FromRawFd as _; + +use crate::{ + AsRawFd as _, BorrowedFd, CStr, Errno, Fat, OwnedFd, Result, Thin, + fs::{Mode, OFlags}, +}; // Linux UAPI `PATH_MAX`. pub(super) const PATH_MAX: usize = 4096; +fn syscall_fd(fd: BorrowedFd<'_>) -> Result { + let fd = isize::try_from(fd.as_raw_fd()).map_err(|_| Errno::OVERFLOW)?; + Ok(fd.cast_unsigned()) +} + +#[expect(clippy::needless_pass_by_value, reason = "CStr is a borrowed value type")] +pub(super) fn openat( + dirfd: BorrowedFd<'_>, + path: CStr<'_, R>, + flags: OFlags, + mode: Mode, +) -> Result { + // SAFETY: `dirfd` remains borrowed and `path` is NUL-terminated. The + // kernel reads no variadic arguments; all four syscall arguments are + // passed explicitly. + let fd = unsafe { + syscalls::syscall4( + syscalls::Sysno::openat, + syscall_fd(dirfd)?, + path.as_ptr().addr(), + usize::try_from(flags.bits()).map_err(|_| Errno::INVAL)?, + usize::try_from(mode.bits()).map_err(|_| Errno::INVAL)?, + ) + } + .map_err(|errno| Errno::from_raw_os_error(errno.into_raw()))?; + let fd = i32::try_from(fd).map_err(|_| Errno::OVERFLOW)?; + + // SAFETY: a successful `openat` returns a new owned descriptor. + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) +} + /// Reads the target of `path` relative to `dirfd` into `buf`. /// /// The returned bytes borrow the initialized prefix of `buf`. As with the @@ -27,7 +63,7 @@ pub fn readlinkat<'buf>( let initialized = unsafe { syscalls::syscall4( syscalls::Sysno::readlinkat, - (dirfd.as_raw_fd() as isize).cast_unsigned(), + syscall_fd(dirfd)?, path.as_ptr().addr(), buf.as_mut_ptr().addr(), buf.len(), diff --git a/crates/sigsafe/src/fs/mac.rs b/crates/sigsafe/src/fs/mac.rs index 53283f80..4727f9ce 100644 --- a/crates/sigsafe/src/fs/mac.rs +++ b/crates/sigsafe/src/fs/mac.rs @@ -7,10 +7,13 @@ use rustix::{ use crate::{BorrowedFd, CStr, CWD, Errno, Fat, Result, Thin}; -pub(super) const PATH_MAX: usize = libc::PATH_MAX as usize; +// Darwin UAPI `MAXPATHLEN`. Keep the Rust array length checked against libc's +// declaration without using an unchecked integer cast. +pub(super) const PATH_MAX: usize = 1024; +const _: () = assert!(libc::PATH_MAX == 1024); #[expect(clippy::needless_pass_by_value, reason = "CStr is a borrowed value type")] -fn openat( +pub(super) fn openat( dirfd: BorrowedFd<'_>, path: CStr<'_, R>, flags: OFlags, diff --git a/crates/sigsafe/src/fs/mod.rs b/crates/sigsafe/src/fs/mod.rs index ea4e5084..3ab71432 100644 --- a/crates/sigsafe/src/fs/mod.rs +++ b/crates/sigsafe/src/fs/mod.rs @@ -2,7 +2,9 @@ use core::mem::MaybeUninit; -use crate::{CStr, Fat, Result}; +pub use rustix::fs::{Mode, OFlags}; + +use crate::{BorrowedFd, CStr, Fat, OwnedFd, Result}; #[cfg(target_os = "linux")] mod linux; @@ -21,6 +23,20 @@ pub use mac::fcntl_getpath; /// The platform's maximum pathname size, including the terminating NUL. pub const PATH_MAX: usize = imp::PATH_MAX; +/// Opens `path` relative to `dirfd` and returns its owned descriptor. +/// +/// # Errors +/// +/// Returns the error reported by `openat`. +pub fn openat( + dirfd: BorrowedFd<'_>, + path: CStr<'_, R>, + flags: OFlags, + mode: Mode, +) -> Result { + imp::openat(dirfd, path, flags, mode) +} + /// Writes the absolute pathname of the current working directory into `buf`. /// /// The returned C string borrows `buf`, starts at the same address as `buf`, diff --git a/crates/sigsafe/src/lib.rs b/crates/sigsafe/src/lib.rs index b46d2f6c..5be7bc45 100644 --- a/crates/sigsafe/src/lib.rs +++ b/crates/sigsafe/src/lib.rs @@ -13,6 +13,7 @@ // preload library. #![cfg(unix)] #![cfg_attr(not(test), no_std)] +#![deny(clippy::as_conversions)] mod c_str; pub mod env; @@ -22,7 +23,7 @@ pub mod param; pub use c_str::{Bytes, CStr, Fat, Thin}; pub use rustix::{ - fd::{AsRawFd, BorrowedFd}, + fd::{AsRawFd, BorrowedFd, IntoRawFd, OwnedFd}, fs::CWD, io::{Errno, Errno as Error, Result}, };