diff --git a/crates/fspy_shm/Cargo.toml b/crates/fspy_shm/Cargo.toml index a9533cd4..eb339b4f 100644 --- a/crates/fspy_shm/Cargo.toml +++ b/crates/fspy_shm/Cargo.toml @@ -7,12 +7,12 @@ license.workspace = true publish = false rust-version.workspace = true -[dependencies] -memmap2 = { workspace = true } - [target.'cfg(unix)'.dependencies] sigsafe = { workspace = true } +[target.'cfg(windows)'.dependencies] +memmap2 = { workspace = true } + [target.'cfg(target_os = "windows")'.dependencies] windows-sys = { workspace = true, features = [ "Win32_Foundation", diff --git a/crates/fspy_shm/README.md b/crates/fspy_shm/README.md index 3b85ee89..cdcb557e 100644 --- a/crates/fspy_shm/README.md +++ b/crates/fspy_shm/README.md @@ -27,14 +27,14 @@ One implementation serves every platform: a sparse file at the full path supplie Only written pages ever occupy memory or disk. The multi-gigabyte capacity fspy asks for therefore costs about as much as the data a run actually records. -Mapping goes through `memmap2` on every platform. The remaining platform-specific parts are three short passages: +Unix opens and maps through `sigsafe`; on Linux, its raw-syscall backend works before libc initialization. Windows maps through `memmap2`. The remaining platform-specific parts are short passages: | Concern | Unix | Windows | | ----------------- | ---------------------------------------- | --------------------------------------------------------------------------- | | Same-user access | `mode(0o600)` on the backing file | the per-user `%TEMP%` ACL | | Sparseness | file holes, produced by setting a length | `FSCTL_SET_SPARSE` before setting a length, or NTFS allocates every cluster | | Keeper cleanup | unlink the path | unlink the path; see the fallback below | -| Descriptor safety | `O_CLOEXEC`, the Rust standard default | non-inheritable handles, the Rust standard default | +| Descriptor safety | raw `openat` with `O_CLOEXEC` | non-inheritable handles, the Rust standard default | `FILE_ATTRIBUTE_TEMPORARY` asks Windows to keep the data in memory when it can. Creation fails on a volume without sparse-file support. diff --git a/crates/fspy_shm/src/unix.rs b/crates/fspy_shm/src/unix.rs index 166c3074..376a8a6f 100644 --- a/crates/fspy_shm/src/unix.rs +++ b/crates/fspy_shm/src/unix.rs @@ -4,12 +4,12 @@ use std::{ ffi::OsStr, fs::{self, File, OpenOptions}, io, + num::NonZeroUsize, os::unix::{ffi::OsStrExt as _, fs::OpenOptionsExt as _, io::IntoRawFd as _}, path::PathBuf, + ptr::{self, NonNull}, }; -use memmap2::{MmapOptions, MmapRaw}; - /// Keeps the shared memory's backing-file name alive and removes it on drop. /// /// Removal is cleanup, not a stop signal: later opens fail, but existing @@ -25,7 +25,7 @@ pub struct ShmKeeper { /// view of the same bytes. Drop the handle once the mappings exist. pub struct ShmHandle { file: sigsafe::OwnedFd, - size: usize, + size: NonZeroUsize, } /// The mapped shared bytes. @@ -33,9 +33,16 @@ pub struct ShmHandle { /// A `Mapping` keeps the bytes alive until it is dropped and cannot affect the /// shared memory's path. pub struct Mapping { - raw: MmapRaw, + ptr: NonNull, + len: NonZeroUsize, } +// SAFETY: a mapping owns no thread-affine state; access synchronization is +// supplied by the fspy channel built on top of it. +unsafe impl Send for Mapping {} +// SAFETY: sharing a `Mapping` does not itself access its bytes, and all actual +// concurrent access is synchronized by the fspy channel. +unsafe impl Sync for Mapping {} /// Creates `size` bytes of zero-initialized shared memory at `path`. /// /// Returns its [`ShmKeeper`] and an already opened [`ShmHandle`], so the @@ -48,13 +55,10 @@ pub struct Mapping { /// /// Returns an error if the shared memory cannot be created or sized. pub fn create(path: &OsStr, size: usize) -> io::Result<(ShmKeeper, ShmHandle)> { - if size == 0 { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "shared-memory size must be nonzero", - )); - } - let size_u64 = u64::try_from(size).map_err(|_| { + let size = NonZeroUsize::new(size).ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size must be nonzero") + })?; + let size_u64 = u64::try_from(size.get()).map_err(|_| { io::Error::new(io::ErrorKind::InvalidInput, "shared-memory size exceeds u64") })?; let path = PathBuf::from(path); @@ -89,9 +93,7 @@ pub fn open(path: &OsStr) -> io::Result { // resize cannot make a mapping access invalid memory. let size = usize::try_from(sigsafe::fs::fstat(&file).map_err(errno_to_io)?.st_size) .map_err(|_| io::ErrorKind::InvalidData)?; - if size == 0 { - return Err(io::ErrorKind::InvalidData.into()); - } + let size = NonZeroUsize::new(size).ok_or(io::ErrorKind::InvalidData)?; Ok(ShmHandle { file, size }) } @@ -137,8 +139,36 @@ impl ShmHandle { /// /// Returns an error if the mapping cannot be established. pub fn map(&self) -> io::Result { - let file = sigsafe::AsRawFd::as_raw_fd(&self.file); - Ok(Mapping { raw: MmapOptions::new().len(self.size).map_raw(file)? }) + // SAFETY: the address is only a hint, the nonzero length is the + // validated backing-file size, the descriptor remains borrowed, + // and the resulting shared mapping is owned by `Mapping`. + let mapped = unsafe { + sigsafe::mm::mmap( + ptr::null_mut(), + self.size.get(), + sigsafe::mm::ProtFlags::READ | sigsafe::mm::ProtFlags::WRITE, + sigsafe::mm::MapFlags::SHARED, + &self.file, + 0, + ) + } + .map_err(errno_to_io)?; + let Some(ptr) = NonNull::new(mapped.cast()) else { + // A non-fixed mapping should not be placed at address zero, + // which Rust cannot represent as a non-null allocation. + // SAFETY: release the successful mapping before rejecting it. + let _ = unsafe { sigsafe::mm::munmap(mapped, self.size.get()) }; + return Err(io::ErrorKind::Other.into()); + }; + Ok(Mapping { ptr, len: self.size }) + } +} + +impl Drop for Mapping { + fn drop(&mut self) { + // SAFETY: this is the complete mapping owned by `self`, and dropping + // it proves that no safe borrow through `self` remains. + let _ = unsafe { sigsafe::mm::munmap(self.ptr.as_ptr().cast(), self.len.get()) }; } } @@ -146,14 +176,14 @@ impl ShmHandle { impl Mapping { /// Returns the mapped length in bytes. #[must_use] - pub fn len(&self) -> usize { - self.raw.len() + pub const fn len(&self) -> usize { + self.len.get() } /// Returns a raw pointer to the first mapped byte. #[must_use] - pub fn as_ptr(&self) -> *mut u8 { - self.raw.as_mut_ptr() + pub const fn as_ptr(&self) -> *mut u8 { + self.ptr.as_ptr() } /// Returns the mapped bytes as a shared slice. @@ -163,7 +193,7 @@ impl Mapping { /// The caller must ensure that no process or thread mutates the mapping for /// the lifetime of the returned slice. #[must_use] - pub unsafe fn as_slice(&self) -> &[u8] { + pub const unsafe fn as_slice(&self) -> &[u8] { // SAFETY: The mapping is valid for its full length, and the caller // guarantees that it is not mutated while the slice is borrowed. unsafe { std::slice::from_raw_parts(self.as_ptr().cast_const(), self.len()) } diff --git a/crates/sigsafe/README.md b/crates/sigsafe/README.md index e708d0da..8216c9f7 100644 --- a/crates/sigsafe/README.md +++ b/crates/sigsafe/README.md @@ -28,9 +28,9 @@ rustix can be built with a libc backend instead of raw syscalls, and anything in Functions whose rustix implementation already meets the rules are re-exposed as-is; being listed in a module here is what marks a call as allowed, and the backend check above is what keeps that true. -- `mm` — anonymous memory mappings: `mmap_anonymous`, `munmap`. +- `mm` — anonymous and file-backed memory mappings, protection changes, and unmapping. - `env` — allocation-free iteration over process arguments and environment entries. -- `fs` — caller-buffer filesystem operations: `getcwd`, plus macOS `fcntl_getpath`. +- `fs` — caller-buffer and descriptor operations such as `openat`, `fstat`, `getcwd`, and `readlinkat`. - `param` — `page_size`. Allocation without malloc lives in [`sigsafe_alloc`](../sigsafe_alloc). diff --git a/crates/sigsafe/src/mm.rs b/crates/sigsafe/src/mm.rs index c7cdecd0..7de3763f 100644 --- a/crates/sigsafe/src/mm.rs +++ b/crates/sigsafe/src/mm.rs @@ -8,4 +8,4 @@ //! pre-libc startup) and the crate-level backend check, which guarantees //! they cannot silently turn into libc calls on Linux. -pub use rustix::mm::{MapFlags, MprotectFlags, ProtFlags, mmap_anonymous, mprotect, munmap}; +pub use rustix::mm::{MapFlags, MprotectFlags, ProtFlags, mmap, mmap_anonymous, mprotect, munmap};