diff --git a/crates/sandlock-core/src/blocking.rs b/crates/sandlock-core/src/blocking.rs new file mode 100644 index 000000000..109f8e9c6 --- /dev/null +++ b/crates/sandlock-core/src/blocking.rs @@ -0,0 +1,151 @@ +//! Bounded synchronous work, isolated from the embedding runtime's blocking pool. +//! +//! Dropping a waiter does not stop running work. Its closure retains admission +//! until it returns; process-wide pools keep that bound across sandbox restarts. + +use std::io; +use std::sync::{Arc, OnceLock}; + +use tokio::runtime::{Builder, Runtime}; +use tokio::sync::Semaphore; +use tokio::task::JoinHandle; + +struct BlockingPool { + runtime: Option, + slots: Arc, +} + +impl BlockingPool { + fn new(name: &str, capacity: usize) -> io::Result { + let runtime = Builder::new_current_thread() + .max_blocking_threads(capacity) + .thread_name(name) + .build()?; + Ok(Self { + runtime: Some(runtime), + slots: Arc::new(Semaphore::new(capacity)), + }) + } + + fn spawn(&self, work: F) -> io::Result> + where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, + { + let permit = Arc::clone(&self.slots) + .try_acquire_owned() + .map_err(|_| io::Error::from_raw_os_error(libc::EAGAIN))?; + Ok(self.runtime.as_ref().unwrap().spawn_blocking(move || { + let _permit = permit; + work() + })) + } +} + +impl Drop for BlockingPool { + fn drop(&mut self) { + self.slots.close(); + if let Some(runtime) = self.runtime.take() { + runtime.shutdown_background(); + } + } +} + +fn pool( + storage: &'static OnceLock>, + name: &str, + capacity: usize, +) -> io::Result<&'static BlockingPool> { + storage + .get_or_init(|| BlockingPool::new(name, capacity)) + .as_ref() + .map_err(|e| io::Error::new(e.kind(), e.to_string())) +} + +async fn run_on(pool: io::Result<&BlockingPool>, work: F) -> io::Result +where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, +{ + pool?.spawn(work)?.await.map_err(io::Error::other) +} + +/// Run synchronous supervised work with bounded admission. +/// +/// Saturation returns EAGAIN without executing the closure. A timeout or dropped +/// future stops waiting only; running work and its owned resources may outlive +/// the caller. Capture owners rather than borrowed fd numbers or pointers. +pub async fn run(work: F) -> io::Result +where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, +{ + static POOL: OnceLock> = OnceLock::new(); + run_on(pool(&POOL, "sandlock-blocking", 64), work).await +} + +pub(crate) async fn filesystem(work: F) -> io::Result +where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, +{ + // A stalled external peer must not occupy the workers needed by filesystem work. + static POOL: OnceLock> = OnceLock::new(); + run_on(pool(&POOL, "sandlock-filesystem", 16), work).await +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn dropped_waiter_keeps_running_work_admitted() { + let pool = BlockingPool::new("blocking-test", 1).unwrap(); + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let job = pool + .spawn(move || { + let _ = started_tx.send(()); + let _ = release_rx.recv(); + }) + .unwrap(); + started_rx.await.unwrap(); + drop(job); + let error = pool.spawn(|| ()).unwrap_err(); + assert_eq!(error.raw_os_error(), Some(libc::EAGAIN)); + release_tx.send(()).unwrap(); + let permit = Arc::clone(&pool.slots).acquire_owned().await.unwrap(); + drop(permit); + assert_eq!(pool.spawn(|| 42).unwrap().await.unwrap(), 42); + } + + #[tokio::test] + async fn shutdown_does_not_wait_for_running_work() { + let pool = BlockingPool::new("blocking-shutdown-test", 1).unwrap(); + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let job = pool + .spawn(move || { + let _ = started_tx.send(()); + let _ = release_rx.recv(); + 42 + }) + .unwrap(); + started_rx.await.unwrap(); + drop(pool); + release_tx.send(()).unwrap(); + assert_eq!(job.await.unwrap(), 42); + } + + #[tokio::test] + async fn panicking_work_releases_admission() { + let pool = BlockingPool::new("blocking-panic-test", 1).unwrap(); + assert!(pool + .spawn(|| panic!("worker panic")) + .unwrap() + .await + .unwrap_err() + .is_panic()); + assert_eq!(pool.spawn(|| 42).unwrap().await.unwrap(), 42); + } +} diff --git a/crates/sandlock-core/src/chroot/dispatch.rs b/crates/sandlock-core/src/chroot/dispatch.rs index 40af237e4..ac970d7b4 100644 --- a/crates/sandlock-core/src/chroot/dispatch.rs +++ b/crates/sandlock-core/src/chroot/dispatch.rs @@ -475,6 +475,39 @@ fn resolve_chroot_path_existing( resolve_existing_in_root(ctx.root, &full_path) } +/// Pin the socket inode a named unix address reaches in the sandbox's view +/// and require write access to it, as the kernel's connect does. +/// +/// The child's real root and cwd are the host's, so the kernel cannot be left +/// to resolve the address: the grant is checked on where the pinned inode +/// actually sits, after symlinks inside the root are followed. +pub(crate) fn pin_writable_unix_target( + notif: &SeccompNotif, + ctx: &ChrootCtx<'_>, + sun_path: &Path, +) -> Result { + let vpath = build_virtual_path( + notif, + libc::AT_FDCWD as i64, + sun_path.to_str().ok_or(libc::ECONNREFUSED)?, + ctx, + ) + .ok_or(libc::ECONNREFUSED)?; + let confined = confine(&vpath); + let (root, sub) = match ctx.mount_target(&confined) { + Some((mount, sub)) => (mount, sub), + None => (ctx.root, confined.to_string_lossy().into_owned()), + }; + let fd = openat2_in_root(root, &sub, libc::O_PATH | libc::O_CLOEXEC, 0) + .map_err(|_| libc::ECONNREFUSED)?; + let pinned = unsafe { OwnedFd::from_raw_fd(fd) }; + let host = std::fs::read_link(format!("/proc/self/fd/{}", fd)).map_err(|_| libc::EACCES)?; + match ctx.host_to_virtual(&host) { + Some(virtual_path) if ctx.can_write(&virtual_path) => Ok(pinned), + _ => Err(libc::EACCES), + } +} + /// Convert a Path to CString, returning Errno on failure. fn path_cstr(path: &Path, err: i32) -> Result { CString::new(path.to_str().unwrap_or("")).map_err(|_| NotifAction::Errno(err)) diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index b65a789e0..477fb3dc7 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -28,7 +28,6 @@ use std::os::unix::io::{FromRawFd, OwnedFd, RawFd}; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; -use tokio::sync::Mutex; use tokio::sync::Mutex as AsyncMutex; use crate::arch; @@ -200,7 +199,7 @@ pub(crate) fn open_confined( /// Handle open/openat/openat2 under workdir: redirect to COW upper/lower. pub(crate) async fn handle_cow_open( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -222,9 +221,9 @@ pub(crate) async fn handle_cow_open( }; let mut path = resolve_at_path_with_virtual(notif, dirfd, &rel_path, virtual_cwd.as_deref()); - // Phase 1: determine plan under lock (no heavy I/O) + // Keep preparation, copy and bookkeeping under the same owned state guard. let (plan, upper_root, workdir_root) = { - let mut st = cow_state.lock().await; + let st = &mut *cow_state; let cow = match st.branch.as_mut() { Some(c) => c, None => return NotifAction::Continue, @@ -253,9 +252,8 @@ pub(crate) async fn handle_cow_open( }; (plan, upper_root, workdir_root) }; - // Lock is released here - // Phase 2: execute I/O plan without holding the lock + // Execute the copy before exposing the upper file to another handler. let real_path = match plan { CowOpenPlan::Skip => return NotifAction::Continue, // Deleted in this branch (whiteout): the lower file still exists, so @@ -268,41 +266,22 @@ pub(crate) async fn handle_cow_open( lower: _lower, file_size, rel_path, - } => { - // Do the potentially-expensive copy on a blocking thread - let root = workdir_root.clone(); - let uroot = upper_root.clone(); - let rel = rel_path.clone(); - let copy_result = tokio::task::spawn_blocking(move || { - crate::cow::seccomp::SeccompCowBranch::execute_copy(&root, &uroot, &rel) - }) - .await; - - match copy_result { - Ok(Ok(digest)) => { - if let Some(d) = digest { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.record_digest(&rel_path, d); - } + } => match SeccompCowBranch::execute_copy(&workdir_root, &upper_root, &rel_path) { + Ok(digest) => { + if let Some(d) = digest { + if let Some(cow) = cow_state.branch.as_mut() { + cow.record_digest(&rel_path, d); } - upper } - Ok(Err(_)) | Err(_) => { - // Continuing would let the kernel open the lower file - // itself, so a write would land outside the branch. - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.rollback_copy(file_size); - } - let errno = match copy_result { - Ok(Err(e)) => e.raw_os_error().unwrap_or(libc::EIO), - _ => libc::EIO, - }; - return NotifAction::Errno(errno); + upper + } + Err(e) => { + if let Some(cow) = cow_state.branch.as_mut() { + cow.rollback_copy(file_size); } + return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EIO)); } - } + }, }; // Phase 3: open the resolved path and inject fd @@ -617,34 +596,27 @@ fn cow_copy_rel<'a>( cow.safe_rel(copy_path).map(|rel| (match_path, rel)) } -/// Execute a deferred `CowCopyPlan::NeedsCopy` on a blocking thread. +/// Execute a copy plan on the filesystem worker. /// Returns the upper path on success, or rolls back quota on failure. async fn execute_deferred_copy( - cow_state: &Arc>, + cow_state: &mut CowState, workdir_root: std::path::PathBuf, upper_root: std::path::PathBuf, rel: String, upper: std::path::PathBuf, file_size: u64, ) -> Option { - let rel_path = rel.clone(); - let copy_result = tokio::task::spawn_blocking(move || { - crate::cow::seccomp::SeccompCowBranch::execute_copy(&workdir_root, &upper_root, &rel) - }) - .await; - match copy_result { - Ok(Ok(digest)) => { + match SeccompCowBranch::execute_copy(&workdir_root, &upper_root, &rel) { + Ok(digest) => { if let Some(d) = digest { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.record_digest(&rel_path, d); + if let Some(cow) = cow_state.branch.as_mut() { + cow.record_digest(&rel, d); } } Some(upper) } - _ => { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { + Err(_) => { + if let Some(cow) = cow_state.branch.as_mut() { cow.rollback_copy(file_size); } None @@ -655,14 +627,11 @@ async fn execute_deferred_copy( /// Handle all write-type syscalls: both *at variants (unlinkat, mkdirat, etc.) /// and legacy variants (unlink, rmdir, mkdir, etc.). /// -/// For operations that modify existing files (chmod, chown, rename, link, -/// truncate), the handler uses a two-phase pattern: prepare the copy plan -/// under the lock, execute the potentially expensive file copy outside the -/// lock, then re-acquire the lock and run the actual operation (which finds -/// the file already in upper). +/// The worker owns COW state through preparation, copying and mutation so +/// another handler or branch finalization cannot observe a partial copy. pub(crate) async fn handle_cow_write( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -672,10 +641,9 @@ pub(crate) async fn handle_cow_write( None => return NotifAction::Continue, }; - // Phase 1: check if we need to pre-copy a file (under lock, no heavy I/O). - // Capture both layer roots here so Phase 2 needs no second lock. + // Pre-copy existing data before applying the mutation. let (copy_plan, copy_workdir, copy_upper, copy_rel) = { - let mut st = cow_state.lock().await; + let st = &mut *cow_state; let cow = match st.branch.as_mut() { Some(c) => c, None => return NotifAction::Continue, @@ -712,9 +680,8 @@ pub(crate) async fn handle_cow_write( ), } }; - // Lock is released here - // Phase 2: execute the file copy outside the lock (if needed) + // Execute the copy before mutating the staged file. if let Some(crate::cow::seccomp::CowCopyPlan::NeedsCopy { upper, lower: _lower, @@ -732,13 +699,12 @@ pub(crate) async fn handle_cow_write( .await .is_none() { - return NotifAction::Continue; + return NotifAction::Errno(libc::EIO); } } - // Phase 3: execute the operation under lock (ensure_cow_copy is now a no-op - // for the pre-copied file since it's already in upper) - let mut st = cow_state.lock().await; + // ensure_cow_copy is now a no-op for the staged file. + let st = &mut *cow_state; let cow = match st.branch.as_mut() { Some(c) => c, None => return NotifAction::Continue, @@ -842,7 +808,7 @@ pub(crate) async fn handle_cow_write( /// don't fail before the COW layer can redirect their writes. pub(crate) async fn handle_cow_access( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -876,7 +842,7 @@ pub(crate) async fn handle_cow_access( return NotifAction::Continue; } - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -905,7 +871,7 @@ pub(crate) async fn handle_cow_access( /// utimensat(dirfd, pathname, times, flags) pub(crate) async fn handle_cow_utimensat( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -925,7 +891,7 @@ pub(crate) async fn handle_cow_utimensat( }; let (upper_path, upper_root, workdir_root) = { - let mut st = cow_state.lock().await; + let st = &mut *cow_state; let cow = match st.branch.as_mut() { Some(c) => c, None => return NotifAction::Continue, @@ -987,7 +953,7 @@ pub(crate) async fn handle_cow_utimensat( /// Actually, simpler: for stat, we do the stat ourselves and write the result. pub(crate) async fn handle_cow_stat( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1019,7 +985,7 @@ pub(crate) async fn handle_cow_stat( }; let (real_path, upper_root, workdir_root) = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1098,7 +1064,7 @@ pub(crate) async fn handle_cow_stat( /// statx the resolved path in the supervisor and write the buffer back. pub(crate) async fn handle_cow_statx( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1123,7 +1089,7 @@ pub(crate) async fn handle_cow_statx( }; let (real_path, upper_root, workdir_root) = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1172,7 +1138,7 @@ pub(crate) async fn handle_cow_statx( /// them alone. pub(crate) async fn handle_cow_exec( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1208,7 +1174,7 @@ pub(crate) async fn handle_cow_exec( let resolved = resolve_at_path_with_virtual(notif, dirfd, &rel_path, virtual_cwd.as_deref()); let (upper_path, upper_root, workdir_root) = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1282,7 +1248,7 @@ pub(crate) async fn handle_cow_exec( /// Handle readlinkat — read symlink from upper/lower, write to child buffer. pub(crate) async fn handle_cow_readlink( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1296,7 +1262,7 @@ pub(crate) async fn handle_cow_readlink( let buf_addr = notif.data.args[2]; let bufsiz = (notif.data.args[3] & 0xFFFFFFFF) as usize; - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1311,8 +1277,6 @@ pub(crate) async fn handle_cow_readlink( Some(t) => t, None => return NotifAction::Errno(libc::ENOENT), }; - drop(st); - let target_bytes = target.as_bytes(); let write_len = target_bytes.len().min(bufsiz); @@ -1334,7 +1298,7 @@ pub(crate) async fn handle_cow_readlink( /// Handle getdents64 for COW directories — merge upper + lower entries. pub(crate) async fn handle_cow_getdents( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1356,10 +1320,9 @@ pub(crate) async fn handle_cow_getdents( return NotifAction::Continue; } - // Compute rel_path under the global COW lock, but do not hold it - // across the per-process lock acquired below. + // COW handlers acquire branch state before per-process state. let rel_path = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1403,7 +1366,7 @@ pub(crate) async fn handle_cow_getdents( // Build cache on first call. if !perproc.cow_dir_cache.contains_key(&child_fd) { let entries = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1481,7 +1444,7 @@ pub(crate) async fn handle_cow_getdents( /// the path arg to /proc/self/fd/N so the kernel chdir succeeds. pub(crate) async fn handle_cow_chdir( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1497,7 +1460,7 @@ pub(crate) async fn handle_cow_chdir( resolve_at_path_with_virtual(notif, libc::AT_FDCWD as i64, &path, virtual_cwd.as_deref()); let (abs_path, upper_path, upper_root, workdir_root) = { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, @@ -1592,7 +1555,7 @@ pub(crate) async fn handle_cow_chdir( /// Handle getcwd after chdir into a COW-only directory. pub(crate) async fn handle_cow_getcwd( notif: &SeccompNotif, - cow_state: &Arc>, + cow_state: &mut CowState, processes: &Arc, notif_fd: RawFd, ) -> NotifAction { @@ -1603,7 +1566,7 @@ pub(crate) async fn handle_cow_getcwd( let virtual_cwd = if let Some(cwd) = cached_virtual_cwd { cwd } else { - let st = cow_state.lock().await; + let st = &*cow_state; let cow = match st.branch.as_ref() { Some(c) => c, None => return NotifAction::Continue, diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index 81101d4d9..57c249b3e 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -1247,7 +1247,7 @@ impl SeccompCowBranch { } /// Execute a file copy synchronously. Used by `ensure_cow_copy` and the - /// async dispatch (via `spawn_blocking`). + /// filesystem dispatch worker. pub fn execute_copy( workdir_root: &Path, upper_root: &Path, diff --git a/crates/sandlock-core/src/image/mod.rs b/crates/sandlock-core/src/image/mod.rs index 6cac1b512..bb1bb6050 100644 --- a/crates/sandlock-core/src/image/mod.rs +++ b/crates/sandlock-core/src/image/mod.rs @@ -206,7 +206,7 @@ fn resolve_working_dir(rootfs: &Path, dir: &str) -> Result( f: impl FnOnce() -> Result + Send + 'static, ) -> Result { - tokio::task::spawn_blocking(f) + crate::blocking::filesystem(f) .await .map_err(|e| SandboxRuntimeError::Child(format!("image task failed: {e}")))? } diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 1418a68b6..355f722c0 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -1,4 +1,5 @@ pub(crate) mod arch; +pub mod blocking; pub(crate) mod ca_inject; #[cfg(feature = "checkpoint")] pub(crate) mod checkpoint; diff --git a/crates/sandlock-core/src/network/connect.rs b/crates/sandlock-core/src/network/connect.rs index 2dfad96df..ccdac2f5b 100644 --- a/crates/sandlock-core/src/network/connect.rs +++ b/crates/sandlock-core/src/network/connect.rs @@ -17,7 +17,7 @@ use super::materialize::{ }; use super::query_socket_protocol; use super::unix::connect_named_unix_on_behalf; -use super::verdict::{classify_destination, path_under_any, Destination}; +use super::verdict::{classify_destination, Destination}; // ============================================================ // connect_on_behalf — perform connect() on behalf of the child (TOCTOU-safe) @@ -101,7 +101,7 @@ pub(super) async fn connect_on_behalf( }; let metadata_addr = addr_bytes.clone(); - NotifAction::prepare(&metadata_addr, dup_fd.as_raw_fd(), move || { + NotifAction::prepare_blocking(&metadata_addr, dup_fd.as_raw_fd(), move || { // Record the original destination before connect so the proxy // cannot receive the connection before its mapping is available. if plan.record_orig_dest { @@ -126,28 +126,11 @@ pub(super) async fn connect_on_behalf( // never return Continue on the deny path, so it is TOCTOU-safe. // Abstract sockets (no path) are handled by the Landlock abstract scope. match named_unix_socket_path(&addr_bytes) { + // Resolve the symlink-followed target and connect on-behalf to + // the pinned inode, so a symlink inside a granted dir cannot + // redirect to an ungranted socket. Some(path) if ctx.policy.supervises_unix_connect() => { - if ctx.policy.chroot_root.is_some() { - // Chroot mode: the child's paths are virtual, so a lexical - // check against the (virtual) write grants is consistent, - // and host socket paths are absent from the chroot view - // anyway. Deny unless under a write grant. - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - } - } else { - // Non-chroot: resolve the symlink-followed real target and - // connect on-behalf to the pinned inode, so a symlink inside - // a granted dir cannot redirect to an ungranted socket. - connect_named_unix_on_behalf( - notif.pid, - sockfd, - &path, - &ctx.policy.chroot_writable, - ) - } + connect_named_unix_on_behalf(notif, ctx, sockfd, &path) } // Abstract/unnamed socket, non-AF_UNIX family, or gate disabled. _ => NotifAction::Continue, diff --git a/crates/sandlock-core/src/network/mod.rs b/crates/sandlock-core/src/network/mod.rs index 566044407..be821724b 100644 --- a/crates/sandlock-core/src/network/mod.rs +++ b/crates/sandlock-core/src/network/mod.rs @@ -154,19 +154,18 @@ fn socket_is_unix(fd: RawFd) -> bool { /// 1. Non-IP families (AF_UNIX etc.): the IP policy doesn't apply; /// Landlock IPC scoping is the enforcement boundary. One sub-case is /// narrower: `sendto`'s non-IP fall-through does NOT Continue once a -/// destination policy is active. That fall-through has no chroot check — -/// it is whatever misses the named-path gate, i.e. a pathless AF_UNIX or a -/// non-AF_UNIX destination — so it fails closed under `--chroot` too. +/// destination policy is active. That fall-through is whatever misses the +/// named-path gate, i.e. a pathless AF_UNIX or a non-AF_UNIX destination, +/// so it fails closed under `--chroot` too. /// There the send goes on-behalf on a /// pinned fd — a named unix target resolved in the child's root view, or a /// non-unix destination on a unix socket (the NETLINK_ROUTE /// virtualization) — or fails closed with EAFNOSUPPORT for an AF_UNIX /// address with no pathname to pin, notably an abstract one: the /// supervisor has no Landlock domain, so an on-behalf abstract send would -/// escape the child's scope. The other non-IP arms still Continue under a -/// destination policy — `connect`'s abstract/unnamed/non-AF_UNIX arm and -/// both chroot named-unix arms (`connect`, `sendto`/`sendmsg`), which is -/// the residue tracked by issues #27 / #143. +/// escape the child's scope. `connect`'s abstract/unnamed/non-AF_UNIX +/// arm still Continues under a destination policy, which is the residue +/// tracked by issues #27 / #143. /// 2. Connected sockets with addr_ptr == 0 — the address was already /// validated at connect time, so the kernel re-read of (nothing) is /// moot. diff --git a/crates/sandlock-core/src/network/send.rs b/crates/sandlock-core/src/network/send.rs index 312c738e2..59d5b133c 100644 --- a/crates/sandlock-core/src/network/send.rs +++ b/crates/sandlock-core/src/network/send.rs @@ -20,9 +20,7 @@ use super::unix::{ mmsg_entry_named_unix_path, sendmmsg_named_unix_on_behalf, sendto_named_unix_on_behalf, sendto_pinned_unix_on_behalf, unix_sendmsg_gate, }; -use super::verdict::{ - check_ip_destination, classify_send_path, path_under_any, DestShape, Destination, SendPath, -}; +use super::verdict::{check_ip_destination, classify_send_path, DestShape, Destination, SendPath}; use super::{query_socket_protocol, socket_is_unix, Protocol}; // ============================================================ @@ -110,26 +108,9 @@ pub(super) async fn sendto_on_behalf( // sendto to a named socket is a WRITE on its inode, so deny unless the // resolved real target is under an fs-write grant. match named_unix_socket_path(&addr_bytes) { - Some(path) if ctx.policy.supervises_unix_send() => { - if ctx.policy.chroot_root.is_some() { - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - } - } else { - sendto_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - buf_ptr, - buf_len, - flags, - &path, - &ctx.policy.chroot_writable, - ) - } - } + Some(path) if ctx.policy.supervises_unix_send() => sendto_named_unix_on_behalf( + notif, ctx, notif_fd, sockfd, buf_ptr, buf_len, flags, &path, + ), _ => { // Non-IP destination with no fs-path gate: an abstract unix // address, a named unix address with the fs-gate off, a non-unix @@ -475,29 +456,9 @@ pub(super) async fn sendmmsg_on_behalf( } } if named_unix { - if ctx.policy.chroot_root.is_some() { - // Chroot: lexical check; deny the whole call if any named-unix - // entry is outside the (virtual) write grants. - for i in 0..vlen { - let entry_ptr = mmsg_entry_ptr(msgvec_ptr, i); - if let Some(path) = mmsg_entry_named_unix_path(notif, notif_fd, entry_ptr) { - if !path_under_any(&path, &ctx.policy.chroot_writable) { - return NotifAction::Errno(libc::EACCES); - } - } - } - // All granted: fall through to the existing path. - } else { - return sendmmsg_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - msgvec_ptr, - vlen, - flags, - &ctx.policy.chroot_writable, - ); - } + return sendmmsg_named_unix_on_behalf( + notif, ctx, notif_fd, sockfd, msgvec_ptr, vlen, flags, + ); } } diff --git a/crates/sandlock-core/src/network/unix.rs b/crates/sandlock-core/src/network/unix.rs index ed8ea3591..0f1bf5c19 100644 --- a/crates/sandlock-core/src/network/unix.rs +++ b/crates/sandlock-core/src/network/unix.rs @@ -18,7 +18,7 @@ use super::materialize::{ MaterializedMsg, }; use super::send_engine::{prepare_batch, prepare_send, wants_blocking}; -use super::verdict::{path_under_any, real_path_under_any}; +use super::verdict::real_path_under_any; /// Address `sun_path` in child's context. Absolute paths use `/proc//root`, /// relative paths use `/proc//cwd`. @@ -34,7 +34,7 @@ fn child_lookup_path(child_pid: u32, sun_path: &std::path::Path) -> String { } /// Pin the inode a named unix socket `sun_path` resolves to **in the child's -/// root view** (see [`child_root_path`]), following the child's symlinks rather +/// root view** (see [`child_lookup_path`]), following the child's symlinks rather /// than the supervisor's. Returns an `O_PATH` fd to that exact inode; callers /// address it through `/proc/self/fd` so the resolved inode is the one acted on, /// immune to a path swap after the check (TOCTOU- and symlink-safe). @@ -55,20 +55,25 @@ fn pin_child_unix_target( } /// Resolve a named unix socket `sun_path` to its real, symlink-followed inode -/// in the child's root view (see [`pin_child_unix_target`]) and verify that -/// inode is under an fs-write grant. On success returns the pinned `O_PATH` fd; -/// on failure returns the deny/refuse `NotifAction`. +/// in the child's view and verify that inode is under an fs-write grant. On +/// success returns the pinned `O_PATH` fd; on failure returns the deny/refuse +/// `NotifAction`. fn resolve_named_unix_target( - child_pid: u32, + notif: &SeccompNotif, + ctx: &SupervisorCtx, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> Result { - let pinned = pin_child_unix_target(child_pid, sun_path)?; + if ctx.policy.chroot_root.is_some() { + let chroot = crate::chroot::dispatch::ChrootCtx::new(&ctx.policy, &ctx.processes); + return crate::chroot::dispatch::pin_writable_unix_target(notif, &chroot, sun_path) + .map_err(NotifAction::Errno); + } + let pinned = pin_child_unix_target(notif.pid, sun_path)?; // Canonical path of the pinned inode in our mount namespace. let real = std::fs::read_link(format!("/proc/self/fd/{}", pinned.as_raw_fd())) .map_err(|_| NotifAction::Errno(libc::EACCES))?; - if real_path_under_any(&real, writable) { + if real_path_under_any(&real, &ctx.policy.chroot_writable) { Ok(pinned) } else { Err(NotifAction::Errno(libc::EACCES)) @@ -104,16 +109,17 @@ fn sockaddr_un_bytes(sun: &libc::sockaddr_un, len: libc::socklen_t) -> Vec { .to_vec() } -/// On-behalf `connect()` for a NAMED `AF_UNIX` socket in non-chroot mode: -/// resolve+verify the target, then connect the child's socket to the pinned -/// inode through `/proc/self/fd`. +/// On-behalf `connect()` for a NAMED `AF_UNIX` socket: resolve+verify the +/// target, then connect the child's socket to the pinned inode through +/// `/proc/self/fd`. pub(super) fn connect_named_unix_on_behalf( - child_pid: u32, + notif: &SeccompNotif, + ctx: &SupervisorCtx, sockfd: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - let pinned = match resolve_named_unix_target(child_pid, sun_path, writable) { + let child_pid = notif.pid; + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(action) => return action, }; @@ -126,7 +132,7 @@ pub(super) fn connect_named_unix_on_behalf( Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)), }; let addr = sockaddr_un_bytes(&sun, len); - NotifAction::prepare(&addr.clone(), dup_fd.as_raw_fd(), move || { + NotifAction::prepare_blocking(&addr.clone(), dup_fd.as_raw_fd(), move || { let _pinned = pinned; let ret = unsafe { libc::connect( @@ -143,20 +149,20 @@ pub(super) fn connect_named_unix_on_behalf( }) } -/// On-behalf `sendto()` for a NAMED `AF_UNIX` datagram in non-chroot mode: -/// resolve+verify the target, copy the child's data, then send to the pinned -/// inode through `/proc/self/fd`. +/// On-behalf `sendto()` for a NAMED `AF_UNIX` datagram: resolve+verify the +/// target, copy the child's data, then send to the pinned inode through +/// `/proc/self/fd`. pub(super) fn sendto_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, buf_ptr: u64, buf_len: usize, flags: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - let pinned = match resolve_named_unix_target(notif.pid, sun_path, writable) { + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(action) => return action, }; @@ -274,38 +280,24 @@ pub(super) fn unix_sendmsg_gate( .ok()?; // None unless this is a NAMED AF_UNIX target; IP/abstract fall through. let path = named_unix_socket_path(&addr_bytes)?; - - if ctx.policy.chroot_root.is_some() { - return Some(if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - }); - } Some(sendmsg_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - msghdr_ptr, - flags, - &path, - &ctx.policy.chroot_writable, + notif, ctx, notif_fd, sockfd, msghdr_ptr, flags, &path, )) } -/// On-behalf `sendmsg()` for a NAMED `AF_UNIX` datagram in non-chroot mode: -/// resolve+verify the target, copy the message's iovecs and control data, then -/// send to the pinned inode through `/proc/self/fd`. +/// On-behalf `sendmsg()` for a NAMED `AF_UNIX` datagram: resolve+verify the +/// target, copy the message's iovecs and control data, then send to the pinned +/// inode through `/proc/self/fd`. fn sendmsg_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msghdr_ptr: u64, flags: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - match send_named_unix_msghdr(notif, notif_fd, sockfd, msghdr_ptr, sun_path, writable) { + match send_named_unix_msghdr(notif, ctx, notif_fd, sockfd, msghdr_ptr, sun_path) { Ok((dup_fd, m)) => { let blocking = wants_blocking(dup_fd.as_raw_fd(), flags); prepare_send(dup_fd, m, flags, blocking) @@ -322,13 +314,13 @@ fn sendmsg_named_unix_on_behalf( /// the per-entry `sendmmsg` path. fn send_named_unix_msghdr( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msghdr_ptr: u64, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> Result<(OwnedFd, MaterializedMsg), i32> { - let pinned = match resolve_named_unix_target(notif.pid, sun_path, writable) { + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(NotifAction::Errno(e)) => return Err(e), Err(_) => return Err(libc::EACCES), @@ -374,20 +366,19 @@ pub(super) fn mmsg_entry_named_unix_path( named_unix_socket_path(&addr_bytes) } -/// On-behalf `sendmmsg` for a batch containing NAMED `AF_UNIX` entries -/// (non-chroot). Each named-unix entry is resolved, verified, and sent to its +/// On-behalf `sendmmsg` for a batch containing NAMED `AF_UNIX` entries. Each named-unix entry is resolved, verified, and sent to its /// pinned inode; the loop stops at the first entry it cannot gate on-behalf /// (connected/abstract) or that is denied, returning the count sent so far /// (standard short-`sendmmsg` semantics). Never returns `Continue`, so a unix /// entry cannot ride out via the binary whole-call passthrough. pub(super) fn sendmmsg_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msgvec_ptr: u64, vlen: usize, flags: i32, - writable: &[crate::grant::Grant], ) -> NotifAction { let mut entries = Vec::new(); let mut batch_bytes = 0usize; @@ -401,7 +392,7 @@ pub(super) fn sendmmsg_named_unix_on_behalf( None => break, }; let (dup_fd, m) = - match send_named_unix_msghdr(notif, notif_fd, sockfd, entry_ptr, &path, writable) { + match send_named_unix_msghdr(notif, ctx, notif_fd, sockfd, entry_ptr, &path) { Ok(pair) => pair, Err(errno) => { first_errno = Some(errno); diff --git a/crates/sandlock-core/src/network/verdict.rs b/crates/sandlock-core/src/network/verdict.rs index 9175b0c45..102339ac8 100644 --- a/crates/sandlock-core/src/network/verdict.rs +++ b/crates/sandlock-core/src/network/verdict.rs @@ -140,16 +140,6 @@ pub(crate) fn real_path_under_any(real: &std::path::Path, grants: &[crate::grant }) } -/// True if `path`, lexically normalized (`.`/`..` resolved without touching the -/// filesystem), is at or under any of the granted `prefixes`. Mirrors the -/// prefix matching the chroot fs enforcement uses. -pub(crate) fn path_under_any(path: &std::path::Path, grants: &[crate::grant::Grant]) -> bool { - let norm = crate::chroot::resolve::confine(&path.to_string_lossy()); - grants - .iter() - .any(|grant| grant.covers(std::path::Path::new(&norm))) -} - /// The shape of a non-IP destination sockaddr, keyed on its ADDRESS FAMILY /// first. Produced by `materialize::classify_dest_shape` from the copied /// address bytes. diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 7eec5b221..9801eee4b 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -17,7 +17,8 @@ // them approve a syscall based on user-memory contents. use std::collections::HashMap; -use std::os::unix::io::RawFd; +use std::os::unix::io::{AsRawFd, OwnedFd, RawFd}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use super::ctx::SupervisorCtx; @@ -76,24 +77,82 @@ pub trait Handler: Send + Sync + 'static { pub struct HandlerCtx { pub notif: SeccompNotif, pub notif_fd: RawFd, + listener: Arc, + active_work: Arc, /// The open-family request decoded once for this notification. open: Option>, } impl HandlerCtx { - pub fn new(notif: SeccompNotif, notif_fd: RawFd) -> Self { + pub fn new(notif: SeccompNotif, notif_fd: OwnedFd) -> Self { + Self::with_listener( + notif, + Arc::new(notif_fd), + None, + Arc::new(AtomicUsize::new(0)), + ) + } + + fn with_listener( + notif: SeccompNotif, + listener: Arc, + open: Option>, + active_work: Arc, + ) -> Self { Self { notif, - notif_fd, - open: None, + notif_fd: listener.as_raw_fd(), + listener, + active_work, + open, } } + pub fn listener(&self) -> Arc { + Arc::clone(&self.listener) + } + + /// Retain this guard inside native work that can access child memory. + /// Exec argument validation requires all such work to have finished. + pub fn blocking_guard(&self) -> BlockingGuard { + self.active_work.fetch_add(1, Ordering::Relaxed); + BlockingGuard(Arc::clone(&self.active_work)) + } + + fn filesystem( + &self, + work: impl std::future::Future + Send + 'static, + ) -> NotifAction { + let guard = self.blocking_guard(); + let listener = self.listener(); + let runtime = tokio::runtime::Handle::current(); + NotifAction::suspend_filesystem(async move { + crate::blocking::filesystem(move || { + let _guard = guard; + let _listener = listener; + // Filesystem handlers await state locks, but perform synchronous I/O. + runtime.block_on(work) + }) + .await + .unwrap_or_else(|e| NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EIO))) + }) + } + pub(crate) fn open(&self) -> Option> { self.open.clone() } } +/// Tracks native child-memory access until its worker finishes. +#[must_use = "retain the guard until native child-memory access finishes"] +pub struct BlockingGuard(Arc); + +impl Drop for BlockingGuard { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::Release); + } +} + // Blanket impl: any Fn(&HandlerCtx) -> Future is a Handler. // // Lets lightweight closure-style handlers work without ceremony at the @@ -246,28 +305,74 @@ impl DispatchTable { .push(handler); } - /// Dispatch a notification through the handler chain for its syscall number. + /// Dispatch until a handler produces an action or suspends. pub(crate) async fn dispatch( &self, notif: SeccompNotif, - notif_fd: RawFd, + listener: Arc, open: Option>, - ) -> NotifAction { - let nr = notif.data.nr as i64; - if let Some(chain) = self.chains.get(&nr) { - let handler_ctx = HandlerCtx { - notif, - notif_fd, - open, + active_work: Arc, + ) -> DispatchState { + let handlers = self + .chains + .get(&(notif.data.nr as i64)) + .map(|chain| chain.handlers.clone()) + .unwrap_or_default(); + DispatchState { + action: NotifAction::Continue, + handlers: handlers.into_iter(), + cx: HandlerCtx::with_listener(notif, listener, open, active_work), + deadline: tokio::time::Instant::now() + super::notif::DEFER_TIMEOUT, + } + .advance() + .await + } +} + +pub(crate) struct DispatchState { + action: NotifAction, + handlers: std::vec::IntoIter>, + cx: HandlerCtx, + deadline: tokio::time::Instant, +} + +impl DispatchState { + pub(crate) fn pending(&self) -> Option { + match &self.action { + NotifAction::Suspend(work) => Some(work.filesystem), + _ => None, + } + } + + pub(crate) async fn wait(mut self) -> Self { + if let NotifAction::Suspend(work) = + std::mem::replace(&mut self.action, NotifAction::Errno(libc::EIO)) + { + self.action = tokio::time::timeout_at(self.deadline, work.run()) + .await + .unwrap_or(NotifAction::Errno(libc::EIO)); + } + self + } + + // Only the notification loop advances handlers; they may write child memory. + pub(crate) async fn advance(mut self) -> Self { + while matches!(self.action, NotifAction::Continue) { + let Some(handler) = self.handlers.next() else { + break; }; - for handler in &chain.handlers { - let action = handler.handle(&handler_ctx).await; - if !matches!(action, NotifAction::Continue) { - return action; - } + self.action = handler.handle(&self.cx).await; + } + self + } + + pub(crate) fn into_action(self) -> NotifAction { + match self.action { + NotifAction::Defer(work) => { + NotifAction::Defer(work.retain_listener(self.cx.listener())) } + other => other, } - NotifAction::Continue } } @@ -291,20 +396,6 @@ pub(crate) fn build_dispatch_table( ) -> DispatchTable { let mut table = DispatchTable::new(); - // ------------------------------------------------------------------ - // Fork/clone family (always on) - // ------------------------------------------------------------------ - for nr in arch::fork_like_syscalls() { - let policy_for_fork = Arc::clone(policy); - let ctx_for_fork = Arc::clone(ctx); - table.register(nr, move |cx: &HandlerCtx| { - let notif = cx.notif; - let policy = Arc::clone(&policy_for_fork); - let ctx = Arc::clone(&ctx_for_fork); - async move { crate::resource::handle_fork(¬if, &ctx, &policy).await } - }); - } - // ------------------------------------------------------------------ // Process groups (always on) // ------------------------------------------------------------------ @@ -814,10 +905,11 @@ fn register_chroot_handlers( let processes = Arc::clone(&processes); let notif_fd = cx.notif_fd; let policy = Arc::clone(&policy); - async move { + let action = cx.filesystem(async move { let chroot_ctx = ChrootCtx::new(&policy, &processes); $handler(¬if, &chroot_state, &cow_state, notif_fd, &chroot_ctx).await - } + }); + async move { action } } }}; } @@ -837,10 +929,11 @@ fn register_chroot_handlers( let processes = Arc::clone(&processes); let notif_fd = cx.notif_fd; let policy = Arc::clone(&policy); - async move { + let action = cx.filesystem(async move { let chroot_ctx = ChrootCtx::new(&policy, &processes); $handler(¬if, &chroot_state, &cow_state, notif_fd, &chroot_ctx).await - } + }); + async move { action } } }}; } @@ -1109,7 +1202,11 @@ fn register_cow_handlers(table: &mut DispatchTable, ctx: &Arc) { let cow_state = Arc::clone(&cow_state); let processes_state = Arc::clone(&processes_state); let notif_fd = cx.notif_fd; - async move { $handler(¬if, &cow_state, &processes_state, notif_fd).await } + let action = cx.filesystem(async move { + let mut state = cow_state.lock_owned().await; + $handler(¬if, &mut state, &processes_state, notif_fd).await + }); + async move { action } } }}; } @@ -1369,8 +1466,14 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1, None) - .await; + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await + .into_action(); assert!(matches!(action, NotifAction::Continue)); let recorded = order.lock().unwrap(); @@ -1416,8 +1519,14 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1, None) - .await; + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await + .into_action(); assert!(matches!(action, NotifAction::Continue)); let recorded = order.lock().unwrap(); @@ -1461,8 +1570,14 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1, None) - .await; + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await + .into_action(); match action { NotifAction::Errno(e) => assert_eq!(e, libc::EACCES), @@ -1475,6 +1590,133 @@ mod handler_tests { ); } + fn fake_listener() -> Arc { + Arc::new(std::fs::File::open("/dev/null").unwrap().into()) + } + + #[tokio::test] + async fn native_memory_tracking_does_not_limit_filesystem_admission() { + let cx = HandlerCtx::new( + fake_notif(libc::SYS_openat as i32), + std::fs::File::open("/dev/null").unwrap().into(), + ); + let _stalled: Vec<_> = (0..super::super::notif::DEFER_MAX_INFLIGHT) + .map(|_| cx.blocking_guard()) + .collect(); + let NotifAction::Suspend(work) = cx.filesystem(async { NotifAction::ReturnValue(42) }) + else { + panic!("filesystem admission depends on unrelated native work"); + }; + assert!(matches!(work.run().await, NotifAction::ReturnValue(42))); + } + + #[tokio::test] + async fn suspended_fork_does_not_reserve_a_process_slot() { + let ctx = fake_supervisor_ctx(); + ctx.resource.lock().await.max_processes = 2; + let mut table = build_dispatch_table(&ctx.policy, &ctx.resource, &ctx, Vec::new()); + table.register(libc::SYS_clone, |_cx: &HandlerCtx| async { + NotifAction::suspend(async { NotifAction::Errno(libc::EIO) }) + }); + let state = table + .dispatch( + fake_notif(libc::SYS_clone as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await; + assert!(state.pending().is_some()); + assert_eq!(ctx.resource.lock().await.proc_count, 0); + let action = state.wait().await.advance().await.into_action(); + assert!(matches!(action, NotifAction::Errno(libc::EIO))); + assert_eq!(ctx.resource.lock().await.proc_count, 0); + } + + #[tokio::test] + async fn filesystem_work_retains_state_and_exec_guard_after_waiter_abort() { + let cx = HandlerCtx::new( + fake_notif(libc::SYS_openat as i32), + std::fs::File::open("/dev/null").unwrap().into(), + ); + let state = Arc::new(Mutex::new(0)); + let work_state = Arc::clone(&state); + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let action = cx.filesystem(async move { + let mut state = work_state.lock_owned().await; + let _ = started_tx.send(()); + let _ = release_rx.recv(); + *state = 42; + NotifAction::Continue + }); + let NotifAction::Suspend(work) = action else { + panic!("expected suspension") + }; + let waiter = tokio::spawn(work.run()); + started_rx.await.unwrap(); + waiter.abort(); + let _ = waiter.await; + assert!(state.try_lock().is_err()); + assert_eq!(cx.active_work.load(Ordering::Acquire), 1); + release_tx.send(()).unwrap(); + assert_eq!(*state.lock().await, 42); + while cx.active_work.load(Ordering::Acquire) != 0 { + tokio::task::yield_now().await; + } + } + + #[tokio::test] + async fn suspended_continue_resumes_remaining_handlers() { + let mut table = DispatchTable::new(); + let calls = Arc::new(AtomicUsize::new(0)); + table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { + NotifAction::suspend(async { NotifAction::Continue }) + }); + let later_calls = Arc::clone(&calls); + table.register(libc::SYS_openat, move |_cx: &HandlerCtx| { + later_calls.fetch_add(1, Ordering::SeqCst); + async { NotifAction::ReturnValue(42) } + }); + let action = table + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await; + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert!(action.pending().is_some()); + let state = action.wait().await; + assert_eq!(calls.load(Ordering::SeqCst), 0); + let action = state.advance().await.into_action(); + assert!(matches!(action, NotifAction::ReturnValue(42))); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn suspended_terminal_action_stops_remaining_handlers() { + let mut table = DispatchTable::new(); + table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { + NotifAction::suspend(async { NotifAction::Errno(libc::EACCES) }) + }); + table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { + panic!("handler ran after a terminal action") + }); + let action = table + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await; + assert!(action.pending().is_some()); + let action = action.wait().await.advance().await.into_action(); + assert!(matches!(action, NotifAction::Errno(libc::EACCES))); + } + /// A handler returning `Defer` is non-`Continue`, so it must short-circuit /// the chain exactly like `Errno`/`ReturnValue`: later handlers on the same /// syscall do not run. Deferral is therefore a terminal decision. @@ -1498,8 +1740,14 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1, None) - .await; + .dispatch( + fake_notif(libc::SYS_openat as i32), + fake_listener(), + None, + Arc::new(AtomicUsize::new(0)), + ) + .await + .into_action(); assert!( matches!(action, NotifAction::Defer(_)), @@ -1562,7 +1810,7 @@ mod handler_tests { let _sup = fake_supervisor_ctx(); let notif = fake_notif(libc::SYS_openat as i32); - let cx = HandlerCtx::new(notif, -1); + let cx = HandlerCtx::new(notif, std::fs::File::open("/dev/null").unwrap().into()); let action = h.handle(&cx).await; assert!(matches!(action, NotifAction::Continue)); @@ -1614,7 +1862,10 @@ mod handler_tests { // Walker MUST hit the struct's handle() each time, accumulating // state on &self.calls. for _ in 0..3 { - let action = table.dispatch(notif, -1, None).await; + let action = table + .dispatch(notif, fake_listener(), None, Arc::new(AtomicUsize::new(0))) + .await + .into_action(); assert!(matches!(action, NotifAction::Continue)); } diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index a0c604ee4..f2fefc88f 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -8,6 +8,7 @@ use std::io; use std::net::IpAddr; use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, IntoRawFd, OwnedFd, RawFd}; use std::pin::Pin; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use crate::arch; @@ -51,9 +52,8 @@ impl OnInjectSuccess { /// single supervisor task that gates every other trapped syscall. The /// supervisor moves the future onto a worker, lets the loop proceed, and /// sends the response (via the still-valid `notif.id`) when the future -/// resolves. The future is `'static` because it outlives the borrowed -/// `HandlerCtx` — capture what you need (`notif` is `Copy`, `notif_fd` is a -/// `RawFd`) by value rather than borrowing `&self`. +/// resolves. The future outlives the borrowed `HandlerCtx`; capture owned +/// state and `HandlerCtx::listener()` when using the notification listener. /// /// The deferred future need only be `Send` (not `Sync`): the supervisor /// moves it onto a worker task and never shares it by reference. Requiring @@ -62,6 +62,7 @@ impl OnInjectSuccess { pub struct Deferred { future: Pin + Send + 'static>>, patient: bool, + pub(crate) filesystem: bool, } // Safety: `NotifAction` must stay `Sync` so it can live in `Sync` contexts @@ -86,6 +87,7 @@ impl Deferred { Self { future: Box::pin(f), patient: false, + filesystem: false, } } @@ -95,6 +97,7 @@ impl Deferred { Self { future: Box::pin(f), patient: true, + filesystem: false, } } @@ -103,6 +106,22 @@ impl Deferred { pub async fn run(self) -> NotifAction { self.future.await } + + pub(crate) fn retain_listener(self, listener: Arc) -> Self { + let Self { + future, + patient, + filesystem, + } = self; + Self { + future: Box::pin(async move { + let _listener = listener; + future.await + }), + patient, + filesystem, + } + } } /// An owned operation whose side effects require callback approval. @@ -170,11 +189,21 @@ pub enum NotifAction { /// so it short-circuits the handler chain — a deferring handler makes a /// terminal decision. See [`Deferred`]. Defer(Deferred), + /// Suspend dispatch, then resume the handler chain and policy processing. + Suspend(Deferred), /// Execute only after the policy callback permits the operation. Prepared(PreparedOperation), } impl NotifAction { + pub(crate) fn prepare_blocking( + addr: &[u8], + socket: RawFd, + execute: impl FnOnce() -> NotifAction + Send + Sync + 'static, + ) -> Self { + Self::prepare(addr, socket, move || Self::blocking(execute)) + } + pub(crate) fn prepare( addr: &[u8], socket: RawFd, @@ -212,6 +241,29 @@ impl NotifAction { NotifAction::Defer(Deferred::new(fut)) } + pub fn suspend + Send + 'static>(fut: F) -> Self { + NotifAction::Suspend(Deferred::new(fut)) + } + + pub(crate) fn suspend_filesystem + Send + 'static>( + fut: F, + ) -> Self { + let mut work = Deferred::new(fut); + work.filesystem = true; + Self::Suspend(work) + } + + /// Defer owned synchronous work with admission held until the work returns. + /// Work accessing child memory must also retain `HandlerCtx::blocking_guard()`. + pub fn blocking(work: impl FnOnce() -> NotifAction + Send + 'static) -> Self { + Self::defer(async move { + match crate::blocking::run(work).await { + Ok(action) => action, + Err(e) => Self::Errno(e.raw_os_error().unwrap_or(libc::EIO)), + } + }) + } + /// Inject `content` into the child as the syscall's returned fd, backed by /// a sealed (read-only, fixed-size), `O_CLOEXEC` in-memory file. /// @@ -298,7 +350,9 @@ pub(crate) fn named_content_memfd(content: &[u8], seal: bool, name: &str) -> io: /// definite response instead of wedging forever waiting for one. fn finalize_deferred(action: NotifAction) -> NotifAction { match action { - NotifAction::Defer(_) | NotifAction::Prepared(_) => NotifAction::Errno(libc::EIO), + NotifAction::Defer(_) | NotifAction::Suspend(_) | NotifAction::Prepared(_) => { + NotifAction::Errno(libc::EIO) + } other => other, } } @@ -1366,15 +1420,17 @@ pub struct NotifPolicy { impl NotifPolicy { /// Whether a named unix `connect()` is decided by the supervisor. It then /// connects on-behalf, so the peer's `SO_PEERCRED` names the supervisor. + /// Under chroot the kernel would resolve the address against the host's + /// root and the child's frozen cwd, so Landlock cannot take it over. pub(crate) fn supervises_unix_connect(&self) -> bool { - self.has_unix_fs_gate && !self.landlock_unix_gate + self.has_unix_fs_gate && (!self.landlock_unix_gate || self.chroot_root.is_some()) } /// Whether a named unix send is decided by the supervisor. A destination /// policy keeps sends on-behalf even under Landlock: a `Continue` would let /// a racing `dup2` move the send onto an IP socket. pub(crate) fn supervises_unix_send(&self) -> bool { - self.has_unix_fs_gate && (!self.landlock_unix_gate || self.has_net_destination_policy) + self.supervises_unix_connect() || (self.has_unix_fs_gate && self.has_net_destination_policy) } /// Whether an open may land on something the supervisor answers for @@ -2110,7 +2166,7 @@ fn send_response(fd: RawFd, id: u64, action: NotifAction) -> io::Result<()> { }, NotifAction::ReturnValue(val) => respond_value(fd, id, val), NotifAction::Hold => Ok(()), // Don't send a response. - NotifAction::Defer(_) | NotifAction::Prepared(_) => { + NotifAction::Defer(_) | NotifAction::Suspend(_) | NotifAction::Prepared(_) => { // Unexecuted work must never become a successful syscall response. debug_assert!(false, "unexecuted operation reached send_response"); respond_errno(fd, id, libc::EIO) @@ -2854,16 +2910,15 @@ async fn emit_policy_event( /// Process a single seccomp notification: vDSO re-patch, path denial check, /// dispatch, policy event emission, and response. -/// Maximum number of deferred handler futures running concurrently. Caps -/// the worker fan-out (and any resources those workers hold, e.g. memfds or -/// sockets) so a burst of deferrals cannot exhaust the supervisor process. -const DEFER_MAX_INFLIGHT: usize = 64; +/// Maximum parked futures per admission class. Native work has separate +/// process-wide bounds that remain occupied after a waiter times out. +pub(crate) const DEFER_MAX_INFLIGHT: usize = 64; /// Maximum time a deferred handler future may run before the supervisor gives /// up and fails the trapped syscall closed. Bounds the worst case so a hung /// future (e.g. a stalled network fetch in a token-injection handler) cannot /// park the child forever or permanently leak its `DEFER_MAX_INFLIGHT` slot. -const DEFER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +pub(crate) const DEFER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); /// Drive a deferred future to its terminal action, bounded by `limit`. /// @@ -2889,6 +2944,12 @@ async fn run_deferred_within(deferred: Deferred, limit: std::time::Duration) -> enum Parked { Approval(Box, crate::policy_fn::Verdict), Deferred(u64, NotifAction), + Dispatch { + notif: SeccompNotif, + state: super::dispatch::DispatchState, + open: Option>, + has_denied: bool, + }, } /// Parked notifications and the tids waiting on them, which the exec freeze @@ -2896,6 +2957,8 @@ enum Parked { struct ParkedSet { tasks: tokio::task::JoinSet, callers: HashMap, + active_work: Arc, + filesystem_sem: Arc, } impl ParkedSet { @@ -2903,9 +2966,24 @@ impl ParkedSet { Self { tasks: tokio::task::JoinSet::new(), callers: HashMap::new(), + active_work: Arc::new(AtomicUsize::new(0)), + filesystem_sem: Arc::new(tokio::sync::Semaphore::new(DEFER_MAX_INFLIGHT)), } } + fn admit( + &self, + filesystem: bool, + general: &Arc, + ) -> Result { + let admission = if filesystem { + &self.filesystem_sem + } else { + general + }; + Arc::clone(admission).try_acquire_owned() + } + fn spawn(&mut self, notif: &SeccompNotif, task: impl Future + Send + 'static) { let handle = self.tasks.spawn(task); self.callers @@ -2920,7 +2998,25 @@ impl ParkedSet { return Some(parked); } Err(e) => { - self.callers.remove(&e.id()); + if let Some((_, id)) = self.callers.remove(&e.id()) { + return Some(Parked::Deferred(id, NotifAction::Errno(libc::EIO))); + } + } + } + } + } + + fn try_join_next(&mut self) -> Option { + loop { + match self.tasks.try_join_next_with_id()? { + Ok((id, parked)) => { + self.callers.remove(&id); + return Some(parked); + } + Err(e) => { + if let Some((_, id)) = self.callers.remove(&e.id()) { + return Some(Parked::Deferred(id, NotifAction::Errno(libc::EIO))); + } } } } @@ -2941,7 +3037,6 @@ impl ParkedSet { struct Prepared { notif: SeccompNotif, action: NotifAction, - fork_counted: bool, exec_image: Option, } @@ -2966,10 +3061,11 @@ async fn handle_notification( notif: SeccompNotif, ctx: &Arc, dispatch_table: &super::dispatch::DispatchTable, - fd: RawFd, + listener: &Arc, parked: &mut ParkedSet, defer_sem: &Arc, ) { + let fd = listener.as_raw_fd(); let policy = &ctx.policy; // Ensure every pid that produces a notification has per-process @@ -2997,39 +3093,100 @@ async fn handle_notification( .then(|| OpenRequest::decode(¬if, fd, policy, &ctx.processes)) .flatten() .map(Arc::new); - let mut action = { - let denied = has_denied - && crate::seccomp_plan::fs_denied_path_syscalls().contains(&nr) - && is_path_denied_for_notif(&*ctx.policy_fn.lock().await, ¬if, fd); - if denied { - NotifAction::Errno(libc::EACCES) - } else { - // Dispatch runs first so /proc virtualization and the other - // handlers still win for their paths. - let action = dispatch_table.dispatch(notif, fd, open.clone()).await; - // A bare Continue for an open is the racy window: the kernel - // re-resolves the path after it, and a racing thread can swap a - // link to reach a denied carve-out inside a granted tree (issue - // #111) or a procfs entry the handlers hide by name (issue - // #236). The open is done against the pinned inode instead. - let resolves = policy.chroot_root.is_none() && (has_denied || policy.resolves_opens()); - match (&action, &open) { - (NotifAction::Continue, Some(open)) if resolves => { - let pfs = ctx.policy_fn.lock().await; - on_behalf_open(¬if, open, ctx, &pfs, fd, has_denied).await + let denied = has_denied + && crate::seccomp_plan::fs_denied_path_syscalls().contains(&nr) + && is_path_denied_for_notif(&*ctx.policy_fn.lock().await, ¬if, fd); + if denied { + finish_dispatch( + notif, + NotifAction::Errno(libc::EACCES), + open, + has_denied, + ctx, + fd, + parked, + defer_sem, + ) + .await; + return; + } + let state = dispatch_table + .dispatch( + notif, + Arc::clone(listener), + open.clone(), + Arc::clone(&parked.active_work), + ) + .await; + finish_dispatch_state(notif, state, open, has_denied, ctx, fd, parked, defer_sem).await; +} + +async fn finish_dispatch_state( + notif: SeccompNotif, + state: super::dispatch::DispatchState, + open: Option>, + has_denied: bool, + ctx: &Arc, + fd: RawFd, + parked: &mut ParkedSet, + defer_sem: &Arc, +) { + if let Some(filesystem) = state.pending() { + match parked.admit(filesystem, defer_sem) { + Ok(permit) => parked.spawn(¬if, async move { + let _permit = permit; + let state = state.wait().await; + Parked::Dispatch { + notif, + state, + open, + has_denied, } - _ => action, + }), + Err(_) => { + let _ = send_response(fd, notif.id, NotifAction::Errno(libc::EAGAIN)); } } - }; + return; + } + finish_dispatch( + notif, + state.into_action(), + open, + has_denied, + ctx, + fd, + parked, + defer_sem, + ) + .await; +} + +async fn finish_dispatch( + notif: SeccompNotif, + mut action: NotifAction, + open: Option>, + has_denied: bool, + ctx: &Arc, + fd: RawFd, + parked: &mut ParkedSet, + defer_sem: &Arc, +) { + let policy = &ctx.policy; + let nr = notif.data.nr as i64; + // Resuming a handler must still pass through pinned-open enforcement. + let resolves = policy.chroot_root.is_none() && (has_denied || policy.resolves_opens()); + if let (NotifAction::Continue, Some(open)) = (&action, &open) { + if resolves { + let pfs = ctx.policy_fn.lock().await; + action = on_behalf_open(¬if, open, ctx, &pfs, fd, has_denied).await; + } + } if let Some(open) = &open { action = crate::procfs::net_dispatch::guard_injection(action, ¬if, open, ctx).await; } - let fork_counted = matches!(action, NotifAction::Continue) - && crate::resource::fork_counted_on_continue(¬if); - // Read without freezing so the sandbox keeps running while policy_fn // decides; `freeze_exec` re-reads it under the freeze before Continue. let exec_image = (matches!(action, NotifAction::Continue) @@ -3041,7 +3198,6 @@ async fn handle_notification( let prepared = Prepared { notif, action, - fork_counted, exec_image, }; match wait { @@ -3106,11 +3262,13 @@ async fn finish_notification( parked: &mut ParkedSet, defer_sem: &Arc, ) { + if id_valid(fd, prepared.notif.id).is_err() { + return; + } let policy = &ctx.policy; let Prepared { notif, mut action, - fork_counted, exec_image, } = prepared; let nr = notif.data.nr as i64; @@ -3131,8 +3289,11 @@ async fn finish_notification( action = operation.execute(); } - if fork_counted && !matches!(action, NotifAction::Continue) { - crate::resource::rollback_fork_count(&ctx.resource).await; + let mut fork_counted = false; + if matches!(action, NotifAction::Continue) && arch::fork_like_syscalls().contains(&nr) { + action = crate::resource::handle_fork(¬if, ctx, policy).await; + fork_counted = matches!(action, NotifAction::Continue) + && crate::resource::fork_counted_on_continue(¬if); } // With policy_fn active, fork-like syscalls are traced for exactly @@ -3156,6 +3317,7 @@ async fn finish_notification( ); if fork_counted { crate::resource::rollback_fork_count(&ctx.resource).await; + fork_counted = false; } action = NotifAction::Errno(libc::EPERM); } @@ -3167,11 +3329,11 @@ async fn finish_notification( // trapped child stays parked; the loop sends the result when it lands. // // Deferral is refused on syscalls whose Continue path requires the - // execve argv-safety freeze or fork creation-tracking: answering later + // execve argv-safety freeze or fork accounting: answering later // would skip that TOCTOU-closing work. if let NotifAction::Defer(deferred) = action { if crate::freeze::requires_freeze_on_continue(nr) - || crate::resource::requires_process_creation_tracking(¬if, policy) + || arch::fork_like_syscalls().contains(&nr) { let _ = send_response(fd, notif.id, NotifAction::Errno(libc::EPERM)); return; @@ -3191,9 +3353,15 @@ async fn finish_notification( // thread, and the loop must not release any parked task meanwhile. let mut exec_freeze = None; if let (NotifAction::Continue, Some(judged)) = (&action, &exec_image) { - let (frozen_action, freeze) = freeze_exec(¬if, judged, ctx, fd, parked); - action = frozen_action; - exec_freeze = Some(freeze); + // Guards are acquired only while advancing handlers on this loop. + // Native workers can release them, but cannot start new guarded work here. + if parked.active_work.load(Ordering::Acquire) == 0 { + let (frozen_action, freeze) = freeze_exec(¬if, judged, ctx, fd, parked); + action = frozen_action; + exec_freeze = Some(freeze); + } else { + action = NotifAction::Errno(libc::EAGAIN); + } } // Ignore error — child may have exited between recv and response. @@ -3212,6 +3380,9 @@ async fn finish_notification( crate::freeze::reap_pending(&freeze.pending_tids); } + if fork_counted && send_result.is_err() && creation_trace.is_none() { + crate::resource::rollback_fork_count(&ctx.resource).await; + } if let Some(trace) = creation_trace { if send_result.is_ok() { match crate::resource::finish_process_creation_tracking(trace).await { @@ -3243,6 +3414,18 @@ async fn settle_parked( defer_sem: &Arc, ) { match parked { + Parked::Dispatch { + notif, + state, + open, + has_denied, + } => { + if id_valid(fd, notif.id).is_ok() { + let state = state.advance().await; + finish_dispatch_state(notif, state, open, has_denied, ctx, fd, set, defer_sem) + .await; + } + } Parked::Approval(prepared, verdict) => { finish_notification(*prepared, Some(verdict), ctx, fd, set, defer_sem).await } @@ -3271,14 +3454,16 @@ pub async fn supervisor( ) { // Register the notif fd with the Tokio IO driver so we can wait for // readiness via epoll instead of a dedicated blocking thread. - let async_fd = - match tokio::io::unix::AsyncFd::with_interest(notif_fd, tokio::io::Interest::READABLE) { - Ok(fd) => fd, - Err(err) => { - let _ = startup.send(Err(err)); - return; - } - }; + let async_fd = match tokio::io::unix::AsyncFd::with_interest( + Arc::new(notif_fd), + tokio::io::Interest::READABLE, + ) { + Ok(fd) => fd, + Err(err) => { + let _ = startup.send(Err(err)); + return; + } + }; let fd = async_fd.get_ref().as_raw_fd(); // Build the dispatch table once at startup. @@ -3335,6 +3520,9 @@ pub async fn supervisor( drop(ready); loop { + if let Some(done) = parked.try_join_next() { + settle_parked(done, &ctx, fd, &mut parked, &defer_sem).await; + } match probe_notif_fd(fd) { NotifFdState::Pending => { let notif = match recv_notif(fd) { @@ -3351,8 +3539,15 @@ pub async fn supervisor( Err(e) if e.raw_os_error() == Some(libc::ENOENT) => continue, Err(_) => break 'outer, }; - handle_notification(notif, &ctx, &dispatch_table, fd, &mut parked, &defer_sem) - .await; + handle_notification( + notif, + &ctx, + &dispatch_table, + async_fd.get_ref(), + &mut parked, + &defer_sem, + ) + .await; } NotifFdState::Empty => break, NotifFdState::Terminal => break 'outer, @@ -3812,6 +4007,48 @@ mod tests { assert!(matches!(deferred.run().await, NotifAction::ReturnValue(7))); } + #[tokio::test] + async fn prepared_blocking_work_starts_only_when_admitted() { + use std::sync::atomic::{AtomicBool, Ordering}; + let ran = Arc::new(AtomicBool::new(false)); + let work_ran = Arc::clone(&ran); + let action = NotifAction::prepare_blocking(&[], -1, move || { + work_ran.store(true, Ordering::SeqCst); + NotifAction::ReturnValue(7) + }); + let NotifAction::Prepared(operation) = action else { + panic!("expected prepared operation"); + }; + let action = operation.execute(); + assert!(!ran.load(Ordering::SeqCst)); + let NotifAction::Defer(deferred) = action else { + panic!("expected deferred work"); + }; + assert!(matches!(deferred.run().await, NotifAction::ReturnValue(7))); + assert!(ran.load(Ordering::SeqCst)); + } + + #[test] + fn supervised_saturation_preserves_filesystem_admission() { + let parked = ParkedSet::new(); + let general = Arc::new(tokio::sync::Semaphore::new(DEFER_MAX_INFLIGHT)); + let _occupied = Arc::clone(&general) + .try_acquire_many_owned(DEFER_MAX_INFLIGHT as u32) + .unwrap(); + assert!(parked.admit(false, &general).is_err()); + assert!(parked.admit(true, &general).is_ok()); + } + + #[tokio::test] + async fn rejected_blocking_work_never_starts() { + let action = NotifAction::prepare_blocking(&[], -1, || { + panic!("denied operation executed"); + }); + drop(action); + let action = NotifAction::blocking(|| panic!("unadmitted operation executed")); + drop(action); + } + #[tokio::test(start_paused = true)] async fn deferred_times_out_to_eio() { // A deferred future that exceeds its limit must fail closed (EIO) so diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index 8a855b53b..d942c652c 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -393,11 +393,12 @@ pub(crate) fn notif_syscalls_resolved(resolved: &ResolvedSandbox) -> Vec { if features.network_supervision { nrs.extend(NETWORK_POLICY_SYSCALLS); - } else if features.unix_fs_gate && !features.landlock_unix_gate { + } else if features.unix_fs_gate && (!features.landlock_unix_gate || features.chroot) { // Named-unix gate: trap connect() (stream) and sendto()/sendmsg()/ // sendmmsg() (datagram) so reaching a unix socket outside the fs-write // grants is denied, even when no IP network rules are present. Landlock - // before ABI v9 cannot gate this. Handlers bail cheaply on addr-less + // before ABI v9 cannot gate this, and under chroot no Landlock can, + // since the kernel would resolve the address on the host. Handlers bail cheaply on addr-less // (connected) sends. nrs.push(libc::SYS_connect); nrs.push(libc::SYS_sendto); diff --git a/crates/sandlock-core/tests/integration/test_chroot.rs b/crates/sandlock-core/tests/integration/test_chroot.rs index 2486510eb..3d7d08d8c 100644 --- a/crates/sandlock-core/tests/integration/test_chroot.rs +++ b/crates/sandlock-core/tests/integration/test_chroot.rs @@ -2646,3 +2646,110 @@ async fn test_chroot_shebang_loop_is_eloop() { ); cleanup_rootfs(&rootfs); } + +/// Run `rootfs-helper unix-send` inside a chroot granting write on virtual +/// `/tmp`, returning the child's result. +async fn chroot_unix_send( + rootfs: &PathBuf, + kind: &str, + dir: &str, + path: &str, + payload: &str, +) -> sandlock_core::RunResult { + minimal_exec_policy(rootfs) + .fs_write("/tmp") + .build() + .unwrap() + .run(&["rootfs-helper", "unix-send", kind, dir, path, payload]) + .await + .unwrap() +} + +/// Named unix sockets are virtual paths in chroot mode: an absolute target +/// reaches the socket inside the rootfs, and a relative one resolves against +/// the child's virtual cwd. +#[tokio::test] +async fn test_chroot_unix_connect_resolves_in_root_and_cwd() { + use std::io::Read; + use std::os::unix::net::UnixListener; + + let rootfs = build_test_rootfs("unix-connect"); + let listener = UnixListener::bind(rootfs.join("tmp/svc.sock")).unwrap(); + listener.set_nonblocking(true).unwrap(); + + for (dir, path) in [ + ("/", "/tmp/svc.sock"), + ("/tmp", "svc.sock"), + ("/tmp", "./svc.sock"), + ] { + let r = chroot_unix_send(&rootfs, "stream", dir, path, "hello").await; + assert!( + r.success(), + "connect {path} from {dir}: {}", + r.stderr_str().unwrap_or("") + ); + let (mut conn, _) = listener + .accept() + .expect("connect must reach the rootfs socket"); + let mut buf = [0u8; 5]; + conn.read_exact(&mut buf).unwrap(); + assert_eq!(&buf, b"hello"); + } + + cleanup_rootfs(&rootfs); +} + +/// A relative datagram target resolves against the virtual cwd too. +#[tokio::test] +async fn test_chroot_unix_sendto_relative_after_chdir() { + use std::os::unix::net::UnixDatagram; + + let rootfs = build_test_rootfs("unix-sendto"); + let listener = UnixDatagram::bind(rootfs.join("tmp/svc.dgram")).unwrap(); + listener.set_nonblocking(true).unwrap(); + + let r = chroot_unix_send(&rootfs, "dgram", "/tmp", "svc.dgram", "datagram").await; + assert!(r.success(), "sendto: {}", r.stderr_str().unwrap_or("")); + let mut buf = [0u8; 16]; + let n = listener + .recv(&mut buf) + .expect("datagram must reach the rootfs socket"); + assert_eq!(&buf[..n], b"datagram"); + + cleanup_rootfs(&rootfs); +} + +/// The grant is checked on the virtual path the relative target resolves to: +/// `tmp/svc.sock` from cwd `/etc` is `/etc/tmp/svc.sock`, outside the `/tmp` +/// write grant, and must not be mistaken for `/tmp/svc.sock`. +#[tokio::test] +async fn test_chroot_unix_connect_relative_outside_grant_denied() { + use std::os::unix::net::UnixListener; + + let rootfs = build_test_rootfs("unix-deny"); + fs::create_dir_all(rootfs.join("etc/tmp")).unwrap(); + let outside = UnixListener::bind(rootfs.join("etc/tmp/svc.sock")).unwrap(); + outside.set_nonblocking(true).unwrap(); + let _granted = UnixListener::bind(rootfs.join("tmp/svc.sock")).unwrap(); + + let r = chroot_unix_send(&rootfs, "stream", "/etc", "tmp/svc.sock", "hello").await; + assert!(!r.success(), "connect outside the write grant must fail"); + assert!( + outside.accept().is_err(), + "the ungranted socket must not be reached" + ); + + // A symlink inside the grant is judged by the socket it reaches. + std::os::unix::fs::symlink("/etc/tmp/svc.sock", rootfs.join("tmp/link.sock")).unwrap(); + let r = chroot_unix_send(&rootfs, "stream", "/tmp", "link.sock", "hello").await; + assert!( + !r.success(), + "a symlink out of the write grant must be denied" + ); + assert!( + outside.accept().is_err(), + "the ungranted socket must not be reached" + ); + + cleanup_rootfs(&rootfs); +} diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index a172cadba..258c8aa7f 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -1841,6 +1841,8 @@ void sandlock_action_set_kill(sandlock_action_out_t *out, int32_t sig, int32_t p * remains valid until either (a) `sandlock_handler_free` is called or * (b) the supervisor takes ownership via `sandlock_run_with_handlers` * and the run completes. + * The run waits for active callbacks before returning, even after a syscall + * timeout or child exit. A callback that never returns prevents run completion. * If `on_exception` does not match a defined `sandlock_exception_policy_t` * discriminant (0, 1, 2, or 3), the call returns null and no allocation occurs. */ @@ -1853,16 +1855,16 @@ sandlock_handler_t *sandlock_handler_new(int32_t (*handler_fn)(void *ud, uint32_t on_exception); /** - * Mark a handler as deferred: the supervisor will run its callback off the - * notification loop so a slow callback does not block other trapped - * syscalls. Must be called before the handler is registered with a run. + * Select terminal deferral. All callbacks run off the notification loop; + * the default resumes the handler chain after the callback returns Continue. + * Must be called before the handler is registered with a run. * * A deferred handler makes a *terminal* decision: deferral short-circuits * the handler chain, so if a deferred handler would `Continue` and other * user handlers are registered on the same syscall, those later handlers do * not run. Builtins always run first regardless. Deferral is refused at * dispatch time for syscalls whose Continue path needs the execve - * argv-safety freeze (`execve`/`execveat`) or fork creation-tracking. + * argv-safety freeze (`execve`/`execveat`) or fork accounting. * * # Safety * `h` must be a non-null pointer returned by `sandlock_handler_new` that has diff --git a/crates/sandlock-ffi/src/handler/abi.rs b/crates/sandlock-ffi/src/handler/abi.rs index 53016875e..c1183b355 100644 --- a/crates/sandlock-ffi/src/handler/abi.rs +++ b/crates/sandlock-ffi/src/handler/abi.rs @@ -458,11 +458,8 @@ pub struct sandlock_handler_t { pub(super) ud: *mut std::ffi::c_void, pub(super) ud_drop: Option, pub(super) on_exception: sandlock_exception_policy_t, - /// When true, the supervisor runs this handler's callback off the - /// notification loop (as a deferred future) instead of inline, so a - /// slow callback (network round-trip, blocking syscall) does not stall - /// every other trapped syscall. The container is an opaque box — C - /// never sees this field's layout — so adding it is not a C ABI break. + /// Terminal deferral skips later handlers; ordinary callbacks suspend + /// dispatch and resume the chain on Continue. Both run off the loop. pub(super) deferred: bool, } @@ -509,6 +506,8 @@ impl Drop for sandlock_handler_t { /// remains valid until either (a) `sandlock_handler_free` is called or /// (b) the supervisor takes ownership via `sandlock_run_with_handlers` /// and the run completes. +/// The run waits for active callbacks before returning, even after a syscall +/// timeout or child exit. A callback that never returns prevents run completion. /// If `on_exception` does not match a defined `sandlock_exception_policy_t` /// discriminant (0, 1, 2, or 3), the call returns null and no allocation occurs. // The two callback parameters are spelled inline rather than via the @@ -555,16 +554,16 @@ pub unsafe extern "C" fn sandlock_handler_new( Box::into_raw(h) } -/// Mark a handler as deferred: the supervisor will run its callback off the -/// notification loop so a slow callback does not block other trapped -/// syscalls. Must be called before the handler is registered with a run. +/// Select terminal deferral. All callbacks run off the notification loop; +/// the default resumes the handler chain after the callback returns Continue. +/// Must be called before the handler is registered with a run. /// /// A deferred handler makes a *terminal* decision: deferral short-circuits /// the handler chain, so if a deferred handler would `Continue` and other /// user handlers are registered on the same syscall, those later handlers do /// not run. Builtins always run first regardless. Deferral is refused at /// dispatch time for syscalls whose Continue path needs the execve -/// argv-safety freeze (`execve`/`execveat`) or fork creation-tracking. +/// argv-safety freeze (`execve`/`execveat`) or fork accounting. /// /// # Safety /// `h` must be a non-null pointer returned by `sandlock_handler_new` that has diff --git a/crates/sandlock-ffi/src/handler/adapter.rs b/crates/sandlock-ffi/src/handler/adapter.rs index 5131e5cd7..fbbb80fc9 100644 --- a/crates/sandlock-ffi/src/handler/adapter.rs +++ b/crates/sandlock-ffi/src/handler/adapter.rs @@ -5,8 +5,9 @@ //! dispatch loop can invoke C callbacks transparently. use std::future::Future; -use std::os::unix::io::FromRawFd; +use std::os::unix::io::{AsRawFd, FromRawFd}; use std::pin::Pin; +use std::sync::{Arc, RwLock}; use sandlock_core::seccomp::dispatch::{Handler, HandlerCtx}; use sandlock_core::seccomp::notif::NotifAction; @@ -44,7 +45,29 @@ pub(crate) const UNSAFE_PGID: i32 = i32::MIN; /// `Handler`. Constructed when the supervisor accepts handlers passed /// through `sandlock_run_with_handlers`. pub struct FfiHandler { - inner: Box, + inner: Arc, + on_exception: sandlock_exception_policy_t, + deferred: bool, +} + +struct CallbackState { + handler: RwLock>>, +} + +pub(super) struct CallbackRunGuard(Arc); + +impl Drop for CallbackRunGuard { + fn drop(&mut self) { + // Userdata may be borrowed until the FFI run returns. Drain readers + // and remove the callback so queued work cannot borrow it afterward. + let handler = self + .0 + .handler + .write() + .unwrap_or_else(|e| e.into_inner()) + .take(); + drop(handler); + } } impl FfiHandler { @@ -57,13 +80,22 @@ impl FfiHandler { /// supervisor owns the container. pub unsafe fn from_raw(raw: *mut sandlock_handler_t) -> Self { assert!(!raw.is_null(), "FfiHandler::from_raw on null pointer"); + let handler = Box::from_raw(raw); Self { - inner: Box::from_raw(raw), + on_exception: handler.on_exception, + deferred: handler.deferred, + inner: Arc::new(CallbackState { + handler: RwLock::new(Some(handler)), + }), } } + pub(super) fn run_guard(&self) -> CallbackRunGuard { + CallbackRunGuard(Arc::clone(&self.inner)) + } + fn exception_action(&self, child_pgid: i32) -> NotifAction { - match self.inner.on_exception { + match self.on_exception { sandlock_exception_policy_t::Kill => { if child_pgid == UNSAFE_PGID { // No safe pgid is available (nested PID namespace, @@ -88,25 +120,14 @@ impl FfiHandler { } } -/// `Send`-only wrapper around the C user-data pointer so it can travel -/// into `spawn_blocking`. Only the move (not sharing across threads) is -/// required; the deeper Send/Sync rationale for the underlying handler -/// container lives on `sandlock_handler_t`. -struct UdPtr(*mut std::ffi::c_void); -// Safety: ud is opaque to Rust; the spawn_blocking pipeline only moves -// (not shares) the wrapper. See `sandlock_handler_t` for the deeper -// Send/Sync rationale that justifies the underlying handler container. -unsafe impl Send for UdPtr {} - impl Handler for FfiHandler { fn handle<'a>( &'a self, cx: &'a HandlerCtx, ) -> Pin + Send + 'a>> { - // Capture the pieces we need by value so spawn_blocking can run - // the C callback on a worker thread without &self lifetime games. + // Native work retains both the callback state and the listener after timeout. let notif_snap = crate::notif_repr::sandlock_notif_data_t::from(&cx.notif); - let notif_fd = cx.notif_fd; + let listener = cx.listener(); let notif_id = cx.notif.id; let pid = cx.notif.pid; // Resolve the trapped child's process group id for use as a fallback @@ -156,73 +177,58 @@ impl Handler for FfiHandler { } } }; - let handler_fn = self.inner.handler_fn; - let ud = UdPtr(self.inner.ud); + let handler = Arc::clone(&self.inner); let on_exception_fallback = self.exception_action(child_pgid); - let deferred = self.inner.deferred; + let deferred = self.deferred; + let work_guard = cx.blocking_guard(); - // The whole callback (including its slow work) runs on a blocking - // worker. When `deferred` is set we hand this future to the - // supervisor as `NotifAction::Defer` so it runs off the notification - // loop; otherwise the supervisor awaits it inline as before. The - // future is fully owned (`'static`) either way — nothing borrows - // `self`/`cx`. let run = async move { - let join = tokio::task::spawn_blocking(move || { - // Rust 2021 disjoint closure captures (RFC 2229) would - // otherwise capture `ud.0` (a bare `*mut c_void`, not - // `Send`) rather than the whole `UdPtr`. Binding `ud` to - // a fresh local at the top of the closure forces a - // whole-struct capture so the `Send` impl on `UdPtr` - // applies to the outer closure. - let ud = ud; - let UdPtr(ud_raw) = ud; - let mut mem = sandlock_mem_handle_t::new(notif_fd, notif_id, pid); + sandlock_core::blocking::run(move || { + let _work_guard = work_guard; + let callback = handler.handler.read().unwrap_or_else(|e| e.into_inner()); + let Some(handler) = callback.as_ref() else { + return NotifAction::Errno(libc::EIO); + }; + let mut mem = sandlock_mem_handle_t::new(listener.as_raw_fd(), notif_id, pid); let mut out = sandlock_action_out_t::zeroed(); - let rc = match handler_fn { + let rc_or_panic = match handler.handler_fn { Some(f) => std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - f(ud_raw, ¬if_snap, &mut mem, &mut out) + f(handler.ud, ¬if_snap, &mut mem, &mut out) })), None => Ok(-1), }; - (rc, out) - }) - .await; - - let (rc_or_panic, out) = match join { - Ok(pair) => pair, - Err(_join_err) => return on_exception_fallback, - }; - - match rc_or_panic { - Ok(0) => match translate_action(&out, child_pgid) { - Some(action) => action, - None => { - // Action kind ended up Unset, unknown, or the - // reserved InjectFdSendTracked discriminant. - // Drain any inject-fd payload before falling - // back to the exception policy — otherwise the - // supervisor leaks the srcfd that was armed by - // the (failed) callback. - // SAFETY: `drain_pending_inject_fd` inspects - // `out.kind` itself before touching the union, - // and `out.kind` matches the union variant per - // the action setters' contract. + match rc_or_panic { + Ok(0) => match translate_action(&out, child_pgid) { + Some(action) => action, + None => { + // Action kind ended up Unset, unknown, or the + // reserved InjectFdSendTracked discriminant. + // Drain any inject-fd payload before falling + // back to the exception policy — otherwise the + // supervisor leaks the srcfd that was armed by + // the (failed) callback. + // SAFETY: `drain_pending_inject_fd` inspects + // `out.kind` itself before touching the union, + // and `out.kind` matches the union variant per + // the action setters' contract. + unsafe { drain_pending_inject_fd(&out) }; + on_exception_fallback + } + }, + _ => { + // Either the callback returned a non-zero rc OR + // `catch_unwind` caught a panic. The callback may + // have armed an InjectFdSend{,Tracked} payload + // before failing; drain it so its srcfd doesn't + // leak in the supervisor. + // SAFETY: see the `Ok(0) -> None` branch above. unsafe { drain_pending_inject_fd(&out) }; on_exception_fallback } - }, - _ => { - // Either the callback returned a non-zero rc OR - // `catch_unwind` caught a panic. The callback may - // have armed an InjectFdSend{,Tracked} payload - // before failing; drain it so its srcfd doesn't - // leak in the supervisor. - // SAFETY: see the `Ok(0) -> None` branch above. - unsafe { drain_pending_inject_fd(&out) }; - on_exception_fallback } - } + }) + .await + .unwrap_or_else(|e| NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EIO))) }; if deferred { @@ -230,7 +236,7 @@ impl Handler for FfiHandler { // a worker task and sends the response when it completes. Box::pin(async move { NotifAction::defer(run) }) } else { - Box::pin(run) + Box::pin(async move { NotifAction::suspend(run) }) } } } @@ -358,3 +364,134 @@ fn translate_action(out: &sandlock_action_out_t, child_pgid: i32) -> Option HandlerCtx { + HandlerCtx::new( + SeccompNotif { + id: 1, + pid: 0, + flags: 0, + data: SeccompData { + nr: libc::SYS_getpid as i32, + arch: 0, + instruction_pointer: 0, + args: [0; 6], + }, + }, + std::fs::File::open("/dev/null").unwrap().into(), + ) + } + + #[tokio::test] + async fn run_guard_waits_for_abandoned_callback_using_borrowed_data() { + struct State { + started: Mutex>>, + release: Mutex>, + finished: Mutex>>, + } + extern "C-unwind" fn callback( + ud: *mut std::ffi::c_void, + _: *const crate::notif_repr::sandlock_notif_data_t, + _: *mut sandlock_mem_handle_t, + out: *mut sandlock_action_out_t, + ) -> i32 { + let state = unsafe { &*ud.cast::() }; + let _ = state.started.lock().unwrap().take().unwrap().send(()); + let _ = state.release.lock().unwrap().recv(); + let finished = state.finished.lock().unwrap().take().unwrap(); + unsafe { sandlock_action_set_return_value(out, 42) }; + let _ = finished.send(()); + 0 + } + + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let (finished_tx, finished_rx) = tokio::sync::oneshot::channel(); + let state = Box::new(State { + started: Mutex::new(Some(started_tx)), + release: Mutex::new(release_rx), + finished: Mutex::new(Some(finished_tx)), + }); + let raw = unsafe { + sandlock_handler_new( + Some(callback), + (&*state as *const State).cast_mut().cast(), + None, + sandlock_exception_policy_t::DenyEio as u32, + ) + }; + let handler = unsafe { FfiHandler::from_raw(raw) }; + let run_guard = handler.run_guard(); + let cx = context(); + let NotifAction::Suspend(work) = handler.handle(&cx).await else { + panic!("expected suspension"); + }; + let waiter = tokio::spawn(work.run()); + started_rx.await.unwrap(); + waiter.abort(); + let _ = waiter.await; + drop(handler); + + let (draining_tx, draining_rx) = tokio::sync::oneshot::channel(); + let (done_tx, mut done_rx) = tokio::sync::oneshot::channel(); + let drain = std::thread::spawn(move || { + let _ = draining_tx.send(()); + drop(run_guard); + let _ = done_tx.send(()); + }); + draining_rx.await.unwrap(); + let waited = tokio::time::timeout(std::time::Duration::from_millis(50), &mut done_rx) + .await + .is_err(); + release_tx.send(()).unwrap(); + finished_rx.await.unwrap(); + drain.join().unwrap(); + assert!( + waited, + "run returned while the callback could still access userdata" + ); + } + + #[tokio::test] + async fn run_guard_releases_userdata_and_rejects_unstarted_callbacks() { + extern "C-unwind" fn callback( + _: *mut std::ffi::c_void, + _: *const crate::notif_repr::sandlock_notif_data_t, + _: *mut sandlock_mem_handle_t, + _: *mut sandlock_action_out_t, + ) -> i32 { + panic!("callback started after its run ended"); + } + extern "C-unwind" fn destroy(ud: *mut std::ffi::c_void) { + unsafe { &*ud.cast::() }.store(true, Ordering::Release); + } + + let dropped = Box::new(AtomicBool::new(false)); + let raw = unsafe { + sandlock_handler_new( + Some(callback), + (&*dropped as *const AtomicBool).cast_mut().cast(), + Some(destroy), + sandlock_exception_policy_t::Continue as u32, + ) + }; + let handler = unsafe { FfiHandler::from_raw(raw) }; + let run_guard = handler.run_guard(); + let cx = context(); + let NotifAction::Suspend(work) = handler.handle(&cx).await else { + panic!("expected suspension"); + }; + drop(run_guard); + assert!(dropped.load(Ordering::Acquire)); + assert!(matches!(work.run().await, NotifAction::Errno(libc::EIO))); + } +} diff --git a/crates/sandlock-ffi/src/handler/run.rs b/crates/sandlock-ffi/src/handler/run.rs index 0ed5a4e23..4241ded0e 100644 --- a/crates/sandlock-ffi/src/handler/run.rs +++ b/crates/sandlock-ffi/src/handler/run.rs @@ -104,6 +104,7 @@ fn block_on_run( handlers: Vec<(i64, FfiHandler)>, interactive: bool, ) -> Option> { + let _callback_guards: Vec<_> = handlers.iter().map(|(_, h)| h.run_guard()).collect(); let cmd_refs: Vec<&str> = cmd.iter().map(String::as_str).collect(); // Apply `name` via the builder method on a clone — mirrors the // pattern used by `sandlock_run` in lib.rs. A `None` here means diff --git a/crates/sandlock-ffi/tests/handler_smoke.rs b/crates/sandlock-ffi/tests/handler_smoke.rs index ee6498c4e..4dcc5867c 100644 --- a/crates/sandlock-ffi/tests/handler_smoke.rs +++ b/crates/sandlock-ffi/tests/handler_smoke.rs @@ -280,6 +280,14 @@ use sandlock_core::seccomp::notif::NotifAction; use sandlock_core::{SeccompData, SeccompNotif}; use sandlock_ffi::handler::FfiHandler; +async fn resolve_handler(handler: &FfiHandler, cx: &HandlerCtx) -> NotifAction { + let mut action = handler.handle(cx).await; + while let NotifAction::Suspend(work) = action { + action = work.run().await; + } + action +} + fn fake_ctx() -> HandlerCtx { HandlerCtx::new( SeccompNotif { @@ -293,7 +301,7 @@ fn fake_ctx() -> HandlerCtx { args: [0; 6], }, }, - -1, + std::fs::File::open("/dev/null").unwrap().into(), ) } @@ -358,7 +366,7 @@ fn fake_ctx_with_isolated_child() -> (HandlerCtx, std::process::Child) { args: [0; 6], }, }, - -1, + std::fs::File::open("/dev/null").unwrap().into(), ); (ctx, child) } @@ -394,7 +402,7 @@ async fn ffi_handler_translates_return_value() { }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; assert!(matches!(action, NotifAction::ReturnValue(42))); } @@ -414,7 +422,7 @@ async fn deferred_ffi_handler_returns_defer_that_resolves_to_callback() { unsafe { sandlock_ffi::handler::sandlock_handler_set_deferred(raw, true) }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; let deferred = match action { NotifAction::Defer(d) => d, other => panic!("deferred handler must return Defer, got {other:?}"), @@ -423,8 +431,7 @@ async fn deferred_ffi_handler_returns_defer_that_resolves_to_callback() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn non_deferred_ffi_handler_stays_inline() { - // Without the flag, handle() resolves the action inline (no Defer). +async fn non_deferred_ffi_handler_suspends_dispatch() { let raw = unsafe { sandlock_ffi::handler::sandlock_handler_new( Some(return_value_42), @@ -435,7 +442,68 @@ async fn non_deferred_ffi_handler_stays_inline() { }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - assert!(matches!(h.handle(&cx).await, NotifAction::ReturnValue(42))); + let NotifAction::Suspend(work) = h.handle(&cx).await else { + panic!("callback must suspend dispatch"); + }; + assert!(matches!(work.run().await, NotifAction::ReturnValue(42))); +} + +#[tokio::test] +async fn abandoned_callback_keeps_user_data_until_worker_exits() { + struct State { + started: Option>, + release: std::sync::mpsc::Receiver<()>, + dropped: Option>, + } + extern "C-unwind" fn callback( + ud: *mut std::ffi::c_void, + _: *const sandlock_ffi::notif_repr::sandlock_notif_data_t, + _: *mut sandlock_ffi::handler::sandlock_mem_handle_t, + out: *mut sandlock_ffi::handler::sandlock_action_out_t, + ) -> i32 { + let state = unsafe { &mut *ud.cast::() }; + let _ = state.started.take().unwrap().send(()); + let _ = state.release.recv(); + unsafe { sandlock_ffi::handler::sandlock_action_set_return_value(out, 42) }; + 0 + } + extern "C-unwind" fn destroy(ud: *mut std::ffi::c_void) { + let mut state = unsafe { Box::from_raw(ud.cast::()) }; + let _ = state.dropped.take().unwrap().send(()); + } + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let (dropped_tx, mut dropped_rx) = tokio::sync::oneshot::channel(); + let state = Box::new(State { + started: Some(started_tx), + release: release_rx, + dropped: Some(dropped_tx), + }); + let raw = unsafe { + sandlock_ffi::handler::sandlock_handler_new( + Some(callback), + Box::into_raw(state).cast(), + Some(destroy), + sandlock_exception_policy_t::DenyEio as u32, + ) + }; + let handler = unsafe { FfiHandler::from_raw(raw) }; + let cx = fake_ctx(); + let NotifAction::Suspend(work) = handler.handle(&cx).await else { + panic!("expected suspension"); + }; + let waiter = tokio::spawn(work.run()); + started_rx.await.unwrap(); + drop(handler); + drop(cx); + waiter.abort(); + let _ = waiter.await; + assert!(matches!( + dropped_rx.try_recv(), + Err(tokio::sync::oneshot::error::TryRecvError::Empty) + )); + release_tx.send(()).unwrap(); + dropped_rx.await.unwrap(); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -450,7 +518,7 @@ async fn ffi_handler_applies_exception_policy_on_failure() { }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; assert!(matches!(action, NotifAction::Errno(e) if e == libc::EPERM)); } @@ -709,7 +777,7 @@ async fn ffi_handler_translates_continue() { // Safety: `raw` was just produced by `sandlock_handler_new` and is // non-null; ownership transfers into the adapter. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Continue)); } @@ -725,7 +793,7 @@ async fn ffi_handler_translates_errno() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Errno(e) if e == 13)); } @@ -741,7 +809,7 @@ async fn ffi_handler_translates_hold() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Hold)); } @@ -757,7 +825,7 @@ async fn ffi_handler_translates_kill_with_explicit_pgid() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!(matches!( action, NotifAction::KillGroup { sig, pgid } if sig == libc::SIGTERM && pgid == 1234 @@ -837,9 +905,9 @@ async fn ffi_handler_translates_kill_zero_pgid_substitutes_child_pgid() { args: [0; 6], }, }, - -1, + std::fs::File::open("/dev/null").unwrap().into(), ); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; // Reap the child regardless of assertion outcome. let _ = child.kill(); @@ -874,7 +942,7 @@ fn fake_ctx_with_pid(pid: u32) -> HandlerCtx { args: [0; 6], }, }, - -1, + std::fs::File::open("/dev/null").unwrap().into(), ) } @@ -911,7 +979,7 @@ async fn k1_pgid_resolution_rejects_pid_zero() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(0); - let action = handler.handle(&cx).await; + let action = resolve_handler(&handler, &cx).await; assert!( matches!(action, NotifAction::Continue), "expected exception-policy fallback (Continue) when no safe pgid available, got {action:?}", @@ -965,7 +1033,7 @@ async fn k1_no_supervisor_signal_on_pid_zero_kill() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(0); // pid=0 -> UNSAFE_PGID - let action = handler.handle(&cx).await; + let action = resolve_handler(&handler, &cx).await; // The action must be Continue (exception-policy fallback), NOT a // Kill that send_response would forward to killpg(0). @@ -1019,7 +1087,7 @@ async fn k2_pgid_resolution_rejects_supervisor_pgid_match() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(child_pid as u32); - let action = handler.handle(&cx).await; + let action = resolve_handler(&handler, &cx).await; // Reap the child regardless of assertion outcome. let _ = child.kill(); @@ -1054,7 +1122,7 @@ async fn k3_pgid_resolution_falls_back_on_esrch() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(dead_pid); - let action = handler.handle(&cx).await; + let action = resolve_handler(&handler, &cx).await; assert!( matches!(action, NotifAction::Continue), "expected exception-policy fallback (Continue) on ESRCH, got {action:?}", @@ -1081,7 +1149,7 @@ async fn ffi_handler_kill_policy_on_callback_rc_nonzero() { // suicide, and the assertion below would not exercise the // Kill-path the test exists to cover. let (cx, mut child) = fake_ctx_with_isolated_child(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; let _ = child.kill(); let _ = child.wait(); assert!(matches!(action, NotifAction::KillGroup { sig, .. } if sig == libc::SIGKILL)); @@ -1099,7 +1167,7 @@ async fn ffi_handler_continue_policy_on_callback_rc_nonzero() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Continue)); } @@ -1136,7 +1204,7 @@ async fn ffi_handler_recovers_from_callback_panic() { // Use an isolated child so the Kill exception policy is observable // (rationale identical to `ffi_handler_kill_policy_on_callback_rc_nonzero`). let (cx, mut child) = fake_ctx_with_isolated_child(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; let _ = child.kill(); let _ = child.wait(); // The `catch_unwind` inside `spawn_blocking` swallows the panic and @@ -1167,7 +1235,7 @@ async fn ffi_handler_callback_returns_zero_but_never_sets_action_triggers_fallba }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; // `translate_action` returns `None` for `Unset`, which routes the // dispatcher onto the exception policy fallback. assert!(matches!(action, NotifAction::Errno(e) if e == libc::EPERM)); @@ -2028,7 +2096,7 @@ async fn a1_ffi_handler_drains_inject_fd_on_panic() { }; // SAFETY: `raw` was just produced and is non-null. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EPERM), "panic must route to the exception-policy fallback (Kill degraded to EPERM under UNSAFE_PGID), got {action:?}", @@ -2103,7 +2171,7 @@ async fn a2_ffi_handler_drains_inject_fd_tracked_discriminant() { }; // SAFETY: `raw` was just produced and is non-null. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve_handler(&h, &fake_ctx()).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EPERM), "unsupported tracked discriminant must route to the exception-policy fallback (Kill degraded to EPERM under UNSAFE_PGID), got {action:?}", @@ -2318,7 +2386,7 @@ async fn ffi_handler_deny_eio_policy_on_callback_rc_nonzero() { }; let h = unsafe { sandlock_ffi::handler::FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve_handler(&h, &cx).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EIO), "expected Errno(EIO), got {:?}", diff --git a/crates/sandlock-fixtures/rootfs-helper.c b/crates/sandlock-fixtures/rootfs-helper.c index abe0659c8..ad46e6941 100644 --- a/crates/sandlock-fixtures/rootfs-helper.c +++ b/crates/sandlock-fixtures/rootfs-helper.c @@ -19,8 +19,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -877,10 +879,41 @@ static int cmd_fexecve(int argc, char **argv) { return 1; } +/* ── unix-send (deliver a payload to a named unix socket) ───── */ +/* + * unix-send stream|dgram : chdir() so a relative + * has a cwd to resolve against, then connect+send (stream) or sendto + * (dgram) . + */ +static int cmd_unix_send(int argc, char **argv) { + if (argc < 4) { fprintf(stderr, "unix-send: need stream|dgram \n"); return 1; } + int dgram = strcmp(argv[0], "dgram") == 0; + if (chdir(argv[1]) != 0) { fprintf(stderr, "unix-send: chdir %s: %s\n", argv[1], strerror(errno)); return 1; } + struct sockaddr_un sun = { .sun_family = AF_UNIX }; + strncpy(sun.sun_path, argv[2], sizeof(sun.sun_path) - 1); + int s = socket(AF_UNIX, dgram ? SOCK_DGRAM : SOCK_STREAM, 0); + if (s < 0) { fprintf(stderr, "unix-send: socket: %s\n", strerror(errno)); return 1; } + size_t len = strlen(argv[3]); + ssize_t n; + if (dgram) { + n = sendto(s, argv[3], len, 0, (struct sockaddr *)&sun, sizeof(sun)); + } else { + if (connect(s, (struct sockaddr *)&sun, sizeof(sun)) != 0) { + fprintf(stderr, "unix-send: connect %s: %s\n", argv[2], strerror(errno)); + return 1; + } + n = send(s, argv[3], len, 0); + } + if (n != (ssize_t)len) { fprintf(stderr, "unix-send: send %s: %s\n", argv[2], strerror(errno)); return 1; } + close(s); + return 0; +} + /* ── dispatch ───────────────────────────────────────────────── */ static int dispatch(const char *cmd, int argc, char **argv) { if (strcmp(cmd, "chdir") == 0) return cmd_chdir(argc, argv); + if (strcmp(cmd, "unix-send") == 0) return cmd_unix_send(argc, argv); if (strcmp(cmd, "fchdir") == 0) return cmd_fchdir(argc, argv); if (strcmp(cmd, "openat2") == 0) return cmd_openat2(argc, argv); if (strcmp(cmd, "chdir-self") == 0) return cmd_chdir_self(argc, argv);