From 99bda46ccd87bea70764e24ea1528076d136513f Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 15:14:24 -0700 Subject: [PATCH 01/18] pgroup: signal groups by number on kernels without pidfd group signals Kernels before 6.9 reject PIDFD_SIGNAL_PROCESS_GROUP with EINVAL, so every group sweep failed there and a session could not be killed. Fall back to kill(-pgid) while the group's key process has not exited: that process still holds the number, so no other group can have it. Once it has exited the number may be reused, but a probe that finds no group at all is still conclusive, because an empty group can never regain members. Only a populated group whose key process is gone stays out of reach, and it is reported. Add live() to list the groups that still have members. Cover each case in a child whose seccomp filter rejects group pidfd signals, so the tests run on any kernel. Signed-off-by: Cong Wang --- crates/sandlock-core/src/pgroup.rs | 101 ++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/crates/sandlock-core/src/pgroup.rs b/crates/sandlock-core/src/pgroup.rs index 0edd30f9..26e4f3a6 100644 --- a/crates/sandlock-core/src/pgroup.rs +++ b/crates/sandlock-core/src/pgroup.rs @@ -47,11 +47,27 @@ impl ProcessGroups { .map(|leaders| { leaders .get(&pgid) - .is_some_and(|leader| pidfd_signal_group(leader, 0).is_ok()) + .is_some_and(|leader| signal_group(pgid, leader, 0).is_ok()) }) .unwrap_or(false) } + /// Sorted pgids that still have members. + pub fn live(&self) -> io::Result> { + let leaders = self.leaders.lock() + .map_err(|_| io::Error::other("process group table poisoned"))?; + let mut groups = Vec::new(); + for (&pgid, leader) in leaders.iter() { + match signal_group(pgid, leader, 0) { + Ok(()) => groups.push(pgid), + Err(e) if e.raw_os_error() == Some(libc::ESRCH) => {} + Err(e) => return Err(e), + } + } + groups.sort_unstable(); + Ok(groups) + } + /// Signal every group, returning how many had members. Holding the lock /// for the whole sweep keeps a group recorded meanwhile from being /// skipped: its creator is still in a group this sweep reaches. @@ -62,7 +78,7 @@ impl ProcessGroups { .map_err(|_| io::Error::other("process group table poisoned"))?; let mut reached = 0; let mut failure = None; - leaders.retain(|_, leader| match pidfd_signal_group(leader, sig) { + leaders.retain(|&pgid, leader| match signal_group(pgid, leader, sig) { Ok(()) => { reached += 1; true @@ -88,7 +104,23 @@ impl Default for ProcessGroups { } } -fn has_exited(pidfd: RawFd) -> bool { +/// Kernels before 6.9 refuse group signals by pidfd. While the key process +/// has not exited it holds the number, so no other group can have taken it. +/// After that only a probe finding no group at all is conclusive, since an +/// empty group can never regain members. +fn signal_group(pgid: i32, key: &OwnedFd, sig: i32) -> io::Result<()> { + let refused = match pidfd_signal_group(key, sig) { + Err(e) if e.raw_os_error() == Some(libc::EINVAL) => e, + result => return result, + }; + let reachable = !has_exited(key.as_raw_fd()); + if unsafe { libc::kill(-pgid, if reachable { sig } else { 0 }) } != 0 { + return Err(io::Error::last_os_error()); + } + if reachable { Ok(()) } else { Err(refused) } +} + +pub(crate) fn has_exited(pidfd: RawFd) -> bool { let mut pfd = libc::pollfd { fd: pidfd, events: libc::POLLIN, @@ -259,6 +291,69 @@ mod tests { reap(leader); } + /// Runs `f` in a child whose kernel appears to predate group pidfd signals. + fn on_old_kernel(f: impl FnOnce() -> bool) -> bool { + let pid = unsafe { libc::fork() }; + if pid == 0 { + let filter = [ + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 0 }, + libc::sock_filter { code: 0x15, jt: 0, jf: 3, k: libc::SYS_pidfd_send_signal as u32 }, + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 16 + 3 * 8 }, + libc::sock_filter { code: 0x45, jt: 0, jf: 1, k: 1 << 2 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ERRNO | libc::EINVAL as u32 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ALLOW }, + ]; + let prog = libc::sock_fprog { len: filter.len() as u16, filter: filter.as_ptr() as *mut _ }; + unsafe { + libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + libc::prctl(libc::PR_SET_SECCOMP, libc::SECCOMP_MODE_FILTER, &prog); + libc::_exit(if f() { 0 } else { 1 }); + } + } + let mut status = 0; + unsafe { libc::waitpid(pid, &mut status, 0) }; + libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0 + } + + #[test] + fn old_kernel_signals_a_group_whose_leader_lives() { + assert!(on_old_kernel(|| { + let (leader, member) = spawn_group(true); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + let reached = groups.live().unwrap() == [leader] && groups.signal(libc::SIGKILL).unwrap() == 1; + reap(leader); + wait_gone(member.unwrap()); + reached + })); + } + + #[test] + fn old_kernel_drops_an_empty_group_whose_leader_is_gone() { + assert!(on_old_kernel(|| { + let (leader, _) = spawn_group(false); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + unsafe { libc::kill(leader, libc::SIGKILL) }; + reap(leader); + groups.signal(libc::SIGKILL).unwrap() == 0 && groups.live().unwrap().is_empty() + })); + } + + #[test] + fn old_kernel_reports_a_populated_group_whose_leader_is_gone() { + assert!(on_old_kernel(|| { + let (leader, member) = spawn_group(true); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + unsafe { libc::kill(leader, libc::SIGKILL) }; + reap(leader); + let refused = groups.signal(libc::SIGKILL).map_err(|e| e.raw_os_error()) == Err(Some(libc::EINVAL)); + unsafe { libc::kill(member.unwrap(), libc::SIGKILL) }; + refused + })); + } + #[test] fn owns_rejects_an_unrecorded_group() { let groups = ProcessGroups::new(); From ec70b384f017d5d385b76dc0b29fba842acc6a24 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 15:17:02 -0700 Subject: [PATCH 02/18] reaper: add a session reaper that names main's process group Background jobs that outlive the main process need two things the supervisor cannot give them. Under Yama, the supervisor can only read memory and fds of its own descendants, but an orphaned job is reparented to init. And once main is reaped, the group named after main's pid can be reused, so nothing can signal it safely by number on kernels without group pidfd signals. Fork a reaper between the supervisor and main. It is a subreaper, so orphans stay below the supervisor, and it reaps them so the embedder never has to. It creates process group R from its own pid, forks main into R and moves itself back to the supervisor's group: while the reaper lives no other process can hold that number, so R stays exact to signal for the whole session, and a sweep of R never kills the reaper itself. It relays main's wait status over a pipe, exits once it has no children, kills R if its owning process dies, and on request exits early to release jobs nobody can track. Test the status relay and group naming, outliving main and the creating thread, releasing jobs on request, and killing R when the owner dies. Signed-off-by: Cong Wang --- crates/sandlock-core/src/lib.rs | 1 + crates/sandlock-core/src/reaper.rs | 331 +++++++++++++++++++++++++++++ 2 files changed, 332 insertions(+) create mode 100644 crates/sandlock-core/src/reaper.rs diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index a8847558..5630a4ef 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -16,6 +16,7 @@ pub mod protection; pub mod seccomp; pub(crate) mod resource; pub(crate) mod pgroup; +pub(crate) mod reaper; pub(crate) mod network; pub mod context; pub(crate) mod resolved; diff --git a/crates/sandlock-core/src/reaper.rs b/crates/sandlock-core/src/reaper.rs new file mode 100644 index 00000000..dd98f7b7 --- /dev/null +++ b/crates/sandlock-core/src/reaper.rs @@ -0,0 +1,331 @@ +//! Main's parent and the session's subreaper, outside the sandbox. + +use std::io; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; + +pub(crate) struct Reaper { + pub pid: i32, + pub pidfd: OwnedFd, + status: Option, + finish: Option, +} + +impl Reaper { + /// Carries main's raw wait status as four little-endian bytes, then EOF. + pub fn take_status(&mut self) -> Option { + self.status.take() + } + + /// Exit after reaping what has exited; jobs still alive are released. + /// A byte rather than EOF: children parked before exec may hold a copy. + pub fn finish(&mut self) { + if let Some(fd) = self.finish.take() { + unsafe { send(fd.as_raw_fd(), &[0]) }; + } + } + + pub fn exited(&self) -> bool { + crate::pgroup::has_exited(self.pidfd.as_raw_fd()) + } + + pub async fn join(&self) { + if let Ok(fd) = tokio::io::unix::AsyncFd::new(self.pidfd.as_raw_fd()) { + let _ = fd.readable().await; + } + reap(self.pid); + } + + pub fn join_blocking(&self) { + reap(self.pid); + } +} + +/// Group R is named after the reaper's pid: while the reaper lives no other +/// process can hold that number, so signalling R by number stays exact on +/// every kernel, even after main has been reaped. +pub(crate) fn fork() -> io::Result<(i32, Option)> { + let (status_r, status_w) = pipe()?; + let (finish_r, finish_w) = pipe()?; + let owner = unsafe { libc::getpid() }; + let owner_pgid = unsafe { libc::getpgrp() }; + let reaper = crate::control::fork_without_control_fds(); + if reaper < 0 { + return Err(io::Error::last_os_error()); + } + if reaper == 0 { + unsafe { + let mut all = std::mem::zeroed(); + let mut previous_mask = std::mem::zeroed(); + libc::sigfillset(&mut all); + libc::sigprocmask(libc::SIG_SETMASK, &all, &mut previous_mask); + let mut previous_chld = std::mem::zeroed(); + let mut default_chld: libc::sigaction = std::mem::zeroed(); + default_chld.sa_sigaction = libc::SIG_DFL; + libc::sigaction(libc::SIGCHLD, &default_chld, &mut previous_chld); + // A pidfd of the owning process, not PDEATHSIG: that fires when + // the forking thread exits and would end the session with it. + let owner_fd = libc::syscall(libc::SYS_pidfd_open, owner, 0) as i32; + let mut chld = std::mem::zeroed(); + libc::sigemptyset(&mut chld); + libc::sigaddset(&mut chld, libc::SIGCHLD); + let signal_fd = libc::signalfd(-1, &chld, libc::SFD_CLOEXEC | libc::SFD_NONBLOCK); + if owner_fd < 0 + || signal_fd < 0 + || libc::getppid() != owner + || libc::prctl(libc::PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) != 0 + || libc::setpgid(0, 0) != 0 + { + libc::_exit(127); + } + let me = libc::getpid(); + let main = libc::fork(); + if main < 0 { + libc::_exit(127); + } + if main == 0 { + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) != 0 || libc::getppid() != me { + libc::_exit(127); + } + libc::sigaction(libc::SIGCHLD, &previous_chld, std::ptr::null_mut()); + libc::sigprocmask(libc::SIG_SETMASK, &previous_mask, std::ptr::null_mut()); + libc::close(owner_fd); + libc::close(signal_fd); + drop((status_r, status_w, finish_r, finish_w)); + return Ok((0, None)); + } + // Out of R, so a sweep of R never takes the reaper with it. + if libc::setpgid(0, owner_pgid) != 0 { + libc::kill(main, libc::SIGKILL); + libc::_exit(127); + } + close_all_but([status_w.as_raw_fd(), finish_r.as_raw_fd(), signal_fd, owner_fd]); + if !send(status_w.as_raw_fd(), &main.to_le_bytes()) { + libc::kill(main, libc::SIGKILL); + } + supervise(main, me, status_w.as_raw_fd(), finish_r.as_raw_fd(), signal_fd, owner_fd); + } + } + drop(status_w); + drop(finish_r); + let main = loop { + match crate::context::read_u32_fd(status_r.as_raw_fd()) { + Err(e) if e.kind() == io::ErrorKind::Interrupted => continue, + result => break result, + } + }; + let pidfd = crate::sys::syscall::pidfd_open(reaper as u32, 0); + match (main, pidfd) { + (Ok(main), Ok(pidfd)) if main > 0 => Ok(( + main as i32, + Some(Reaper { pid: reaper, pidfd, status: Some(status_r), finish: Some(finish_w) }), + )), + (main, pidfd) => { + unsafe { libc::kill(reaper, libc::SIGKILL) }; + reap(reaper); + Err(main.err().or(pidfd.err()).unwrap_or_else(|| io::Error::other("session reaper failed to start"))) + } + } +} + +unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signal_fd: i32, owner_fd: i32) -> ! { + let mut poll = [ + libc::pollfd { fd: finish_fd, events: libc::POLLIN, revents: 0 }, + libc::pollfd { fd: signal_fd, events: libc::POLLIN, revents: 0 }, + libc::pollfd { fd: owner_fd, events: libc::POLLIN, revents: 0 }, + ]; + loop { + loop { + let mut status = 0; + match libc::waitpid(-1, &mut status, libc::WNOHANG | libc::__WALL) { + 0 => break, + pid if pid == main => { + send(status_fd, &status.to_le_bytes()); + libc::close(status_fd); + } + pid if pid > 0 => {} + _ if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} + // No child left means no session process left. + _ => libc::_exit(0), + } + } + libc::poll(poll.as_mut_ptr(), 3, -1); + if poll[0].revents != 0 { + libc::_exit(0); + } + if poll[2].revents != 0 { + libc::kill(-group, libc::SIGKILL); + libc::_exit(0); + } + if poll[1].revents != 0 { + let mut info: libc::signalfd_siginfo = std::mem::zeroed(); + libc::read(signal_fd, (&mut info as *mut libc::signalfd_siginfo).cast(), std::mem::size_of_val(&info)); + } + } +} + +fn pipe() -> io::Result<(OwnedFd, OwnedFd)> { + let mut fds = [0; 2]; + if unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + return Err(io::Error::last_os_error()); + } + Ok(unsafe { (OwnedFd::from_raw_fd(fds[0]), OwnedFd::from_raw_fd(fds[1])) }) +} + +unsafe fn close_all_but(mut keep: [i32; 4]) { + keep.sort_unstable(); + let mut next = 0u32; + for kept in keep { + if kept as u32 > next { + libc::syscall(libc::SYS_close_range, next, kept as u32 - 1, 0u32); + } + next = kept as u32 + 1; + } + libc::syscall(libc::SYS_close_range, next, u32::MAX, 0u32); +} + +fn reap(pid: i32) { + while unsafe { libc::waitpid(pid, std::ptr::null_mut(), 0) } < 0 + && io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) + {} +} + +unsafe fn send(fd: i32, bytes: &[u8]) -> bool { + loop { + if libc::write(fd, bytes.as_ptr().cast(), bytes.len()) == bytes.len() as isize { + return true; + } + if io::Error::last_os_error().raw_os_error() != Some(libc::EINTR) { + return false; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn read_status(fd: &OwnedFd) -> i32 { + let mut raw = [0u8; 4]; + assert_eq!(unsafe { libc::read(fd.as_raw_fd(), raw.as_mut_ptr().cast(), 4) }, 4); + i32::from_le_bytes(raw) + } + + #[tokio::test] + async fn relays_main_status_and_names_its_group() { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { libc::_exit(if libc::getpgrp() == libc::getppid() { 42 } else { 1 }) } + } + let mut reaper = reaper.unwrap(); + let status = read_status(&reaper.take_status().unwrap()); + assert_eq!(libc::WEXITSTATUS(status), 42); + assert_ne!(unsafe { libc::getpgid(reaper.pid) }, reaper.pid, "reaper left group R"); + reaper.join().await; + } + + #[tokio::test] + async fn outlives_main_until_orphans_are_reaped() { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + if libc::fork() == 0 { + libc::usleep(300_000); + libc::_exit(0); + } + libc::_exit(7); + } + } + let mut reaper = reaper.unwrap(); + assert_eq!(libc::WEXITSTATUS(read_status(&reaper.take_status().unwrap())), 7); + assert!(!reaper.exited()); + reaper.join().await; + } + + #[tokio::test] + async fn outlives_the_thread_that_created_it() { + let reaper = std::thread::spawn(|| { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + libc::pause(); + libc::_exit(0); + } + } + reaper + }) + .join() + .unwrap() + .unwrap(); + std::thread::sleep(std::time::Duration::from_millis(50)); + assert!(!reaper.exited()); + unsafe { libc::kill(-reaper.pid, libc::SIGKILL) }; + reaper.join().await; + } + + #[tokio::test] + async fn finish_releases_jobs_still_running() { + let mut pipe = [0; 2]; + assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + let job = libc::fork(); + if job == 0 { + libc::setsid(); + libc::pause(); + libc::_exit(0); + } + libc::write(pipe[1], (&job as *const i32).cast(), 4); + libc::_exit(0); + } + } + let mut job = 0i32; + assert_eq!(unsafe { libc::read(pipe[0], (&mut job as *mut i32).cast(), 4) }, 4); + unsafe { + libc::close(pipe[0]); + libc::close(pipe[1]); + } + let mut reaper = reaper.unwrap(); + read_status(&reaper.take_status().unwrap()); + reaper.finish(); + reaper.join().await; + assert_eq!(unsafe { libc::kill(job, libc::SIGKILL) }, 0); + } + + #[test] + fn owner_death_kills_group_r() { + let mut pipe = [0; 2]; + assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); + // The owner is a forked helper so its death can be observed here. + let owner = unsafe { libc::fork() }; + if owner == 0 { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + if libc::fork() == 0 { + libc::pause(); + } + libc::pause(); + libc::_exit(0); + } + } + let group = reaper.unwrap().pid; + unsafe { + libc::write(pipe[1], (&group as *const i32).cast(), 4); + libc::_exit(0); + } + } + let mut group = 0i32; + assert_eq!(unsafe { libc::read(pipe[0], (&mut group as *mut i32).cast(), 4) }, 4); + unsafe { + libc::close(pipe[0]); + libc::close(pipe[1]); + libc::waitpid(owner, std::ptr::null_mut(), 0); + } + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while unsafe { libc::kill(-group, 0) } == 0 { + assert!(std::time::Instant::now() < deadline, "group R outlived its owner"); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } +} From 21c25ba153911e8caa180a143e513e585a079f7b Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 15:26:38 -0700 Subject: [PATCH 03/18] sandbox: run every sandbox under a session reaper The main process led its own process group, so once main was reaped its group number was free for reuse and nothing could signal the jobs left in it by number. Orphaned jobs were reparented to init, out of the supervisor's reach under Yama. Fork each sandbox through the session reaper. Main now runs in the reaper's group R, recorded with the reaper's pidfd, so the group stays signallable after main exits on every kernel. Main's exit status comes from the reaper through a watch slot that a background task fills as soon as main is reaped. The reaper also sets main's parent-death signal and checks its parent, so confine_child no longer repeats that. The terminal foreground is handed back by comparing against R. wait() still ends the session with main: it sweeps the groups and waits for the reaper to exit, releasing untracked jobs when there is no supervisor. The tty handover test now expects the foreground to go to the child's group rather than its pid, and the no-supervisor control test expects main to run in the reaper's group. All core, CLI and FFI tests pass. Signed-off-by: Cong Wang --- crates/sandlock-core/src/context.rs | 18 --- crates/sandlock-core/src/reaper.rs | 1 + crates/sandlock-core/src/sandbox.rs | 109 ++++++++++++------ .../tests/integration/test_control.rs | 2 +- .../tests/integration/test_tty.rs | 3 +- 5 files changed, 75 insertions(+), 58 deletions(-) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 7a2b5a22..6780cd60 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -237,10 +237,6 @@ pub(crate) struct ChildSpawnArgs<'a> { /// Merged into the child's BPF notif list so the kernel actually /// raises USER_NOTIF for them. pub extra_syscalls: &'a [u32], - /// PID of the parent process captured before fork. Used to detect - /// parent death in the child without assuming PID 1 is always init - /// (incorrect in containers where the entrypoint runs as PID 1). - pub parent_pid: libc::pid_t, /// Make the child the terminal's foreground process group before exec. /// Only interactive (fully inherited) stdio wants this; a captured or /// piped run taking the foreground demotes the embedding process to a @@ -291,7 +287,6 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { keep_fds, sandbox_name, extra_syscalls, - parent_pid, foreground, } = args; // Helper: abort child on error. Includes the OS error automatically. @@ -319,19 +314,6 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { } } - // 2. Die if parent exits - if unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) } != 0 { - fail!("prctl(PR_SET_PDEATHSIG)"); - } - - // 3. Check parent didn't die between fork and prctl. - // Compare against the actual parent PID captured before fork rather than - // hardcoding 1, since containers often run the entrypoint as PID 1 and a - // child forked from it legitimately has getppid() == 1. - if unsafe { libc::getppid() } != parent_pid { - fail!("parent died before confinement"); - } - // 4. Optional: disable ASLR if sandbox.no_randomize_memory { const ADDR_NO_RANDOMIZE: libc::c_ulong = 0x0040000; diff --git a/crates/sandlock-core/src/reaper.rs b/crates/sandlock-core/src/reaper.rs index dd98f7b7..26a2770b 100644 --- a/crates/sandlock-core/src/reaper.rs +++ b/crates/sandlock-core/src/reaper.rs @@ -24,6 +24,7 @@ impl Reaper { } } + #[cfg(test)] pub fn exited(&self) -> bool { crate::pgroup::has_exited(self.pidfd.as_raw_fd()) } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 840d6e22..ac56b2cf 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -288,6 +288,10 @@ struct Runtime { state: RuntimeState, child_pid: Option, pidfd: Option, + reaper: Option, + /// Main's process group: the reaper's pid, or a clone's own pid. + pgid: i32, + main_exit: tokio::sync::watch::Receiver>, groups: Arc, notif_handle: Option>, policy_fn_worker: Option, @@ -1010,19 +1014,33 @@ impl Sandbox { // race the worker for fork events and hang it). Mirrors // `spawn_pid_watcher`. Falls back to a blocking `waitpid` only when no // pidfd is available (kernel without `pidfd_open`). - let exit_status = match self.rt_mut().pidfd.take() { - Some(pidfd) => wait_child_exit_via_pidfd(pidfd, pid).await, - None => wait_child_exit_blocking(pid).await, + let exit_status = if self.rt().reaper.is_some() { + let mut main_exit = self.rt().main_exit.clone(); + let status = main_exit.wait_for(|s| s.is_some()).await + .map(|s| s.clone().expect("checked")) + .unwrap_or(crate::result::ExitStatus::Killed); + status + } else { + match self.rt_mut().pidfd.take() { + Some(pidfd) => wait_child_exit_via_pidfd(pidfd, pid).await, + None => wait_child_exit_blocking(pid).await, + } }; // The sandbox ends with its main process: descendants left behind // would run on with every supervised syscall failing, holding the // caller's stdio. - let _ = self.rt().groups.signal(libc::SIGKILL); + let swept = self.rt().groups.signal(libc::SIGKILL); + let no_supervisor = self.no_supervisor; + if let Some(mut reaper) = self.rt_mut().reaper.take() { + // Only jobs no sweep reached can keep it alive. + if no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join().await; + } self.rt_mut().state = RuntimeState::Stopped(exit_status.clone()); if self.rt().tty_foreground_taken { - sandbox_restore_tty_foreground(pid); + sandbox_restore_tty_foreground(self.rt().pgid); self.rt_mut().tty_foreground_taken = false; } @@ -1688,6 +1706,9 @@ impl Sandbox { }), child_pid: Some(clone_pid), pidfd: None, + reaper: None, + pgid: clone_pid, + main_exit: tokio::sync::watch::channel(None).1, groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -1793,6 +1814,9 @@ impl Sandbox { state: RuntimeState::Created, child_pid: None, pidfd: None, + reaper: None, + pgid: 0, + main_exit: tokio::sync::watch::channel(None).1, groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -2050,10 +2074,6 @@ impl Sandbox { None }; - // Capture our PID before fork so the child can detect parent death - // without assuming PID 1 is always init (wrong in containers). - let parent_pid = unsafe { libc::getpid() }; - // Interactive (fully inherited) stdio on a terminal: the child will // take the tty foreground group, and this process must take it back // once the child is reaped. @@ -2085,20 +2105,11 @@ impl Sandbox { }; let child_publisher = control_socket.as_ref().and_then(|s| s.claim.as_ref()).map(|c| c.child_publisher()); - let pid = crate::control::fork_without_control_fds(); - if pid < 0 { - return Err(SandboxRuntimeError::Fork(std::io::Error::last_os_error()).into()); - } + let (pid, reaper) = crate::reaper::fork().map_err(SandboxRuntimeError::Fork)?; if pid == 0 { // ===== CHILD PROCESS ===== - // Publish only after the process group exists, before confinement denies IPC. - if unsafe { libc::setpgid(0, 0) } != 0 { - use std::io::Write; - let err = std::io::Error::last_os_error(); - let _ = writeln!(std::io::stderr(), "sandlock child: setpgid: {err}"); - unsafe { libc::_exit(127) }; - } + // Publish before confinement denies IPC. if let Some(publisher) = child_publisher { if !publisher.publish() { unsafe { libc::_exit(127); } } } @@ -2172,7 +2183,6 @@ impl Sandbox { keep_fds: &gather_keep_fds, sandbox_name: Some(sandbox_name.as_str()), extra_syscalls: &extra_syscalls, - parent_pid, foreground, }); } @@ -2190,14 +2200,14 @@ impl Sandbox { // State remains `Created` until `do_start` writes ready_w to release // the child to execve. - let pidfd = match syscall::pidfd_open(pid as u32, 0) { - Ok(fd) => Some(fd), - Err(_) => None, - }; - // The child's first act is setpgid(0, 0), so its pid names its group. - if let Some(leader) = pidfd.as_ref().and_then(|pfd| pfd.try_clone().ok()) { - self.rt_mut().groups.track(pid, leader); - } + let mut reaper = reaper.expect("parent has a reaper"); + let status = reaper.take_status().expect("fresh reaper"); + let key = reaper.pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; + self.rt_mut().groups.track(reaper.pid, key); + self.rt_mut().pgid = reaper.pid; + self.rt_mut().main_exit = spawn_main_exit_watch(status); + self.rt_mut().reaper = Some(reaper); + let pidfd = syscall::pidfd_open(pid as u32, 0).ok(); let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; @@ -2595,9 +2605,9 @@ impl Process<'_> { /// to someone else is left alone. SIGTTOU is blocked around `tcsetpgrp`: /// this process is a background group at that moment, and an unblocked /// SIGTTOU would stop it, which is the exact symptom being prevented. -fn sandbox_restore_tty_foreground(child_pid: i32) { +fn sandbox_restore_tty_foreground(pgid: i32) { unsafe { - if libc::isatty(0) != 1 || libc::tcgetpgrp(0) != child_pid { + if libc::isatty(0) != 1 || libc::tcgetpgrp(0) != pgid { return; } let mut block: libc::sigset_t = std::mem::zeroed(); @@ -2616,19 +2626,23 @@ fn sandbox_restore_tty_foreground(child_pid: i32) { impl Drop for Sandbox { fn drop(&mut self) { + let no_supervisor = self.no_supervisor; if let Some(ref mut rt) = self.runtime { if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } if let Some(pid) = rt.child_pid { if matches!(rt.state, RuntimeState::Created | RuntimeState::Running | RuntimeState::Paused) { - let _ = rt.groups.signal(libc::SIGKILL); - // The waitpid below must not hang on a child whose group - // was never recorded. Unreaped, its pid is still ours. - unsafe { libc::kill(pid, libc::SIGKILL) }; - let mut status: i32 = 0; - unsafe { libc::waitpid(pid, &mut status, 0) }; + let swept = rt.groups.signal(libc::SIGKILL); + if let Some(mut reaper) = rt.reaper.take() { + if no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join_blocking(); + } else { + // Unreaped, a clone's pid is still ours. + unsafe { libc::kill(pid, libc::SIGKILL) }; + unsafe { libc::waitpid(pid, std::ptr::null_mut(), 0) }; + } } if rt.tty_foreground_taken { - sandbox_restore_tty_foreground(pid); + sandbox_restore_tty_foreground(rt.pgid); rt.tty_foreground_taken = false; } } @@ -2925,6 +2939,25 @@ fn sandbox_read_fd_to_end(fd: std::os::fd::OwnedFd) -> Vec { buf } +/// Main's status arrives once on the reaper's pipe; the slot keeps it for +/// `wait()` and for `ps`, whether or not anyone is waiting yet. +fn spawn_main_exit_watch( + status: std::os::fd::OwnedFd, +) -> tokio::sync::watch::Receiver> { + use tokio::io::AsyncReadExt; + let (tx, rx) = tokio::sync::watch::channel(None); + tokio::spawn(async move { + let Ok(mut pipe) = tokio::net::unix::pipe::Receiver::from_owned_fd(status) else { return }; + let mut raw = [0u8; 4]; + let exit = match pipe.read_exact(&mut raw).await { + Ok(_) => sandbox_wait_status_to_exit(i32::from_le_bytes(raw)), + Err(_) => crate::result::ExitStatus::Killed, + }; + let _ = tx.send(Some(exit)); + }); + rx +} + fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { use crate::result::ExitStatus; if libc::WIFEXITED(status) { diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index a0eb34bf..b56048ec 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -419,7 +419,7 @@ fn test_control_no_supervisor() { let pids = sandlock_core::control::sandbox_pids(&name).expect("pids"); assert_eq!(pids.supervisor, child.id() as i32, "supervisor pid: {:?}", pids); - assert_eq!(unsafe { libc::getpgid(pids.child) }, pids.child, "child leads its group: {:?}", pids); + assert_ne!(unsafe { libc::getpgid(pids.child) }, pids.child, "main runs in the reaper's group: {:?}", pids); let info = sandlock_core::control::sandbox_info(&name).expect("info"); assert_eq!(info.mode, None); diff --git a/crates/sandlock-core/tests/integration/test_tty.rs b/crates/sandlock-core/tests/integration/test_tty.rs index 42368a98..66ea2ca0 100644 --- a/crates/sandlock-core/tests/integration/test_tty.rs +++ b/crates/sandlock-core/tests/integration/test_tty.rs @@ -148,7 +148,8 @@ fn interactive_run_hands_tty_foreground_to_child_while_alive() { Some(p) => p, None => return EXIT_RUN_FAILED, }; - if unsafe { libc::tcgetpgrp(0) } != child { + // The child runs in its reaper's group, not one it leads. + if unsafe { libc::tcgetpgrp(0) != libc::getpgid(child) } { return EXIT_FOREGROUND_NOT_TAKEN; } if policy.kill().is_err() { From 2f8aee6fbdf7016af94e3e55eef507cc2dd50880 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 15:58:04 -0700 Subject: [PATCH 04/18] reaper: kill the session when its owner goes away When the owning process died, its end of the finish pipe closed too, so the reaper saw the finish descriptor readable and exited before it looked at the owner pidfd. Group R was never killed: main died of its parent-death signal, but every other job in the group kept running with nobody supervising it. Check the owner pidfd first, and tell the finish byte apart from EOF. A byte still releases the remaining jobs; EOF means the handle is gone without a finish or a join, so the session is killed like on owner death. The owner-death test now re-executes itself as the owner instead of running the reaper in a child forked from the multithreaded test binary, and reads only up to the reported group, since concurrent forks in the test binary can hold its stdout pipe open. Signed-off-by: Cong Wang --- crates/sandlock-core/src/reaper.rs | 59 ++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/crates/sandlock-core/src/reaper.rs b/crates/sandlock-core/src/reaper.rs index 26a2770b..491e4466 100644 --- a/crates/sandlock-core/src/reaper.rs +++ b/crates/sandlock-core/src/reaper.rs @@ -150,11 +150,13 @@ unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signa } } libc::poll(poll.as_mut_ptr(), 3, -1); - if poll[0].revents != 0 { + // Owner death also closes its end of the finish pipe, so it must be + // seen first; EOF alone likewise means nobody can finish or join. + if poll[2].revents != 0 || (poll[0].revents != 0 && !read_finish(finish_fd)) { + libc::kill(-group, libc::SIGKILL); libc::_exit(0); } - if poll[2].revents != 0 { - libc::kill(-group, libc::SIGKILL); + if poll[0].revents != 0 { libc::_exit(0); } if poll[1].revents != 0 { @@ -164,6 +166,18 @@ unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signa } } +/// True for the finish byte, false for EOF. +unsafe fn read_finish(fd: i32) -> bool { + let mut byte = 0u8; + loop { + match libc::read(fd, (&mut byte as *mut u8).cast(), 1) { + 1 => return true, + -1 if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} + _ => return false, + } + } +} + fn pipe() -> io::Result<(OwnedFd, OwnedFd)> { let mut fds = [0; 2]; if unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { @@ -293,13 +307,12 @@ mod tests { assert_eq!(unsafe { libc::kill(job, libc::SIGKILL) }, 0); } + /// The owner is a re-executed copy of this test, so its death can be + /// observed here without running Rust code in a forked child. #[test] fn owner_death_kills_group_r() { - let mut pipe = [0; 2]; - assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); - // The owner is a forked helper so its death can be observed here. - let owner = unsafe { libc::fork() }; - if owner == 0 { + const OWNER: &str = "SANDLOCK_TEST_REAPER_OWNER"; + if std::env::var_os(OWNER).is_some() { let (main, reaper) = fork().unwrap(); if main == 0 { unsafe { @@ -310,19 +323,25 @@ mod tests { libc::_exit(0); } } - let group = reaper.unwrap().pid; - unsafe { - libc::write(pipe[1], (&group as *const i32).cast(), 4); - libc::_exit(0); - } - } - let mut group = 0i32; - assert_eq!(unsafe { libc::read(pipe[0], (&mut group as *mut i32).cast(), 4) }, 4); - unsafe { - libc::close(pipe[0]); - libc::close(pipe[1]); - libc::waitpid(owner, std::ptr::null_mut(), 0); + use std::io::Write; + let mut out = std::io::stdout(); + let _ = writeln!(out, "group={}", reaper.unwrap().pid); + let _ = out.flush(); + unsafe { libc::_exit(0) } } + let mut owner = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["reaper::tests::owner_death_kills_group_r", "--exact", "--nocapture"]) + .env(OWNER, "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap(); + // Read only up to the report: concurrent forks in this test binary + // may hold the pipe's write end, so EOF can come late. + let group: i32 = std::io::BufRead::lines(std::io::BufReader::new(owner.stdout.take().unwrap())) + .find_map(|l| l.ok()?.strip_prefix("group=")?.parse().ok()) + .expect("owner reported group R"); + owner.wait().unwrap(); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); while unsafe { libc::kill(-group, 0) } == 0 { assert!(std::time::Instant::now() < deadline, "group R outlived its owner"); From 2464823f038ccc18e59e1ecb6d867ce4641b9f10 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:14:19 -0700 Subject: [PATCH 05/18] sandbox: keep background jobs supervised after main exits wait() killed every remaining process as soon as the main process exited, so a command that started background work lost it, and an embedder had no way to let that work finish under supervision. Split the lifetime in two. wait() now returns main's exit status and leaves the session running. shutdown() kills what is left, waits until the session has no process, then settles the COW branch and collects the captured output. A Controller, obtained from the sandbox, signals and lists the session's process groups from another task or thread while wait() holds the sandbox, and cancels a held policy_fn verdict on kill. Capture drains are cut once jobs are released without a supervisor, since those may hold the write ends forever. Runs, popen processes, pipelines, transactions, the OCI supervisor and sandlock learn call shutdown() where they need the full result. Add tests for jobs surviving wait(), shutdown() with a job holding the captured stdout, shutdown() of a paused session, killing through the Controller during a blocked wait(), a cancelled wait(), and dropping the sandbox after wait(). An emulation of kernels without group pidfd signals checks that a backgrounded job is killed, a setsid job with a live leader is killed, and a double-forked daemon is reported as unreachable. All core, CLI and FFI tests pass. Fixes #292 Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 6 +- crates/sandlock-core/src/controller.rs | 51 +++++ crates/sandlock-core/src/lib.rs | 2 + crates/sandlock-core/src/pipeline.rs | 6 +- crates/sandlock-core/src/sandbox.rs | 214 ++++++++++-------- crates/sandlock-core/src/transaction.rs | 3 +- .../tests/integration/test_control.rs | 6 +- .../tests/integration/test_fork.rs | 8 +- .../tests/integration/test_policy_fn.rs | 2 +- .../tests/integration/test_popen.rs | 30 +-- .../tests/integration/test_process_groups.rs | 81 +++++++ .../tests/integration/test_protection.rs | 94 ++++++++ .../tests/integration/test_resource.rs | 4 +- .../tests/integration/test_restore.rs | 8 +- .../tests/integration/test_sandbox.rs | 4 +- crates/sandlock-ffi/src/lib.rs | 13 +- crates/sandlock-oci/src/supervisor.rs | 4 +- 17 files changed, 405 insertions(+), 131 deletions(-) create mode 100644 crates/sandlock-core/src/controller.rs diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 95668ca1..1652fccc 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -632,7 +632,7 @@ pub async fn run(args: LearnArgs) -> Result<()> { match tokio::time::timeout(deadline, sandbox.wait()).await { Ok(r) => { let result = r.map_err(|e| anyhow!("sandbox error: {e}"))?; - require_clean_exit(&result.exit_status)?; + require_clean_exit(&result)?; false } Err(_elapsed) => { @@ -647,10 +647,12 @@ pub async fn run(args: LearnArgs) -> Result<()> { } else { let result = sandbox.wait().await .map_err(|e| anyhow!("sandbox error: {e}"))?; - require_clean_exit(&result.exit_status)?; + require_clean_exit(&result)?; false }; + sandbox.shutdown().await.map_err(|e| anyhow!("sandbox error: {e}"))?; + if timed_out { eprintln!("sandlock learn: writing partial profile from observations before timeout"); } diff --git a/crates/sandlock-core/src/controller.rs b/crates/sandlock-core/src/controller.rs new file mode 100644 index 00000000..1ddb9802 --- /dev/null +++ b/crates/sandlock-core/src/controller.rs @@ -0,0 +1,51 @@ +//! Acts on a running session while `Sandbox::wait()` holds it mutably. + +use std::sync::Arc; + +use crate::error::{SandboxRuntimeError, SandlockError}; +use crate::result::ExitStatus; + +#[derive(Clone)] +pub struct Controller { + pub(crate) groups: Arc, + pub(crate) stop: tokio_util::sync::CancellationToken, + pub(crate) main_exit: tokio::sync::watch::Receiver>, +} + +impl Controller { + pub fn kill(&self) -> Result<(), SandlockError> { + self.signal(libc::SIGKILL).map(|_| ()) + } + + pub fn pause(&self) -> Result<(), SandlockError> { + self.signal_live(libc::SIGSTOP) + } + + pub fn resume(&self) -> Result<(), SandlockError> { + self.signal_live(libc::SIGCONT) + } + + /// Sorted pgids that still have members; may change as soon as it returns. + pub fn process_groups(&self) -> Result, SandlockError> { + Ok(self.groups.live().map_err(SandboxRuntimeError::Io)?) + } + + pub fn main_exited(&self) -> bool { + self.main_exit.borrow().is_some() + } + + /// Signal every group, returning how many had members left. + pub fn signal(&self, sig: i32) -> Result { + if sig == libc::SIGKILL { + self.stop.cancel(); + } + Ok(self.groups.signal(sig).map_err(SandboxRuntimeError::Io)?) + } + + fn signal_live(&self, sig: i32) -> Result<(), SandlockError> { + if self.signal(sig)? == 0 { + return Err(SandboxRuntimeError::Io(std::io::Error::from_raw_os_error(libc::ESRCH)).into()); + } + Ok(()) + } +} diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 5630a4ef..7482fa6d 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -44,12 +44,14 @@ pub mod fork; pub(crate) mod ca_inject; pub(crate) mod chroot; pub mod control; +pub mod controller; #[cfg(feature = "http")] mod transparent_proxy; #[cfg(not(feature = "http"))] #[path = "transparent_proxy/disabled.rs"] mod transparent_proxy; +pub use controller::Controller; pub use error::SandlockError; pub use policy_fn::PolicyFn; pub use sys::structs::{SeccompData, SeccompNotif}; diff --git a/crates/sandlock-core/src/pipeline.rs b/crates/sandlock-core/src/pipeline.rs index 1c41d5a5..f018c1b5 100644 --- a/crates/sandlock-core/src/pipeline.rs +++ b/crates/sandlock-core/src/pipeline.rs @@ -247,7 +247,8 @@ async fn run_pipeline(stages: Vec) -> Result { }; for (i, mut sb) in sandboxes.into_iter().enumerate() { - let result = sb.wait().await?; + sb.wait().await?; + let result = sb.shutdown().await?; if i == n - 1 { last_result.exit_status = result.exit_status; } @@ -459,7 +460,8 @@ async fn run_gather( changes: Vec::new(), }; for (i, mut sb) in sandboxes.into_iter().enumerate() { - let result = sb.wait().await?; + sb.wait().await?; + let result = sb.shutdown().await?; if i == total - 1 { last_result.exit_status = result.exit_status; } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index ac56b2cf..febf10c8 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -293,6 +293,10 @@ struct Runtime { pgid: i32, main_exit: tokio::sync::watch::Receiver>, groups: Arc, + /// Cancelled by a kill, so a held policy_fn verdict cannot stall it. + stop: tokio_util::sync::CancellationToken, + /// Ends the drains once released jobs might hold the write ends forever. + drain_cut: tokio_util::sync::CancellationToken, notif_handle: Option>, policy_fn_worker: Option, throttle_handle: Option>, @@ -360,6 +364,7 @@ enum RuntimeState { Created, Running, Paused, + MainExited(crate::result::ExitStatus), Stopped(crate::result::ExitStatus), } @@ -867,7 +872,7 @@ impl Sandbox { /// Return whether the child is currently running or paused. pub fn is_running(&self) -> bool { self.runtime.as_ref().map(|r| { - matches!(r.state, RuntimeState::Running | RuntimeState::Paused) + matches!(r.state, RuntimeState::Running | RuntimeState::Paused | RuntimeState::MainExited(_)) }).unwrap_or(false) } @@ -875,41 +880,43 @@ impl Sandbox { /// members left. Groups stay reachable after the main process has been /// reaped, so this can sweep what a `wait()` left behind. pub fn signal(&self, sig: i32) -> Result { - use crate::error::SandboxRuntimeError; - let rt = self.runtime.as_ref() - .filter(|rt| rt.child_pid.is_some()) - .ok_or(SandboxRuntimeError::NotRunning)?; - if sig == libc::SIGKILL { - if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } - } - Ok(rt.groups.signal(sig).map_err(SandboxRuntimeError::Io)?) + self.controller()?.signal(sig) } - fn signal_live_groups(&self, sig: i32) -> Result<(), crate::error::SandlockError> { - if self.signal(sig)? == 0 { - let gone = std::io::Error::from_raw_os_error(libc::ESRCH); - return Err(crate::error::SandboxRuntimeError::Io(gone).into()); - } - Ok(()) + /// A handle that acts on this session from another task or thread, + /// including while `wait()` holds the `Sandbox`. + pub fn controller(&self) -> Result { + let rt = self.runtime.as_ref() + .filter(|rt| rt.child_pid.is_some()) + .ok_or(crate::error::SandboxRuntimeError::NotRunning)?; + Ok(crate::Controller { + groups: Arc::clone(&rt.groups), + stop: rt.stop.clone(), + main_exit: rt.main_exit.clone(), + }) } /// Send SIGSTOP to the sandbox's process groups. pub fn pause(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal_live_groups(libc::SIGSTOP)?; - self.rt_mut().state = RuntimeState::Paused; + self.controller()?.pause()?; + if !matches!(self.rt().state, RuntimeState::MainExited(_)) { + self.rt_mut().state = RuntimeState::Paused; + } Ok(()) } /// Send SIGCONT to the sandbox's process groups. pub fn resume(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal_live_groups(libc::SIGCONT)?; - self.rt_mut().state = RuntimeState::Running; + self.controller()?.resume()?; + if !matches!(self.rt().state, RuntimeState::MainExited(_)) { + self.rt_mut().state = RuntimeState::Running; + } Ok(()) } /// Send SIGKILL to the sandbox's process groups. pub fn kill(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal(libc::SIGKILL).map(|_| ()) + self.controller()?.kill() } /// Set a callback invoked whenever a port bind is recorded. @@ -942,35 +949,18 @@ impl Sandbox { } } - /// Wait for the child process to exit. + /// Wait for the main process to exit; background jobs stay supervised + /// until [`Self::shutdown`]. Returns the same status on repeated calls. /// - /// Dropping the returned future does not cost the captured output: the pipe - /// drains belong to the `Sandbox`, so a caller that cancels this `wait()` - /// (a `timeout` or `select!` around it) and calls `wait()` again still gets - /// what the child wrote. That holds wherever the cancellation lands — - /// before the drains start, while the child is still running, or while the - /// final join is blocked because a descendant is still holding the write - /// end open past the child's own exit. - pub async fn wait(&mut self) -> Result { + /// Dropping the returned future costs nothing: the pipe drains belong to + /// the `Sandbox`, so a cancelled `wait()` can be followed by another + /// `wait()` or by `shutdown()`, which still collects the output. + pub async fn wait(&mut self) -> Result { use crate::error::SandboxRuntimeError; - use crate::result::RunResult; let pid = self.rt().child_pid.ok_or(SandboxRuntimeError::NotRunning)?; - - // Already reaped: hand back the same status again, plus whatever the - // drains collected. They outlive the `wait()` that started them, so a - // second call still reports the output of a first one that was - // cancelled — including one cancelled while joining them here, since a - // join only unparks a drain once it has completed. A `wait()` that ran - // to the end took them with it, and both streams are then `None`. - let stopped = match self.rt().state { - RuntimeState::Stopped(ref es) => Some(es.clone()), - _ => None, - }; - if let Some(exit_status) = stopped { - let changes = self.settle_branch().await; - let (stdout, stderr) = self.collect_pipe_drains().await; - return Ok(RunResult { exit_status, stdout, stderr, changes }); + if let RuntimeState::MainExited(ref s) | RuntimeState::Stopped(ref s) = self.rt().state { + return Ok(s.clone()); } // Deliver EOF to a piped stdin the caller never took: otherwise a child @@ -997,12 +987,12 @@ impl Sandbox { // `Process::take_stdout`/`take_stderr` is `None` here and stays theirs. if self.rt().stdout_drain.is_none() { if let Some(fd) = self.rt_mut()._stdout_read.take() { - self.rt_mut().stdout_drain = sandbox_spawn_pipe_drain(fd); + self.rt_mut().stdout_drain = sandbox_spawn_pipe_drain(fd, self.rt().drain_cut.clone()); } } if self.rt().stderr_drain.is_none() { if let Some(fd) = self.rt_mut()._stderr_read.take() { - self.rt_mut().stderr_drain = sandbox_spawn_pipe_drain(fd); + self.rt_mut().stderr_drain = sandbox_spawn_pipe_drain(fd, self.rt().drain_cut.clone()); } } @@ -1026,39 +1016,53 @@ impl Sandbox { None => wait_child_exit_blocking(pid).await, } }; - // The sandbox ends with its main process: descendants left behind - // would run on with every supervised syscall failing, holding the - // caller's stdio. - let swept = self.rt().groups.signal(libc::SIGKILL); - let no_supervisor = self.no_supervisor; - if let Some(mut reaper) = self.rt_mut().reaper.take() { - // Only jobs no sweep reached can keep it alive. - if no_supervisor || swept.is_err() { reaper.finish(); } - reaper.join().await; - } - - self.rt_mut().state = RuntimeState::Stopped(exit_status.clone()); + self.rt_mut().state = RuntimeState::MainExited(exit_status.clone()); if self.rt().tty_foreground_taken { sandbox_restore_tty_foreground(self.rt().pgid); self.rt_mut().tty_foreground_taken = false; } + Ok(exit_status) + } - let rt = self.rt_mut(); - rt.control_claim = None; - if let Some(h) = rt.notif_handle.take() { h.abort(); } - rt.policy_fn_worker = None; - if let Some(h) = rt.throttle_handle.take() { h.abort(); } - if let Some(h) = rt.loadavg_handle.take() { h.abort(); } - // Finish shutting down the old endpoint before wait() returns. - if let Some(h) = rt.control_handle.take() { - h.abort(); - let _ = h.await; + /// Kill every remaining job, wait until the session has no process left, + /// then settle COW and collect the captured output. Safe to call again. + pub async fn shutdown(&mut self) -> Result { + use crate::error::SandboxRuntimeError; + use crate::result::RunResult; + + self.rt().child_pid.ok_or(SandboxRuntimeError::NotRunning)?; + if !matches!(self.rt().state, RuntimeState::Stopped(_)) { + let swept = self.controller()?.kill(); + let exit_status = self.wait().await?; + let no_supervisor = self.no_supervisor; + if let Some(mut reaper) = self.rt_mut().reaper.take() { + // Untracked jobs (no supervisor, or a group out of reach) + // would keep it alive; release them instead. + if no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join().await; + self.rt().drain_cut.cancel(); + } + self.rt_mut().state = RuntimeState::Stopped(exit_status); + + let rt = self.rt_mut(); + rt.control_claim = None; + if let Some(h) = rt.notif_handle.take() { h.abort(); } + rt.policy_fn_worker = None; + if let Some(h) = rt.throttle_handle.take() { h.abort(); } + if let Some(h) = rt.loadavg_handle.take() { h.abort(); } + // Finish shutting down the old endpoint before releasing the name. + if let Some(h) = rt.control_handle.take() { + h.abort(); + let _ = h.await; + } + swept?; } + let RuntimeState::Stopped(ref exit_status) = self.rt().state else { unreachable!() }; + let exit_status = exit_status.clone(); let changes = self.settle_branch().await; let (stdout, stderr) = self.collect_pipe_drains().await; - Ok(RunResult { exit_status, stdout, stderr, changes }) } @@ -1101,7 +1105,7 @@ impl Sandbox { /// exited non-zero. fn branch_action(&self) -> BranchAction { let failed = self.runtime.as_ref().is_some_and(|rt| { - matches!(rt.state, RuntimeState::Stopped(ref s) if !matches!(s, crate::result::ExitStatus::Code(0))) + matches!(rt.state, RuntimeState::MainExited(ref s) | RuntimeState::Stopped(ref s) if !matches!(s, crate::result::ExitStatus::Code(0))) }); if failed { self.on_error.clone() } else { self.on_exit.clone() } } @@ -1533,7 +1537,8 @@ impl Sandbox { ) -> Result { self.do_create(cmd, true).await?; self.do_start()?; - self.wait().await + self.wait().await?; + self.shutdown().await } /// Run with inherited stdio (interactive mode). @@ -1543,7 +1548,8 @@ impl Sandbox { ) -> Result { self.do_create(cmd, false).await?; self.do_start()?; - self.wait().await + self.wait().await?; + self.shutdown().await } /// One-shot run with user-supplied syscall handlers. @@ -1562,7 +1568,8 @@ impl Sandbox { self.rt_mut().handlers = pending; self.do_create(cmd, true).await?; self.do_start()?; - self.wait().await + self.wait().await?; + self.shutdown().await } /// Interactive-stdio counterpart of `run_with_handlers`. @@ -1581,7 +1588,8 @@ impl Sandbox { self.rt_mut().handlers = pending; self.do_create(cmd, false).await?; self.do_start()?; - self.wait().await + self.wait().await?; + self.shutdown().await } /// Create N COW clones of this sandbox. @@ -1709,6 +1717,8 @@ impl Sandbox { reaper: None, pgid: clone_pid, main_exit: tokio::sync::watch::channel(None).1, + stop: tokio_util::sync::CancellationToken::new(), + drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -1787,7 +1797,8 @@ impl Sandbox { unsafe { libc::close(stdin_fds[0]) }; let _ = write_handle.await; - reducer.wait().await + reducer.wait().await?; + reducer.shutdown().await } /// Whether named (pathname) `AF_UNIX` connects should be gated by the @@ -1817,6 +1828,8 @@ impl Sandbox { reaper: None, pgid: 0, main_exit: tokio::sync::watch::channel(None).1, + stop: tokio_util::sync::CancellationToken::new(), + drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -2383,6 +2396,8 @@ impl Sandbox { policy_fn_state.verdict_timeout = policy_fn.verdict_timeout; policy_fn_state.cancellation = worker.cancellation(); policy_fn_state.event_tx = Some(worker.sender()); + let (stop, worker_cancel) = (self.rt().stop.clone(), worker.cancellation()); + tokio::spawn(async move { stop.cancelled().await; worker_cancel.cancel() }); self.rt_mut().policy_fn_worker = Some(worker); } @@ -2541,12 +2556,12 @@ impl Sandbox { /// stream opened with [`StdioMode::Piped`] (each available once); the caller /// reads/writes those while the process runs. Unlike `std::process::Child`, /// this *borrows* the originating [`Sandbox`] rather than owning the process: -/// the process is killed and reaped when that `Sandbox` is dropped, or eagerly -/// via [`Process::kill`]. +/// the session is killed and reaped when that `Sandbox` is dropped, or eagerly +/// via [`Process::shutdown`]. /// -/// A `Process` that is dropped without [`Process::wait`] leaves the child -/// running until the `Sandbox` is dropped — call `wait` (or `kill`) to end it. -#[must_use = "a Process is a live confined child; call wait() (or kill()) or it runs until the Sandbox is dropped"] +/// A `Process` dropped without [`Process::shutdown`] leaves the session +/// running until the `Sandbox` is dropped. +#[must_use = "a Process is a live confined session; call shutdown() or it runs until the Sandbox is dropped"] pub struct Process<'a> { sandbox: &'a mut Sandbox, } @@ -2589,14 +2604,22 @@ impl Process<'_> { self.sandbox.kill() } - /// Wait for the child to exit. Any `Piped` stdout/stderr the caller did not - /// take is drained into the returned `RunResult`; taken streams are `None` - /// because the caller owns them. An untaken piped stdin is closed here so the - /// child sees EOF; a *taken* stdin the caller must close itself first (see - /// [`Process::take_stdin`]) or this blocks forever. - pub async fn wait(self) -> Result { + /// Wait for the main process; background jobs stay supervised. An untaken + /// piped stdin is closed here so the child sees EOF; a *taken* stdin the + /// caller must close itself first (see [`Process::take_stdin`]). + pub async fn wait(&mut self) -> Result { self.sandbox.wait().await } + + /// End the session. Any `Piped` stdout/stderr the caller did not take is + /// drained into the returned `RunResult`; taken streams are `None`. + pub async fn shutdown(self) -> Result { + self.sandbox.shutdown().await + } + + pub fn controller(&self) -> Result { + self.sandbox.controller() + } } /// Hand the terminal's foreground process group back to this process after @@ -2877,7 +2900,10 @@ enum ParkedDrain { /// dropping output. The conversion can only fail if the fd is not a readable /// pipe, which cannot happen for the pipes `do_create_stdio` creates; if it /// somehow did, the stream is left uncaptured rather than the run failing. -fn sandbox_spawn_pipe_drain(fd: std::os::fd::OwnedFd) -> Option { +fn sandbox_spawn_pipe_drain( + fd: std::os::fd::OwnedFd, + cut: tokio_util::sync::CancellationToken, +) -> Option { let mut rx = match tokio::net::unix::pipe::Receiver::from_owned_fd(fd) { Ok(rx) => rx, Err(_) => return None, @@ -2885,7 +2911,17 @@ fn sandbox_spawn_pipe_drain(fd: std::os::fd::OwnedFd) -> Option { Some(ParkedDrain::Running(tokio::spawn(async move { use tokio::io::AsyncReadExt; let mut buf = Vec::new(); - let _ = rx.read_to_end(&mut buf).await; + loop { + buf.reserve(8192); + tokio::select! { + biased; + _ = cut.cancelled() => { + while matches!(rx.try_read_buf(&mut buf), Ok(n) if n > 0) {} + break; + } + n = rx.read_buf(&mut buf) => if !matches!(n, Ok(n) if n > 0) { break }, + } + } buf }))) } diff --git a/crates/sandlock-core/src/transaction.rs b/crates/sandlock-core/src/transaction.rs index 14d8fc9e..9a9dd76e 100644 --- a/crates/sandlock-core/src/transaction.rs +++ b/crates/sandlock-core/src/transaction.rs @@ -738,7 +738,8 @@ async fn drive_txn_stages( let drain = tokio::task::spawn_blocking(move || drain_capped_tee(read_fd, STAGE_STDERR_CAP, tee)); - let mut result = sb.wait().await.map_err(at)?; + sb.wait().await.map_err(at)?; + let mut result = sb.shutdown().await.map_err(at)?; // wait() returns stderr: None on the inherit path; the captured bytes are // authoritative. result.stderr = Some(drain.await.unwrap_or_default()); diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index b56048ec..cae2073f 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -748,7 +748,7 @@ async fn test_control_parked_child_does_not_pin_other_names() { parked.create(&["true"]).await.unwrap(); first.start().unwrap(); - first.wait().await.unwrap(); + async { first.wait().await?; first.shutdown().await }.await.unwrap(); let again = policy .clone() @@ -756,7 +756,7 @@ async fn test_control_parked_child_does_not_pin_other_names() { .run(&["true"]) .await; parked.start().unwrap(); - let _ = parked.wait().await; + let _ = async { parked.wait().await?; parked.shutdown().await }.await; assert!(again.is_ok(), "a parked sibling must not pin the name: {:?}", again.err()); } @@ -773,7 +773,7 @@ async fn test_control_child_pid_is_published_before_start() { assert_eq!(pids.supervisor, std::process::id() as i32); } sb.start().unwrap(); - assert!(sb.wait().await.unwrap().success()); + assert!(async { sb.wait().await?; sb.shutdown().await }.await.unwrap().success()); } // ============================================================ diff --git a/crates/sandlock-core/tests/integration/test_fork.rs b/crates/sandlock-core/tests/integration/test_fork.rs index 363f9119..55fd2dac 100644 --- a/crates/sandlock-core/tests/integration/test_fork.rs +++ b/crates/sandlock-core/tests/integration/test_fork.rs @@ -29,7 +29,7 @@ async fn test_fork_basic() { // Each clone is a live Sandbox — wait for all for c in clones.iter_mut() { - let r = c.wait().await.unwrap(); + let r = async { c.wait().await?; c.shutdown().await }.await.unwrap(); assert!(r.success()); } @@ -61,7 +61,7 @@ async fn test_fork_cow_sharing() { }); let mut clones = sb.fork(3).await.unwrap(); - for c in clones.iter_mut() { let _ = c.wait().await; } + for c in clones.iter_mut() { let _ = async { c.wait().await?; c.shutdown().await }.await; } for i in 0..3u32 { let content = std::fs::read_to_string(out_dir.join(format!("{}", i))).unwrap_or_default(); @@ -87,7 +87,7 @@ async fn test_fork_clone_id_env() { }); let mut clones = sb.fork(3).await.unwrap(); - for c in clones.iter_mut() { let _ = c.wait().await; } + for c in clones.iter_mut() { let _ = async { c.wait().await?; c.shutdown().await }.await; } for i in 0..3u32 { let content = std::fs::read_to_string(out_dir.join(format!("{}", i))).unwrap_or_default(); @@ -142,7 +142,7 @@ async fn test_fork_clone_exit_status() { // All clones should have succeeded (already finished) for (i, c) in clones.iter_mut().enumerate() { - let r = c.wait().await.unwrap(); + let r = async { c.wait().await?; c.shutdown().await }.await.unwrap(); assert!(r.success(), "clone {} should succeed", i); } diff --git a/crates/sandlock-core/tests/integration/test_policy_fn.rs b/crates/sandlock-core/tests/integration/test_policy_fn.rs index b613f505..0ff2de66 100644 --- a/crates/sandlock-core/tests/integration/test_policy_fn.rs +++ b/crates/sandlock-core/tests/integration/test_policy_fn.rs @@ -881,7 +881,7 @@ async fn unlimited_approval_survives_old_five_second_limit() { .unwrap(); let mut sandbox = policy.clone(); sandbox.spawn(&["/bin/sleep", "0.1"]).await.unwrap(); - let result = sandbox.wait().await.unwrap(); + let result = async { sandbox.wait().await?; sandbox.shutdown().await }.await.unwrap(); assert!( result.success(), "unlimited approval must accept a late verdict" diff --git a/crates/sandlock-core/tests/integration/test_popen.rs b/crates/sandlock-core/tests/integration/test_popen.rs index ace53d22..e82729a9 100644 --- a/crates/sandlock-core/tests/integration/test_popen.rs +++ b/crates/sandlock-core/tests/integration/test_popen.rs @@ -44,7 +44,7 @@ async fn test_popen_pipe_roundtrip() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "hello\n"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); } @@ -62,7 +62,7 @@ async fn test_popen_stdout_only() { let mut out = String::new(); stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "hi\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Null stdout is actually wired to `/dev/null`, not left inherited. The child @@ -71,7 +71,7 @@ async fn test_popen_stdout_only() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_null_stdout_is_dev_null() { let mut sb = base().with_name("popen-null"); - let child = sb + let mut child = sb .popen( // Stat the *shell's* fd 1 via $$ — `$(...)` would rebind fd 1 for the // inner stat itself, so /proc/self/fd/1 there is the substitution pipe. @@ -82,7 +82,7 @@ async fn test_popen_null_stdout_is_dev_null() { ) .await .unwrap(); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success(), "fd 1 was not /dev/null (1:3): exit={:?}", res.exit_status); } @@ -100,7 +100,7 @@ async fn test_popen_stderr_piped() { let mut out = String::new(); stderr.read_to_string(&mut out).unwrap(); assert_eq!(out, "oops\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// A piped stdin the caller never takes must still reach EOF at `wait`, or a @@ -109,12 +109,12 @@ async fn test_popen_stderr_piped() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_untaken_stdin_reaches_eof() { let mut sb = base().with_name("popen-stdin-eof"); - let child = sb + let mut child = sb .popen(&["cat"], StdioMode::Piped, StdioMode::Null, StdioMode::Inherit) .await .unwrap(); // Deliberately do not take stdin. cat reads to EOF; wait must deliver it. - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success(), "cat did not exit — untaken stdin never EOF'd"); } @@ -178,7 +178,7 @@ async fn test_wait_kills_descendants_the_main_process_left_behind() { let bg_pid: i32 = line.trim().parse().expect("background pid"); assert!(unsafe { libc::kill(bg_pid, 0) } == 0, "background job should be alive"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); // A killed orphan stays a zombie until init/subreaper reaps it. @@ -225,18 +225,18 @@ async fn test_popen_all_three_piped() { stderr.read_to_string(&mut err).unwrap(); assert_eq!(out, "data\n"); assert_eq!(err, "err\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Null stdin delivers immediate EOF: `cat` reading /dev/null exits 0 at once. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_null_stdin_is_eof() { let mut sb = base().with_name("popen-null-stdin"); - let child = sb + let mut child = sb .popen(&["cat"], StdioMode::Null, StdioMode::Null, StdioMode::Inherit) .await .unwrap(); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Explicit Process::kill terminates the child; wait still collects a @@ -249,7 +249,7 @@ async fn test_popen_explicit_kill_then_wait() { .await .unwrap(); child.kill().expect("kill"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(!res.success(), "a killed process must not report success"); } @@ -258,11 +258,11 @@ async fn test_popen_explicit_kill_then_wait() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_rejects_second_spawn() { let mut sb = base().with_name("popen-twice"); - let first = sb + let mut first = sb .popen(&["echo", "first"], StdioMode::Inherit, StdioMode::Inherit, StdioMode::Inherit) .await .unwrap(); - first.wait().await.unwrap(); // consume the Process → releases the &mut borrow + async { first.wait().await?; first.shutdown().await }.await.unwrap(); // consume the Process → releases the &mut borrow let second = sb .popen(&["echo", "second"], StdioMode::Inherit, StdioMode::Inherit, StdioMode::Inherit) @@ -293,7 +293,7 @@ async fn test_popen_taken_stdout_is_not_drained_by_wait() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "out", "the taken stream must still carry the child's output"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); assert!(res.stdout.is_none(), "a taken stdout must be None in the RunResult"); assert_eq!( diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index bdc3b6aa..1ed3d1c3 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -206,3 +206,84 @@ os.rename("{0}.tmp", "{0}") let leader: i32 = std::fs::read_to_string(&pid_file).unwrap().trim().parse().unwrap(); assert!(wait_gone(leader), "session leader {leader} outlived the main process"); } + +fn alive(pid: i32) -> bool { + unsafe { libc::kill(pid, 0) == 0 } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_wait_keeps_jobs_and_shutdown_ends_them() { + use std::io::BufRead; + let mut sb = policy(); + let mut proc = sb + .popen(&["sh", "-c", "sleep 60 & echo $!; exit 3"], sandlock_core::StdioMode::Inherit, sandlock_core::StdioMode::Piped, sandlock_core::StdioMode::Inherit) + .await + .unwrap(); + let mut job = String::new(); + std::io::BufReader::new(std::fs::File::from(proc.take_stdout().unwrap())).read_line(&mut job).unwrap(); + let job: i32 = job.trim().parse().unwrap(); + assert_eq!(proc.wait().await.unwrap(), sandlock_core::ExitStatus::Code(3)); + assert!(alive(job), "the job survives wait()"); + assert!(!proc.controller().unwrap().process_groups().unwrap().is_empty()); + assert_eq!(proc.shutdown().await.unwrap().exit_status, sandlock_core::ExitStatus::Code(3)); + assert!(wait_gone(job), "shutdown() ends the job"); +} + +#[tokio::test] +async fn test_shutdown_returns_output_while_a_job_holds_stdout() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & echo main-output"]).await.unwrap(); + sb.wait().await.unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().unwrap(); + assert_eq!(result.stdout.as_deref(), Some(b"main-output\n".as_slice())); +} + +#[tokio::test] +async fn test_shutdown_ends_a_paused_session() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + sb.controller().unwrap().pause().unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_controller_kills_during_a_blocked_wait() { + let mut sb = policy(); + sb.spawn(&["sleep", "60"]).await.unwrap(); + let controller = sb.controller().unwrap(); + let killer = tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + controller.kill().unwrap(); + }); + let status = tokio::time::timeout(std::time::Duration::from_secs(5), sb.wait()).await.unwrap().unwrap(); + assert_ne!(status, sandlock_core::ExitStatus::Code(0)); + killer.await.unwrap(); + sb.shutdown().await.unwrap(); +} + +#[tokio::test] +async fn test_cancelled_wait_then_shutdown() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 0.3; echo done"]).await.unwrap(); + assert!(tokio::time::timeout(std::time::Duration::from_millis(50), sb.wait()).await.is_err()); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(0)); + assert_eq!(sb.shutdown().await.unwrap().stdout.as_deref(), Some(b"done\n".as_slice())); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_drop_after_wait_ends_jobs() { + let dir = tempfile::tempdir().unwrap(); + let pidfile = dir.path().join("job"); + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc").fs_read("/dev").fs_write(dir.path()) + .build() + .unwrap(); + sb.spawn(&["sh", "-c", &format!("sleep 60 & echo $! > {}", pidfile.display())]).await.unwrap(); + sb.wait().await.unwrap(); + let job: i32 = std::fs::read_to_string(&pidfile).unwrap().trim().parse().unwrap(); + assert!(alive(job)); + drop(sb); + assert!(wait_gone(job)); +} diff --git a/crates/sandlock-core/tests/integration/test_protection.rs b/crates/sandlock-core/tests/integration/test_protection.rs index d12ed2b9..22a6efb1 100644 --- a/crates/sandlock-core/tests/integration/test_protection.rs +++ b/crates/sandlock-core/tests/integration/test_protection.rs @@ -534,3 +534,97 @@ async fn degraded_policy_confines_and_runs_below_v6() { "degraded-mode confinement must still execute the child" ); } + +/// Kernels before 6.9 reject PIDFD_SIGNAL_PROCESS_GROUP. Re-run this test in +/// a child that emulates one, so the filter never touches the test process. +#[test] +fn shutdown_without_pidfd_group_signals() { + const CHILD: &str = "SANDLOCK_TEST_NO_PIDFD_GROUP_SIGNALS"; + if std::env::var_os(CHILD).is_none() { + let mut child = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["test_protection::shutdown_without_pidfd_group_signals", "--exact", "--nocapture"]) + .env(CHILD, "1") + .spawn() + .unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + if let Some(status) = child.try_wait().unwrap() { + assert!(status.success()); + return; + } + if std::time::Instant::now() >= deadline { + child.kill().unwrap(); + child.wait().unwrap(); + panic!("shutdown hung without pidfd group signals"); + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + + let filter = [ + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 0 }, + libc::sock_filter { code: 0x15, jt: 0, jf: 3, k: libc::SYS_pidfd_send_signal as u32 }, + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 16 + 3 * 8 }, + libc::sock_filter { code: 0x45, jt: 0, jf: 1, k: 1 << 2 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ERRNO | libc::EINVAL as u32 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ALLOW }, + ]; + let program = libc::sock_fprog { len: filter.len() as u16, filter: filter.as_ptr() as *mut _ }; + unsafe { + assert_eq!(libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), 0); + assert_eq!(libc::prctl(libc::PR_SET_SECCOMP, libc::SECCOMP_MODE_FILTER, &program), 0); + } + + let dir = tempfile::tempdir().unwrap(); + let d = dir.path().display().to_string(); + let mut builder = sandlock_core::Sandbox::builder(); + for protection in Protection::all() { + builder = builder.allow_degraded(protection); + } + let policy = builder + .fs_read("/usr") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/bin") + .fs_read("/etc") + .fs_read("/proc") + .fs_read("/dev") + .fs_write(dir.path()) + .build() + .unwrap(); + let job = |name: &str| -> i32 { + std::fs::read_to_string(dir.path().join(name)).unwrap().trim().parse().unwrap() + }; + let publish = |name: &str| format!("open('{d}/{name}.tmp','w').write(str(os.getpid())); os.rename('{d}/{name}.tmp','{d}/{name}'); signal.pause()"); + let wait_for = |name: &str| format!("while not os.path.exists('{d}/{name}'): time.sleep(0.01)"); + let gone = |pid: i32| unsafe { libc::kill(pid, 0) } != 0; + + tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap().block_on(async { + // `cmd &` stays in main's group, which the live reaper pins. + let mut sb = policy.clone(); + sb.spawn(&["sh", "-c", &format!("sleep 60 & echo $! > {d}/r; exit 7")]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(7)); + let background = job("r"); + assert_eq!(sb.shutdown().await.unwrap().exit_status, sandlock_core::ExitStatus::Code(7)); + assert!(gone(background)); + + // A setsid job whose leader lives is reached by number. + let mut sb = policy.clone(); + let script = format!("import os,signal,time\nif os.fork()==0:\n os.setsid(); {}\n{}", publish("s"), wait_for("s")); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let leader = job("s"); + sb.shutdown().await.unwrap(); + assert!(gone(leader)); + + // A double-fork daemon outlived its leader: unreachable, so reported. + let mut sb = policy.clone(); + let script = format!("import os,signal,time\nif os.fork()==0:\n os.setsid()\n if os.fork()==0:\n {}\n os._exit(0)\n{}", publish("d"), wait_for("d")); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let daemon = job("d"); + assert!(tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().is_err()); + drop(sb); + assert_eq!(unsafe { libc::kill(daemon, libc::SIGKILL) }, 0); + }); +} diff --git a/crates/sandlock-core/tests/integration/test_resource.rs b/crates/sandlock-core/tests/integration/test_resource.rs index 8371d21d..63d74de2 100644 --- a/crates/sandlock-core/tests/integration/test_resource.rs +++ b/crates/sandlock-core/tests/integration/test_resource.rs @@ -492,7 +492,7 @@ async fn test_spawn_and_kill() { sb.start().unwrap(); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + let result = async { sb.wait().await?; sb.shutdown().await }.await.unwrap(); assert!( matches!(result.exit_status, ExitStatus::Signal(_) | ExitStatus::Killed), "expected Signal or Killed, got {:?}", @@ -681,7 +681,7 @@ async fn test_pause_resume() { sb.resume().expect("resume should succeed"); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + let result = async { sb.wait().await?; sb.shutdown().await }.await.unwrap(); // Process should have been killed cleanly after pause/resume cycle assert!( diff --git a/crates/sandlock-core/tests/integration/test_restore.rs b/crates/sandlock-core/tests/integration/test_restore.rs index 7c28733a..54902dd1 100644 --- a/crates/sandlock-core/tests/integration/test_restore.rs +++ b/crates/sandlock-core/tests/integration/test_restore.rs @@ -138,7 +138,7 @@ async fn test_restore_glibc_vdso_program_resumes() { // Kill the original so only the restored process can advance the file. sb.kill().unwrap(); - let _ = sb.wait().await; + let _ = async { sb.wait().await?; sb.shutdown().await }.await; // Sentinel: prove the *restored* process (not a leftover original) is writing. std::fs::write(&counter, b"0\n").unwrap(); @@ -168,7 +168,7 @@ async fn test_restore_glibc_vdso_program_resumes() { // Clean up before asserting so a failure never leaks the child/files. let _ = sb2.kill(); - let exit = sb2.wait().await.map(|r| r.exit_status); + let exit = async { sb2.wait().await?; sb2.shutdown().await }.await.map(|r| r.exit_status); let _ = std::fs::remove_dir_all(&tmp); assert!( @@ -251,7 +251,7 @@ async fn test_restore_resumes_inside_a_chroot() { let cp = sb.checkpoint().await.unwrap(); let baseline = read_counter().expect("counter file should exist with a value"); sb.kill().unwrap(); - let _ = sb.wait().await; + let _ = async { sb.wait().await?; sb.shutdown().await }.await; std::fs::write(&counter, b"0\n").unwrap(); let mut sb2 = policy.clone().with_name("chroot-restore-dst"); @@ -267,7 +267,7 @@ async fn test_restore_resumes_inside_a_chroot() { tokio::time::sleep(std::time::Duration::from_millis(50)).await; } let _ = sb2.kill(); - let _ = sb2.wait().await; + let _ = async { sb2.wait().await?; sb2.shutdown().await }.await; let _ = std::fs::remove_dir_all(&rootfs); restored.expect("restore under chroot"); diff --git a/crates/sandlock-core/tests/integration/test_sandbox.rs b/crates/sandlock-core/tests/integration/test_sandbox.rs index fce2666d..0ba30a4d 100644 --- a/crates/sandlock-core/tests/integration/test_sandbox.rs +++ b/crates/sandlock-core/tests/integration/test_sandbox.rs @@ -180,7 +180,7 @@ async fn test_nested_sandbox() { // separately verifying that running Sandbox::run after another // Sandbox::run works (the seccomp filter stacks). outer_sb.kill().unwrap(); - let _ = outer_sb.wait().await; + let _ = async { outer_sb.wait().await?; outer_sb.shutdown().await }.await; // Sequential sandboxes: first sandbox applies Landlock + seccomp, // second sandbox from the same parent gets EBUSY on seccomp @@ -905,7 +905,7 @@ async fn test_cancelled_wait_keeps_the_capture_for_the_next_wait() { sb.kill().unwrap(); - let result = tokio::time::timeout(std::time::Duration::from_secs(30), sb.wait()) + let result = tokio::time::timeout(std::time::Duration::from_secs(30), async { sb.wait().await?; sb.shutdown().await }) .await .expect("the second wait() hung") .unwrap(); diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index fa0c91c4..cfc55d34 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1440,7 +1440,7 @@ pub unsafe extern "C" fn sandlock_handle_wait(h: *mut sandlock_handle_t) -> *mut return ptr::null_mut(); } let h = &mut *h; - match block_on_runtime(&h.runtime, h.sandbox.wait()) { + match block_on_runtime(&h.runtime, async { h.sandbox.wait().await?; h.sandbox.shutdown().await }) { Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), _ => ptr::null_mut(), } @@ -1464,7 +1464,7 @@ pub unsafe extern "C" fn sandlock_handle_wait_timeout( if timeout_ms == 0 { // No timeout -- same as sandlock_handle_wait. - return match block_on_runtime(&h.runtime, h.sandbox.wait()) { + return match block_on_runtime(&h.runtime, async { h.sandbox.wait().await?; h.sandbox.shutdown().await }) { Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), _ => ptr::null_mut(), }; @@ -1472,7 +1472,11 @@ pub unsafe extern "C" fn sandlock_handle_wait_timeout( let dur = Duration::from_millis(timeout_ms); match block_on_runtime(&h.runtime, async { - tokio::time::timeout(dur, h.sandbox.wait()).await + match tokio::time::timeout(dur, h.sandbox.wait()).await { + Ok(Ok(_)) => Ok(h.sandbox.shutdown().await), + Ok(Err(e)) => Ok(Err(e)), + Err(elapsed) => Err(elapsed), + } }) { Some(Ok(Ok(result))) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), Some(Ok(Err(_))) | None => ptr::null_mut(), @@ -2608,7 +2612,8 @@ pub unsafe extern "C" fn sandlock_wait(sb: *mut Sandbox) -> c_int { let sb = &mut *sb; match with_runtime(|rt| rt.block_on(sb.wait())) { - Some(Ok(r)) => r.code().unwrap_or(-1), + Some(Ok(ExitStatus::Code(code))) => code, + Some(Ok(_)) => -1, _ => -1, } } diff --git a/crates/sandlock-oci/src/supervisor.rs b/crates/sandlock-oci/src/supervisor.rs index 9a14e515..0c04d442 100644 --- a/crates/sandlock-oci/src/supervisor.rs +++ b/crates/sandlock-oci/src/supervisor.rs @@ -907,7 +907,7 @@ async fn serve_running( RunningCmd::Continue => {} RunningCmd::Shutdown => { let _ = sandbox.kill(); - return exit_info_from(sandbox.wait().await); + return exit_info_from(sandbox.shutdown().await); } } } @@ -928,7 +928,7 @@ async fn serve_running( async fn reap_and_collapse( sandbox: &mut sandlock_core::Sandbox, ) -> Option { - let info = exit_info_from(sandbox.wait().await); + let info = exit_info_from(sandbox.shutdown().await); let _ = sandbox.signal(libc::SIGKILL); info } From b9d01917e3f7178d83ec781033b8b6e1c8842f7f Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:21:50 -0700 Subject: [PATCH 06/18] ffi: separate main-process waits from session shutdown sandlock_handle_wait returned only when the whole session was done and killed everything left at main's exit. Killing a session from another thread meant racing the handle that the blocked wait held. sandlock_handle_wait now takes a timeout and an out-pointer: it returns 0 with main's status once main exits, or 1 if the timeout elapsed, and leaves the session running either way. sandlock_handle_shutdown ends the session and returns the full result, replacing sandlock_handle_wait_timeout. Kill, pause, resume and the new process-group listing go through a Controller stored beside the Sandbox in the handle, so they are safe while another thread is blocked in sandlock_handle_wait. Existing tests wait, then shut down. New tests kill a session from another thread during a blocked wait, and check that a timed-out wait leaves the session running. They live in their own test binary, since the popen fd-leak test counts the whole process's descriptors. Signed-off-by: Cong Wang --- crates/sandlock-ffi/include/sandlock.h | 63 +++++++--- crates/sandlock-ffi/src/lib.rs | 154 ++++++++++++++++--------- crates/sandlock-ffi/tests/changes.rs | 11 +- crates/sandlock-ffi/tests/defer.rs | 15 ++- crates/sandlock-ffi/tests/popen.rs | 24 ++-- crates/sandlock-ffi/tests/restore.rs | 14 ++- crates/sandlock-ffi/tests/session.rs | 81 +++++++++++++ 7 files changed, 277 insertions(+), 85 deletions(-) create mode 100644 crates/sandlock-ffi/tests/session.rs diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 22d83f5c..1020be11 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -70,7 +70,7 @@ enum sandlock_exit_reason */ SANDLOCK_EXIT_REASON_KILLED = 2, /** - * Killed by sandlock because it exceeded its timeout. + * A run exceeded its timeout and was killed. */ SANDLOCK_EXIT_REASON_TIMEOUT = 3, }; @@ -877,10 +877,10 @@ int sandlock_start(sandlock_handle_t *h); int32_t sandlock_handle_pid(const sandlock_handle_t *h); /** - * Send SIGKILL to the handle's entire process group. Idempotent: a process - * that already exited is not an error. Returns 0 on success, -1 on error. - * The handle remains valid; call `sandlock_handle_wait` to collect the exit - * status and `sandlock_handle_free` to release it. + * Send SIGKILL to every process group of the session. Idempotent: a session + * whose processes already exited is not an error. Returns 0 on success, -1 + * on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * * # Safety * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. @@ -888,26 +888,59 @@ int32_t sandlock_handle_pid(const sandlock_handle_t *h); int sandlock_handle_kill(sandlock_handle_t *h); /** - * Wait for the sandbox to exit. Returns a result handle with stdout/stderr. + * Send SIGSTOP to every process group of the session. Returns 0 on success, + * -1 on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * - * For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any - * piped `out_stdout_fd`/`out_stderr_fd` *before* calling this, or a child that - * reads to EOF or fills a pipe buffer never exits and this blocks forever. + * # Safety + * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. + */ +int sandlock_handle_pause(sandlock_handle_t *h); + +/** + * Send SIGCONT to every process group of the session. Returns 0 on success, + * -1 on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * * # Safety * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. */ -sandlock_result_t *sandlock_handle_wait(sandlock_handle_t *h); +int sandlock_handle_resume(sandlock_handle_t *h); /** - * Wait for the sandbox to exit with a timeout in milliseconds. - * Returns a result handle, or null on error. On timeout the sandbox is - * killed and a result with `ExitStatus::Timeout` is returned. + * Sorted ids of the session's process groups that still have members, as a + * JSON array. Free with `sandlock_string_free`. Returns null on error. Safe + * to call while another thread is blocked in `sandlock_handle_wait`. * * # Safety - * `h` must be a valid handle from `sandlock_create`. + * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. + */ +char *sandlock_handle_process_groups(const sandlock_handle_t *h); + +/** + * Wait for the main process; background jobs stay supervised. Returns 0 and + * a status-only result in `*status`, 1 if `timeout_ms` (0 waits forever) + * elapsed first and the session is untouched, or -1 on error. + * + * For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any + * piped `out_stdout_fd`/`out_stderr_fd` while waiting, or a child that reads + * to EOF or fills a pipe buffer never exits. + * + * # Safety + * `h` must be a valid handle, `status` a valid pointer, and no other call on + * `h` except kill, pause, resume and process_groups may run concurrently. + */ +int sandlock_handle_wait(sandlock_handle_t *h, uint64_t timeout_ms, sandlock_result_t **status); + +/** + * Kill what is left of the session, wait until it has no process, and return + * main's status with the captured output and filesystem changes. Returns + * null on error. + * + * # Safety + * `h` must be a valid handle, and no other call on `h` may run concurrently. */ -sandlock_result_t *sandlock_handle_wait_timeout(sandlock_handle_t *h, uint64_t timeout_ms); +sandlock_result_t *sandlock_handle_shutdown(sandlock_handle_t *h); /** * Get port mappings as a JSON string (e.g. `{"80":9001,"443":9002}`). diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index cfc55d34..59d0c64f 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1155,6 +1155,8 @@ pub unsafe extern "C" fn sandlock_run( pub struct sandlock_handle_t { sandbox: Sandbox, runtime: tokio::runtime::Runtime, + // Beside the Sandbox so signals need no access to it during a wait. + controller: Option, } /// Shared entry-point prologue for the create/popen family: validate the @@ -1218,6 +1220,7 @@ unsafe fn sandlock_create_with_runtime( } Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) @@ -1373,6 +1376,7 @@ pub unsafe extern "C" fn sandlock_popen( write_or_close_fd(out_stderr_fd, err_fd.map(|f| f.into_raw_fd()).unwrap_or(-1)); Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) @@ -1407,85 +1411,128 @@ pub unsafe extern "C" fn sandlock_handle_pid(h: *const sandlock_handle_t) -> i32 (*h).sandbox.pid().unwrap_or(0) } -/// Send SIGKILL to the handle's entire process group. Idempotent: a process -/// that already exited is not an error. Returns 0 on success, -1 on error. -/// The handle remains valid; call `sandlock_handle_wait` to collect the exit -/// status and `sandlock_handle_free` to release it. +/// Send SIGKILL to every process group of the session. Idempotent: a session +/// whose processes already exited is not an error. Returns 0 on success, -1 +/// on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// /// # Safety /// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. #[no_mangle] pub unsafe extern "C" fn sandlock_handle_kill(h: *mut sandlock_handle_t) -> c_int { - if h.is_null() { - return -1; - } - let h = &mut *h; - if h.sandbox.kill().is_err() { - return -1; + match handle_controller(h).map(|c| c.kill()) { + Some(Ok(())) => 0, + _ => -1, } - 0 } -/// Wait for the sandbox to exit. Returns a result handle with stdout/stderr. +/// Send SIGSTOP to every process group of the session. Returns 0 on success, +/// -1 on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// -/// For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any -/// piped `out_stdout_fd`/`out_stderr_fd` *before* calling this, or a child that -/// reads to EOF or fills a pipe buffer never exits and this blocks forever. +/// # Safety +/// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_pause(h: *mut sandlock_handle_t) -> c_int { + match handle_controller(h).map(|c| c.pause()) { + Some(Ok(())) => 0, + _ => -1, + } +} + +/// Send SIGCONT to every process group of the session. Returns 0 on success, +/// -1 on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// /// # Safety /// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. #[no_mangle] -pub unsafe extern "C" fn sandlock_handle_wait(h: *mut sandlock_handle_t) -> *mut sandlock_result_t { - if h.is_null() { - return ptr::null_mut(); +pub unsafe extern "C" fn sandlock_handle_resume(h: *mut sandlock_handle_t) -> c_int { + match handle_controller(h).map(|c| c.resume()) { + Some(Ok(())) => 0, + _ => -1, } - let h = &mut *h; - match block_on_runtime(&h.runtime, async { h.sandbox.wait().await?; h.sandbox.shutdown().await }) { - Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), +} + +/// Sorted ids of the session's process groups that still have members, as a +/// JSON array. Free with `sandlock_string_free`. Returns null on error. Safe +/// to call while another thread is blocked in `sandlock_handle_wait`. +/// +/// # Safety +/// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_process_groups(h: *const sandlock_handle_t) -> *mut c_char { + match handle_controller(h).map(|c| c.process_groups()) { + Some(Ok(groups)) => CString::new(serde_json::to_string(&groups).unwrap_or_default()) + .map(CString::into_raw) + .unwrap_or(ptr::null_mut()), _ => ptr::null_mut(), } } -/// Wait for the sandbox to exit with a timeout in milliseconds. -/// Returns a result handle, or null on error. On timeout the sandbox is -/// killed and a result with `ExitStatus::Timeout` is returned. +unsafe fn handle_controller<'a>(h: *const sandlock_handle_t) -> Option<&'a sandlock_core::Controller> { + if h.is_null() { None } else { (*h).controller.as_ref() } +} + +/// Wait for the main process; background jobs stay supervised. Returns 0 and +/// a status-only result in `*status`, 1 if `timeout_ms` (0 waits forever) +/// elapsed first and the session is untouched, or -1 on error. +/// +/// For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any +/// piped `out_stdout_fd`/`out_stderr_fd` while waiting, or a child that reads +/// to EOF or fills a pipe buffer never exits. /// /// # Safety -/// `h` must be a valid handle from `sandlock_create`. +/// `h` must be a valid handle, `status` a valid pointer, and no other call on +/// `h` except kill, pause, resume and process_groups may run concurrently. #[no_mangle] -pub unsafe extern "C" fn sandlock_handle_wait_timeout( +pub unsafe extern "C" fn sandlock_handle_wait( h: *mut sandlock_handle_t, timeout_ms: u64, -) -> *mut sandlock_result_t { + status: *mut *mut sandlock_result_t, +) -> c_int { + if h.is_null() || status.is_null() { + return -1; + } + *status = ptr::null_mut(); + let sandbox = &mut (*h).sandbox; + let outcome = block_on_runtime(&(*h).runtime, async { + if timeout_ms == 0 { + sandbox.wait().await.map(Some) + } else { + match tokio::time::timeout(Duration::from_millis(timeout_ms), sandbox.wait()).await { + Ok(r) => r.map(Some), + Err(_) => Ok(None), + } + } + }); + match outcome { + Some(Ok(Some(exit_status))) => { + *status = Box::into_raw(Box::new(sandlock_result_t { + _private: RunResult { exit_status, stdout: None, stderr: None, changes: Vec::new() }, + })); + 0 + } + Some(Ok(None)) => 1, + _ => -1, + } +} + +/// Kill what is left of the session, wait until it has no process, and return +/// main's status with the captured output and filesystem changes. Returns +/// null on error. +/// +/// # Safety +/// `h` must be a valid handle, and no other call on `h` may run concurrently. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_shutdown(h: *mut sandlock_handle_t) -> *mut sandlock_result_t { if h.is_null() { return ptr::null_mut(); } let h = &mut *h; - - if timeout_ms == 0 { - // No timeout -- same as sandlock_handle_wait. - return match block_on_runtime(&h.runtime, async { h.sandbox.wait().await?; h.sandbox.shutdown().await }) { - Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), - _ => ptr::null_mut(), - }; - } - - let dur = Duration::from_millis(timeout_ms); - match block_on_runtime(&h.runtime, async { - match tokio::time::timeout(dur, h.sandbox.wait()).await { - Ok(Ok(_)) => Ok(h.sandbox.shutdown().await), - Ok(Err(e)) => Ok(Err(e)), - Err(elapsed) => Err(elapsed), - } - }) { - Some(Ok(Ok(result))) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), - Some(Ok(Err(_))) | None => ptr::null_mut(), - Some(Err(_)) => { - // Timeout -- kill the process and return a timeout result. - let _ = h.sandbox.kill(); - let result = RunResult::timeout(); - Box::into_raw(Box::new(sandlock_result_t { _private: result })) - } + match block_on_runtime(&h.runtime, h.sandbox.shutdown()) { + Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), + _ => ptr::null_mut(), } } @@ -1643,7 +1690,7 @@ pub enum sandlock_exit_reason_t { Signaled = 1, /// Killed with no recoverable signal number. Killed = 2, - /// Killed by sandlock because it exceeded its timeout. + /// A run exceeded its timeout and was killed. Timeout = 3, } @@ -2957,6 +3004,7 @@ pub unsafe extern "C" fn sandlock_restore_interactive( return ptr::null_mut(); } Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs index 562d7ce0..09330a18 100644 --- a/crates/sandlock-ffi/tests/changes.rs +++ b/crates/sandlock-ffi/tests/changes.rs @@ -10,7 +10,6 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_entry_kind_t, sandlock_entry_t, sandlock_handle_free, - sandlock_handle_wait, sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_change_target, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, @@ -94,7 +93,7 @@ fn both_sides_of_every_change_are_readable() { unsafe { sandlock_sandbox_free(policy) }; assert!(!h.is_null()); assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(unsafe { sandlock_result_success(r) }); let n = unsafe { sandlock_result_changes_len(r) }; @@ -137,3 +136,11 @@ fn both_sides_of_every_change_are_readable() { unsafe { sandlock_result_free(r) }; unsafe { sandlock_handle_free(h) }; } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/defer.rs b/crates/sandlock-ffi/tests/defer.rs index 381c1c45..104adea1 100644 --- a/crates/sandlock-ffi/tests/defer.rs +++ b/crates/sandlock-ffi/tests/defer.rs @@ -10,8 +10,7 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_handle_abort, sandlock_handle_commit, sandlock_handle_free, - sandlock_handle_pending, sandlock_handle_upper_dir, sandlock_handle_wait, - sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_changes_len, + sandlock_handle_pending, sandlock_handle_upper_dir, sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, sandlock_sandbox_builder_fs_write, @@ -57,7 +56,7 @@ fn run_deferred(workdir: &Path, storage: &Path) -> *mut sandlock_ffi::sandlock_h unsafe { sandlock_sandbox_free(policy) }; assert!(!h.is_null(), "create failed"); assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(!r.is_null(), "wait failed"); assert!(unsafe { sandlock_result_success(r) }); @@ -131,7 +130,7 @@ fn a_committing_handle_is_never_pending() { let h = unsafe { sandlock_create_for_run(policy, ptr::null(), av.as_ptr(), av.len() as c_uint) }; unsafe { sandlock_sandbox_free(policy) }; assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(unsafe { sandlock_result_success(r) }); assert_eq!(unsafe { sandlock_result_changes_len(r) }, 1); unsafe { sandlock_result_free(r) }; @@ -141,3 +140,11 @@ fn a_committing_handle_is_never_pending() { assert_eq!(std::fs::read_to_string(workdir.path().join("out.txt")).unwrap(), "hi\n"); unsafe { sandlock_handle_free(h) }; } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/popen.rs b/crates/sandlock-ffi/tests/popen.rs index a0740c4e..20e1a4df 100644 --- a/crates/sandlock-ffi/tests/popen.rs +++ b/crates/sandlock-ffi/tests/popen.rs @@ -11,7 +11,7 @@ use std::os::raw::{c_char, c_int, c_uint}; use std::ptr; use sandlock_ffi::{ - sandlock_handle_free, sandlock_handle_kill, sandlock_handle_wait, sandlock_popen, + sandlock_handle_free, sandlock_handle_kill, sandlock_popen, sandlock_result_exit_code, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_new, sandlock_sandbox_free, sandlock_sandbox_t, @@ -76,7 +76,7 @@ fn popen_streams_stdout_and_collects_exit() { .unwrap(); assert_eq!(out, "ffi-hi\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert!(!res.is_null()); assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { @@ -120,7 +120,7 @@ fn popen_stdin_stdout_roundtrip() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "ping\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -153,7 +153,7 @@ fn popen_null_mode_yields_no_fd() { }; assert!(!h.is_null()); assert_eq!(fd_out, -1, "Null stdout yields no caller fd"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -194,7 +194,7 @@ fn popen_stderr_piped_through_ffi() { .unwrap(); assert_eq!(out, "err\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -230,7 +230,7 @@ fn popen_kill_then_wait_then_free() { assert!(!h.is_null()); assert_eq!(unsafe { sandlock_handle_kill(h) }, 0, "kill should succeed"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert!(!res.is_null(), "wait must still return after kill"); assert!( !unsafe { sandlock_result_success(res) }, @@ -307,7 +307,7 @@ fn popen_kill_is_idempotent() { 0, "second kill must be idempotent" ); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; unsafe { sandlock_result_free(res); sandlock_handle_free(h); @@ -389,7 +389,7 @@ fn popen_piped_stream_with_null_out_ptr_is_closed_not_leaked() { ); assert_eq!(fd_in, -1, "stdin inherited → no fd"); assert_eq!(fd_err, -1, "stderr inherited → no fd"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; unsafe { sandlock_result_free(res); sandlock_handle_free(h); @@ -410,3 +410,11 @@ fn popen_piped_stream_with_null_out_ptr_is_closed_not_leaked() { stream with a null out-pointer is leaking its fd instead of being closed" ); } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/restore.rs b/crates/sandlock-ffi/tests/restore.rs index 2542174b..f4840569 100644 --- a/crates/sandlock-ffi/tests/restore.rs +++ b/crates/sandlock-ffi/tests/restore.rs @@ -16,7 +16,7 @@ use sandlock_ffi::{ sandlock_checkpoint_free, sandlock_handle_checkpoint, sandlock_handle_free, sandlock_handle_kill, sandlock_handle_pid, sandlock_handle_restore_skipped_fd, sandlock_handle_restore_skipped_len, sandlock_handle_restore_skipped_path, - sandlock_handle_wait, sandlock_result_free, sandlock_restore_interactive, + sandlock_result_free, sandlock_restore_interactive, sandlock_sandbox_build, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_write, sandlock_sandbox_builder_new, sandlock_sandbox_free, sandlock_sandbox_t, sandlock_start, sandlock_string_free, @@ -149,7 +149,7 @@ fn restore_interactive_resumes_via_c_abi() { assert!(baseline > 2, "counter should have advanced, got {baseline}"); assert_eq!(unsafe { sandlock_handle_kill(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; if !r.is_null() { unsafe { sandlock_result_free(r) }; } @@ -197,7 +197,7 @@ fn restore_interactive_resumes_via_c_abi() { // Clean up before asserting so a failure never leaks the child/files. unsafe { sandlock_handle_kill(h2); - let r2 = sandlock_handle_wait(h2); + let r2 = finish(h2); if !r2.is_null() { sandlock_result_free(r2); } @@ -225,3 +225,11 @@ fn restore_skipped_accessors_tolerate_null_handle() { assert_eq!(unsafe { sandlock_handle_restore_skipped_fd(ptr::null(), 0) }, -1); assert!(unsafe { sandlock_handle_restore_skipped_path(ptr::null(), 0) }.is_null()); } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/session.rs b/crates/sandlock-ffi/tests/session.rs new file mode 100644 index 00000000..7035e44b --- /dev/null +++ b/crates/sandlock-ffi/tests/session.rs @@ -0,0 +1,81 @@ +//! Session lifetime through the C ABI: a wait that keeps the session, and +//! signals from another thread while that wait is blocked. + +use std::ffi::{CStr, CString}; +use std::os::raw::{c_char, c_int, c_uint}; +use std::ptr; + +use sandlock_ffi::{ + sandlock_handle_free, sandlock_handle_kill, sandlock_handle_process_groups, + sandlock_handle_shutdown, sandlock_handle_t, sandlock_handle_wait, sandlock_popen, + sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, + sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_new, sandlock_sandbox_free, + sandlock_sandbox_t, sandlock_string_free, +}; + +const INHERIT: u32 = 0; + +fn build_policy() -> *mut sandlock_sandbox_t { + let mut b = sandlock_sandbox_builder_new(); + for p in ["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc", "/dev"] { + if p == "/lib64" && !std::path::Path::new("/lib64").exists() { continue; } + let c = CString::new(p).unwrap(); + b = unsafe { sandlock_sandbox_builder_fs_read(b, c.as_ptr()) }; + } + let mut err: c_int = 0; + let policy = unsafe { sandlock_sandbox_build(b, &mut err, ptr::null_mut()) }; + assert_eq!(err, 0, "policy build failed"); + policy +} + +fn popen(policy: *mut sandlock_sandbox_t, cmd: &[&str]) -> *mut sandlock_handle_t { + let owned: Vec = cmd.iter().map(|s| CString::new(*s).unwrap()).collect(); + let av: Vec<*const c_char> = owned.iter().map(|c| c.as_ptr()).collect(); + let (mut fd_in, mut fd_out, mut fd_err): (c_int, c_int, c_int) = (-1, -1, -1); + let h = unsafe { + sandlock_popen(policy, ptr::null(), av.as_ptr(), av.len() as c_uint, INHERIT, INHERIT, INHERIT, + &mut fd_in, &mut fd_out, &mut fd_err) + }; + assert!(!h.is_null(), "sandlock_popen returned null"); + h +} + +#[test] +fn kill_reaches_a_blocked_wait() { + let policy = build_policy(); + let h = popen(policy, &["sleep", "60"]); + let addr = h as usize; + let killer = std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(100)); + unsafe { sandlock_handle_kill(addr as *mut sandlock_handle_t) } + }); + let mut status = ptr::null_mut(); + assert_eq!(unsafe { sandlock_handle_wait(h, 5000, &mut status) }, 0); + assert_eq!(killer.join().unwrap(), 0); + assert_eq!(unsafe { sandlock_result_success(status) }, false); + unsafe { + sandlock_result_free(status); + sandlock_result_free(sandlock_handle_shutdown(h)); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} + +#[test] +fn wait_timeout_leaves_the_session_alive() { + let policy = build_policy(); + let h = popen(policy, &["sleep", "60"]); + let mut status = ptr::null_mut(); + assert_eq!(unsafe { sandlock_handle_wait(h, 50, &mut status) }, 1); + assert!(status.is_null()); + let groups = unsafe { sandlock_handle_process_groups(h) }; + assert!(!groups.is_null()); + let listed = unsafe { CStr::from_ptr(groups) }.to_string_lossy().into_owned(); + assert_ne!(listed, "[]", "the session is still running"); + unsafe { + sandlock_string_free(groups); + sandlock_result_free(sandlock_handle_shutdown(h)); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} From 95f0b98fbe3182664b113d4c6ef1063c573389a1 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:27:29 -0700 Subject: [PATCH 07/18] python: wait for main, shut down the session explicitly Sandbox.wait() and Process.wait() freed the handle and returned the full Result, so background jobs died with the main process. Kill, pause and resume signalled main's pid by number, which after this change can be reused while the session lives on. wait() now returns an ExitStatus when main exits, keeps the session, and raises TimeoutError on a timeout without killing anything. shutdown() ends the session and returns the Result, and process_groups() lists what is left. Signals go through the native controller instead of os.killpg. Every native call except a wait runs under a lock, and waits only count themselves in and out, so a signal from another thread reaches a blocked wait while nothing can use a handle that shutdown() is freeing. Leaving the context or dropping a Process shuts it down. Existing tests now wait and then shut down where they read output, and expect a wait timeout to raise. New session tests cover jobs surviving wait(), a timed-out wait, killing from another thread, shutdown() ending a blocked wait, context exit ending leftover jobs, and run() timeouts. All 443 Python tests pass. Signed-off-by: Cong Wang --- python/README.md | 20 +- python/src/sandlock/__init__.py | 3 +- python/src/sandlock/_sdk.py | 26 +- python/src/sandlock/sandbox.py | 474 ++++++++++++++++---------------- python/tests/test_lifecycle.py | 6 +- python/tests/test_popen.py | 45 +-- python/tests/test_sandbox.py | 7 +- python/tests/test_session.py | 88 ++++++ 8 files changed, 394 insertions(+), 275 deletions(-) create mode 100644 python/tests/test_session.py diff --git a/python/README.md b/python/README.md index edf37a3a..6b0d538b 100644 --- a/python/README.md +++ b/python/README.md @@ -331,7 +331,7 @@ result = sandbox.run(["python3", "-c", "print(42)"], timeout=10.0) #### `sandbox.spawn(cmd) -> None` Spawn `cmd` without waiting. Use `pid`, `pause()`, `resume()`, `kill()`, -and `wait()` to manage the process lifecycle. +`wait()` and `shutdown()` to manage the session. Raises `RuntimeError` if a process is already running. @@ -353,9 +353,21 @@ Release a previously `create()`d child to `execve`. Raises `RuntimeError` if no child has been created. -#### `sandbox.wait() -> Result` +#### `sandbox.wait(timeout=None) -> ExitStatus` -Wait for the running process to finish and return its `Result`. +Wait for the main process and return its `ExitStatus`. Background jobs it +started keep running under supervision. A `timeout` raises `TimeoutError` +and leaves the session running. + +#### `sandbox.shutdown() -> Result` + +Kill whatever is left of the session, wait until it has no process, and +return the `Result` with captured output and filesystem changes. + +`kill()`, `pause()`, `resume()` and `process_groups()` work from any +thread, including while another thread is blocked in `wait()`. A +`popen()` `Process` has the same `wait()`, `shutdown()` and +`process_groups()`. #### Inspecting and deferring COW changes @@ -419,7 +431,7 @@ Only contains entries where port remapping occurred. Requires `port_remap=True`. #### `sandbox.pause()` / `sandbox.resume()` / `sandbox.kill()` -Send SIGSTOP / SIGCONT / SIGKILL to the sandbox process group. +Send SIGSTOP / SIGCONT / SIGKILL to every process group of the session. Raises `RuntimeError` if the sandbox is not running. #### `sandbox.checkpoint(save_fn=None) -> Checkpoint` diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index e4fd45bb..e79384b5 100644 --- a/python/src/sandlock/__init__.py +++ b/python/src/sandlock/__init__.py @@ -7,7 +7,7 @@ from ._version import __version__ from ._sdk import ( - Stage, Pipeline, Result, ExitReason, SyscallEvent, PolicyContext, Checkpoint, SkippedFd, + Stage, Pipeline, Result, ExitReason, ExitStatus, SyscallEvent, PolicyContext, Checkpoint, SkippedFd, NamedStage, Gather, GatherPipeline, Protection, landlock_abi_version, min_landlock_abi, confine, pull_image, @@ -46,6 +46,7 @@ "Pipeline", "Result", "ExitReason", + "ExitStatus", "SyscallEvent", "PolicyContext", "Checkpoint", diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index 01b973d8..e8ddb41b 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -313,11 +313,18 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_handle_pid.restype = ctypes.c_int _lib.sandlock_handle_pid.argtypes = [_c_handle_p] -_lib.sandlock_handle_wait.restype = _c_result_p -_lib.sandlock_handle_wait.argtypes = [_c_handle_p] +_lib.sandlock_handle_wait.restype = ctypes.c_int +_lib.sandlock_handle_wait.argtypes = [_c_handle_p, ctypes.c_uint64, ctypes.POINTER(_c_result_p)] -_lib.sandlock_handle_wait_timeout.restype = _c_result_p -_lib.sandlock_handle_wait_timeout.argtypes = [_c_handle_p, ctypes.c_uint64] +_lib.sandlock_handle_shutdown.restype = _c_result_p +_lib.sandlock_handle_shutdown.argtypes = [_c_handle_p] + +_lib.sandlock_handle_process_groups.restype = ctypes.c_void_p +_lib.sandlock_handle_process_groups.argtypes = [_c_handle_p] + +for _name in ("sandlock_handle_kill", "sandlock_handle_pause", "sandlock_handle_resume"): + getattr(_lib, _name).restype = ctypes.c_int + getattr(_lib, _name).argtypes = [_c_handle_p] _lib.sandlock_handle_free.restype = None _lib.sandlock_handle_free.argtypes = [_c_handle_p] @@ -904,7 +911,16 @@ class ExitReason(IntEnum): KILLED = 2 """Killed with no recoverable signal number.""" TIMEOUT = 3 - """Killed by sandlock because it exceeded its timeout.""" + """A run exceeded its timeout and was killed.""" + + +@dataclass(frozen=True) +class ExitStatus: + """Main-process status from ``wait()``; the session may still be running.""" + success: bool + exit_code: int + reason: ExitReason + signal: int = -1 @dataclass diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 2e139313..5467f0a0 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -12,6 +12,7 @@ import itertools import os import re +import threading import weakref from dataclasses import dataclass, field from enum import Enum, IntEnum @@ -534,6 +535,12 @@ def __post_init__(self): # own handle (see `_popen_process`), this is only a # non-owning busy marker self._restore_skipped = [] # SkippedFd entries from the last restore + self._pid = None + # Every native call on _handle except a wait runs under this lock, so + # none can use a handle that shutdown() is freeing; waits only count + # themselves in and out, so a signal can reach a blocked wait. + self._idle = threading.Condition() + self._waiting = 0 @staticmethod def _next_name() -> str: @@ -617,25 +624,14 @@ def __enter__(self): return self def __exit__(self, exc_type, exc_val, exc_tb): - # Reap a still-running popen() Process (it owns its own handle) so leaving - # the sandbox context never strands a confined child. proc = self._popen_process() if proc is not None and proc._handle is not None: - try: - proc.kill() - except Exception: - pass - try: - proc.wait() - except Exception: - pass + proc.__exit__(exc_type, exc_val, exc_tb) if self._handle is not None: - from ._sdk import _lib try: - _lib.sandlock_handle_free(self._handle) + self.shutdown() except Exception: pass - self._handle = None # Freeing an undecided branch preserves it, same as KEEP. self._release_pending() return False @@ -653,10 +649,7 @@ def pid(self) -> int | None: # The Process owns the handle; its pid is lock-guarded and cached, so # reading it can't race a concurrent wait() freeing the handle. return proc.pid - if self._handle is None: - return None - from ._sdk import _lib - return _lib.sandlock_handle_pid(self._handle) or None + return self._pid if self._handle is not None else None @property def is_running(self) -> bool: @@ -748,7 +741,7 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): killed and a timeout result is returned if exceeded. None means no timeout. """ - from ._sdk import _lib, _make_argv, _read_result_bytes, _read_result_changes, Result, ExitReason + from ._sdk import _lib, _make_argv, Result, ExitReason self._check_not_running() @@ -772,34 +765,19 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): handle, self._handle = self._handle, None try: - # None -> wait forever (0). A finite timeout clamps up to 1ms so - # timeout=0 / sub-ms don't collapse to 0 (= wait forever). - timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 - result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) + try: + _wait(handle, timeout) + timed_out = False + except TimeoutError: + timed_out = True + result = _take_result(_lib.sandlock_handle_shutdown(handle)) + except RuntimeError as e: + return Result(success=False, exit_code=-1, error=str(e)) finally: self._park_or_free(handle) - - if not result_p: - return Result(success=False, exit_code=-1, error="sandlock_handle_wait failed") - - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - stdout = _read_result_bytes(result_p, _lib.sandlock_result_stdout_bytes) - stderr = _read_result_bytes(result_p, _lib.sandlock_result_stderr_bytes) - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) - - return Result( - success=bool(success), - exit_code=exit_code, - reason=reason, - signal=signal, - stdout=stdout, - stderr=stderr, - changes=changes, - ) + if timed_out: + result.exit_code, result.success, result.reason, result.signal = -1, False, ExitReason.TIMEOUT, -1 + return result def run_with_handlers( self, @@ -1014,6 +992,7 @@ def create(self, cmd: Sequence[str]) -> None: ) if not self._handle: raise RuntimeError("sandlock_create failed") + self._pid = _lib.sandlock_handle_pid(self._handle) or None def start(self) -> None: """Release a previously ``create()``d child to ``execve`` the @@ -1024,12 +1003,13 @@ def start(self) -> None: """ from ._sdk import _lib - if self._handle is None: - raise RuntimeError("sandbox has not been created") - if _lib.sandlock_start(self._handle) != 0: - _lib.sandlock_handle_free(self._handle) - self._handle = None - raise RuntimeError("sandlock_start failed") + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox has not been created") + if _lib.sandlock_start(self._handle) != 0: + _lib.sandlock_handle_free(self._handle) + self._handle = None + raise RuntimeError("sandlock_start failed") def spawn(self, cmd: Sequence[str]) -> None: """Spawn ``cmd`` in the sandbox without waiting for it to finish. @@ -1044,47 +1024,57 @@ def spawn(self, cmd: Sequence[str]) -> None: self.create(cmd) self.start() - def wait(self): - """Wait for the running process to finish and return its Result. + def wait(self, timeout: float | None = None) -> "ExitStatus": + """Wait for the main process and return its :class:`ExitStatus`. + + Background jobs stay supervised until :meth:`shutdown`. A timeout + raises ``TimeoutError`` and leaves the session running. Repeated + calls return the same status. Raises: - RuntimeError: If the sandbox is not running, or if it is running a - :meth:`popen` :class:`Process` (wait on that Process instead — - freeing its handle here would break it). + RuntimeError: If the sandbox is not running, or a :meth:`popen` + :class:`Process` owns the child (wait on that Process instead). """ - from ._sdk import _lib, _read_result_bytes, _read_result_changes, Result, ExitReason - self._reject_if_popen() - if self._handle is None: - raise RuntimeError("sandbox is not running") - - handle, self._handle = self._handle, None + with self._idle: + handle = self._handle + if handle is None: + raise RuntimeError("sandbox is not running") + self._waiting += 1 try: - result_p = _lib.sandlock_handle_wait_timeout(handle, 0) + return _wait(handle, timeout) finally: - self._park_or_free(handle) - - if not result_p: - return Result(success=False, exit_code=-1, error="sandlock_handle_wait failed") - - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - stdout = _read_result_bytes(result_p, _lib.sandlock_result_stdout_bytes) - stderr = _read_result_bytes(result_p, _lib.sandlock_result_stderr_bytes) - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) + with self._idle: + self._waiting -= 1 + self._idle.notify_all() + + def shutdown(self) -> "Result": + """Kill what is left of the session, wait until it has no process, and + return the :class:`Result` with captured output and changes. A wait + blocked on another thread is ended first.""" + from ._sdk import _lib + self._reject_if_popen() + with self._idle: + handle = self._handle + if handle is None: + raise RuntimeError("sandbox is not running") + if self._waiting: + _send_signal(handle, "sandlock_handle_kill") + while self._waiting: + self._idle.wait() + self._handle = None + try: + return _take_result(_lib.sandlock_handle_shutdown(handle)) + finally: + self._park_or_free(handle) - return Result( - success=bool(success), - exit_code=exit_code, - reason=reason, - signal=signal, - stdout=stdout, - stderr=stderr, - changes=changes, - ) + def process_groups(self) -> list[int]: + """Sorted ids of the session's process groups that still have members.""" + self._reject_if_popen() + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running") + return _process_groups(self._handle) def popen( self, @@ -1288,46 +1278,28 @@ def reduce(self, cmd: list[str], fork_result: "ForkResult") -> "Result": # Lifecycle methods # ------------------------------------------------------------------ - def pause(self) -> None: - """Send SIGSTOP to the sandbox process group. - - Raises RuntimeError if a :meth:`popen` Process owns the child (manage it - through the Process).""" - import signal + def _signal(self, name: str) -> None: self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - os.killpg(pid, signal.SIGSTOP) + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running") + _send_signal(self._handle, name) - def resume(self) -> None: - """Send SIGCONT to the sandbox process group. + def pause(self) -> None: + """Send SIGSTOP to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child.""" + self._signal("sandlock_handle_pause") - Raises RuntimeError if a :meth:`popen` Process owns the child.""" - import signal - self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - os.killpg(pid, signal.SIGCONT) + def resume(self) -> None: + """Send SIGCONT to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child.""" + self._signal("sandlock_handle_resume") def kill(self) -> None: - """Send SIGKILL to the sandbox process group. - - Raises RuntimeError if a :meth:`popen` Process owns the child (call + """Send SIGKILL to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child (call ``proc.kill()`` instead).""" - import signal - self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - try: - os.killpg(pid, signal.SIGKILL) - except ProcessLookupError: - pass + self._signal("sandlock_handle_kill") def ports(self) -> dict[int, int]: """Return current port mappings {virtual_port: real_port}. @@ -1340,11 +1312,13 @@ def ports(self) -> dict[int, int]: proc = self._popen_process() if proc is not None: # The Process owns the handle; its ports() reads it under the lock so - # this can't race a concurrent wait() freeing it. + # this can't race a concurrent shutdown() freeing it. return proc.ports() - if self._handle is None: - return {} - return _read_port_mappings(self._handle) + with self._idle: + # The wait holds the native sandbox; only signals may run beside it. + if self._handle is None or self._waiting: + return {} + return _read_port_mappings(self._handle) def checkpoint( self, @@ -1373,9 +1347,12 @@ def checkpoint( # would fight the streaming Process for it, so reject rather than reach # past it — consistent with wait/pause/resume/kill. self._reject_if_popen() - if self._handle is None: - raise RuntimeError("sandbox is not running (use start() or run() first)") - ptr = _lib.sandlock_handle_checkpoint(self._handle) + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running (use start() or run() first)") + if self._waiting: + raise RuntimeError("cannot checkpoint while a wait() is in progress") + ptr = _lib.sandlock_handle_checkpoint(self._handle) if not ptr: raise RuntimeError("checkpoint capture failed") cp = Checkpoint(ptr) @@ -1424,8 +1401,9 @@ def restore_interactive(self, cp: "Checkpoint") -> None: if not handle: raise RuntimeError("checkpoint restore failed") self._handle = handle + self._pid = _lib.sandlock_handle_pid(handle) or None # Copy the skipped-fd diagnostics out eagerly: the native entries live - # on the handle, which wait() frees. + # on the handle, which shutdown() frees. skipped = [] for i in range(_lib.sandlock_handle_restore_skipped_len(handle)): fd = _lib.sandlock_handle_restore_skipped_fd(handle, i) @@ -1534,17 +1512,17 @@ class Process: :meth:`Sandbox.popen`. ``.stdin`` / ``.stdout`` / ``.stderr`` are unbuffered binary file objects - for streams opened with :attr:`StdioMode.PIPED`, else ``None``. The process - keeps running until :meth:`wait` (or :meth:`kill`) is called, or the - ``Process`` context manager exits. Prefer the context manager, which closes - the streams and reaps the child even on error:: + for streams opened with :attr:`StdioMode.PIPED`, else ``None``. The session + keeps running until :meth:`shutdown` is called or the ``Process`` context + manager exits. Prefer the context manager, which closes the streams and + reaps the session even on error:: with sandbox.popen(["cat"], stdin=StdioMode.PIPED, stdout=StdioMode.PIPED) as proc: proc.stdin.write(b"hi\\n") proc.stdin.close() # EOF so cat exits data = proc.stdout.read() - result = proc.wait() + status = proc.wait() """ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, stderr_fd: int): @@ -1553,11 +1531,10 @@ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, st from ._sdk import _lib self._sandbox = sandbox - # _lock guards _handle (and the _waiting reservation) so kill()/pid on - # one thread can't observe or act on a handle wait() is freeing on - # another — the "kill from another thread while wait() blocks" pattern. - self._lock = threading.Lock() - self._waiting = False + # As on Sandbox: native calls other than a wait run under the lock, and + # waits only count themselves in and out. + self._idle = threading.Condition() + self._waiting = 0 # The handle and pid are set only AFTER the fds are wrapped below: if an # fdopen raises, this Process never takes ownership, so popen()'s caller # frees the handle exactly once (no double free via __del__). @@ -1590,145 +1567,100 @@ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, st pass raise self.stdin, self.stdout, self.stderr = opened - # Take ownership now that construction can no longer fail. Cache the pid - # so kill()/pid never dereference the handle (avoiding a race with a - # concurrent wait() free) — they signal by pid, like the sandbox and the - # Go binding. + # Take ownership now that construction can no longer fail. self._pid = _lib.sandlock_handle_pid(handle) or -1 self._handle = handle @property def pid(self) -> int | None: - """The child PID while running, else ``None`` (after :meth:`wait`).""" - with self._lock: + """The child PID until :meth:`shutdown`, else ``None``.""" + with self._idle: return self._pid if self._handle is not None and self._pid > 0 else None def ports(self) -> dict[int, int]: """Current virtual→real port mappings while running, else ``{}`` (needs - ``port_remap``). Reads the handle under the lock, and reports ``{}`` - while a :meth:`wait` holds it, so it can't race the wait's free.""" - with self._lock: + ``port_remap``); ``{}`` too while a :meth:`wait` holds the session.""" + with self._idle: if self._handle is None or self._waiting: return {} return _read_port_mappings(self._handle) def kill(self) -> None: - """SIGKILL the child's entire process group by pid (like the sandbox and - the Go binding), so a kill from another thread is safe while :meth:`wait` - blocks on the handle. Idempotent: a child that already exited is not an - error, and after :meth:`wait` has reaped it this is a no-op. - - Raises: - RuntimeError: If the process has no pid (never a valid popen child). - OSError: If the kill genuinely fails (e.g. EPERM) — a - ``ProcessLookupError`` (already-exited group) is swallowed. - """ - import os - import signal - - with self._lock: + """SIGKILL every process group of the session; safe from another thread + while :meth:`wait` blocks. A no-op after :meth:`shutdown`.""" + with self._idle: + if self._handle is not None: + _send_signal(self._handle, "sandlock_handle_kill") + + def process_groups(self) -> list[int]: + """Sorted ids of the session's process groups that still have members.""" + with self._idle: if self._handle is None: - return # already reaped by wait() — no-op - if self._pid <= 0: - raise RuntimeError("process has no pid") - pid = self._pid - try: - os.killpg(pid, signal.SIGKILL) - except ProcessLookupError: - pass # the group already exited — idempotent no-op + raise RuntimeError("process is not running") + return _process_groups(self._handle) - def wait(self, timeout: float | None = None) -> "Result": - """Wait for the child to exit and return its :class:`Result`. + def wait(self, timeout: float | None = None) -> "ExitStatus": + """Wait for the main process and return its :class:`ExitStatus`. A still-open piped ``stdin`` is closed first so a child that reads to EOF - can exit; piped ``stdout``/``stderr`` stay open for the caller to finish - reading. Frees the underlying handle; a second call returns the cached - result. - - The returned ``Result`` carries no ``stdout``/``stderr`` (those were - streamed to you as fds — read them off ``.stdout``/``.stderr``). - - Args: - timeout: Maximum seconds to wait. On timeout the child is killed and - a non-success ``Result`` is returned (same semantics as - :meth:`Sandbox.run`'s ``timeout``) — the wait never hangs. ``None`` - (default) waits indefinitely. Note: without a ``timeout``, a piped - ``stdout``/``stderr`` you have not drained can block the child on a - full pipe and hang the wait forever; pass a ``timeout`` or drain first. + can exit. Background jobs stay supervised until :meth:`shutdown`. A + timeout raises ``TimeoutError`` and leaves the session running. """ - from ._sdk import _lib, _read_result_changes, Result, ExitReason - - # Reserve the handle under the lock so a concurrent kill()/pid sees a - # consistent state, then run the blocking wait WITHOUT the lock so kill() - # (which signals by pid, not through the handle) can interrupt it. - with self._lock: + from ._sdk import ExitStatus + with self._idle: handle = self._handle if handle is None: if self._result is not None: - return self._result + return ExitStatus(self._result.success, self._result.exit_code, + self._result.reason, self._result.signal) raise RuntimeError("process is not running") - if self._waiting: - raise RuntimeError("wait already in progress") - self._waiting = True - - # Deliver EOF to a still-open piped stdin so a reader child can exit and - # the wait below does not block forever. + self._waiting += 1 if self.stdin is not None and not self.stdin.closed: try: self.stdin.close() except OSError: pass - try: - # None -> wait forever (the FFI treats 0 as "no timeout"). A finite - # timeout maps to milliseconds, clamped up to 1ms so timeout=0 and - # sub-millisecond values don't collapse to 0 and wait forever — the - # opposite of the caller's intent. - timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 - result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) + return _wait(handle, timeout) finally: - with self._lock: + with self._idle: + self._waiting -= 1 + self._idle.notify_all() + + def shutdown(self) -> "Result": + """Kill what is left of the session, wait until it has no process, and + return its :class:`Result`. The result carries no ``stdout``/``stderr``: + those were streamed to you. A repeated call returns the same result.""" + from ._sdk import _lib + with self._idle: + handle = self._handle + if handle is None: + if self._result is not None: + return self._result + raise RuntimeError("process is not running") + if self._waiting: + _send_signal(handle, "sandlock_handle_kill") + while self._waiting: + self._idle.wait() + self._handle = None + try: + self._result = _take_result(_lib.sandlock_handle_shutdown(handle)) + finally: self._sandbox._park_or_free(handle) - self._handle = None - self._waiting = False - # Release the sandbox's busy marker so it can be reused. Guard that it - # still points at us (the weakref may already be dead during __del__). - if self._sandbox._popen_process() is self: - self._sandbox._process = None - - if not result_p: - self._result = Result( - success=False, exit_code=-1, error="sandlock_handle_wait failed" - ) + # Guard: the weakref may already be dead during __del__. + if self._sandbox._popen_process() is self: + self._sandbox._process = None return self._result - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - # stdout/stderr were handed to the caller as fds, so the RunResult holds - # none — read them off the streams, not the Result. - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) - self._result = Result( - success=bool(success), exit_code=exit_code, reason=reason, signal=signal, - changes=changes, - ) - return self._result - def __enter__(self) -> "Process": return self def __exit__(self, exc_type, exc_val, exc_tb) -> None: - # Terminate and reap a still-running child so no confined process is left - # behind, then close every stream we own. + # End the session so no confined process is left behind, then close + # every stream we own. if self._handle is not None: try: - self.kill() - except Exception: - pass - try: - self.wait() + self.shutdown() except Exception: pass for stream in (self.stdin, self.stdout, self.stderr): @@ -1739,7 +1671,7 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> None: pass def __del__(self): - # Safety net for a Process dropped without wait()/context: a live handle + # Safety net for a Process dropped without shutdown()/context: a live handle # would otherwise leak the confined child + its runtime and wedge the # Sandbox in "already running". Reap it, warning like subprocess.Popen so # the missing wait()/`with` is visible. __del__ must never raise, and may @@ -1751,13 +1683,71 @@ def __del__(self): import warnings warnings.warn( - "Process was not waited on or used as a context manager; " - "reaping the confined child. Use `with sandbox.popen(...)` or " - "call wait().", + "Process was not shut down or used as a context manager; " + "reaping the confined session. Use `with sandbox.popen(...)` or " + "call shutdown().", ResourceWarning, stacklevel=2, ) - self.kill() - self.wait() + self.shutdown() except Exception: pass + + +def _wait(handle, timeout): + import ctypes + import math + from ._sdk import _c_result_p, _lib, ExitReason, ExitStatus + ms = 0 if timeout is None else max(1, math.ceil(timeout * 1000)) + ptr = _c_result_p() + rc = _lib.sandlock_handle_wait(handle, ms, ctypes.byref(ptr)) + if rc == 1: + raise TimeoutError("main process has not exited") + if rc != 0: + raise RuntimeError("sandlock_handle_wait failed") + try: + return ExitStatus( + success=bool(_lib.sandlock_result_success(ptr)), + exit_code=_lib.sandlock_result_exit_code(ptr), + reason=ExitReason(_lib.sandlock_result_reason(ptr)), + signal=_lib.sandlock_result_signal(ptr), + ) + finally: + _lib.sandlock_result_free(ptr) + + +def _take_result(ptr): + from ._sdk import _lib, _read_result_bytes, _read_result_changes, Result, ExitReason + if not ptr: + raise RuntimeError("sandlock_handle_shutdown failed") + try: + return Result( + success=bool(_lib.sandlock_result_success(ptr)), + exit_code=_lib.sandlock_result_exit_code(ptr), + reason=ExitReason(_lib.sandlock_result_reason(ptr)), + signal=_lib.sandlock_result_signal(ptr), + stdout=_read_result_bytes(ptr, _lib.sandlock_result_stdout_bytes), + stderr=_read_result_bytes(ptr, _lib.sandlock_result_stderr_bytes), + changes=_read_result_changes(ptr), + ) + finally: + _lib.sandlock_result_free(ptr) + + +def _process_groups(handle): + import ctypes + import json + from ._sdk import _lib + ptr = _lib.sandlock_handle_process_groups(handle) + if not ptr: + raise RuntimeError("process group inspection failed") + try: + return json.loads(ctypes.string_at(ptr)) + finally: + _lib.sandlock_string_free(ctypes.cast(ptr, ctypes.c_char_p)) + + +def _send_signal(handle, name): + from ._sdk import _lib + if getattr(_lib, name)(handle) != 0: + raise RuntimeError("sandbox signal failed") diff --git a/python/tests/test_lifecycle.py b/python/tests/test_lifecycle.py index 423c0aa7..16f19819 100644 --- a/python/tests/test_lifecycle.py +++ b/python/tests/test_lifecycle.py @@ -70,7 +70,8 @@ class TestSpawn: def test_spawn_then_wait_returns_result(self): with _policy() as sb: sb.spawn(["sh", "-c", "echo hello; exit 0"]) - result = sb.wait() + assert sb.wait().exit_code == 0 + result = sb.shutdown() assert result.exit_code == 0 assert b"hello" in result.stdout @@ -102,7 +103,8 @@ def test_create_then_start_runs_command(self): with _policy() as sb: sb.create(["sh", "-c", "echo two-step"]) sb.start() - result = sb.wait() + sb.wait() + result = sb.shutdown() assert result.exit_code == 0 assert b"two-step" in result.stdout diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py index 0283d87e..f125fa53 100644 --- a/python/tests/test_popen.py +++ b/python/tests/test_popen.py @@ -149,9 +149,10 @@ def test_wait_is_idempotent_and_cached(): proc = _policy().popen(["echo", "hi"], stdout=StdioMode.PIPED) assert proc.stdout.read() == b"hi\n" first = proc.wait() - second = proc.wait() # handle already freed → cached result, no double-free - assert first is second + second = proc.wait() + assert first == second assert first.exit_code == 0 + assert proc.shutdown() is proc.shutdown() def test_popen_unknown_stdio_mode_raises(): @@ -181,17 +182,19 @@ def test_popen_rejects_second_process_on_same_sandbox(): proc.wait() -def test_wait_timeout_kills_and_returns_nonsuccess(): - # A child that never exits within the timeout: wait(timeout) must RETURN (not - # hang), killing the child and yielding a non-success result — parity with - # Sandbox.run(timeout=...). Run under a watchdog so a regression (ignored - # timeout → hang) surfaces as a failure, not a stuck job. +def test_wait_timeout_raises_and_keeps_the_session(): + # A child that never exits within the timeout: wait(timeout) must return + # promptly by raising TimeoutError and leave the session running. Run under + # a watchdog so an ignored timeout surfaces as a failure, not a stuck job. proc = _policy().popen(["sleep", "100"]) box = {} def _run(): - box["result"] = proc.wait(timeout=0.5) + try: + proc.wait(timeout=0.5) + except TimeoutError as e: + box["result"] = e t = threading.Thread(target=_run, daemon=True) t.start() @@ -201,7 +204,9 @@ def _run(): t.join(timeout=5) pytest.fail("wait(timeout) did not return — timeout was ignored") - assert not box["result"].success, "a timed-out (killed) process is not success" + assert isinstance(box["result"], TimeoutError) + assert proc.pid is not None, "a timeout leaves the session running" + proc.shutdown() def test_wait_without_timeout_completes_for_quick_child(): @@ -256,18 +261,17 @@ def test_stale_process_does_not_alias_reused_sandbox(): sandbox = _policy() p1 = sandbox.popen(["echo", "one"], stdout=StdioMode.PIPED) assert p1.stdout.read() == b"one\n" - p1.wait() # frees p1's handle, releases the sandbox + p1.shutdown() # frees p1's handle, releases the sandbox p2 = sandbox.popen(["sleep", "100"]) try: assert sandbox.is_running p1.kill() # no-op on the reaped p1 — must NOT kill p2 - p1.wait() # cached result — must NOT reap p2 + p1.wait() # cached status — must NOT reap p2 assert sandbox.is_running, "stale p1 must not have reaped p2" assert p2.pid is not None, "p2 must still be alive" finally: - p2.kill() - p2.wait() + p2.shutdown() assert not sandbox.is_running @@ -299,22 +303,24 @@ def test_sandbox_pid_and_ports_delegate_to_popen_process(): assert sandbox.pid == proc.pid assert sandbox.ports() == proc.ports() # both {} without port_remap finally: - proc.kill() - proc.wait() + proc.shutdown() assert sandbox.pid is None assert sandbox.ports() == {} def test_wait_timeout_zero_does_not_wait_forever(): # timeout=0 (and sub-millisecond values) must not collapse to "wait forever"; - # they return promptly with a non-success (killed) result. Watchdog so a - # regression surfaces as a failure, not a hang. + # they raise TimeoutError promptly. Watchdog so a regression surfaces as a + # failure, not a hang. proc = _policy().popen(["sleep", "100"]) box = {} def _run(): - box["result"] = proc.wait(timeout=0) + try: + proc.wait(timeout=0) + except TimeoutError as e: + box["result"] = e t = threading.Thread(target=_run, daemon=True) t.start() @@ -324,7 +330,8 @@ def _run(): t.join(timeout=5) pytest.fail("wait(timeout=0) hung — 0 collapsed to wait-forever") - assert not box["result"].success + assert isinstance(box["result"], TimeoutError) + proc.shutdown() def test_abandoned_process_is_reaped_by_del(): diff --git a/python/tests/test_sandbox.py b/python/tests/test_sandbox.py index e2b5624b..55e451e2 100644 --- a/python/tests/test_sandbox.py +++ b/python/tests/test_sandbox.py @@ -795,6 +795,8 @@ def test_pid_available_during_run(self): assert pid > 0 sb.wait() + assert sb.pid == pid, "the session outlives its main process" + sb.shutdown() assert sb.pid is None def test_pause_not_running_raises(self): @@ -1008,8 +1010,9 @@ def test_spawn_wait_defers_too(self, tmp_path): workdir.mkdir() p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.DEFER) p.spawn(["sh", "-c", f"echo hi > {workdir}/out.txt"]) - result = p.wait() - assert result.success + assert p.wait().success + assert not p.pending, "the branch settles at shutdown, not at main's exit" + assert p.shutdown().success assert p.pending p.commit() assert (workdir / "out.txt").read_text() == "hi\n" diff --git a/python/tests/test_session.py b/python/tests/test_session.py new file mode 100644 index 00000000..1f471220 --- /dev/null +++ b/python/tests/test_session.py @@ -0,0 +1,88 @@ +"""wait() returns with the main process; shutdown() ends the session.""" +import os +import threading +import time + +import pytest + +from sandlock import Sandbox, ExitStatus, ExitReason, StdioMode + + +def policy(**kw): + readable = ["/usr", "/lib", "/bin", "/etc", "/proc", "/dev"] + if os.path.exists("/lib64"): + readable.append("/lib64") + return Sandbox(fs_readable=readable, **kw) + + +def alive(pid): + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + + +def gone(pid, seconds=5): + deadline = time.monotonic() + seconds + while alive(pid): + if time.monotonic() > deadline: + return False + time.sleep(0.01) + return True + + +def test_wait_keeps_jobs_until_shutdown(): + with policy().popen(["sh", "-c", "sleep 60 & echo $!; exit 7"], stdout=StdioMode.PIPED) as proc: + job = int(proc.stdout.readline()) + status = proc.wait(timeout=5) + assert isinstance(status, ExitStatus) and status.exit_code == 7 + assert alive(job) and proc.process_groups() + assert proc.shutdown().exit_code == 7 + assert gone(job) + + +def test_wait_timeout_keeps_the_session(): + sb = policy() + sb.spawn(["sleep", "60"]) + with pytest.raises(TimeoutError): + sb.wait(timeout=0.05) + assert sb.process_groups() + sb.shutdown() + + +def test_kill_from_another_thread_ends_a_blocked_wait(): + sb = policy() + sb.spawn(["sleep", "60"]) + threading.Timer(0.1, sb.kill).start() + status = sb.wait(timeout=5) + assert not status.success + assert status.reason in (ExitReason.KILLED, ExitReason.SIGNALED) + sb.shutdown() + + +def test_shutdown_during_a_wait_ends_it(): + sb = policy() + sb.spawn(["sleep", "60"]) + waiter = threading.Thread(target=sb.wait) + waiter.start() + time.sleep(0.1) + sb.shutdown() + waiter.join(timeout=5) + assert not waiter.is_alive() + + +def test_context_exit_ends_jobs_left_after_wait(tmp_path): + pidfile = tmp_path / "job" + with policy(fs_writable=[str(tmp_path)]) as sb: + sb.spawn(["sh", "-c", f"sleep 60 & echo $! > {pidfile}"]) + sb.wait(timeout=5) + job = int(pidfile.read_text()) + assert alive(job) + assert gone(job) + + +def test_run_timeout_reports_timeout(): + result = policy().run(["sleep", "60"], timeout=0.2) + assert result.reason == ExitReason.TIMEOUT + assert not result.success From 087cbb16914e1babc60ddf36c734a52c3dce4f6a Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:31:18 -0700 Subject: [PATCH 08/18] go: wait for main, shut down the session explicitly Process.Wait returned only when the whole session was done, and Kill, Pause and Resume signalled main's pid by number, which can be reused once main is reaped while the session lives on. Wait now returns main's ExitStatus and keeps background jobs supervised; WaitTimeout returns ErrTimeout and leaves the session untouched. Shutdown ends the session and returns the Result, and ProcessGroups lists what is left. Signals go through the native controller, which is safe beside a blocked native wait, so the wait runs without the mutex; Shutdown and Close kill an in-flight Wait and let it return before they touch the handle. That hand-off uses a channel rather than a sync.Cond on the mutex, since a Cond would point back into the Process and keep Go from running its finalizer. Run waits with the context deadline, then shuts down. Existing tests shut down where they need changes or a deferred branch. New tests cover jobs surviving Wait, a timed-out wait, and Shutdown ending a blocked Wait. All Go tests pass. Signed-off-by: Cong Wang --- go/README.md | 28 ++-- go/errors.go | 4 + go/sandbox.go | 10 +- go/sandlock_linux.go | 270 +++++++++++++++++++++++++------------- go/sandlock_linux_test.go | 14 +- go/session_linux_test.go | 95 ++++++++++++++ 6 files changed, 314 insertions(+), 107 deletions(-) create mode 100644 go/session_linux_test.go diff --git a/go/README.md b/go/README.md index b9b7c8bd..9ae1009e 100644 --- a/go/README.md +++ b/go/README.md @@ -138,11 +138,13 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) (`Stdin`/`Stdout`/`Stderr`) you read/write while the child runs. The zero `Stdio` inherits all three (identical to Spawn). Close a piped `Stdin` before `Wait` (or let `Wait` close it for you) so a reader child sees EOF; drain a - piped `Stdout`/`Stderr` before `Wait`, or `Kill` from another goroutine to - interrupt a blocked `Wait`. `Wait` returns a `Result` with the exit status - only — a Popen'd process sends piped output to the `Stdout`/`Stderr` fields - (inherited/null streams go to the parent fd or `/dev/null`), so (unlike `Run`) - `Result.Stdout`/`Result.Stderr` are always empty. + piped `Stdout`/`Stderr` while waiting, or `Kill` from another goroutine to + interrupt a blocked `Wait`. `Wait` returns the main process's `ExitStatus` + and leaves its background jobs supervised; `Shutdown` ends the session and + returns a `Result`. A Popen'd process sends piped output to the + `Stdout`/`Stderr` fields (inherited/null streams go to the parent fd or + `/dev/null`), so (unlike `Run`) `Result.Stdout`/`Result.Stderr` are always + empty. ```go p, _ := sb.Popen(sandlock.Stdio{Stdin: sandlock.StdioPiped, Stdout: sandlock.StdioPiped}, "cat") @@ -150,7 +152,8 @@ defer p.Close() p.Stdin.Write([]byte("hi\n")) p.Stdin.Close() // EOF so cat exits out, _ := io.ReadAll(p.Stdout) // "hi\n" -res, _ := p.Wait() +st, _ := p.Wait() // main's ExitStatus +res, _ := p.Shutdown() // ends the session ``` ### Container images @@ -260,14 +263,19 @@ sb := &sandlock.Sandbox{ ```go func (p *Process) Pid() int -func (p *Process) Wait() (*Result, error) -func (p *Process) Pause() error // SIGSTOP to the process group +func (p *Process) Wait() (*ExitStatus, error) // main exited; background jobs keep running +func (p *Process) WaitTimeout(d time.Duration) (*ExitStatus, error) // ErrTimeout leaves the session +func (p *Process) Shutdown() (*Result, error) // kill what is left, reap it, collect the result +func (p *Process) ProcessGroups() ([]int, error) // groups that still have members +func (p *Process) Pause() error // SIGSTOP to every process group func (p *Process) Resume() error // SIGCONT func (p *Process) Kill() error // SIGKILL func (p *Process) Ports() (map[int]int, error) // virtual→real, with PortRemap -func (p *Process) Close() error // release the handle (kills if running), close piped streams +func (p *Process) Close() error // release the handle (ends the session), close piped streams -// BranchActionDefer only: after Wait, the change set stays on the Process. +// Kill, Pause and Resume work while another goroutine is blocked in Wait. + +// BranchActionDefer only: after Shutdown, the change set stays on the Process. func (p *Process) Pending() bool // exited under Defer and undecided func (p *Process) UpperDir() string // new bytes of added/modified files, laid out like Workdir func (p *Process) Commit() error // merge into Workdir (blocks up to 5s on a contended workdir) diff --git a/go/errors.go b/go/errors.go index 9d9684ea..22194df4 100644 --- a/go/errors.go +++ b/go/errors.go @@ -9,3 +9,7 @@ var ErrInvalidString = errors.New("sandlock: string contains NUL byte") // ErrNotRunning is returned by *Process lifecycle methods when no process is // currently running in the handle. var ErrNotRunning = errors.New("sandlock: process is not running") + +// ErrTimeout is returned by WaitTimeout when the main process is still +// running; the session is left untouched. +var ErrTimeout = errors.New("sandlock: main process has not exited") diff --git a/go/sandbox.go b/go/sandbox.go index 28d6c2fa..977bcda5 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -317,10 +317,18 @@ const ( ReasonSignaled ExitReason = 1 // ReasonKilled: killed with no recoverable signal number. ReasonKilled ExitReason = 2 - // ReasonTimeout: killed by sandlock because it exceeded its timeout. + // ReasonTimeout: a run exceeded its timeout and was killed. ReasonTimeout ExitReason = 3 ) +// ExitStatus describes the main process; its session may still be running. +type ExitStatus struct { + ExitCode int + Reason ExitReason + Signal int + Success bool +} + // Result is the outcome of a captured run. type Result struct { ExitCode int // process exit code, or -1 if terminated abnormally diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index 91ebe7e2..8844c835 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -731,13 +731,23 @@ func (s *Sandbox) Run(ctx context.Context, cmd ...string) (*Result, error) { C.sandlock_handle_free(h) return nil, fmt.Errorf("sandlock: failed to start sandbox") } - r := C.sandlock_handle_wait_timeout(h, timeoutMs(ctx)) + var status *C.sandlock_result_t + rc := C.sandlock_handle_wait(h, timeoutMs(ctx), &status) + if rc < 0 { + C.sandlock_handle_free(h) + return nil, fmt.Errorf("sandlock: wait failed") + } + C.sandlock_result_free(status) + r := C.sandlock_handle_shutdown(h) C.sandlock_handle_free(h) if r == nil { - return nil, fmt.Errorf("sandlock: wait failed") + return nil, fmt.Errorf("sandlock: shutdown failed") } res := readResult(r) C.sandlock_result_free(r) + if rc == 1 { + res.ExitCode, res.Reason, res.Signal, res.Success = -1, ReasonTimeout, -1, false + } return res, nil } @@ -852,21 +862,24 @@ func SyscallNr(name string) (int, error) { return int(nr), nil } -// Process is a live sandboxed process started by Spawn. It supports PID -// inspection, pause/resume/kill via the process group, and Wait. A Process -// holds at most one running command; create separate Spawns for concurrency. +// Process is a sandbox session started by Spawn or Popen. Wait returns when +// the main process exits and leaves its background jobs supervised; Shutdown +// ends the session. A Process holds one session; create separate Spawns for +// concurrency. // -// The underlying FFI handle is not safe for concurrent access, so all handle -// operations are serialized. Pause/Resume/Kill act on the OS process group by -// PID and touch no handle state, so they remain usable while Wait blocks on -// the handle — that is how Kill interrupts a blocked Wait. Ports, by contrast, -// reads the handle and is reported as empty while a Wait is in flight. +// Handle operations are serialized by the mutex, except the native wait: +// native signals are safe beside it, so Kill, Pause and Resume reach a +// blocked Wait, while Shutdown and Close wait for it to return before they +// touch the handle. Ports is reported as empty while a Wait is in flight. type Process struct { - mu sync.Mutex - h *C.sandlock_handle_t - pid int - waiting bool // a Wait owns the handle; other handle ops must defer to it - pending bool // the run exited under BranchActionDefer; Commit/Abort own the handle now + mu sync.Mutex + h *C.sandlock_handle_t + pid int + waiting int // Waits in the native call + waitDone chan struct{} // closed when the last of them returns + status *ExitStatus // main's status once a Wait has seen it + result *Result // the session's result once Shutdown has run + pending bool // the run exited under BranchActionDefer; Commit/Abort own the handle now // Caller-owned stdio for a process started by Popen. Each is non-nil only // for a stream wired StdioPiped; it owns the pipe fd (closing the file @@ -906,9 +919,9 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error) { return nil, fmt.Errorf("sandlock: failed to start sandbox") } p := &Process{h: h, pid: int(C.sandlock_handle_pid(h))} - // Last-resort cleanup if the caller drops the Process without Wait/Close: - // kill the child and release the handle so neither is leaked. Wait and - // Close clear this once they have done the cleanup themselves. + // Last-resort cleanup if the caller drops the Process without + // Shutdown/Close: freeing the handle kills and reaps the session. + // Shutdown and Close clear this once they have done the cleanup. runtime.SetFinalizer(p, (*Process).finalize) return p, nil } @@ -931,9 +944,9 @@ func fdFile(fd C.int, name string) *os.File { // // Deadlock warning (as with os/exec pipes): a piped Stdin is yours — close it // before Wait, or a child that reads to EOF (e.g. cat) never exits and Wait -// blocks. Likewise drain a piped Stdout/Stderr before Wait, or a child that +// blocks. Likewise drain a piped Stdout/Stderr while waiting, or a child that // fills the pipe buffer blocks on write. As an escape hatch, Kill from another -// goroutine interrupts a blocked Wait; Close reaps the child and closes the +// goroutine interrupts a blocked Wait; Close ends the session and closes the // streams. Wait closes a still-open piped Stdin for you to deliver EOF. func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { if len(cmd) == 0 { @@ -980,7 +993,7 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { } // finalize is the SetFinalizer cleanup for a Process abandoned without -// Wait/Close. It can only run once the Process is unreachable, which implies +// Shutdown/Close. It can only run once the Process is unreachable, which implies // no Wait is in flight (a blocked Wait keeps the Process reachable), so the // handle is not concurrently borrowed and is safe to free here. The piped // streams are intentionally left alone: once the Process is unreachable so are @@ -991,9 +1004,6 @@ func (p *Process) finalize() { if p.h == nil { return } - if p.pid > 0 { - _ = syscall.Kill(-p.pid, syscall.SIGKILL) - } C.sandlock_handle_free(p.h) p.h = nil } @@ -1005,59 +1015,144 @@ func (p *Process) Pid() int { return p.pid } -// Wait blocks until the process exits, returns its Result, and releases the -// handle. After Wait the Process is no longer running. -// -// The Result carries the exit status only. Unlike Run, a Popen'd process sends -// any StdioPiped output to the caller through the Stdout/Stderr fields (and -// StdioInherit/StdioNull output to the inherited fd or /dev/null), so it is -// never captured into Result.Stdout/Result.Stderr — read piped bytes off -// p.Stdout/p.Stderr, not off the Result. -// -// The blocking native wait runs without holding the mutex so that Kill (and -// Pause/Resume), which signal the process group by PID, can run concurrently -// and interrupt it. The waiting flag reserves exclusive use of the handle for -// the duration, so no other handle operation aliases it. +// Wait blocks until the main process exits and returns its status. Background +// jobs stay supervised until Shutdown. Repeated calls return the same status. // // Wait closes a still-open piped Stdin to deliver EOF, so do not write to Stdin // from another goroutine while Wait runs — the write would race a closing fd. -func (p *Process) Wait() (*Result, error) { +func (p *Process) Wait() (*ExitStatus, error) { return p.wait(0) } + +// WaitTimeout is Wait with a deadline. It returns ErrTimeout if main is still +// running when d elapses, leaving the session untouched. +func (p *Process) WaitTimeout(d time.Duration) (*ExitStatus, error) { + ms := d.Milliseconds() + if ms < 1 { + ms = 1 + } + return p.wait(uint64(ms)) +} + +func (p *Process) wait(ms uint64) (*ExitStatus, error) { p.mu.Lock() - if p.h == nil || p.waiting { + if p.status != nil { + s := *p.status + p.mu.Unlock() + return &s, nil + } + if p.h == nil || p.pending { p.mu.Unlock() return nil, ErrNotRunning } h := p.h + if p.waiting == 0 { + p.waitDone = make(chan struct{}) + } + p.waiting++ stdin := p.Stdin - p.waiting = true p.mu.Unlock() - - // Deliver EOF to a still-open piped stdin so a reader child (e.g. cat) can - // exit; otherwise the native wait blocks forever. Harmless if already closed. if stdin != nil { _ = stdin.Close() } - r := C.sandlock_handle_wait(h) + var r *C.sandlock_result_t + rc := C.sandlock_handle_wait(h, C.uint64_t(ms), &r) p.mu.Lock() defer p.mu.Unlock() - p.waiting = false + p.waiting-- + if p.waiting == 0 { + close(p.waitDone) + } + switch rc { + case 0: + case 1: + return nil, ErrTimeout + default: + return nil, fmt.Errorf("sandlock: wait failed") + } + s := ExitStatus{ + ExitCode: int(C.sandlock_result_exit_code(r)), + Reason: ExitReason(C.sandlock_result_reason(r)), + Signal: int(C.sandlock_result_signal(r)), + Success: bool(C.sandlock_result_success(r)), + } + C.sandlock_result_free(r) + p.status = &s + out := s + return &out, nil +} + +// Shutdown kills whatever is left of the session, waits until it has no +// process, and returns the result. The Result carries no Stdout/Stderr for a +// Popen'd process: read those off p.Stdout/p.Stderr. A Wait blocked in +// another goroutine is ended first. A deferred branch stays available for +// Commit or Abort. Repeated calls return the same result. +func (p *Process) Shutdown() (*Result, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.result != nil { + return p.result, nil + } + if p.h == nil { + return nil, ErrNotRunning + } + if p.waiting > 0 { + C.sandlock_handle_kill(p.h) + p.drainWaits() + if p.result != nil { + return p.result, nil + } + if p.h == nil { + return nil, ErrNotRunning + } + } + r := C.sandlock_handle_shutdown(p.h) + if r == nil { + return nil, fmt.Errorf("sandlock: shutdown failed") + } + p.result = readResult(r) + C.sandlock_result_free(r) // A deferred branch keeps the handle alive until Commit/Abort/Close; the // finalizer stays armed so a leaked Process still preserves it. - if C.sandlock_handle_pending(h) != 0 { + if C.sandlock_handle_pending(p.h) != 0 { p.pending = true } else { - C.sandlock_handle_free(h) + C.sandlock_handle_free(p.h) p.h = nil runtime.SetFinalizer(p, nil) } - if r == nil { - return nil, fmt.Errorf("sandlock: wait failed") + return p.result, nil +} + +// drainWaits releases p.mu until no Wait is in flight; callers recheck state. +// A channel, not a Cond on p.mu, which would cycle into p and stall its finalizer. +func (p *Process) drainWaits() { + for p.waiting > 0 { + done := p.waitDone + p.mu.Unlock() + <-done + p.mu.Lock() } - res := readResult(r) - C.sandlock_result_free(r) - return res, nil +} + +// ProcessGroups returns the sorted ids of the session's process groups that +// still have members. +func (p *Process) ProcessGroups() ([]int, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.h == nil || p.pending { + return nil, ErrNotRunning + } + raw := C.sandlock_handle_process_groups(p.h) + if raw == nil { + return nil, fmt.Errorf("sandlock: process group inspection failed") + } + defer C.sandlock_string_free(raw) + var groups []int + if err := json.Unmarshal([]byte(C.GoString(raw)), &groups); err != nil { + return nil, err + } + return groups, nil } // Pending reports whether the process exited under BranchActionDefer and its @@ -1118,45 +1213,37 @@ func (p *Process) dispose(op func(*C.sandlock_handle_t) C.int, name string) erro func (p *Process) signal(sig syscall.Signal) error { p.mu.Lock() defer p.mu.Unlock() - if p.h == nil || p.pid <= 0 || p.pending { + if p.h == nil || p.pending { + if sig == syscall.SIGKILL { + return nil + } return ErrNotRunning } - // The sandbox child leads its own process group; signal the whole group. - return syscall.Kill(-p.pid, sig) + var rc C.int + switch sig { + case syscall.SIGSTOP: + rc = C.sandlock_handle_pause(p.h) + case syscall.SIGCONT: + rc = C.sandlock_handle_resume(p.h) + default: + rc = C.sandlock_handle_kill(p.h) + } + if rc != 0 { + return fmt.Errorf("sandlock: signal failed") + } + return nil } -// Pause sends SIGSTOP to the sandbox process group. +// Pause sends SIGSTOP to every process group of the session. func (p *Process) Pause() error { return p.signal(syscall.SIGSTOP) } -// Resume sends SIGCONT to the sandbox process group. +// Resume sends SIGCONT to every process group of the session. func (p *Process) Resume() error { return p.signal(syscall.SIGCONT) } -// Kill sends SIGKILL to the sandbox process group. It is idempotent: a process -// that already exited (ESRCH) or was already reaped by Wait (ErrNotRunning) is -// not an error — killing it is a no-op success, matching the FFI -// (sandlock_handle_kill returns 0) and the Python binding (silent no-op after -// wait()). This keeps the "Kill from another goroutine" escape hatch race-free: -// a Kill that fires just as Wait reaps the child still returns nil. -func (p *Process) Kill() error { - err := p.signal(syscall.SIGKILL) - if err == syscall.ESRCH { - return nil // child already exited (still unreaped) — no-op - } - if err == ErrNotRunning { - // signal() returns ErrNotRunning for two causes: the handle was already - // reaped by Wait (h == nil) — an idempotent no-op — or a live handle with - // no pid. Only the reaped case is a success; a missing pid is a genuine - // error we must surface (and must never turn into killpg(-0), which would - // signal our own process group). - p.mu.Lock() - reaped := p.h == nil - p.mu.Unlock() - if reaped { - return nil - } - } - return err -} +// Kill sends SIGKILL to every process group of the session. It is idempotent: +// a session that already ended is not an error, so a Kill racing Shutdown in +// another goroutine still returns nil. +func (p *Process) Kill() error { return p.signal(syscall.SIGKILL) } // Ports returns the current virtual-to-real TCP port mappings while the // process is running. It is non-empty only when PortRemap is enabled and at @@ -1166,7 +1253,7 @@ func (p *Process) Ports() (map[int]int, error) { defer p.mu.Unlock() // While a Wait holds the handle, reading port mappings would alias it; // report empty rather than touch the handle concurrently. - if p.h == nil || p.waiting { + if p.h == nil || p.waiting > 0 { return map[int]int{}, nil } c := C.sandlock_handle_port_mappings(p.h) @@ -1190,8 +1277,8 @@ func (p *Process) Ports() (map[int]int, error) { return out, nil } -// Close releases the process handle, killing the process if it is still -// running. It is safe to call multiple times. +// Close releases the process handle, ending the session if it is still +// running, without returning its result. It is safe to call multiple times. func (p *Process) Close() error { p.mu.Lock() defer p.mu.Unlock() @@ -1205,13 +1292,12 @@ func (p *Process) Close() error { if p.h == nil { return nil } - if p.waiting { - // A Wait owns the handle and will free it; just kill the process - // group by PID to unblock that Wait, without touching the handle. - if p.pid > 0 { - _ = syscall.Kill(-p.pid, syscall.SIGKILL) + if p.waiting > 0 { + C.sandlock_handle_kill(p.h) + p.drainWaits() + if p.h == nil { + return nil } - return nil } C.sandlock_handle_free(p.h) p.h = nil diff --git a/go/sandlock_linux_test.go b/go/sandlock_linux_test.go index d1c26137..e49b66af 100644 --- a/go/sandlock_linux_test.go +++ b/go/sandlock_linux_test.go @@ -393,15 +393,18 @@ func TestDeferHoldsTheBranchForTheProcess(t *testing.T) { t.Fatalf("Spawn: %v", err) } defer p.Close() - res, err := p.Wait() - if err != nil { + if _, err := p.Wait(); err != nil { t.Fatalf("Wait: %v", err) } + res, err := p.Shutdown() + if err != nil { + t.Fatalf("Shutdown: %v", err) + } if !res.Success || len(res.Changes) != 1 { t.Fatalf("unexpected result: %+v", res) } if !p.Pending() { - t.Fatal("a Defer run must be pending after Wait") + t.Fatal("a Defer run must be pending after Shutdown") } if _, statErr := os.Stat(dir + "/out.txt"); statErr == nil { t.Fatal("nothing may land before Commit") @@ -441,6 +444,9 @@ func TestDeferAbortDiscards(t *testing.T) { if _, err := p.Wait(); err != nil { t.Fatalf("Wait: %v", err) } + if _, err := p.Shutdown(); err != nil { + t.Fatalf("Shutdown: %v", err) + } if err := p.Abort(); err != nil { t.Fatalf("Abort: %v", err) } @@ -556,7 +562,7 @@ func TestPopenWaitClosesUnclosedStdin(t *testing.T) { // stdout concurrently so a full pipe can never be the reason cat blocks. go io.ReadAll(p.Stdout) - done := make(chan *sandlock.Result, 1) + done := make(chan *sandlock.ExitStatus, 1) go func() { res, _ := p.Wait() done <- res diff --git a/go/session_linux_test.go b/go/session_linux_test.go new file mode 100644 index 00000000..6a1396a4 --- /dev/null +++ b/go/session_linux_test.go @@ -0,0 +1,95 @@ +//go:build linux + +package sandlock_test + +import ( + "bufio" + "strconv" + "strings" + "syscall" + "testing" + "time" + + sandlock "github.com/multikernel/sandlock/go" +) + +func gone(pid int) bool { + deadline := time.Now().Add(5 * time.Second) + for syscall.Kill(pid, 0) == nil { + if time.Now().After(deadline) { + return false + } + time.Sleep(10 * time.Millisecond) + } + return true +} + +func TestWaitKeepsJobsUntilShutdown(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Popen(sandlock.Stdio{Stdout: sandlock.StdioPiped}, "sh", "-c", "sleep 60 & echo $!; exit 7") + if err != nil { + t.Fatalf("Popen: %v", err) + } + defer p.Close() + line, err := bufio.NewReader(p.Stdout).ReadString('\n') + if err != nil { + t.Fatalf("read job pid: %v", err) + } + job, _ := strconv.Atoi(strings.TrimSpace(line)) + st, err := p.Wait() + if err != nil || st.ExitCode != 7 { + t.Fatalf("Wait: %+v %v", st, err) + } + if syscall.Kill(job, 0) != nil { + t.Fatal("the job died with main") + } + if groups, err := p.ProcessGroups(); err != nil || len(groups) == 0 { + t.Fatalf("ProcessGroups after Wait: %v %v", groups, err) + } + if res, err := p.Shutdown(); err != nil || res.ExitCode != 7 { + t.Fatalf("Shutdown: %+v %v", res, err) + } + if !gone(job) { + t.Fatal("the job outlived Shutdown") + } +} + +func TestWaitTimeoutKeepsTheSession(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sleep", "60") + if err != nil { + t.Fatalf("Spawn: %v", err) + } + defer p.Close() + if _, err := p.WaitTimeout(50 * time.Millisecond); err != sandlock.ErrTimeout { + t.Fatalf("WaitTimeout: want ErrTimeout, got %v", err) + } + if groups, err := p.ProcessGroups(); err != nil || len(groups) == 0 { + t.Fatalf("a timed-out wait must leave the session: %v %v", groups, err) + } +} + +func TestShutdownEndsABlockedWait(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sleep", "60") + if err != nil { + t.Fatalf("Spawn: %v", err) + } + done := make(chan error, 1) + go func() { + _, werr := p.Wait() + done <- werr + }() + time.Sleep(100 * time.Millisecond) + if _, err := p.Shutdown(); err != nil { + t.Fatalf("Shutdown: %v", err) + } + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Shutdown did not end a blocked Wait") + } +} From 90183365ec90bf29b7fe33cada7c03f25ec3ccf8 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:39:21 -0700 Subject: [PATCH 09/18] cli: kill and list sessions through the supervisor sandlock kill sent the kill request, then also called killpg on main's recorded pid and SIGKILLed the supervisor. A session now outlives its main process, so that pid can be reaped and reused while the name is still registered, and the killpg could hit an unrelated group. Killing the supervisor on every call also took down a Python or Go host that embeds the sandbox. sandlock ps had the same stale pid problem when it read main's uptime and command line from /proc. The control loop now holds the session's Controller, with or without a seccomp supervisor. sandlock kill goes only through it and reports a refusal instead of claiming success. Only a supervisor that accepts the connection and then never answers is killed, through a pidfd opened while SO_PEERCRED vouches for it, and the reaper then kills main's group; a closed connection means the host is tearing down and is left alone. The kill request waits longer than the server's per-client cap, so queueing behind another client is not mistaken for a wedge. The info verb reports whether main has exited, and ps shows main's pid, uptime and command only while it is alive. Add tests for killing a sandbox without a supervisor, for ps after main exits, and for kill racing a host that is shutting down. All core and CLI tests pass. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 63 ++++----- crates/sandlock-cli/tests/cli_test.rs | 23 +++ crates/sandlock-core/src/control.rs | 131 +++++++++++------- crates/sandlock-core/src/sandbox.rs | 4 +- crates/sandlock-core/src/sys/syscall.rs | 8 ++ .../tests/integration/test_control.rs | 53 +++++++ docs/cli.md | 6 +- 7 files changed, 200 insertions(+), 88 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 08b4c083..d8ad5dd5 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -285,23 +285,31 @@ async fn main() -> Result<()> { } else { println!("NAME PID UPTIME STATUS PORTS CMD"); for name in &names { - // The pids need no answer from the supervisor; the rest - // does, and a supervisor that does not answer is still - // alive because its socket is. - let (status, ports) = match sandlock_core::control::sandbox_info(name) { - Ok(info) => ( + // The registry's main pid outlives main, so /proc is read + // only while the supervisor vouches main is alive. + let pid = sandlock_core::control::sandbox_pids(name).map(|p| p.child); + let (status, ports, pid, uptime, cmd) = match (sandlock_core::control::sandbox_info(name), pid) { + (Ok(info), _) if info.main_exited => ( info.mode.unwrap_or_else(|| "running".to_string()), query_ports(name), + "-".to_string(), + "-".to_string(), + "(main exited)".to_string(), ), - Err(_) => ("unresponsive".to_string(), "?".to_string()), - }; - let (pid, uptime, cmd) = match sandlock_core::control::sandbox_pids(name) { - Ok(pids) => ( - pids.child.to_string(), - proc_uptime(pids.child).unwrap_or_else(|| "?".to_string()), - proc_cmdline(pids.child).unwrap_or_else(|| "?".to_string()), + (Ok(info), Ok(pid)) => ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), + pid.to_string(), + proc_uptime(pid).unwrap_or_else(|| "?".to_string()), + proc_cmdline(pid).unwrap_or_else(|| "?".to_string()), + ), + (info, pid) => ( + if info.is_ok() { "running" } else { "unresponsive" }.to_string(), + "?".to_string(), + pid.map_or_else(|_| "?".to_string(), |p| p.to_string()), + "?".to_string(), + "?".to_string(), ), - Err(_) => ("?".to_string(), "?".to_string(), "?".to_string()), }; println!( "{:<32} {:>8} {:>12} {:<10} {:<24} {}", @@ -358,30 +366,11 @@ async fn main() -> Result<()> { eprintln!("sandlock: {e}"); std::process::exit(1); } - // Both pids come from the kernel, so this works on a supervisor - // that is stopped or wedged. - let pids = match sandlock_core::control::sandbox_pids(&name) { - Ok(p) => p, - Err(e) => { - eprintln!("sandlock: {}", e); - std::process::exit(1); - } - }; - // Groups the sandbox created are known only to the supervisor, - // and a wedged one cannot answer, so the child's own group is - // signalled here regardless. The supervisor may sit in a - // different group, so signal it directly too. - let _ = sandlock_core::control::send_control_request( - &name, - "kill", - serde_json::Value::Object(Default::default()), - ); - unsafe { libc::killpg(pids.child, libc::SIGKILL) }; - unsafe { libc::kill(pids.supervisor, libc::SIGKILL) }; - println!( - "Killed sandbox '{}' (child PID {}, supervisor PID {})", - name, pids.child, pids.supervisor - ); + if let Err(e) = sandlock_core::control::kill_sandbox(&name) { + eprintln!("sandlock: kill '{}': {}", name, e); + std::process::exit(1); + } + println!("Killed sandbox '{}'", name); } Command::Check => { diff --git a/crates/sandlock-cli/tests/cli_test.rs b/crates/sandlock-cli/tests/cli_test.rs index 06411b0a..f48e7574 100644 --- a/crates/sandlock-cli/tests/cli_test.rs +++ b/crates/sandlock-cli/tests/cli_test.rs @@ -671,6 +671,29 @@ fn test_kill_stops_sandbox() { } } +#[test] +fn test_kill_stops_sandbox_without_supervisor() { + let name = format!("test-kill-nosup-{}", std::process::id()); + let mut args = sandlock_sleep_args(&name); + args.insert(1, "--no-supervisor".into()); + let mut child = std::process::Command::new(env!("CARGO_BIN_EXE_sandlock")) + .args(&args) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("spawn sandlock"); + if let Err(e) = wait_for_sandbox(&mut child, &name) { + panic!("{}", e); + } + let out = sandlock_bin().args(["kill", &name]).output().expect("sandlock kill"); + assert!( + out.status.success(), + "kill should succeed without a supervisor: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(!child.wait().expect("wait for killed child").success()); +} + #[test] fn test_kill_nonexistent_sandbox() { let out = sandlock_bin() diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 9601a8e7..0cc8b38f 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -169,6 +169,9 @@ pub(crate) fn bind_control_socket(name: &str) -> std::io::Result #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct SandboxInfo { pub mode: Option, + /// Main's pid in the registry is free for reuse once this is set. + #[serde(default)] + pub main_exited: bool, } /// Spawn the control-loop task. `ctx` is `None` for sandboxes without a @@ -177,6 +180,7 @@ pub struct SandboxInfo { pub(crate) fn spawn_control_loop( sockets: ControlSocket, ctx: Option>, + controller: crate::Controller, sandbox: Sandbox, info: SandboxInfo, ) -> tokio::task::JoinHandle<()> { @@ -185,7 +189,7 @@ pub(crate) fn spawn_control_loop( let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); tokio::spawn(async move { let ControlSocket { control, claim: _claim } = sockets; - control_loop(control, ctx, sandbox, info, unsafe { libc::getuid() }).await; + control_loop(control, ctx, controller, sandbox, info, unsafe { libc::getuid() }).await; }) } @@ -271,6 +275,7 @@ impl tokio::io::AsyncWrite for ControlStream { async fn control_loop( listener: ControlFd, ctx: Option>, + controller: crate::Controller, sandbox: Arc>, info: SandboxInfo, my_uid: u32, @@ -289,7 +294,7 @@ async fn control_loop( let Ok(stream) = AsyncFd::new(stream).map(ControlStream) else { continue }; let _ = tokio::time::timeout( std::time::Duration::from_secs(5), - serve_one(stream, ctx.as_ref(), &sandbox, &info), + serve_one(stream, ctx.as_ref(), &controller, &sandbox, &info), ) .await; } @@ -321,6 +326,7 @@ pub struct ControlResponse { async fn serve_one( stream: ControlStream, ctx: Option<&Arc>, + controller: &crate::Controller, sandbox: &Arc>, info: &SandboxInfo, ) { @@ -367,10 +373,10 @@ async fn serve_one( } match req.verb.as_str() { - "info" => handle_info(&mut stream, info).await, + "info" => handle_info(&mut stream, info, controller).await, "config" => handle_config(&mut stream, ctx, sandbox).await, "ports" => handle_ports(&mut stream, ctx).await, - "kill" => handle_kill(&mut stream, ctx).await, + "kill" => handle_kill(&mut stream, controller).await, _ => { let resp = ControlResponse { v: 1, @@ -383,8 +389,9 @@ async fn serve_one( } } -async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo) { - let resp = match serde_json::to_value(info) { +async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo, controller: &crate::Controller) { + let info = SandboxInfo { main_exited: controller.main_exited(), ..info.clone() }; + let resp = match serde_json::to_value(&info) { Ok(data) => ControlResponse { v: 1, ok: true, data: Some(data), err: None }, Err(e) => ControlResponse { v: 1, @@ -437,29 +444,10 @@ async fn handle_config( /// Only the supervisor knows the groups a process has moved into, so /// `sandlock kill` asks here before it signals the child's own group. -async fn handle_kill(stream: &mut ControlStream, ctx: Option<&Arc>) { - if let Some(ctx) = ctx { - ctx.policy_fn.lock().await.cancellation.cancel(); - } - let resp = match ctx.map(|ctx| ctx.groups.signal(libc::SIGKILL)) { - Some(Ok(groups)) => ControlResponse { - v: 1, - ok: true, - data: Some(serde_json::json!({ "groups": groups })), - err: None, - }, - Some(Err(e)) => ControlResponse { - v: 1, - ok: false, - data: None, - err: Some(format!("signal process groups: {}", e)), - }, - None => ControlResponse { - v: 1, - ok: false, - data: None, - err: Some("no supervisor".into()), - }, +async fn handle_kill(stream: &mut ControlStream, controller: &crate::Controller) { + let resp = match controller.kill() { + Ok(()) => ControlResponse { v: 1, ok: true, data: None, err: None }, + Err(e) => ControlResponse { v: 1, ok: false, data: None, err: Some(e.to_string()) }, }; let _ = write_response(stream, &resp).await; } @@ -616,7 +604,7 @@ fn connect_entry(name: &str, entry: &ownership::Entry, my_uid: u32) -> Result<(U Ok((stream, cred)) } -/// The two PIDs `kill` needs, stamped by the kernel at semaphore publication. +/// The sandbox's PIDs, stamped by the kernel at semaphore publication. #[derive(Debug, Clone, Copy)] pub struct SandboxPids { /// The child, which leads its own process group. @@ -654,18 +642,62 @@ fn send_control_request_as( args: serde_json::Value, my_uid: u32, ) -> Result { - use std::io::{Read, Write}; + let (stream, _) = connect_control(name, my_uid)?; + // A short timeout so a wedged supervisor does not block the CLI forever. + exchange(name, stream, verb, args, std::time::Duration::from_secs(2)).map_err(|e| e.message) +} - let (mut stream, _) = connect_control(name, my_uid)?; +/// Kill through the supervisor's group table. Only a supervisor that accepts +/// the connection and then never answers is killed instead, and the reaper +/// then kills main's group; a closed connection means it is tearing down. +pub fn kill_sandbox(name: &str) -> Result<(), String> { + let (stream, cred) = connect_control(name, unsafe { libc::getuid() })?; + // Opened while connected, so it names the peer SO_PEERCRED vouched for. + let supervisor = crate::sys::syscall::pidfd_open(cred.pid as u32, 0) + .map_err(|e| format!("pidfd_open supervisor: {e}"))?; + let args = serde_json::Value::Object(Default::default()); + // Longer than the server's per-client cap, so waiting in its queue is not + // mistaken for a wedge. + match exchange(name, stream, "kill", args, std::time::Duration::from_secs(8)) { + Ok(resp) if resp.ok => Ok(()), + Ok(resp) => Err(resp.err.unwrap_or_else(|| "kill failed".into())), + Err(e) if e.timed_out => crate::sys::syscall::pidfd_send_signal(&supervisor, libc::SIGKILL) + .map_err(|e| format!("kill unresponsive supervisor: {e}")), + Err(e) => Err(e.message), + } +} + +struct ExchangeError { + timed_out: bool, + message: String, +} + +impl ExchangeError { + fn other(message: String) -> Self { + Self { timed_out: false, message } + } + + fn read(name: &str, e: std::io::Error) -> Self { + let timed_out = matches!(e.kind(), std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut); + Self { timed_out, message: unresponsive(name, e) } + } +} + +fn exchange( + name: &str, + mut stream: UnixStream, + verb: &str, + args: serde_json::Value, + timeout: std::time::Duration, +) -> Result { + use std::io::{Read, Write}; - // Set a 2-second timeout on reads so a wedged supervisor does not - // block the CLI forever. stream - .set_read_timeout(Some(std::time::Duration::from_secs(2))) - .map_err(|e| format!("set_read_timeout: {}", e))?; + .set_read_timeout(Some(timeout)) + .map_err(|e| ExchangeError::other(format!("set_read_timeout: {}", e)))?; stream - .set_write_timeout(Some(std::time::Duration::from_secs(2))) - .map_err(|e| format!("set_write_timeout: {}", e))?; + .set_write_timeout(Some(timeout)) + .map_err(|e| ExchangeError::other(format!("set_write_timeout: {}", e)))?; let req = serde_json::json!({ "v": 1, @@ -673,24 +705,24 @@ fn send_control_request_as( "args": args, }); let body = serde_json::to_vec(&req) - .map_err(|e| format!("serialize request: {}", e))?; + .map_err(|e| ExchangeError::other(format!("serialize request: {}", e)))?; let len = (body.len() as u32).to_be_bytes(); - stream.write_all(&len).map_err(|e| format!("write len: {}", e))?; - stream.write_all(&body).map_err(|e| format!("write body: {}", e))?; + stream.write_all(&len).map_err(|e| ExchangeError::other(format!("write len: {}", e)))?; + stream.write_all(&body).map_err(|e| ExchangeError::other(format!("write body: {}", e)))?; // Read response. let mut len_buf = [0u8; 4]; - stream.read_exact(&mut len_buf).map_err(|e| unresponsive(name, e))?; + stream.read_exact(&mut len_buf).map_err(|e| ExchangeError::read(name, e))?; let resp_len = u32::from_be_bytes(len_buf) as usize; if resp_len > 65536 { - return Err("response too large".to_string()); + return Err(ExchangeError::other("response too large".to_string())); } let mut resp_body = vec![0u8; resp_len]; - stream.read_exact(&mut resp_body).map_err(|e| unresponsive(name, e))?; + stream.read_exact(&mut resp_body).map_err(|e| ExchangeError::read(name, e))?; serde_json::from_slice(&resp_body) - .map_err(|e| format!("parse response: {}", e)) + .map_err(|e| ExchangeError::other(format!("parse response: {}", e))) } /// Ask a sandbox for its mode. @@ -806,7 +838,7 @@ mod tests { } fn info() -> SandboxInfo { - SandboxInfo { mode: Some("test".into()) } + SandboxInfo { mode: Some("test".into()), main_exited: false } } /// Bind a listener for `name`, run the control loop on it with @@ -816,7 +848,12 @@ mod tests { let sandbox = Arc::new(tokio::sync::Mutex::new(test_sandbox())); tokio::spawn(async move { let _claim = sockets.claim; - control_loop(sockets.control, None, sandbox, info(), expected_uid).await; + let controller = crate::Controller { + groups: Arc::new(crate::pgroup::ProcessGroups::new()), + stop: Default::default(), + main_exit: tokio::sync::watch::channel(None).1, + }; + control_loop(sockets.control, None, controller, sandbox, info(), expected_uid).await; }) } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index febf10c8..84f86ed4 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2225,7 +2225,7 @@ impl Sandbox { let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; - let control_info = crate::control::SandboxInfo { mode: self.mode.clone() }; + let control_info = crate::control::SandboxInfo { mode: self.mode.clone(), main_exited: false }; let is_nested_mode = notif_fd_num == 0; @@ -2479,6 +2479,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, Some(control_ctx), + self.controller()?, sandbox_snapshot, control_info.clone(), )); @@ -2504,6 +2505,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, None, + self.controller()?, self.clone(), control_info, )); diff --git a/crates/sandlock-core/src/sys/syscall.rs b/crates/sandlock-core/src/sys/syscall.rs index a043e5b8..95bba159 100644 --- a/crates/sandlock-core/src/sys/syscall.rs +++ b/crates/sandlock-core/src/sys/syscall.rs @@ -194,6 +194,14 @@ pub fn pidfd_getfd(pidfd: &OwnedFd, targetfd: i32, flags: u32) -> io::Result io::Result<()> { + use std::os::unix::io::AsRawFd; + unsafe { + syscall4(crate::arch::SYS_PIDFD_SEND_SIGNAL, pidfd.as_raw_fd() as u64, sig as u64, 0, 0)? + }; + Ok(()) +} + /// Signal the process group led by the process behind `pidfd`. The group /// is named by the pidfd rather than by number, so a recycled pgid cannot /// redirect the signal, and it stays reachable after its leader is reaped. diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index cae2073f..78be2095 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -875,3 +875,56 @@ async fn test_control_cancelled_creation_releases_its_name() { assert!(!sandlock_core::control::list_sandboxes().unwrap().contains(&name)); drop(cancelled); } + +/// A session can outlive its main process, whose pid is then free for +/// reuse; ps must stop reporting it instead of reading a recycled /proc entry. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_ps_hides_the_pid_of_an_exited_main() { + let name = format!("test-ctrl-main-exited-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/proc") + .fs_read("/dev") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + assert!(sandlock_core::control::sandbox_info(&name).expect("info").main_exited); + let out = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + let stdout = String::from_utf8_lossy(&out.stdout); + let row = stdout.lines().find(|l| l.starts_with(&name)).expect("ps lists the session"); + assert_eq!(row.split_whitespace().nth(1), Some("-"), "exited main has no pid: {row}"); + assert!(row.ends_with("(main exited)"), "{row}"); + sb.shutdown().await.unwrap(); +} + +/// A host in shutdown drops its control socket mid-request; kill must not +/// take the host down for it. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_kill_does_not_kill_a_host_in_teardown() { + let name = format!("test-ctrl-teardown-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["sleep", "60"]).await.unwrap(); + let killer = std::thread::spawn({ + let name = name.clone(); + move || { + for _ in 0..50 { + let _ = sandlock_core::control::kill_sandbox(&name); + } + } + }); + sb.shutdown().await.unwrap(); + killer.join().unwrap(); + // Reaching this line means the host survived every kill attempt. +} diff --git a/docs/cli.md b/docs/cli.md index 02532519..5a7ff53c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -226,8 +226,8 @@ sandlock run -p build -- make test # trailing command overrides [program] `sandlock ps` lists every running sandbox with its uptime, port mappings, and command. `sandlock inspect NAME` prints the effective policy as JSON -(`--toml` prints it in profile syntax). `sandlock kill NAME` stops a -sandbox and its supervisor. +(`--toml` prints it in profile syntax). `sandlock kill NAME` asks the +supervisor to kill every process group the sandbox created. ``` $ sandlock ps @@ -243,7 +243,7 @@ http_inject_ca = [] ... $ sandlock kill web.local -Killed sandbox 'web.local' (child PID 12346, supervisor PID 12340) +Killed sandbox 'web.local' ``` Ports show as `virtual→real` when `--port-remap` is active; see From ac96a5142edc35114d99293fbfee15cf316657c3 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:47:34 -0700 Subject: [PATCH 10/18] sandbox: kill remaining jobs when a sandbox is dropped after wait() Drop swept the process groups only while the main process was created, running or paused. After wait() the state is MainExited, so a sandbox dropped between wait() and shutdown() skipped the sweep. Dropping the reaper handle still made the reaper kill main's group, but jobs that had moved to groups of their own, such as a setsid daemon, kept running with nobody supervising them. Sweep in MainExited too, so Drop kills every tracked group and waits for the reaper, as shutdown() does. Add a test that drops the sandbox after wait() while a setsid job runs in its own group. All Rust, Python and Go tests pass. Signed-off-by: Cong Wang --- crates/sandlock-core/src/sandbox.rs | 2 +- .../tests/integration/test_process_groups.rs | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 84f86ed4..2266cac6 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2655,7 +2655,7 @@ impl Drop for Sandbox { if let Some(ref mut rt) = self.runtime { if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } if let Some(pid) = rt.child_pid { - if matches!(rt.state, RuntimeState::Created | RuntimeState::Running | RuntimeState::Paused) { + if matches!(rt.state, RuntimeState::Created | RuntimeState::Running | RuntimeState::Paused | RuntimeState::MainExited(_)) { let swept = rt.groups.signal(libc::SIGKILL); if let Some(mut reaper) = rt.reaper.take() { if no_supervisor || swept.is_err() { reaper.finish(); } diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index 1ed3d1c3..e205e9f2 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -287,3 +287,23 @@ async fn test_drop_after_wait_ends_jobs() { drop(sb); assert!(wait_gone(job)); } + +/// Dropping the sandbox after wait() must also reach jobs that left main's +/// group, not only those the reaper can kill by itself. +#[tokio::test(flavor = "multi_thread")] +async fn test_drop_after_wait_ends_jobs_in_other_groups() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("leader"); + let script = format!( + "import os, signal\npid = os.fork()\nif pid == 0:\n os.setsid()\n signal.pause()\nopen('{p}.tmp', 'w').write(str(pid))\nos.rename('{p}.tmp', '{p}')\n", + p = pid_file.display() + ); + let mut sb = policy(); + sb.fs_writable.push(dir.path().to_path_buf()); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let leader: i32 = std::fs::read_to_string(&pid_file).unwrap().trim().parse().unwrap(); + assert!(alive(leader), "the setsid job survives wait()"); + drop(sb); + assert!(wait_gone(leader), "Drop must end jobs in groups the sandbox created"); +} From 63499675d068666cbc2b3a77d7d10f057377df13 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 16:53:16 -0700 Subject: [PATCH 11/18] learn: end the whole session on timeout When the observed command ran past --timeout, sandlock learn killed the main process by its recorded pid and waited again. Background jobs it had started were only reached later by shutdown, and the pid was used as an identity at all, which the session lifetime change avoids everywhere else: once main is reaped its pid can be reused. Drop the pid kill. shutdown() already kills every process group of the session and waits for all of them before the partial profile is written. Add a test that runs learn with a one-second timeout over a command that backgrounds a job, and checks that the partial profile is written and the job is gone. All CLI tests pass. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 6 +----- crates/sandlock-cli/tests/learn_test.rs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 1652fccc..7b799cb4 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -622,7 +622,6 @@ pub async fn run(args: LearnArgs) -> Result<()> { let mut sandbox = policy; sandbox.create_interactive(&cmd_refs).await .map_err(|e| anyhow!("sandbox error: {e}"))?; - let child_pid = sandbox.pid().expect("child pid after create") as u32; sandbox.start() .map_err(|e| anyhow!("sandbox error: {e}"))?; @@ -636,11 +635,8 @@ pub async fn run(args: LearnArgs) -> Result<()> { false } Err(_elapsed) => { - // Timeout: kill the child, drain the supervisor, write a partial profile. + // The shutdown below kills the session; the profile is partial. eprintln!("sandlock learn: timeout after {secs}s, killing process"); - unsafe { libc::kill(child_pid as i32, libc::SIGKILL); } - // Drain without timeout so the supervisor releases its resources cleanly. - let _ = sandbox.wait().await; true } } diff --git a/crates/sandlock-cli/tests/learn_test.rs b/crates/sandlock-cli/tests/learn_test.rs index 5b1966d5..a566981f 100644 --- a/crates/sandlock-cli/tests/learn_test.rs +++ b/crates/sandlock-cli/tests/learn_test.rs @@ -1151,3 +1151,27 @@ fn test_learn_merge_preserves_home_variable() { assert!(!reads.iter().any(|r| r.starts_with(&format!("{home}/.config"))), "merge added a machine-specific duplicate of ${{HOME}}/.config: {reads:?}"); } + +/// A timeout ends the whole session, background jobs included, and still +/// writes what was observed before it. +#[test] +fn test_learn_timeout_ends_the_session() { + // A sleep length no other process uses, so the job can be found afterwards. + let marker = format!("{}.{}", 3000 + std::process::id() % 1000, std::process::id()); + let cmd = format!("cat /etc/hostname; sleep {marker} & sleep 60"); + let started = std::time::Instant::now(); + let output = sandlock_bin() + .args(["learn", "--timeout", "1", "--", "sh", "-c", &cmd]) + .output() + .expect("failed to run sandlock learn"); + assert!(started.elapsed() < std::time::Duration::from_secs(30), "learn ignored its timeout"); + let stdout = String::from_utf8_lossy(&output.stdout); + let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or(""); + assert!( + read_line.contains("/etc/hostname"), + "expected a partial profile with /etc/hostname, got: {stdout}; stderr={}", + String::from_utf8_lossy(&output.stderr), + ); + let survivors = Command::new("pgrep").args(["-f", &format!("sleep {marker}")]).output().unwrap(); + assert!(survivors.stdout.is_empty(), "the background job outlived the timeout"); +} From 1891e598120abecaf381e483a0f7ed1ab822b26d Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 17:57:12 -0700 Subject: [PATCH 12/18] sandbox: detect exec completion through a child-owned exec pipe spawn() decided that main had exec'd by polling /proc//exe every 1ms until it stopped matching the supervisor's binary. The session reaper collects main the moment it exits, so a main like `sh -c "sleep 60 & exit 0"` could exec and vanish between two polls and be reported as "child exited before exec() completed". The comparison also could never succeed for a workload that execs the supervisor's own binary. The child now opens a CLOEXEC pipe, hands the read end to the parent alongside the notif fd, marks it right before execve and again if execve returns. A successful exec closes the write end, so spawn() reads to EOF and judges the marks. The pipe is created after fork so that a sibling forked by the parent, such as another sandbox parked before start(), cannot inherit the write end and hold back the EOF. Signed-off-by: Cong Wang --- crates/sandlock-core/src/context.rs | 31 ++++++- crates/sandlock-core/src/sandbox.rs | 80 ++++++++++--------- .../tests/integration/test_process_groups.rs | 28 +++++++ 3 files changed, 98 insertions(+), 41 deletions(-) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 6780cd60..48542f4a 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -65,6 +65,15 @@ impl PipePair { } } +/// Written on the exec pipe right before `execve`, which closes the pipe on +/// success; `EXEC_FAILED` follows if `execve` returns. +pub(crate) const EXEC_REACHED: u8 = b'e'; +pub(crate) const EXEC_FAILED: u8 = b'f'; + +fn mark_exec(exec_w: RawFd, mark: u8) { + unsafe { libc::write(exec_w, [mark].as_ptr().cast(), 1) }; +} + // ============================================================ // Pipe I/O helpers // ============================================================ @@ -300,6 +309,14 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { use std::io::Write; + // Created here rather than before fork so that no sibling forked by the + // parent inherits the write end and holds back its EOF. + let mut exec_fds = [0i32; 2]; + if unsafe { libc::pipe2(exec_fds.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + fail!("exec pipe"); + } + let [exec_r, exec_w] = exec_fds; + // 1. Interactive runs only: if stdin is a terminal, become the // foreground process group so interactive shells can read from the // TTY. Captured/piped runs must not: the embedding process keeps @@ -509,6 +526,10 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { std::mem::forget(notif_fd); } + if let Err(e) = write_u32_fd(pipes.notif_w.as_raw_fd(), exec_r as u32) { + fail!(format!("write exec pipe fd: {}", e)); + } + // 10. Wait for parent to signal ready match read_u32_fd(pipes.ready_r.as_raw_fd()) { Ok(_) => {} @@ -520,6 +541,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { if keep_fd >= 0 { fds_to_keep.push(keep_fd); } + fds_to_keep.push(exec_w); close_fds_above(2, &fds_to_keep); // 13. Apply environment @@ -589,6 +611,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { // supervisor, so `run`'s code is already mapped; running it directly // avoids an execve that Landlock would otherwise have to authorize. set_proc_name(name); + unsafe { libc::close(exec_w) }; run(); unsafe { libc::_exit(0) }; } @@ -610,6 +633,8 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { .chain(std::iter::once(std::ptr::null())) .collect(); + mark_exec(exec_w, EXEC_REACHED); + if let Some(fd) = exec_fd { extern "C" { static environ: *const *const libc::c_char; @@ -644,8 +669,10 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { unsafe { libc::execvp(argv_ptrs[0], argv_ptrs.as_ptr()) }; } - // If we get here, exec failed - fail!(format!("exec '{}'", cmd[0].to_string_lossy())); + let exec_err = io::Error::last_os_error(); + mark_exec(exec_w, EXEC_FAILED); + let _ = write!(std::io::stderr(), "sandlock child: exec '{}': {}\n", cmd[0].to_string_lossy(), exec_err); + unsafe { libc::_exit(127) }; } // ============================================================ diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 2266cac6..029b6bb0 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -330,6 +330,8 @@ struct Runtime { on_bind: Option) + Send + Sync>>, handlers: Vec<(i64, Arc)>, ready_w: Option, + /// The child's exec pipe: EOF after `EXEC_REACHED` alone means it exec'd. + exec_r: Option, /// Set when this sandbox is a stage of a [`Transaction`](crate::transaction::Transaction) /// that shares one COW upper across all stages. When present, `do_create_stdio` /// reuses this `CowState` (instead of building its own branch), and neither @@ -1365,43 +1367,34 @@ impl Sandbox { &self.restore_skipped } - /// Wait for the child to finish `execve`. Detected by `/proc//exe` - /// no longer matching `/proc/self/exe` (before execve the child still - /// shares the supervisor's binary). The kernel offers no direct event - /// for execve completion, so this polls every 1ms. Policy approvals add - /// their configured timeout to the usual five-second startup allowance; - /// unlimited approvals also disable the startup deadline. - async fn wait_until_exec(&self) -> Result<(), crate::error::SandlockError> { + /// Wait for the child to finish `execve`, which closes its exec pipe. + /// Policy approvals add their configured timeout to the usual five-second + /// startup allowance; unlimited approvals also disable the startup deadline. + async fn wait_until_exec(&mut self) -> Result<(), crate::error::SandlockError> { use crate::error::SandboxRuntimeError; - let pid = self.pid().ok_or(SandboxRuntimeError::NotRunning)?; - let Some(our_exe) = std::fs::read_link("/proc/self/exe").ok() else { - return Ok(()); - }; - let child_link = format!("/proc/{}/exe", pid); + use tokio::io::AsyncReadExt; + let exec_r = self.rt_mut().exec_r.take().ok_or(SandboxRuntimeError::NotRunning)?; + let mut exec_r = tokio::net::unix::pipe::Receiver::from_owned_fd(exec_r) + .map_err(SandboxRuntimeError::Io)?; let startup_allowance = Duration::from_secs(5); let timeout = if let Some(policy_fn) = &self.policy_fn { policy_fn.verdict_timeout.map(|timeout| timeout.saturating_add(startup_allowance)) } else { Some(startup_allowance) }; - let deadline = timeout.and_then(|timeout| std::time::Instant::now().checked_add(timeout)); - loop { - match std::fs::read_link(&child_link) { - Ok(child_exe) if child_exe != our_exe => return Ok(()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(SandboxRuntimeError::Child( - "child exited before exec() completed".into(), - ).into()); - } - _ => {} - } - if deadline.is_some_and(|deadline| std::time::Instant::now() >= deadline) { - return Err(SandboxRuntimeError::Child( - "child did not exec() within the startup timeout".into(), - ).into()); - } - tokio::time::sleep(std::time::Duration::from_millis(1)).await; + let mut marks = Vec::new(); + let read = exec_r.read_to_end(&mut marks); + match timeout { + Some(timeout) => tokio::time::timeout(timeout, read).await.map_err(|_| { + SandboxRuntimeError::Child("child did not exec() within the startup timeout".into()) + })?, + None => read.await, + } + .map_err(SandboxRuntimeError::Io)?; + if marks != [context::EXEC_REACHED] { + return Err(SandboxRuntimeError::Child("child exited before exec() completed".into()).into()); } + Ok(()) } /// Create with explicit stdin/stdout/stderr fd redirection. Child is @@ -1743,6 +1736,7 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + exec_r: None, #[cfg(feature = "cow")] shared_cow: None, tty_foreground_taken: false, @@ -1856,6 +1850,7 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + exec_r: None, #[cfg(feature = "cow")] shared_cow: None, tty_foreground_taken: false, @@ -2224,6 +2219,21 @@ impl Sandbox { let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; + let exec_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) + .map_err(|e| SandboxRuntimeError::Child(format!("read exec pipe fd from child: {}", e)))?; + let take_child_fd = |num: u32, flags: i32| -> Result { + if let Some(ref pfd) = pidfd { + return syscall::pidfd_getfd(pfd, num as i32, 0) + .map_err(|e| SandboxRuntimeError::Child(format!("pidfd_getfd: {}", e))); + } + let cpath = CString::new(format!("/proc/{}/fd/{}", pid, num)).unwrap(); + let raw = unsafe { libc::open(cpath.as_ptr(), flags | libc::O_CLOEXEC) }; + if raw < 0 { + return Err(SandboxRuntimeError::Child(format!("failed to open child fd {} from /proc", num))); + } + Ok(unsafe { OwnedFd::from_raw_fd(raw) }) + }; + let exec_r = take_child_fd(exec_fd_num, libc::O_RDONLY)?; let control_info = crate::control::SandboxInfo { mode: self.mode.clone(), main_exited: false }; @@ -2231,17 +2241,8 @@ impl Sandbox { let notif_fd = if is_nested_mode { None - } else if let Some(ref pfd) = pidfd { - Some(syscall::pidfd_getfd(pfd, notif_fd_num as i32, 0) - .map_err(|e| SandboxRuntimeError::Child(format!("pidfd_getfd: {}", e)))?) } else { - let path = format!("/proc/{}/fd/{}", pid, notif_fd_num); - let cpath = CString::new(path).unwrap(); - let raw = unsafe { libc::open(cpath.as_ptr(), libc::O_RDWR) }; - if raw < 0 { - return Err(SandboxRuntimeError::Child("failed to open notif fd from /proc".into()).into()); - } - Some(unsafe { OwnedFd::from_raw_fd(raw) }) + Some(take_child_fd(notif_fd_num, libc::O_RDWR)?) }; if let Some(notif_fd) = notif_fd { @@ -2520,6 +2521,7 @@ impl Sandbox { self.rt_mut().pidfd = pidfd; self.rt_mut().ready_w = Some(pipes.ready_w); + self.rt_mut().exec_r = Some(exec_r); Ok(()) } diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index e205e9f2..d097cb12 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -238,6 +238,34 @@ async fn test_shutdown_returns_output_while_a_job_holds_stdout() { assert_eq!(result.stdout.as_deref(), Some(b"main-output\n".as_slice())); } +/// The reaper collects a main that exits right after exec before spawn() +/// can see its new image; spawn() must still report success. +#[tokio::test] +async fn test_spawn_of_an_instantly_exiting_main() { + for _ in 0..50 { + let mut sb = policy(); + sb.spawn(&["true"]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(0)); + sb.shutdown().await.unwrap(); + } +} + +#[tokio::test] +async fn test_spawn_beside_a_parked_sibling() { + let mut parked = policy(); + parked.create(&["true"]).await.unwrap(); + let mut sb = policy(); + tokio::time::timeout(std::time::Duration::from_secs(5), sb.spawn(&["true"])).await.unwrap().unwrap(); + sb.wait().await.unwrap(); + sb.shutdown().await.unwrap(); +} + +#[tokio::test] +async fn test_spawn_reports_a_failed_exec() { + let mut sb = policy(); + assert!(sb.spawn(&["/nonexistent-sandlock-binary"]).await.is_err()); +} + #[tokio::test] async fn test_shutdown_ends_a_paused_session() { let mut sb = policy(); From 10479d0927afc2014d9b07b054812ce7061ef96c Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 18:21:07 -0700 Subject: [PATCH 13/18] sandbox: refuse to checkpoint once main has exited checkpoint() captured main by its recorded pid. The session reaper reaps main as soon as it exits, so that number can be reused while the session lives on, and a checkpoint could freeze the session and capture an unrelated process, such as one in another sandbox of the same host. Return NotRunning once main has exited. Signed-off-by: Cong Wang --- crates/sandlock-core/src/sandbox.rs | 4 ++++ .../tests/integration/test_checkpoint.rs | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 029b6bb0..e85857ef 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1481,7 +1481,11 @@ impl Sandbox { #[cfg(feature = "checkpoint")] pub async fn checkpoint(&self) -> Result { use crate::error::{SandboxRuntimeError, SandlockError}; + // The reaper reaps main as soon as it exits, so its pid may already + // name an unrelated process. let pid = self.runtime.as_ref() + .filter(|rt| rt.main_exit.borrow().is_none()) + .filter(|rt| !matches!(rt.state, RuntimeState::MainExited(_) | RuntimeState::Stopped(_))) .and_then(|rt| rt.child_pid) .ok_or(SandlockError::Runtime(SandboxRuntimeError::NotRunning))?; self.checkpoint_pid(pid).await diff --git a/crates/sandlock-core/tests/integration/test_checkpoint.rs b/crates/sandlock-core/tests/integration/test_checkpoint.rs index 43237b91..9bfbdbd7 100644 --- a/crates/sandlock-core/tests/integration/test_checkpoint.rs +++ b/crates/sandlock-core/tests/integration/test_checkpoint.rs @@ -152,6 +152,21 @@ async fn test_checkpoint_process_info() { let _ = sb.wait().await; } +#[tokio::test] +async fn test_checkpoint_refused_after_main_exits() { + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .build().unwrap(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + assert!(matches!( + sb.checkpoint().await, + Err(sandlock_core::SandlockError::Runtime(sandlock_core::error::SandboxRuntimeError::NotRunning)) + )); + sb.shutdown().await.unwrap(); +} + /// Test loading from a nonexistent directory fails. #[tokio::test] async fn test_checkpoint_load_nonexistent() { From 793fec70b08757288d37db95a77045a9a984a971 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 18:31:09 -0700 Subject: [PATCH 14/18] sandbox: report main's pid only until it exits The session reaper reaps main as soon as it exits, so its pid can be reused while the session lives on. pid() kept returning it until the Sandbox was dropped, and Python and Go cached it at creation, so a caller reading /proc or signalling that number could reach an unrelated process. pid() now returns None once main has exited, in Rust, through the Controller for the FFI, and in Python and Go, which query it on each call instead of caching it. Main's exit is published on one watch owned by the runtime: the reaper's status relay fills it, and wait() fills it on paths without a reaper, such as fork() clones, so checkpoint() relies on pid() as well. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 2 - crates/sandlock-core/src/control.rs | 1 + crates/sandlock-core/src/controller.rs | 6 +++ crates/sandlock-core/src/sandbox.rs | 50 +++++++++---------- .../tests/integration/test_process_groups.rs | 11 ++++ crates/sandlock-ffi/src/lib.rs | 2 +- go/sandlock_linux.go | 12 +++-- python/README.md | 2 +- python/src/sandlock/sandbox.py | 29 ++++++----- python/tests/test_sandbox.py | 2 +- 10 files changed, 69 insertions(+), 48 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 7b799cb4..4822a894 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -617,8 +617,6 @@ pub async fn run(args: LearnArgs) -> Result<()> { eprintln!("sandlock learn: observing {cmd_str} ..."); - // Use the three-step lifecycle (create/start/wait) so we can get the child - // PID from sandbox.pid() and sample /proc/ for resource peaks. let mut sandbox = policy; sandbox.create_interactive(&cmd_refs).await .map_err(|e| anyhow!("sandbox error: {e}"))?; diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 0cc8b38f..f2af5036 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -852,6 +852,7 @@ mod tests { groups: Arc::new(crate::pgroup::ProcessGroups::new()), stop: Default::default(), main_exit: tokio::sync::watch::channel(None).1, + pid: 0, }; control_loop(sockets.control, None, controller, sandbox, info(), expected_uid).await; }) diff --git a/crates/sandlock-core/src/controller.rs b/crates/sandlock-core/src/controller.rs index 1ddb9802..8f1aa8b8 100644 --- a/crates/sandlock-core/src/controller.rs +++ b/crates/sandlock-core/src/controller.rs @@ -10,6 +10,7 @@ pub struct Controller { pub(crate) groups: Arc, pub(crate) stop: tokio_util::sync::CancellationToken, pub(crate) main_exit: tokio::sync::watch::Receiver>, + pub(crate) pid: i32, } impl Controller { @@ -34,6 +35,11 @@ impl Controller { self.main_exit.borrow().is_some() } + /// Main's pid until it exits; afterwards the number may name another process. + pub fn pid(&self) -> Option { + (!self.main_exited()).then_some(self.pid) + } + /// Signal every group, returning how many had members left. pub fn signal(&self, sig: i32) -> Result { if sig == libc::SIGKILL { diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index e85857ef..0a86aa6c 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -291,7 +291,7 @@ struct Runtime { reaper: Option, /// Main's process group: the reaper's pid, or a clone's own pid. pgid: i32, - main_exit: tokio::sync::watch::Receiver>, + main_exit: tokio::sync::watch::Sender>, groups: Arc, /// Cancelled by a kill, so a held policy_fn verdict cannot stall it. stop: tokio_util::sync::CancellationToken, @@ -866,9 +866,11 @@ impl Sandbox { .or_else(|| self.name.as_deref()) } - /// Return the child PID if spawned. + /// Main's pid until it exits; afterwards the number may name another process. pub fn pid(&self) -> Option { - self.runtime.as_ref().and_then(|r| r.child_pid) + self.runtime.as_ref() + .filter(|rt| rt.main_exit.borrow().is_none()) + .and_then(|rt| rt.child_pid) } /// Return whether the child is currently running or paused. @@ -894,7 +896,8 @@ impl Sandbox { Ok(crate::Controller { groups: Arc::clone(&rt.groups), stop: rt.stop.clone(), - main_exit: rt.main_exit.clone(), + main_exit: rt.main_exit.subscribe(), + pid: rt.child_pid.unwrap_or(0), }) } @@ -1007,7 +1010,7 @@ impl Sandbox { // `spawn_pid_watcher`. Falls back to a blocking `waitpid` only when no // pidfd is available (kernel without `pidfd_open`). let exit_status = if self.rt().reaper.is_some() { - let mut main_exit = self.rt().main_exit.clone(); + let mut main_exit = self.rt().main_exit.subscribe(); let status = main_exit.wait_for(|s| s.is_some()).await .map(|s| s.clone().expect("checked")) .unwrap_or(crate::result::ExitStatus::Killed); @@ -1019,6 +1022,7 @@ impl Sandbox { } }; self.rt_mut().state = RuntimeState::MainExited(exit_status.clone()); + self.rt().main_exit.send_replace(Some(exit_status.clone())); if self.rt().tty_foreground_taken { sandbox_restore_tty_foreground(self.rt().pgid); @@ -1481,13 +1485,7 @@ impl Sandbox { #[cfg(feature = "checkpoint")] pub async fn checkpoint(&self) -> Result { use crate::error::{SandboxRuntimeError, SandlockError}; - // The reaper reaps main as soon as it exits, so its pid may already - // name an unrelated process. - let pid = self.runtime.as_ref() - .filter(|rt| rt.main_exit.borrow().is_none()) - .filter(|rt| !matches!(rt.state, RuntimeState::MainExited(_) | RuntimeState::Stopped(_))) - .and_then(|rt| rt.child_pid) - .ok_or(SandlockError::Runtime(SandboxRuntimeError::NotRunning))?; + let pid = self.pid().ok_or(SandlockError::Runtime(SandboxRuntimeError::NotRunning))?; self.checkpoint_pid(pid).await } @@ -1713,7 +1711,7 @@ impl Sandbox { pidfd: None, reaper: None, pgid: clone_pid, - main_exit: tokio::sync::watch::channel(None).1, + main_exit: tokio::sync::watch::channel(None).0, stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), @@ -1825,7 +1823,7 @@ impl Sandbox { pidfd: None, reaper: None, pgid: 0, - main_exit: tokio::sync::watch::channel(None).1, + main_exit: tokio::sync::watch::channel(None).0, stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), @@ -2217,7 +2215,7 @@ impl Sandbox { let key = reaper.pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; self.rt_mut().groups.track(reaper.pid, key); self.rt_mut().pgid = reaper.pid; - self.rt_mut().main_exit = spawn_main_exit_watch(status); + spawn_main_exit_watch(status, self.rt().main_exit.clone()); self.rt_mut().reaper = Some(reaper); let pidfd = syscall::pidfd_open(pid as u32, 0).ok(); @@ -2599,8 +2597,7 @@ impl Process<'_> { self.sandbox.rt_mut()._stderr_read.take() } - /// The child PID, or `None` if not spawned. Remains `Some` after the child - /// exits (until the `Sandbox` is dropped). + /// Main's pid until it exits. pub fn pid(&self) -> Option { self.sandbox.pid() } @@ -2987,19 +2984,22 @@ fn sandbox_read_fd_to_end(fd: std::os::fd::OwnedFd) -> Vec { /// `wait()` and for `ps`, whether or not anyone is waiting yet. fn spawn_main_exit_watch( status: std::os::fd::OwnedFd, -) -> tokio::sync::watch::Receiver> { + tx: tokio::sync::watch::Sender>, +) { use tokio::io::AsyncReadExt; - let (tx, rx) = tokio::sync::watch::channel(None); tokio::spawn(async move { - let Ok(mut pipe) = tokio::net::unix::pipe::Receiver::from_owned_fd(status) else { return }; let mut raw = [0u8; 4]; - let exit = match pipe.read_exact(&mut raw).await { - Ok(_) => sandbox_wait_status_to_exit(i32::from_le_bytes(raw)), - Err(_) => crate::result::ExitStatus::Killed, + let read = match tokio::net::unix::pipe::Receiver::from_owned_fd(status) { + Ok(mut pipe) => pipe.read_exact(&mut raw).await.is_ok(), + Err(_) => false, + }; + let exit = if read { + sandbox_wait_status_to_exit(i32::from_le_bytes(raw)) + } else { + crate::result::ExitStatus::Killed }; - let _ = tx.send(Some(exit)); + tx.send_replace(Some(exit)); }); - rx } fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index d097cb12..5c1bff69 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -260,6 +260,17 @@ async fn test_spawn_beside_a_parked_sibling() { sb.shutdown().await.unwrap(); } +#[tokio::test] +async fn test_pid_is_gone_once_main_exits() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + let controller = sb.controller().unwrap(); + sb.wait().await.unwrap(); + assert_eq!(sb.pid(), None); + assert_eq!(controller.pid(), None); + sb.shutdown().await.unwrap(); +} + #[tokio::test] async fn test_spawn_reports_a_failed_exec() { let mut sb = policy(); diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index 59d0c64f..53ddc3ae 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1408,7 +1408,7 @@ pub unsafe extern "C" fn sandlock_handle_pid(h: *const sandlock_handle_t) -> i32 if h.is_null() { return 0; } - (*h).sandbox.pid().unwrap_or(0) + handle_controller(h).and_then(|c| c.pid()).unwrap_or(0) } /// Send SIGKILL to every process group of the session. Idempotent: a session diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index 8844c835..3f49b721 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -874,7 +874,6 @@ func SyscallNr(name string) (int, error) { type Process struct { mu sync.Mutex h *C.sandlock_handle_t - pid int waiting int // Waits in the native call waitDone chan struct{} // closed when the last of them returns status *ExitStatus // main's status once a Wait has seen it @@ -918,7 +917,7 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error) { C.sandlock_handle_free(h) return nil, fmt.Errorf("sandlock: failed to start sandbox") } - p := &Process{h: h, pid: int(C.sandlock_handle_pid(h))} + p := &Process{h: h} // Last-resort cleanup if the caller drops the Process without // Shutdown/Close: freeing the handle kills and reaps the session. // Shutdown and Close clear this once they have done the cleanup. @@ -983,7 +982,6 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { } p := &Process{ h: h, - pid: int(C.sandlock_handle_pid(h)), Stdin: fdFile(fdIn, "sandlock-stdin"), Stdout: fdFile(fdOut, "sandlock-stdout"), Stderr: fdFile(fdErr, "sandlock-stderr"), @@ -1008,11 +1006,15 @@ func (p *Process) finalize() { p.h = nil } -// Pid returns the child process ID, or 0 if it is not available. +// Pid returns main's process ID until it exits, else 0: the number may then +// name another process. func (p *Process) Pid() int { p.mu.Lock() defer p.mu.Unlock() - return p.pid + if p.h == nil || p.pending { + return 0 + } + return int(C.sandlock_handle_pid(p.h)) } // Wait blocks until the main process exits and returns its status. Background diff --git a/python/README.md b/python/README.md index 6b0d538b..b13141bd 100644 --- a/python/README.md +++ b/python/README.md @@ -418,7 +418,7 @@ The sandbox name. #### `sandbox.pid -> int | None` -The child PID while running, `None` otherwise. +Main's PID until it exits, `None` otherwise. #### `sandbox.is_running -> bool` diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 5467f0a0..de97a70b 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -535,7 +535,6 @@ def __post_init__(self): # own handle (see `_popen_process`), this is only a # non-owning busy marker self._restore_skipped = [] # SkippedFd entries from the last restore - self._pid = None # Every native call on _handle except a wait runs under this lock, so # none can use a handle that shutdown() is freeing; waits only count # themselves in and out, so a signal can reach a blocked wait. @@ -642,14 +641,15 @@ def __exit__(self, exc_type, exc_val, exc_tb): @property def pid(self) -> int | None: - """Child PID while running, None otherwise. Reflects a running child from - either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" + """Main's PID until it exits, None otherwise. Reflects a running child + from either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" proc = self._popen_process() if proc is not None: - # The Process owns the handle; its pid is lock-guarded and cached, so - # reading it can't race a concurrent wait() freeing the handle. return proc.pid - return self._pid if self._handle is not None else None + with self._idle: + if self._handle is None: + return None + return _main_pid(self._handle) @property def is_running(self) -> bool: @@ -992,7 +992,6 @@ def create(self, cmd: Sequence[str]) -> None: ) if not self._handle: raise RuntimeError("sandlock_create failed") - self._pid = _lib.sandlock_handle_pid(self._handle) or None def start(self) -> None: """Release a previously ``create()``d child to ``execve`` the @@ -1401,7 +1400,6 @@ def restore_interactive(self, cp: "Checkpoint") -> None: if not handle: raise RuntimeError("checkpoint restore failed") self._handle = handle - self._pid = _lib.sandlock_handle_pid(handle) or None # Copy the skipped-fd diagnostics out eagerly: the native entries live # on the handle, which shutdown() frees. skipped = [] @@ -1535,11 +1533,10 @@ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, st # waits only count themselves in and out. self._idle = threading.Condition() self._waiting = 0 - # The handle and pid are set only AFTER the fds are wrapped below: if an + # The handle is set only AFTER the fds are wrapped below: if an # fdopen raises, this Process never takes ownership, so popen()'s caller # frees the handle exactly once (no double free via __del__). self._handle = None - self._pid = -1 self._result = None self.stdin = self.stdout = self.stderr = None # os.fdopen takes ownership of each fd: closing the stream closes the fd, @@ -1568,14 +1565,15 @@ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, st raise self.stdin, self.stdout, self.stderr = opened # Take ownership now that construction can no longer fail. - self._pid = _lib.sandlock_handle_pid(handle) or -1 self._handle = handle @property def pid(self) -> int | None: - """The child PID until :meth:`shutdown`, else ``None``.""" + """Main's PID until it exits, else ``None``.""" with self._idle: - return self._pid if self._handle is not None and self._pid > 0 else None + if self._handle is None: + return None + return _main_pid(self._handle) def ports(self) -> dict[int, int]: """Current virtual→real port mappings while running, else ``{}`` (needs @@ -1747,6 +1745,11 @@ def _process_groups(handle): _lib.sandlock_string_free(ctypes.cast(ptr, ctypes.c_char_p)) +def _main_pid(handle): + from ._sdk import _lib + return _lib.sandlock_handle_pid(handle) or None + + def _send_signal(handle, name): from ._sdk import _lib if getattr(_lib, name)(handle) != 0: diff --git a/python/tests/test_sandbox.py b/python/tests/test_sandbox.py index 55e451e2..631126ac 100644 --- a/python/tests/test_sandbox.py +++ b/python/tests/test_sandbox.py @@ -795,7 +795,7 @@ def test_pid_available_during_run(self): assert pid > 0 sb.wait() - assert sb.pid == pid, "the session outlives its main process" + assert sb.pid is None, "an exited main's pid may already be reused" sb.shutdown() assert sb.pid is None From 36f06d4504e92fbcfc3bfbd772c474a85df1baf4 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 18:42:09 -0700 Subject: [PATCH 15/18] sandbox: read main's exit status without a background task Main's status was read by a task spawned on the runtime that created the sandbox and published on a watch. If that runtime went away, the task died with it, and since the runtime also held a sender, wait() never returned. The reaper also reaped main before sending its status, so for a moment main's pid was free while pid() still reported it. The reaper now reads main's status with WNOWAIT, sends it, and only then reaps. A shared MainExit cell reads the status pipe without blocking for pid(), checkpoint() and the controller, and wait() polls it through an AsyncFd on whichever runtime calls it. Paths without a reaper still publish through wait(). Signed-off-by: Cong Wang --- crates/sandlock-core/src/control.rs | 2 +- crates/sandlock-core/src/controller.rs | 60 ++++++++++++++++++- crates/sandlock-core/src/reaper.rs | 37 +++++++++--- crates/sandlock-core/src/sandbox.rs | 45 ++++---------- .../tests/integration/test_process_groups.rs | 22 +++++++ 5 files changed, 118 insertions(+), 48 deletions(-) diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index f2af5036..cc80ee0c 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -851,7 +851,7 @@ mod tests { let controller = crate::Controller { groups: Arc::new(crate::pgroup::ProcessGroups::new()), stop: Default::default(), - main_exit: tokio::sync::watch::channel(None).1, + main_exit: crate::controller::MainExit::new(None), pid: 0, }; control_loop(sockets.control, None, controller, sandbox, info(), expected_uid).await; diff --git a/crates/sandlock-core/src/controller.rs b/crates/sandlock-core/src/controller.rs index 8f1aa8b8..8deeaf9c 100644 --- a/crates/sandlock-core/src/controller.rs +++ b/crates/sandlock-core/src/controller.rs @@ -1,6 +1,7 @@ //! Acts on a running session while `Sandbox::wait()` holds it mutably. -use std::sync::Arc; +use std::os::fd::{AsRawFd, OwnedFd}; +use std::sync::{Arc, Mutex}; use crate::error::{SandboxRuntimeError, SandlockError}; use crate::result::ExitStatus; @@ -9,7 +10,7 @@ use crate::result::ExitStatus; pub struct Controller { pub(crate) groups: Arc, pub(crate) stop: tokio_util::sync::CancellationToken, - pub(crate) main_exit: tokio::sync::watch::Receiver>, + pub(crate) main_exit: Arc, pub(crate) pid: i32, } @@ -32,7 +33,7 @@ impl Controller { } pub fn main_exited(&self) -> bool { - self.main_exit.borrow().is_some() + self.main_exit.get().is_some() } /// Main's pid until it exits; afterwards the number may name another process. @@ -55,3 +56,56 @@ impl Controller { Ok(()) } } + +/// Main's exit status, read straight from the reaper's status pipe so that no +/// background task, and no particular runtime, has to be alive to learn it. +pub(crate) struct MainExit { + pipe: Option, + status: Mutex>, +} + +impl MainExit { + /// `pipe` carries main's raw wait status as four bytes; without one, the + /// owner publishes the status through [`MainExit::set`]. + pub(crate) fn new(pipe: Option) -> Arc { + if let Some(fd) = &pipe { + unsafe { libc::fcntl(fd.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) }; + } + Arc::new(MainExit { pipe, status: Mutex::new(None) }) + } + + pub(crate) fn get(&self) -> Option { + let mut status = self.status.lock().unwrap(); + if status.is_none() { + if let Some(fd) = &self.pipe { + let mut raw = [0u8; 4]; + let n = unsafe { libc::read(fd.as_raw_fd(), raw.as_mut_ptr().cast(), raw.len()) }; + *status = match n { + 4 => Some(crate::sandbox::sandbox_wait_status_to_exit(i32::from_le_bytes(raw))), + // The write is atomic, so anything else but EAGAIN means + // the reaper died without reporting main. + n if n < 0 && std::io::Error::last_os_error().kind() == std::io::ErrorKind::WouldBlock => None, + _ => Some(ExitStatus::Killed), + }; + } + } + status.clone() + } + + pub(crate) fn set(&self, exit: ExitStatus) { + self.status.lock().unwrap().get_or_insert(exit); + } + + pub(crate) async fn wait(&self) -> std::io::Result { + let Some(fd) = &self.pipe else { + return Err(std::io::Error::other("main's exit has no status pipe")); + }; + let ready = tokio::io::unix::AsyncFd::with_interest(fd.as_raw_fd(), tokio::io::Interest::READABLE)?; + loop { + if let Some(exit) = self.get() { + return Ok(exit); + } + ready.readable().await?.clear_ready(); + } + } +} diff --git a/crates/sandlock-core/src/reaper.rs b/crates/sandlock-core/src/reaper.rs index 491e4466..845d0d16 100644 --- a/crates/sandlock-core/src/reaper.rs +++ b/crates/sandlock-core/src/reaper.rs @@ -136,18 +136,27 @@ unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signa ]; loop { loop { - let mut status = 0; - match libc::waitpid(-1, &mut status, libc::WNOHANG | libc::__WALL) { - 0 => break, - pid if pid == main => { - send(status_fd, &status.to_le_bytes()); - libc::close(status_fd); + // WNOWAIT: main's status is sent before its pid is freed, so no + // reader can see main alive once that number is reusable. + let mut info: libc::siginfo_t = std::mem::zeroed(); + if libc::waitid(libc::P_ALL, 0, &mut info, libc::WEXITED | libc::WNOHANG | libc::WNOWAIT | libc::__WALL) < 0 { + if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) { + continue; } - pid if pid > 0 => {} - _ if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} // No child left means no session process left. - _ => libc::_exit(0), + libc::_exit(0); + } + let pid = info.si_pid(); + if pid == 0 { + break; + } + if pid == main { + send(status_fd, &wait_status(&info).to_le_bytes()); + libc::close(status_fd); } + while libc::waitpid(pid, std::ptr::null_mut(), libc::__WALL) < 0 + && io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) + {} } libc::poll(poll.as_mut_ptr(), 3, -1); // Owner death also closes its end of the finish pipe, so it must be @@ -166,6 +175,16 @@ unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signa } } +/// The raw wait status that `waitpid` would have returned for `info`. +unsafe fn wait_status(info: &libc::siginfo_t) -> i32 { + let status = info.si_status(); + match info.si_code { + libc::CLD_EXITED => (status & 0xff) << 8, + libc::CLD_DUMPED => status | 0x80, + _ => status, + } +} + /// True for the finish byte, false for EOF. unsafe fn read_finish(fd: i32) -> bool { let mut byte = 0u8; diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 0a86aa6c..96209de4 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -291,7 +291,7 @@ struct Runtime { reaper: Option, /// Main's process group: the reaper's pid, or a clone's own pid. pgid: i32, - main_exit: tokio::sync::watch::Sender>, + main_exit: Arc, groups: Arc, /// Cancelled by a kill, so a held policy_fn verdict cannot stall it. stop: tokio_util::sync::CancellationToken, @@ -869,7 +869,7 @@ impl Sandbox { /// Main's pid until it exits; afterwards the number may name another process. pub fn pid(&self) -> Option { self.runtime.as_ref() - .filter(|rt| rt.main_exit.borrow().is_none()) + .filter(|rt| rt.main_exit.get().is_none()) .and_then(|rt| rt.child_pid) } @@ -896,7 +896,7 @@ impl Sandbox { Ok(crate::Controller { groups: Arc::clone(&rt.groups), stop: rt.stop.clone(), - main_exit: rt.main_exit.subscribe(), + main_exit: Arc::clone(&rt.main_exit), pid: rt.child_pid.unwrap_or(0), }) } @@ -1010,11 +1010,8 @@ impl Sandbox { // `spawn_pid_watcher`. Falls back to a blocking `waitpid` only when no // pidfd is available (kernel without `pidfd_open`). let exit_status = if self.rt().reaper.is_some() { - let mut main_exit = self.rt().main_exit.subscribe(); - let status = main_exit.wait_for(|s| s.is_some()).await - .map(|s| s.clone().expect("checked")) - .unwrap_or(crate::result::ExitStatus::Killed); - status + let main_exit = Arc::clone(&self.rt().main_exit); + main_exit.wait().await.map_err(SandboxRuntimeError::Io)? } else { match self.rt_mut().pidfd.take() { Some(pidfd) => wait_child_exit_via_pidfd(pidfd, pid).await, @@ -1022,7 +1019,7 @@ impl Sandbox { } }; self.rt_mut().state = RuntimeState::MainExited(exit_status.clone()); - self.rt().main_exit.send_replace(Some(exit_status.clone())); + self.rt().main_exit.set(exit_status.clone()); if self.rt().tty_foreground_taken { sandbox_restore_tty_foreground(self.rt().pgid); @@ -1711,7 +1708,7 @@ impl Sandbox { pidfd: None, reaper: None, pgid: clone_pid, - main_exit: tokio::sync::watch::channel(None).0, + main_exit: crate::controller::MainExit::new(None), stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), @@ -1823,7 +1820,7 @@ impl Sandbox { pidfd: None, reaper: None, pgid: 0, - main_exit: tokio::sync::watch::channel(None).0, + main_exit: crate::controller::MainExit::new(None), stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), @@ -2215,7 +2212,7 @@ impl Sandbox { let key = reaper.pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; self.rt_mut().groups.track(reaper.pid, key); self.rt_mut().pgid = reaper.pid; - spawn_main_exit_watch(status, self.rt().main_exit.clone()); + self.rt_mut().main_exit = crate::controller::MainExit::new(Some(status)); self.rt_mut().reaper = Some(reaper); let pidfd = syscall::pidfd_open(pid as u32, 0).ok(); @@ -2980,29 +2977,7 @@ fn sandbox_read_fd_to_end(fd: std::os::fd::OwnedFd) -> Vec { buf } -/// Main's status arrives once on the reaper's pipe; the slot keeps it for -/// `wait()` and for `ps`, whether or not anyone is waiting yet. -fn spawn_main_exit_watch( - status: std::os::fd::OwnedFd, - tx: tokio::sync::watch::Sender>, -) { - use tokio::io::AsyncReadExt; - tokio::spawn(async move { - let mut raw = [0u8; 4]; - let read = match tokio::net::unix::pipe::Receiver::from_owned_fd(status) { - Ok(mut pipe) => pipe.read_exact(&mut raw).await.is_ok(), - Err(_) => false, - }; - let exit = if read { - sandbox_wait_status_to_exit(i32::from_le_bytes(raw)) - } else { - crate::result::ExitStatus::Killed - }; - tx.send_replace(Some(exit)); - }); -} - -fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { +pub(crate) fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { use crate::result::ExitStatus; if libc::WIFEXITED(status) { ExitStatus::Code(libc::WEXITSTATUS(status)) diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index 5c1bff69..7f7c003c 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -271,6 +271,28 @@ async fn test_pid_is_gone_once_main_exits() { sb.shutdown().await.unwrap(); } +/// Main's status must not depend on a task of the runtime that created the +/// sandbox. +#[test] +fn test_wait_on_another_runtime() { + let runtime = || tokio::runtime::Builder::new_multi_thread().enable_all().build().unwrap(); + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin") + .no_supervisor(true) + .build() + .unwrap(); + let first = runtime(); + first.block_on(sb.spawn(&["sh", "-c", "sleep 0.2; exit 3"])).unwrap(); + drop(first); + let second = runtime(); + let status = second + .block_on(async { tokio::time::timeout(std::time::Duration::from_secs(5), sb.wait()).await }) + .expect("wait() hangs") + .unwrap(); + assert_eq!(status, sandlock_core::ExitStatus::Code(3)); + second.block_on(sb.shutdown()).unwrap(); +} + #[tokio::test] async fn test_spawn_reports_a_failed_exec() { let mut sb = policy(); From 309b58c75073cda9ef514a32d104171296be1704 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 18:53:11 -0700 Subject: [PATCH 16/18] oci: drop the killpg fallback on the container's init pid kill --all and delete --force fell back to killpg(init pid) when the daemon could not be reached. sandlock-init now runs in its reaper's process group and never leads one, so that number names no group of the session: the call either failed or, once the number was reused as a group id, signalled unrelated processes. The fallback is not needed either: the daemon owns the reaper, which kills the session when its owner dies. Signed-off-by: Cong Wang --- crates/sandlock-oci/src/main.rs | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/crates/sandlock-oci/src/main.rs b/crates/sandlock-oci/src/main.rs index a7d60a06..56f03947 100644 --- a/crates/sandlock-oci/src/main.rs +++ b/crates/sandlock-oci/src/main.rs @@ -723,17 +723,13 @@ fn cmd_kill(id: &str, signal: &str, all: bool) -> Result<()> { let signum = parse_signal(signal)?; if all { - // For group-wide signals, ask the daemon to killpg so it uses the - // correct pgid (sandlock-init's pid, not the workload's). Fall back - // to a direct killpg if the daemon is unreachable (already exited). - let sent = supervisor::send_command( + // Only the daemon knows the session's groups: init does not lead + // one, so its pid names no group. A daemon that is gone took the + // session with it, since its reaper kills the session on owner death. + let _ = supervisor::send_command( id, supervisor::SupervisorCmd::Signal { signum }, ); - if sent.is_err() { - // Daemon gone: best-effort direct killpg on the recorded pid. - unsafe { libc::killpg(state.pid, signum) }; - } } else { let ret = unsafe { libc::kill(state.pid, signum) }; if ret < 0 { @@ -768,15 +764,10 @@ fn cmd_delete(id: &str, force: bool) -> Result<()> { } // If running with --force, ask the daemon to shutdown (it sends Shutdown to - // sandlock-init, which killpg's its group and exits). This correctly targets - // the process group even when state.pid is the workload (not the pgid). - // Fall back to a direct killpg if the daemon is already gone. + // sandlock-init, which killpg's its group and exits). A daemon that is + // already gone took the session with it. if state.status == Status::Running && state.pid > 0 && state.is_alive() { - let sent = supervisor::send_command(id, supervisor::SupervisorCmd::Shutdown); - if sent.is_err() { - // Daemon already gone: kill whatever we can reach. - unsafe { libc::killpg(state.pid, libc::SIGKILL) }; - } + let _ = supervisor::send_command(id, supervisor::SupervisorCmd::Shutdown); std::thread::sleep(std::time::Duration::from_millis(50)); } From 5d04f4e3219b511c555ffb5f7eaa5d1e06a4e17a Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 18:55:39 -0700 Subject: [PATCH 17/18] sandbox: return the terminal from any sandbox group on shutdown wait() handed the terminal back only while main's own group held it, and only once. A job that outlives main can block SIGTTOU and take the terminal back afterwards; shutdown() then killed it and left the caller's group in the background, so its next read from the terminal stopped it. The same happened when main moved itself into another group before taking the terminal. Restore the caller's foreground whenever a group the sandbox tracked holds the terminal, on wait() and again in shutdown() and Drop after the session is gone. Signed-off-by: Cong Wang --- crates/sandlock-core/src/pgroup.rs | 5 ++ crates/sandlock-core/src/sandbox.rs | 25 +++++----- .../tests/integration/test_tty.rs | 47 +++++++++++++++++++ 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/crates/sandlock-core/src/pgroup.rs b/crates/sandlock-core/src/pgroup.rs index 26e4f3a6..93aaff1b 100644 --- a/crates/sandlock-core/src/pgroup.rs +++ b/crates/sandlock-core/src/pgroup.rs @@ -52,6 +52,11 @@ impl ProcessGroups { .unwrap_or(false) } + /// Whether `pgid` was a group of this sandbox, members or not. + pub fn tracks(&self, pgid: i32) -> bool { + self.leaders.lock().map(|leaders| leaders.contains_key(&pgid)).unwrap_or(false) + } + /// Sorted pgids that still have members. pub fn live(&self) -> io::Result> { let leaders = self.leaders.lock() diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 96209de4..dea66e58 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -289,8 +289,6 @@ struct Runtime { child_pid: Option, pidfd: Option, reaper: Option, - /// Main's process group: the reaper's pid, or a clone's own pid. - pgid: i32, main_exit: Arc, groups: Arc, /// Cancelled by a kill, so a held policy_fn verdict cannot stall it. @@ -1022,8 +1020,7 @@ impl Sandbox { self.rt().main_exit.set(exit_status.clone()); if self.rt().tty_foreground_taken { - sandbox_restore_tty_foreground(self.rt().pgid); - self.rt_mut().tty_foreground_taken = false; + sandbox_restore_tty_foreground(&self.rt().groups); } Ok(exit_status) } @@ -1046,6 +1043,11 @@ impl Sandbox { reaper.join().await; self.rt().drain_cut.cancel(); } + // A job that outlived main may have taken the terminal back. + if self.rt().tty_foreground_taken { + sandbox_restore_tty_foreground(&self.rt().groups); + self.rt_mut().tty_foreground_taken = false; + } self.rt_mut().state = RuntimeState::Stopped(exit_status); let rt = self.rt_mut(); @@ -1707,7 +1709,6 @@ impl Sandbox { child_pid: Some(clone_pid), pidfd: None, reaper: None, - pgid: clone_pid, main_exit: crate::controller::MainExit::new(None), stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), @@ -1819,7 +1820,6 @@ impl Sandbox { child_pid: None, pidfd: None, reaper: None, - pgid: 0, main_exit: crate::controller::MainExit::new(None), stop: tokio_util::sync::CancellationToken::new(), drain_cut: tokio_util::sync::CancellationToken::new(), @@ -2211,7 +2211,6 @@ impl Sandbox { let status = reaper.take_status().expect("fresh reaper"); let key = reaper.pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; self.rt_mut().groups.track(reaper.pid, key); - self.rt_mut().pgid = reaper.pid; self.rt_mut().main_exit = crate::controller::MainExit::new(Some(status)); self.rt_mut().reaper = Some(reaper); let pidfd = syscall::pidfd_open(pid as u32, 0).ok(); @@ -2625,14 +2624,14 @@ impl Process<'_> { } /// Hand the terminal's foreground process group back to this process after -/// reaping an interactive child that took it. Restores only while the child's -/// group still owns the terminal, so a foreground the caller has since given -/// to someone else is left alone. SIGTTOU is blocked around `tcsetpgrp`: +/// reaping an interactive child that took it. Restores only while one of the +/// sandbox's groups holds the terminal, so a foreground the caller has since +/// given to someone else is left alone. SIGTTOU is blocked around `tcsetpgrp`: /// this process is a background group at that moment, and an unblocked /// SIGTTOU would stop it, which is the exact symptom being prevented. -fn sandbox_restore_tty_foreground(pgid: i32) { +fn sandbox_restore_tty_foreground(groups: &crate::pgroup::ProcessGroups) { unsafe { - if libc::isatty(0) != 1 || libc::tcgetpgrp(0) != pgid { + if libc::isatty(0) != 1 || !groups.tracks(libc::tcgetpgrp(0)) { return; } let mut block: libc::sigset_t = std::mem::zeroed(); @@ -2667,7 +2666,7 @@ impl Drop for Sandbox { } } if rt.tty_foreground_taken { - sandbox_restore_tty_foreground(rt.pgid); + sandbox_restore_tty_foreground(&rt.groups); rt.tty_foreground_taken = false; } } diff --git a/crates/sandlock-core/tests/integration/test_tty.rs b/crates/sandlock-core/tests/integration/test_tty.rs index 66ea2ca0..790de024 100644 --- a/crates/sandlock-core/tests/integration/test_tty.rs +++ b/crates/sandlock-core/tests/integration/test_tty.rs @@ -170,3 +170,50 @@ fn interactive_run_hands_tty_foreground_to_child_while_alive() { 13=child never got the tty foreground, 14=tty foreground not returned after reap)" ); } + +/// A job that outlives main can take the terminal back after wait() returned +/// it; shutdown() kills that job and must not leave the caller in the +/// background. +#[test] +fn shutdown_returns_tty_foreground_taken_by_a_job() { + if !std::path::Path::new("/usr/bin/python3").exists() { + return; + } + let code = run_in_pty_session(|| { + block_on(async { + let name = format!("tty-job-steal-{}", std::process::id()); + let mut policy = test_policy().with_name(&name); + let script = "import os, signal, time\n\ + signal.signal(signal.SIGTTOU, signal.SIG_IGN)\n\ + if os.fork() == 0:\n \ + time.sleep(0.3)\n \ + os.tcsetpgrp(0, os.getpgrp())\n \ + time.sleep(60)\n"; + if policy.spawn_interactive(&["python3", "-c", script]).await.is_err() { + return EXIT_RUN_FAILED; + } + if policy.wait().await.is_err() { + return EXIT_RUN_FAILED; + } + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while unsafe { libc::tcgetpgrp(0) == libc::getpgrp() } { + if std::time::Instant::now() > deadline { + return EXIT_FOREGROUND_NOT_TAKEN; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + if policy.shutdown().await.is_err() { + return EXIT_RUN_FAILED; + } + if unsafe { libc::tcgetpgrp(0) != libc::getpgrp() } { + return EXIT_FOREGROUND_NOT_RESTORED; + } + EXIT_OK + }) + }); + assert_eq!( + code, EXIT_OK, + "helper exit {code} (10=no tty foreground at start, 11=spawn/wait/shutdown failed, \ + 13=the job never took the tty foreground, 14=tty foreground not returned after shutdown)" + ); +} From 52e0734b60d6ab1e5fb4955e4275137e6d6aead6 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Thu, 15 Oct 2026 19:16:23 -0700 Subject: [PATCH 18/18] cli: make ps exact about main and show what a session has left ps asked the supervisor whether main was alive, then read /proc/. If main exited in between and its pid was reused, the row showed another process's uptime and command. It now opens a pidfd for main before asking, so a positive answer proves the pidfd names main, and keeps what it read from /proc only if main is still alive afterwards. After main exits, a session stays listed until its owner shuts it down, and ps showed "(main exited)" whether jobs were still running or none were. The info verb now reports how many process groups still have members, and ps shows "(main exited, N groups)" or "(main exited, idle)". Signed-off-by: Cong Wang --- crates/sandlock-cli/src/main.rs | 66 ++++++++++++++----- crates/sandlock-core/src/control.rs | 13 +++- crates/sandlock-core/src/sandbox.rs | 2 +- .../tests/integration/test_control.rs | 24 ++++++- 4 files changed, 84 insertions(+), 21 deletions(-) diff --git a/crates/sandlock-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index d8ad5dd5..fad0ab8c 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -285,24 +285,28 @@ async fn main() -> Result<()> { } else { println!("NAME PID UPTIME STATUS PORTS CMD"); for name in &names { - // The registry's main pid outlives main, so /proc is read - // only while the supervisor vouches main is alive. + // The registry's main pid outlives main. A pidfd opened before + // the supervisor vouches main is alive names main, and /proc + // is trusted only if main is still alive after reading it. let pid = sandlock_core::control::sandbox_pids(name).map(|p| p.child); + let main_fd = pid.as_ref().ok().and_then(|&pid| pidfd_open(pid)); let (status, ports, pid, uptime, cmd) = match (sandlock_core::control::sandbox_info(name), pid) { - (Ok(info), _) if info.main_exited => ( - info.mode.unwrap_or_else(|| "running".to_string()), - query_ports(name), - "-".to_string(), - "-".to_string(), - "(main exited)".to_string(), - ), - (Ok(info), Ok(pid)) => ( - info.mode.unwrap_or_else(|| "running".to_string()), - query_ports(name), - pid.to_string(), - proc_uptime(pid).unwrap_or_else(|| "?".to_string()), - proc_cmdline(pid).unwrap_or_else(|| "?".to_string()), - ), + (Ok(info), Ok(pid)) if !info.main_exited => { + let uptime = proc_uptime(pid); + let cmd = proc_cmdline(pid); + if main_fd.as_ref().is_some_and(|fd| !pidfd_exited(fd)) { + ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), + pid.to_string(), + uptime.unwrap_or_else(|| "?".to_string()), + cmd.unwrap_or_else(|| "?".to_string()), + ) + } else { + main_exited_row(name, info) + } + } + (Ok(info), _) if info.main_exited => main_exited_row(name, info), (info, pid) => ( if info.is_ok() { "running" } else { "unresponsive" }.to_string(), "?".to_string(), @@ -945,6 +949,36 @@ fn validate_cli_name(name: &str) -> Result<(), String> { /// Query the control socket for the virtual→real port map, returning a /// compact display string or `"-"` when the socket is missing (e.g. /// `--no-supervisor`). +fn main_exited_row( + name: &str, + info: sandlock_core::control::SandboxInfo, +) -> (String, String, String, String, String) { + let cmd = match info.live_groups { + 0 => "(main exited, idle)".to_string(), + 1 => "(main exited, 1 group)".to_string(), + n => format!("(main exited, {n} groups)"), + }; + ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), + "-".to_string(), + "-".to_string(), + cmd, + ) +} + +fn pidfd_open(pid: i32) -> Option { + use std::os::fd::FromRawFd; + let fd = unsafe { libc::syscall(libc::SYS_pidfd_open, pid, 0) }; + (fd >= 0).then(|| unsafe { std::os::fd::OwnedFd::from_raw_fd(fd as i32) }) +} + +fn pidfd_exited(fd: &std::os::fd::OwnedFd) -> bool { + use std::os::fd::AsRawFd; + let mut poll = libc::pollfd { fd: fd.as_raw_fd(), events: libc::POLLIN, revents: 0 }; + unsafe { libc::poll(&mut poll, 1, 0) != 0 } +} + fn query_ports(name: &str) -> String { use sandlock_core::control::send_control_request; match send_control_request(name, "ports", serde_json::Value::Object(Default::default())) { diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index cc80ee0c..d62bc525 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -172,6 +172,9 @@ pub struct SandboxInfo { /// Main's pid in the registry is free for reuse once this is set. #[serde(default)] pub main_exited: bool, + /// Process groups of the session that still have members. + #[serde(default)] + pub live_groups: usize, } /// Spawn the control-loop task. `ctx` is `None` for sandboxes without a @@ -390,7 +393,11 @@ async fn serve_one( } async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo, controller: &crate::Controller) { - let info = SandboxInfo { main_exited: controller.main_exited(), ..info.clone() }; + let info = SandboxInfo { + main_exited: controller.main_exited(), + live_groups: controller.process_groups().map_or(0, |groups| groups.len()), + ..info.clone() + }; let resp = match serde_json::to_value(&info) { Ok(data) => ControlResponse { v: 1, ok: true, data: Some(data), err: None }, Err(e) => ControlResponse { @@ -607,7 +614,7 @@ fn connect_entry(name: &str, entry: &ownership::Entry, my_uid: u32) -> Result<(U /// The sandbox's PIDs, stamped by the kernel at semaphore publication. #[derive(Debug, Clone, Copy)] pub struct SandboxPids { - /// The child, which leads its own process group. + /// Main, whose pid is free for reuse once it has exited. pub child: i32, pub supervisor: i32, } @@ -838,7 +845,7 @@ mod tests { } fn info() -> SandboxInfo { - SandboxInfo { mode: Some("test".into()), main_exited: false } + SandboxInfo { mode: Some("test".into()), main_exited: false, live_groups: 0 } } /// Bind a listener for `name`, run the control loop on it with diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index dea66e58..a20fa956 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2233,7 +2233,7 @@ impl Sandbox { }; let exec_r = take_child_fd(exec_fd_num, libc::O_RDONLY)?; - let control_info = crate::control::SandboxInfo { mode: self.mode.clone(), main_exited: false }; + let control_info = crate::control::SandboxInfo { mode: self.mode.clone(), main_exited: false, live_groups: 0 }; let is_nested_mode = notif_fd_num == 0; diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index 78be2095..481c24e2 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -898,7 +898,29 @@ async fn test_control_ps_hides_the_pid_of_an_exited_main() { let stdout = String::from_utf8_lossy(&out.stdout); let row = stdout.lines().find(|l| l.starts_with(&name)).expect("ps lists the session"); assert_eq!(row.split_whitespace().nth(1), Some("-"), "exited main has no pid: {row}"); - assert!(row.ends_with("(main exited)"), "{row}"); + assert!(row.ends_with("(main exited, 1 group)"), "{row}"); + sb.shutdown().await.unwrap(); +} + +/// A session whose main exited alone stays listed until shutdown(); ps must +/// tell it apart from one with jobs still running. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_ps_reports_an_idle_session() { + let name = format!("test-ctrl-idle-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["true"]).await.unwrap(); + sb.wait().await.unwrap(); + let out = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + let stdout = String::from_utf8_lossy(&out.stdout); + let row = stdout.lines().find(|l| l.starts_with(&name)).expect("ps lists the session"); + assert!(row.ends_with("(main exited, idle)"), "{row}"); sb.shutdown().await.unwrap(); }