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-cli/src/main.rs b/crates/sandlock-cli/src/main.rs index 08b4c083..5c2e8956 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -285,23 +285,32 @@ 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 itself, so it is + // looked up only while the supervisor vouches for it. One + // that cannot answer is still alive because its socket is. + 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 +367,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..24ce6295 100644 --- a/crates/sandlock-cli/tests/cli_test.rs +++ b/crates/sandlock-cli/tests/cli_test.rs @@ -671,6 +671,61 @@ 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: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(!child.wait().expect("wait for killed child").success()); +} + +#[test] +fn test_kill_wedged_supervisor_kills_main() { + use std::os::unix::process::ExitStatusExt; + let name = format!("test-kill-wedged-{}", std::process::id()); + let mut child = spawn_sandbox(&name); + if let Err(e) = wait_for_sandbox(&mut child, &name) { + panic!("{}", e); + } + let ps = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + let main: i32 = String::from_utf8_lossy(&ps.stdout) + .lines() + .find_map(|line| { + let mut fields = line.split_whitespace(); + (fields.next() == Some(name.as_str())).then(|| fields.next()?.parse().ok())? + }) + .expect("main pid in ps output"); + + unsafe { libc::kill(child.id() as i32, libc::SIGSTOP) }; + let out = sandlock_bin().args(["kill", &name]).output().expect("sandlock kill"); + assert!( + out.status.success(), + "kill should succeed: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(child.wait().expect("wait for supervisor").signal(), Some(libc::SIGKILL)); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while unsafe { libc::kill(main, 0) } == 0 { + assert!(std::time::Instant::now() < deadline, "main {main} outlived its supervisor"); + std::thread::sleep(std::time::Duration::from_millis(20)); + } +} + #[test] fn test_kill_nonexistent_sandbox() { let out = sandlock_bin() 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/control.rs b/crates/sandlock-core/src/control.rs index 9601a8e7..23b4ba51 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -169,6 +169,16 @@ pub(crate) fn bind_control_socket(name: &str) -> std::io::Result #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct SandboxInfo { pub mode: Option, + /// Background jobs can keep a session alive after its main process + /// exits, and the main pid in the registry is then free for reuse. + #[serde(default)] + pub main_exited: bool, +} + +/// The live session the control loop answers for. +pub(crate) struct Session { + pub groups: Arc, + pub main: OwnedFd, } /// Spawn the control-loop task. `ctx` is `None` for sandboxes without a @@ -177,6 +187,7 @@ pub struct SandboxInfo { pub(crate) fn spawn_control_loop( sockets: ControlSocket, ctx: Option>, + session: Session, sandbox: Sandbox, info: SandboxInfo, ) -> tokio::task::JoinHandle<()> { @@ -185,7 +196,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, session, sandbox, info, unsafe { libc::getuid() }).await; }) } @@ -271,6 +282,7 @@ impl tokio::io::AsyncWrite for ControlStream { async fn control_loop( listener: ControlFd, ctx: Option>, + session: Session, sandbox: Arc>, info: SandboxInfo, my_uid: u32, @@ -289,7 +301,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(), &session, &sandbox, &info), ) .await; } @@ -321,6 +333,7 @@ pub struct ControlResponse { async fn serve_one( stream: ControlStream, ctx: Option<&Arc>, + session: &Session, sandbox: &Arc>, info: &SandboxInfo, ) { @@ -367,10 +380,10 @@ async fn serve_one( } match req.verb.as_str() { - "info" => handle_info(&mut stream, info).await, + "info" => handle_info(&mut stream, info, session).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, ctx, &session.groups).await, _ => { let resp = ControlResponse { v: 1, @@ -383,8 +396,12 @@ 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, session: &Session) { + let info = SandboxInfo { + main_exited: crate::pgroup::has_exited(session.main.as_raw_fd()), + ..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 +454,27 @@ 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>) { +async fn handle_kill( + stream: &mut ControlStream, + ctx: Option<&Arc>, + groups: &crate::pgroup::ProcessGroups, +) { 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 { + let resp = match groups.signal(libc::SIGKILL) { + Ok(groups) => ControlResponse { v: 1, ok: true, data: Some(serde_json::json!({ "groups": groups })), err: None, }, - Some(Err(e)) => ControlResponse { + 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()), - }, }; let _ = write_response(stream, &resp).await; } @@ -616,7 +631,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,9 +669,34 @@ 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)?; + exchange(name, stream, verb, args) +} - let (mut stream, _) = connect_control(name, my_uid)?; +/// Kill every process group the supervisor tracks. A supervisor that +/// accepts the connection but never answers is stopped or wedged; it is +/// killed instead, and the session reaper then kills the main process. +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()); + match exchange(name, stream, "kill", args) { + Ok(resp) if resp.ok => Ok(()), + Ok(resp) => Err(resp.err.unwrap_or_else(|| "kill failed".into())), + Err(_) => crate::sys::syscall::pidfd_send_signal(&supervisor, libc::SIGKILL) + .map_err(|e| format!("kill unresponsive supervisor: {e}")), + } +} + +fn exchange( + name: &str, + mut stream: UnixStream, + verb: &str, + args: serde_json::Value, +) -> Result { + use std::io::{Read, Write}; // Set a 2-second timeout on reads so a wedged supervisor does not // block the CLI forever. @@ -806,7 +846,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 +856,11 @@ 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 session = Session { + groups: Arc::new(crate::pgroup::ProcessGroups::new()), + main: crate::sys::syscall::pidfd_open(std::process::id(), 0).unwrap(), + }; + control_loop(sockets.control, None, session, sandbox, info(), expected_uid).await; }) } @@ -825,8 +869,8 @@ mod tests { fn raw_info_request(name: &str) -> Vec { let mut s = UnixStream::connect_addr(&socket_addr(name).unwrap()).unwrap(); let body = br#"{"v":1,"verb":"info","args":{}}"#; - s.write_all(&(body.len() as u32).to_be_bytes()).unwrap(); - s.write_all(body).unwrap(); + // A server refusing the peer may close before the request is written. + let _ = s.write_all(&(body.len() as u32).to_be_bytes()).and_then(|()| s.write_all(body)); s.set_read_timeout(Some(std::time::Duration::from_secs(2))).unwrap(); let mut out = Vec::new(); let _ = s.read_to_end(&mut out); 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/pgroup.rs b/crates/sandlock-core/src/pgroup.rs index 0edd30f9..721b2c98 100644 --- a/crates/sandlock-core/src/pgroup.rs +++ b/crates/sandlock-core/src/pgroup.rs @@ -47,11 +47,26 @@ 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) } + 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 +77,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 +103,23 @@ impl Default for ProcessGroups { } } -fn has_exited(pidfd: RawFd) -> bool { +/// Kernels before 6.9 refuse group signals by pidfd. A leader that has not +/// exited still holds its pid, so no other group can have taken the number. +/// Once it has, the number is reusable: only a probe finding no group at all +/// is conclusive, since an empty group can never regain members. +fn signal_group(pgid: i32, leader: &OwnedFd, sig: i32) -> io::Result<()> { + let refused = match pidfd_signal_group(leader, sig) { + Err(e) if e.raw_os_error() == Some(libc::EINVAL) => e, + result => return result, + }; + let reachable = !has_exited(leader.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, @@ -144,16 +175,19 @@ fn track_new_group( caller: i32, leader: i32, ) -> NotifAction { - let Ok(pidfd) = pidfd_open(leader as u32, 0) else { - return NotifAction::Continue; + // An unrecorded group is out of reach of kill, so refuse rather than allow it. + let pidfd = match pidfd_open(leader as u32, 0) { + Ok(pidfd) => pidfd, + Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EAGAIN)), }; // The kernel moves only the caller or one of its children. Anything - // else fails there with ESRCH, and recording it would aim later signals - // at a process outside the sandbox. The caller is parked in this + // else fails there with ESRCH, so answer that here: recording it would + // aim later signals outside the sandbox, and letting a misread through + // could form a group nobody recorded. The caller is parked in this // syscall, and a leader still alive after the read kept its number // throughout, so neither pid was recycled under the check. if leader != caller && (read_ppid(leader) != Some(caller) || has_exited(pidfd.as_raw_fd())) { - return NotifAction::Continue; + return NotifAction::Errno(libc::ESRCH); } if id_valid(notif_fd, notif.id).is_err() { return NotifAction::Continue; 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/reaper.rs b/crates/sandlock-core/src/reaper.rs new file mode 100644 index 00000000..7056d207 --- /dev/null +++ b/crates/sandlock-core/src/reaper.rs @@ -0,0 +1,383 @@ +use std::io; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; + +pub(crate) struct Reaper { + pub pid: i32, + pub pidfd: OwnedFd, + pub main_pidfd: OwnedFd, + status: OwnedFd, + finish: Option, + /// Set once the main process is known to have been the last one left. + pub empty: bool, +} + +pub(crate) struct MainExit { + pub status: i32, + pub last: bool, +} + +impl Reaper { + pub async fn main_exit(&self) -> io::Result { + let fd = tokio::io::unix::AsyncFd::new(self.status.try_clone()?)?; + loop { + let mut ready = fd.readable().await?; + let result = ready.try_io(|fd| { + let mut message = [0i32; 2]; + let n = unsafe { + libc::read(fd.get_ref().as_raw_fd(), message.as_mut_ptr().cast(), 8) + }; + match n { + 8 => Ok(MainExit { status: message[0], last: message[1] != 0 }), + -1 => Err(io::Error::last_os_error()), + _ => Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "session reaper lost main status", + )), + } + }); + match result { + Ok(Err(e)) if e.kind() == io::ErrorKind::Interrupted => continue, + Ok(result) => return result, + Err(_) => {} + } + } + } + + /// Collect what has exited, then exit. Children still alive are not in + /// any tracked group, so nothing could kill them; they are released. + pub fn finish(&mut self) { + // A byte, not EOF: children parked before exec may hold a copy of this fd. + if let Some(fd) = self.finish.take() { + unsafe { send(fd.as_raw_fd(), &[0]) }; + } + } + + pub async fn join(&self) -> io::Result<()> { + crate::sandbox::wait_child_exit_via_pidfd(self.pidfd.try_clone()?, self.pid).await; + Ok(()) + } + + pub fn join_blocking(self) { + reap(self.pid); + } +} + +/// A private subreaper keeps orphans below the supervisor for Yama checks, +/// without changing the embedding process's child-adoption policy. It never +/// kills: the supervisor kills through the tracked process groups, and the +/// reaper exits once nothing is left to reap or it is told to finish. +pub(crate) fn fork() -> io::Result<(i32, Option)> { + let mut pipe = [0; 2]; + if unsafe { libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + return Err(io::Error::last_os_error()); + } + let read = unsafe { OwnedFd::from_raw_fd(pipe[0]) }; + let write = unsafe { OwnedFd::from_raw_fd(pipe[1]) }; + if unsafe { libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + return Err(io::Error::last_os_error()); + } + let pinned_r = unsafe { OwnedFd::from_raw_fd(pipe[0]) }; + let pinned_w = unsafe { OwnedFd::from_raw_fd(pipe[1]) }; + let owner = unsafe { libc::getpid() }; + let reaper = crate::control::fork_without_control_fds(); + if reaper < 0 { + return Err(io::Error::last_os_error()); + } + if reaper == 0 { + unsafe { + let mut blocked = std::mem::zeroed(); + let mut previous_mask = std::mem::zeroed(); + libc::sigfillset(&mut blocked); + libc::sigprocmask(libc::SIG_SETMASK, &blocked, &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); + // Watch the owning process rather than set PDEATHSIG, which 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; + if libc::prctl(libc::PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) != 0 + || owner_fd < 0 + || libc::getppid() != owner + { + libc::_exit(127); + } + 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 signal_fd < 0 { + libc::_exit(127); + } + let parent = libc::getpid(); + let child = libc::fork(); + if child < 0 { + libc::_exit(127); + } + if child == 0 { + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) != 0 + || libc::getppid() != parent + { + libc::_exit(127); + } + libc::sigaction(libc::SIGCHLD, &previous_chld, std::ptr::null_mut()); + libc::sigprocmask(libc::SIG_SETMASK, &previous_mask, std::ptr::null_mut()); + drop(read); + drop(write); + drop(pinned_r); + drop(pinned_w); + return Ok((0, None)); + } + let fd = write.as_raw_fd(); + let gate = pinned_r.as_raw_fd(); + let mut keep = [fd, gate, signal_fd, owner_fd]; + 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); + if !send(fd, &[child]) { + libc::_exit(127); + } + // Keep the main PID pinned as a child until the supervisor has a pidfd. + let mut ack = 0i32; + let acknowledged = loop { + let n = libc::read(gate, (&mut ack as *mut i32).cast(), 4); + if n >= 0 { + break n == 4; + } + if io::Error::last_os_error().raw_os_error() != Some(libc::EINTR) { + break false; + } + }; + if !acknowledged { + libc::kill(child, libc::SIGKILL); + } + supervise(child, fd, gate, signal_fd, owner_fd); + } + } + drop(write); + drop(pinned_r); + let child = loop { + match crate::context::read_u32_fd(read.as_raw_fd()) { + Err(e) if e.kind() == io::ErrorKind::Interrupted => continue, + result => break result, + } + }; + let opened = match child { + Ok(pid) if pid > 0 => crate::sys::syscall::pidfd_open(pid, 0) + .and_then(|main| Ok((pid as i32, main, crate::sys::syscall::pidfd_open(reaper as u32, 0)?))), + _ => Err(io::Error::other("session reaper failed to start")), + }; + match opened { + Ok((pid, main_pidfd, pidfd)) => { + unsafe { send(pinned_w.as_raw_fd(), &[1]) }; + unsafe { libc::fcntl(read.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) }; + Ok(( + pid, + Some(Reaper { + pid: reaper, + pidfd, + main_pidfd, + status: read, + finish: Some(pinned_w), + empty: false, + }), + )) + } + Err(e) => { + // Without the ack the reaper kills its pinned child itself. + drop(pinned_w); + reap(reaper); + Err(e) + } + } +} + +unsafe fn supervise(main: 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 }, + ]; + let mut finishing = false; + let mut main_alive = true; + loop { + let alive = loop { + let mut status = 0; + match libc::waitpid(-1, &mut status, libc::WNOHANG | libc::__WALL) { + 0 => break true, + pid if pid == main => { + main_alive = false; + let last = !has_children(); + send(status_fd, &[status, last as i32]); + } + pid if pid > 0 => {} + _ if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} + // No child means no descendant: the session is over. + _ => break false, + } + }; + if !alive || finishing { + libc::_exit(0); + } + libc::poll(poll.as_mut_ptr(), 3, -1); + if poll[0].revents != 0 { + finishing = true; + } + if poll[2].revents != 0 { + // Unreaped, the main pid cannot have been reused. + if main_alive { + libc::kill(main, libc::SIGKILL); + } + finishing = true; + } + 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)); + } + } +} + +unsafe fn has_children() -> bool { + loop { + match libc::waitpid(-1, std::ptr::null_mut(), libc::WNOHANG | libc::__WALL) { + 0 => return true, + pid if pid > 0 => {} + _ if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} + _ => return false, + } + } +} + +fn reap(pid: i32) { + loop { + if unsafe { libc::waitpid(pid, std::ptr::null_mut(), 0) } >= 0 { + return; + } + if io::Error::last_os_error().raw_os_error() != Some(libc::EINTR) { + return; + } + } +} + +unsafe fn send(fd: i32, values: &[i32]) -> bool { + let len = std::mem::size_of_val(values); + loop { + if libc::write(fd, values.as_ptr().cast(), len) == 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::*; + + #[tokio::test] + async fn reports_an_exit_before_the_caller_starts_waiting() { + let (pid, reaper) = fork().unwrap(); + if pid == 0 { + unsafe { libc::_exit(42) } + } + let reaper = reaper.unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + let exit = reaper.main_exit().await.unwrap(); + assert!(libc::WIFEXITED(exit.status)); + assert_eq!(libc::WEXITSTATUS(exit.status), 42); + assert!(exit.last); + let mut ready = libc::pollfd { + fd: reaper.main_pidfd.as_raw_fd(), + events: libc::POLLIN, + revents: 0, + }; + assert_eq!(unsafe { libc::poll(&mut ready, 1, 0) }, 1); + reaper.join().await.unwrap(); + } + + #[tokio::test] + async fn outlives_the_main_process_until_orphans_are_reaped() { + let (pid, reaper) = fork().unwrap(); + if pid == 0 { + unsafe { + if libc::fork() == 0 { + libc::usleep(300_000); + libc::_exit(0); + } + libc::_exit(7); + } + } + let reaper = reaper.unwrap(); + let exit = reaper.main_exit().await.unwrap(); + assert_eq!(libc::WEXITSTATUS(exit.status), 7); + assert!(!exit.last); + let mut ready = libc::pollfd { + fd: reaper.pidfd.as_raw_fd(), + events: libc::POLLIN, + revents: 0, + }; + assert_eq!(unsafe { libc::poll(&mut ready, 1, 50) }, 0); + reaper.join().await.unwrap(); + } + + #[tokio::test] + async fn outlives_the_thread_that_created_it() { + let (pid, reaper) = std::thread::spawn(|| { + let (pid, reaper) = fork().unwrap(); + if pid == 0 { + unsafe { + libc::pause(); + libc::_exit(0); + } + } + (pid, reaper) + }) + .join() + .unwrap(); + let mut reaper = reaper.unwrap(); + std::thread::sleep(std::time::Duration::from_millis(50)); + for fd in [&reaper.pidfd, &reaper.main_pidfd] { + let mut ready = libc::pollfd { fd: fd.as_raw_fd(), events: libc::POLLIN, revents: 0 }; + assert_eq!(unsafe { libc::poll(&mut ready, 1, 0) }, 0); + } + unsafe { libc::kill(pid, libc::SIGKILL) }; + reaper.finish(); + reaper.join().await.unwrap(); + } + + #[tokio::test] + async fn finish_releases_children_still_running() { + let mut pipe = [0; 2]; + assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); + let (pid, reaper) = fork().unwrap(); + if pid == 0 { + unsafe { + let orphan = libc::fork(); + if orphan == 0 { + libc::pause(); + libc::_exit(0); + } + libc::write(pipe[1], (&orphan as *const i32).cast(), 4); + libc::_exit(0); + } + } + let mut orphan = 0i32; + assert_eq!(unsafe { libc::read(pipe[0], (&mut orphan as *mut i32).cast(), 4) }, 4); + unsafe { + libc::close(pipe[0]); + libc::close(pipe[1]); + } + let mut reaper = reaper.unwrap(); + assert!(!reaper.main_exit().await.unwrap().last); + reaper.finish(); + reaper.join().await.unwrap(); + assert_eq!(unsafe { libc::kill(orphan, libc::SIGKILL) }, 0); + } +} diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 840d6e22..60fd4450 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -287,6 +287,7 @@ struct Runtime { name: String, state: RuntimeState, child_pid: Option, + reaper: Option, pidfd: Option, groups: Arc, notif_handle: Option>, @@ -302,6 +303,9 @@ struct Runtime { // the runtime rather than to the `wait()` that started them, so a cancelled // `wait()` takes neither the reader nor what it has already produced. stdout_drain: Option, + /// Ends the drains once the reaper is gone: what still holds the write + /// ends was released and may never close them. + drain_cut: tokio_util::sync::CancellationToken, stderr_drain: Option, // Parent-held write end of a piped stdin (popen). The caller takes it via // `Process::take_stdin`; closing it signals EOF to the child. @@ -325,7 +329,7 @@ struct Runtime { /// 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 - /// `wait()` nor `Drop` take/commit/abort the branch — the transaction + /// `shutdown()` nor `Drop` take/commit/abort the branch — the transaction /// coordinator owns the single commit/abort. See [`SharedCow`]. #[cfg(feature = "cow")] shared_cow: Option, @@ -356,6 +360,7 @@ enum RuntimeState { Created, Running, Paused, + MainExited(crate::result::ExitStatus), Stopped(crate::result::ExitStatus), } @@ -892,14 +897,18 @@ impl Sandbox { /// 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; + 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; + if !matches!(self.rt().state, RuntimeState::MainExited(_)) { + self.rt_mut().state = RuntimeState::Running; + } Ok(()) } @@ -938,37 +947,31 @@ impl Sandbox { } } - /// Wait for the child process to exit. - /// - /// 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 { + /// Snapshot of the sandbox's remaining process group IDs, sorted by ID. + /// Membership may change as soon as this returns. + pub fn process_groups(&self) -> Result, crate::error::SandlockError> { use crate::error::SandboxRuntimeError; - use crate::result::RunResult; + let rt = self.runtime.as_ref() + .filter(|rt| rt.child_pid.is_some()) + .ok_or(SandboxRuntimeError::NotRunning)?; + Ok(rt.groups.live().map_err(SandboxRuntimeError::Io)?) + } - 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 }); - } + /// A descriptor that turns readable once the main process has exited. + /// Blocking on it needs no `&mut self`, so other threads can signal the + /// session meanwhile; `wait()` then returns without blocking. + pub fn main_exit_fd(&mut self) -> Result { + use crate::error::SandboxRuntimeError; + let pidfd = self.runtime.as_ref() + .and_then(|rt| rt.pidfd.as_ref()) + .ok_or(SandboxRuntimeError::NotRunning)? + .try_clone() + .map_err(SandboxRuntimeError::Io)?; + self.prepare_wait(); + Ok(pidfd) + } + fn prepare_wait(&mut self) { // Deliver EOF to a piped stdin the caller never took: otherwise a child // that reads stdin (e.g. `cat`) blocks forever and this wait never // returns. A taken stdin is already None here (the caller owns it). @@ -987,52 +990,108 @@ impl Sandbox { // // The drains are parked in the runtime rather than in this future: a // caller that cancels `wait()` (a `timeout` or `select!` around it) must - // be able to `wait()` again and still be given the output. The + // be able to `shutdown()` later and still be given the output. The // `is_none()` guard is what makes that second call reuse them instead of // starting a second reader. A stream the caller took through // `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()); } } + } - // Wait for the top-level child to exit. Prefer the child's pidfd via - // `AsyncFd`: pidfd readiness fires only on *exit*, so — unlike a - // `waitpid` loop — it never consumes the child's ptrace-stops, which - // the `policy_fn` fork-tracking worker reaps (`waitpid` with any flags - // reaps a tracee's ptrace-stops, so a concurrent `waitpid` here would - // 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, - }; - // 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); + /// Wait only for the main process, keeping background jobs supervised. + /// + /// Returns the same status on repeated calls. Capture pipes keep draining + /// until `shutdown()` collects them; this does not wait for EOF + /// or settle the COW branch. Untaken piped stdin is closed here. + /// Call `shutdown()` to terminate remaining jobs and collect their output. + pub async fn wait(&mut self) -> Result { + use crate::error::SandboxRuntimeError; + let rt = self.runtime.as_ref().ok_or(SandboxRuntimeError::NotRunning)?; + let pid = rt.child_pid.ok_or(SandboxRuntimeError::NotRunning)?; + match &rt.state { + RuntimeState::MainExited(status) | RuntimeState::Stopped(status) => return Ok(status.clone()), + _ => {} + } - self.rt_mut().state = RuntimeState::Stopped(exit_status.clone()); + self.prepare_wait(); + + let exit_status = if let Some(reaper) = &self.rt().reaper { + let exit = reaper.main_exit().await.map_err(SandboxRuntimeError::Io)?; + if let Some(reaper) = &mut self.rt_mut().reaper { reaper.empty = exit.last; } + sandbox_wait_status_to_exit(exit.status) + } else { + match self.rt().pidfd.as_ref() { + Some(pidfd) => { + let pidfd = pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; + wait_child_exit_via_pidfd(pidfd, pid).await + } + None => wait_child_exit_blocking(pid).await, + } + }; + self.rt_mut().state = RuntimeState::MainExited(exit_status.clone()); if self.rt().tty_foreground_taken { sandbox_restore_tty_foreground(pid); self.rt_mut().tty_foreground_taken = false; } + Ok(exit_status) + } + + /// Kill remaining jobs, reap owned children, and release supervision. + /// Returns the main process's status and collected output, settling COW + /// only after all tracked groups have exited. Safe to call repeatedly. + pub async fn shutdown(&mut self) -> Result { + use crate::error::SandboxRuntimeError; + use crate::result::RunResult; + use std::os::fd::AsRawFd; + let rt = self.runtime.as_ref().ok_or(SandboxRuntimeError::NotRunning)?; + // A main process that already exited delivers its status promptly, + // and that status says whether it left anything behind to kill. + if rt.reaper.as_ref().is_some_and(|r| crate::pgroup::has_exited(r.main_pidfd.as_raw_fd())) { + self.wait().await?; + } + let rt = self.rt(); + let empty = matches!(rt.state, RuntimeState::Stopped(_)) + || rt.reaper.as_ref().is_some_and(|r| r.empty || crate::pgroup::has_exited(r.pidfd.as_raw_fd())); + if !empty { self.kill()?; } + let exit_status = self.wait().await?; + let no_supervisor = self.no_supervisor; + if let Some(reaper) = &mut self.rt_mut().reaper { + // Supervised, every process was in a swept group, so the reaper + // exits on its own once the last is reaped. Only without a + // supervisor can untracked jobs remain, and there is no COW then + // that must outwait them. + if no_supervisor { reaper.finish(); } + reaper.join().await.map_err(SandboxRuntimeError::Io)?; + self.rt().drain_cut.cancel(); + } else if !empty { + // Nothing reports the end of a clone's descendants. + loop { + if self.rt().groups.signal(libc::SIGKILL).map_err(SandboxRuntimeError::Io)? == 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + } + self.rt_mut().reaper = None; + self.rt_mut().state = RuntimeState::Stopped(exit_status.clone()); let rt = self.rt_mut(); rt.control_claim = None; + rt.http_acl_handle = 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. + // Finish shutting down the old endpoint before releasing the name. if let Some(h) = rt.control_handle.take() { h.abort(); let _ = h.await; @@ -1083,7 +1142,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() } } @@ -1156,7 +1215,7 @@ impl Sandbox { /// completes; see `finish_parked_drain`. /// /// `None` means the stream was never piped, was taken by the caller, or was - /// already collected by an earlier `wait()`; a drain that panicked or was + /// already collected by an earlier `shutdown()`; a drain that panicked or was /// aborted reports empty bytes rather than failing the run. async fn collect_pipe_drains(&mut self) -> (Option>, Option>) { // Finish both first — each stores its bytes into the runtime as it @@ -1174,7 +1233,7 @@ impl Sandbox { /// supervisor + rlimits + landlock + COW + network/HTTP proxies). /// The child is parked between policy install and `execve`; call /// `start()` to release it. Stdout/stderr are captured for later - /// retrieval via `wait()`. + /// retrieval via `shutdown()`. pub async fn create(&mut self, cmd: &[&str]) -> Result<(), crate::error::SandlockError> { self.do_create(cmd, true).await } @@ -1515,7 +1574,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). @@ -1525,7 +1585,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. @@ -1544,7 +1605,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`. @@ -1563,7 +1625,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. @@ -1687,6 +1750,7 @@ impl Sandbox { crate::result::ExitStatus::Killed }), child_pid: Some(clone_pid), + reaper: None, pidfd: None, groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, @@ -1696,6 +1760,7 @@ impl Sandbox { _stdout_read: None, _stderr_read: None, stdout_drain: None, + drain_cut: tokio_util::sync::CancellationToken::new(), stderr_drain: None, _stdin_write: None, #[cfg(feature = "cow")] @@ -1766,7 +1831,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 @@ -1792,6 +1858,7 @@ impl Sandbox { name, state: RuntimeState::Created, child_pid: None, + reaper: None, pidfd: None, groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, @@ -1803,6 +1870,7 @@ impl Sandbox { _stdout_read: None, _stderr_read: None, stdout_drain: None, + drain_cut: tokio_util::sync::CancellationToken::new(), stderr_drain: None, _stdin_write: None, #[cfg(feature = "cow")] @@ -2050,10 +2118,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,10 +2149,8 @@ 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)?; + self.rt_mut().reaper = reaper; if pid == 0 { // ===== CHILD PROCESS ===== @@ -2172,7 +2234,6 @@ impl Sandbox { keep_fds: &gather_keep_fds, sandbox_name: Some(sandbox_name.as_str()), extra_syscalls: &extra_syscalls, - parent_pid, foreground, }); } @@ -2190,19 +2251,25 @@ 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, - }; + let pidfd = Some(self.rt().reaper.as_ref().expect("parent has a reaper") + .main_pidfd.try_clone().map_err(SandboxRuntimeError::Io)?); // 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 leader = pidfd.as_ref().expect("reaper pins the child") + .try_clone().map_err(SandboxRuntimeError::Io)?; + self.rt_mut().groups.track(pid, leader); 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 control_groups = Arc::clone(&self.rt().groups); + let control_session = || -> Result { + Ok(crate::control::Session { + groups: Arc::clone(&control_groups), + main: pidfd.as_ref().expect("reaper pins the child") + .try_clone().map_err(SandboxRuntimeError::Io)?, + }) + }; let is_nested_mode = notif_fd_num == 0; @@ -2454,6 +2521,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, Some(control_ctx), + control_session()?, sandbox_snapshot, control_info.clone(), )); @@ -2479,6 +2547,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, None, + control_session()?, self.clone(), control_info, )); @@ -2534,9 +2603,9 @@ impl Sandbox { /// the process is killed and reaped when that `Sandbox` is dropped, or eagerly /// via [`Process::kill`]. /// -/// 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"] +/// Dropping a `Process` leaves the session owned by the `Sandbox`. Call +/// [`Process::shutdown`] to terminate its jobs and collect captured output. +#[must_use = "a Process is a live confined child; call shutdown() to end the session or it runs until the Sandbox is dropped"] pub struct Process<'a> { sandbox: &'a mut Sandbox, } @@ -2579,14 +2648,21 @@ 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 while background jobs remain supervised. + pub async fn wait(&mut self) -> Result { self.sandbox.wait().await } + + /// Snapshot of the sandbox's remaining process group IDs. + pub fn process_groups(&self) -> Result, crate::error::SandlockError> { + self.sandbox.process_groups() + } + + /// Terminate remaining jobs and collect the main status and untaken output. + pub async fn shutdown(self) -> Result { + self.sandbox.shutdown().await + } + } /// Hand the terminal's foreground process group back to this process after @@ -2618,14 +2694,20 @@ impl Drop for Sandbox { fn drop(&mut self) { if let Some(ref mut rt) = self.runtime { if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } + let swept = rt.groups.signal(libc::SIGKILL); + if let Some(reaper) = &rt.reaper { + use std::os::fd::AsRawFd; + unsafe { libc::syscall(libc::SYS_pidfd_send_signal, reaper.main_pidfd.as_raw_fd(), libc::SIGKILL, 0, 0) }; + } 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) }; + if rt.reaper.is_none() { + unsafe { libc::kill(pid, libc::SIGKILL) }; + let mut status: i32 = 0; + unsafe { libc::waitpid(pid, &mut status, 0) }; + } } if rt.tty_foreground_taken { sandbox_restore_tty_foreground(pid); @@ -2633,6 +2715,12 @@ impl Drop for Sandbox { } } + if let Some(mut reaper) = rt.reaper.take() { + // As in shutdown(): only jobs no sweep reached can keep it alive. + if self.no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join_blocking(); + } + 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(); } @@ -2649,8 +2737,8 @@ impl Drop for Sandbox { } - // wait() settles the branch, so this only sees one it left behind: a - // deferred branch nobody decided on, or one a cancelled wait() never + // shutdown() settles the branch, so this only sees one it left behind: a + // deferred branch nobody decided on, or one a cancelled shutdown() never // reached. let action = self.branch_action(); #[cfg(feature = "cow")] @@ -2863,7 +2951,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, @@ -2871,7 +2962,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 }))) } @@ -2946,7 +3047,7 @@ fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { /// already exited, it does not consume the child's ptrace-stops the way a /// `waitpid`-loop would — so it doesn't race the `policy_fn` fork-tracking /// worker. Falls back to the blocking waiter on any pidfd/`AsyncFd` error. -async fn wait_child_exit_via_pidfd( +pub(crate) async fn wait_child_exit_via_pidfd( pidfd: std::os::unix::io::OwnedFd, pid: libc::pid_t, ) -> crate::result::ExitStatus { 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/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_branch_action.rs b/crates/sandlock-core/tests/integration/test_branch_action.rs index 6dae271f..da32d1c0 100644 --- a/crates/sandlock-core/tests/integration/test_branch_action.rs +++ b/crates/sandlock-core/tests/integration/test_branch_action.rs @@ -201,3 +201,43 @@ async fn commit_and_abort_need_a_pending_branch() { let _ = fs::remove_dir_all(&workdir); let _ = fs::remove_dir_all(&storage); } + +#[tokio::test] +async fn background_changes_are_committed_only_at_shutdown() { + use std::time::Duration; + let workdir = tempfile::tempdir().unwrap(); + let storage = tempfile::tempdir().unwrap(); + let control = tempfile::tempdir().unwrap(); + let ready = control.path().join("ready"); + let release = control.path().join("release"); + let done = control.path().join("done"); + let script = format!(r#" +import os, time +if os.fork() == 0: + open({ready:?}, "w").close() + while not os.path.exists({release:?}): + time.sleep(0.01) + open("late.txt", "w").write("background change") + open({done:?}, "w").close() + while True: + time.sleep(1) +while not os.path.exists({ready:?}): + time.sleep(0.01) +os._exit(0) +"#); + let mut sb = policy(workdir.path(), storage.path()) + .fs_write(control.path()).build().unwrap(); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(0)); + fs::write(release, "go").unwrap(); + tokio::time::timeout(Duration::from_secs(10), async { + while !done.exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }).await.unwrap(); + assert!(!workdir.path().join("late.txt").exists()); + let result = sb.shutdown().await.unwrap(); + assert!(result.success()); + assert_eq!(fs::read_to_string(workdir.path().join("late.txt")).unwrap(), "background change"); + assert!(result.changes.iter().any(|c| c.path == Path::new("late.txt"))); +} diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index a0eb34bf..2a82c299 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()); } // ============================================================ @@ -875,3 +875,33 @@ 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!(!sb.process_groups().unwrap().is_empty()); + + let info = sandlock_core::control::sandbox_info(&name).expect("info"); + assert!(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"); + let fields: Vec<&str> = row.split_whitespace().collect(); + assert_eq!(fields[1], "-", "exited main has no pid: {row}"); + assert!(row.ends_with("(main exited)"), "{row}"); + sb.shutdown().await.unwrap(); +} diff --git a/crates/sandlock-core/tests/integration/test_fork.rs b/crates/sandlock-core/tests/integration/test_fork.rs index 363f9119..53355edc 100644 --- a/crates/sandlock-core/tests/integration/test_fork.rs +++ b/crates/sandlock-core/tests/integration/test_fork.rs @@ -29,7 +29,8 @@ 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(); + c.wait().await.unwrap(); + let r = c.shutdown().await.unwrap(); assert!(r.success()); } @@ -61,7 +62,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 +88,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 +143,8 @@ 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(); + c.wait().await.unwrap(); + let r = c.shutdown().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..ac4d96f0 100644 --- a/crates/sandlock-core/tests/integration/test_policy_fn.rs +++ b/crates/sandlock-core/tests/integration/test_policy_fn.rs @@ -881,7 +881,8 @@ 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(); + sandbox.wait().await.unwrap(); + let result = sandbox.shutdown().await.unwrap(); assert!( result.success(), "unlimited approval must accept a late verdict" @@ -945,7 +946,7 @@ async fn shutdown_cancels_unlimited_approval() { }) .await .expect("kill must cancel before wait or drop"); - tokio::time::timeout(Duration::from_secs(2), sandbox.wait()) + tokio::time::timeout(Duration::from_secs(2), async { sandbox.wait().await?; sandbox.shutdown().await }) .await .unwrap() .unwrap(); diff --git a/crates/sandlock-core/tests/integration/test_popen.rs b/crates/sandlock-core/tests/integration/test_popen.rs index ace53d22..ad0151c7 100644 --- a/crates/sandlock-core/tests/integration/test_popen.rs +++ b/crates/sandlock-core/tests/integration/test_popen.rs @@ -44,7 +44,8 @@ async fn test_popen_pipe_roundtrip() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "hello\n"); - let res = child.wait().await.unwrap(); + child.wait().await.unwrap(); + let res = child.shutdown().await.unwrap(); assert!(res.success()); } @@ -62,7 +63,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 +72,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 +83,8 @@ async fn test_popen_null_stdout_is_dev_null() { ) .await .unwrap(); - let res = child.wait().await.unwrap(); + child.wait().await.unwrap(); + let res = child.shutdown().await.unwrap(); assert!(res.success(), "fd 1 was not /dev/null (1:3): exit={:?}", res.exit_status); } @@ -100,7 +102,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 +111,13 @@ 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(); + child.wait().await.unwrap(); + let res = child.shutdown().await.unwrap(); assert!(res.success(), "cat did not exit — untaken stdin never EOF'd"); } @@ -178,7 +181,8 @@ 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(); + child.wait().await.unwrap(); + let res = child.shutdown().await.unwrap(); assert!(res.success()); // A killed orphan stays a zombie until init/subreaper reaps it. @@ -225,18 +229,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 +253,8 @@ async fn test_popen_explicit_kill_then_wait() { .await .unwrap(); child.kill().expect("kill"); - let res = child.wait().await.unwrap(); + child.wait().await.unwrap(); + let res = child.shutdown().await.unwrap(); assert!(!res.success(), "a killed process must not report success"); } @@ -258,11 +263,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 +298,8 @@ 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(); + child.wait().await.unwrap(); + let res = child.shutdown().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..f1574f62 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -115,9 +115,9 @@ import os, signal pid = os.fork() if pid == 0: os.setsid() + open("{}.tmp", "w").write(str(os.getpid())) + os.rename("{}.tmp", "{}") signal.pause() -open("{}.tmp", "w").write(str(pid)) -os.rename("{}.tmp", "{}") signal.pause() "#, pid_file.display(), @@ -156,7 +156,10 @@ async fn test_pause_stops_a_process_that_left_the_main_group() { let state = || { std::fs::read_to_string(format!("/proc/{leader}/stat")) .ok() - .and_then(|s| s.rsplit_once(") ").map(|(_, rest)| rest.chars().next().unwrap())) + .and_then(|s| { + s.rsplit_once(") ") + .map(|(_, rest)| rest.chars().next().unwrap()) + }) }; let mut stopped = false; for _ in 0..300 { @@ -166,7 +169,10 @@ async fn test_pause_stops_a_process_that_left_the_main_group() { } std::thread::sleep(std::time::Duration::from_millis(10)); } - assert!(stopped, "session leader {leader} kept running through pause()"); + assert!( + stopped, + "session leader {leader} kept running through pause()" + ); sb.resume().unwrap(); sb.kill().unwrap(); @@ -203,6 +209,264 @@ os.rename("{0}.tmp", "{0}") .unwrap(); assert!(result.success()); - 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"); + 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" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_wait_keeps_background_jobs_supervised() { + use sandlock_core::result::ExitStatus; + use std::time::Duration; + + let mut subreaper_before = 0i32; + assert_eq!( + unsafe { libc::prctl(libc::PR_GET_CHILD_SUBREAPER, &mut subreaper_before) }, + 0 + ); + let dir = tempfile::tempdir().unwrap(); + let ready = dir.path().join("ready"); + let release = dir.path().join("release"); + let done = dir.path().join("done"); + let script = format!( + r#" +import os, time +pid = os.fork() +if pid == 0: + os.setsid() + open({ready:?}, "w").write(str(os.getpid())) + while not os.path.exists({release:?}): + time.sleep(0.01) + print("background output", flush=True) + open({done:?}, "w").write("supervised") + while True: + time.sleep(1) +while not os.path.exists({ready:?}): + time.sleep(0.01) +os._exit(7) +"# + ); + let mut sb = policy(); + sb.fs_writable.push(dir.path().to_path_buf()); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + assert_eq!( + tokio::time::timeout(Duration::from_secs(10), sb.wait()) + .await + .unwrap() + .unwrap(), + ExitStatus::Code(7) + ); + let mut subreaper_after = 0i32; + assert_eq!( + unsafe { libc::prctl(libc::PR_GET_CHILD_SUBREAPER, &mut subreaper_after) }, + 0 + ); + assert_eq!(subreaper_before, subreaper_after); + assert_eq!(sb.wait().await.unwrap(), ExitStatus::Code(7)); + assert!(!sb.is_running()); + let leader: i32 = std::fs::read_to_string(&ready).unwrap().parse().unwrap(); + assert_eq!(sb.process_groups().unwrap(), vec![leader]); + sb.pause().unwrap(); + sb.resume().unwrap(); + assert_eq!(sb.wait().await.unwrap(), ExitStatus::Code(7)); + std::fs::write(release, "go").unwrap(); + tokio::time::timeout(Duration::from_secs(10), async { + while !done.exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("background job lost supervision"); + let result = tokio::time::timeout(Duration::from_secs(10), sb.shutdown()) + .await + .unwrap() + .unwrap(); + assert_eq!(result.exit_status, ExitStatus::Code(7)); + assert_eq!(result.stdout_str(), Some("background output")); + assert!(sb.process_groups().unwrap().is_empty()); + assert!(wait_gone(leader)); + assert_eq!( + sb.shutdown().await.unwrap().exit_status, + ExitStatus::Code(7) + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_drop_after_wait_cleans_background_jobs() { + use std::time::Duration; + let mut sb = policy(); + let leader = spawn_with_session_leader(&mut sb).await; + unsafe { libc::kill(sb.pid().unwrap(), libc::SIGTERM) }; + tokio::time::timeout(Duration::from_secs(10), sb.wait()) + .await + .unwrap() + .unwrap(); + drop(sb); + assert!(wait_gone(leader)); +} + +#[tokio::test] +async fn test_wait_without_a_child_returns_an_error() { + let mut sb = policy(); + assert!(sb.wait().await.is_err()); + assert!(sb.shutdown().await.is_err()); + assert!(sb.process_groups().is_err()); +} + +#[tokio::test] +async fn test_cancelled_wait_preserves_status_and_output() { + use sandlock_core::result::ExitStatus; + use std::time::Duration; + let mut sb = policy(); + sb.spawn(&["sh", "-c", "echo before; sleep 0.2; echo after; exit 23"]) + .await + .unwrap(); + assert!(tokio::time::timeout(Duration::from_millis(20), sb.wait()) + .await + .is_err()); + assert_eq!(sb.wait().await.unwrap(), ExitStatus::Code(23)); + let result = sb.shutdown().await.unwrap(); + assert_eq!(result.exit_status, ExitStatus::Code(23)); + assert_eq!(result.stdout_str(), Some("before\nafter")); +} + +#[test] +fn test_shutdown_reaps_adopted_jobs_without_reaping_unrelated_children() { + const CHILD: &str = "SANDLOCK_TEST_BACKGROUND_SUBREAPER"; + if std::env::var_os(CHILD).is_none() { + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["test_process_groups::test_shutdown_reaps_adopted_jobs_without_reaping_unrelated_children", "--exact", "--nocapture"]) + .env(CHILD, "1") + .status().unwrap(); + assert!(status.success()); + return; + } + assert_eq!( + unsafe { libc::prctl(libc::PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) }, + 0 + ); + let mut unrelated = std::process::Command::new("sh") + .args(["-c", "exit 29"]) + .spawn() + .unwrap(); + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + rt.block_on(async { + let mut sb = policy(); + let leader = spawn_with_session_leader(&mut sb).await; + unsafe { libc::kill(sb.pid().unwrap(), libc::SIGTERM) }; + sb.wait().await.unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(10), sb.shutdown()) + .await + .unwrap() + .unwrap(); + assert_eq!( + unsafe { libc::waitpid(leader, std::ptr::null_mut(), libc::WNOHANG) }, + -1 + ); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ECHILD) + ); + assert!(wait_gone(leader)); + }); + assert_eq!(unrelated.wait().unwrap().code(), Some(29)); +} + +#[test] +fn test_creation_under_fd_pressure_does_not_leave_children() { + const CHILD: &str = "SANDLOCK_TEST_BACKGROUND_FD_PRESSURE"; + if std::env::var_os(CHILD).is_none() { + let mut child = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "test_process_groups::test_creation_under_fd_pressure_does_not_leave_children", + "--exact", + "--nocapture", + ]) + .env(CHILD, "1") + .spawn() + .unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(15); + loop { + if let Some(status) = child.try_wait().unwrap() { + assert!(status.success()); + return; + } + if std::time::Instant::now() >= deadline { + child.kill().unwrap(); + let _ = child.wait(); + panic!("sandbox creation or cleanup hung under fd pressure"); + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let mut limit: libc::rlimit = unsafe { std::mem::zeroed() }; + assert_eq!( + unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut limit) }, + 0 + ); + for budget in [16, 20, 24, 28, 32] { + let mut sb = policy(); + let constrained = libc::rlimit { + rlim_cur: budget, + rlim_max: limit.rlim_max, + }; + assert_eq!( + unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &constrained) }, + 0 + ); + let _ = rt.block_on(sb.create(&["sleep", "30"])); + drop(sb); + assert_eq!(unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &limit) }, 0); + assert_eq!( + unsafe { libc::waitpid(-1, std::ptr::null_mut(), libc::WNOHANG) }, + -1 + ); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ECHILD) + ); + } +} + +#[tokio::test] +async fn test_shutdown_releases_untracked_groups_without_syscall_supervision() { + let dir = tempfile::tempdir().unwrap(); + let ready = dir.path().join("ready"); + let script = format!( + r#" +import os, signal, time +if os.fork() == 0: + os.setsid() + open({ready:?}, "w").write(str(os.getpid())) + signal.pause() +while not os.path.exists({ready:?}): + time.sleep(0.01) +"# + ); + let mut sb = policy(); + sb.no_supervisor = true; + 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(ready).unwrap().parse().unwrap(); + // Without a supervisor nothing records the new group, so shutdown + // releases its leader instead of waiting on it forever. + let shutdown = tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await; + let alive = unsafe { libc::kill(leader, libc::SIGKILL) } == 0; + shutdown.expect("shutdown must not wait on untracked descendants").unwrap(); + assert!(alive); + assert!(wait_gone(leader)); } diff --git a/crates/sandlock-core/tests/integration/test_protection.rs b/crates/sandlock-core/tests/integration/test_protection.rs index d12ed2b9..da0c4812 100644 --- a/crates/sandlock-core/tests/integration/test_protection.rs +++ b/crates/sandlock-core/tests/integration/test_protection.rs @@ -534,3 +534,153 @@ async fn degraded_policy_confines_and_runs_below_v6() { "degraded-mode confinement must still execute the child" ); } + +#[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(20); + 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)); + } + } + + // Emulate kernels that reject PIDFD_SIGNAL_PROCESS_GROUP, including 6.8. + 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 mut builder = sandlock_core::Sandbox::builder(); + for protection in Protection::all() { + builder = builder.allow_degraded(protection); + } + let mut sb = builder + .fs_read("/usr") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/bin") + .fs_read("/etc") + .fs_write(dir.path()) + .build() + .unwrap(); + let policy = sb.clone(); + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(async { + let result = sb.run(&["echo", "ok"]).await.unwrap(); + assert!(result.success()); + assert_eq!(result.stdout.as_deref(), Some(b"ok\n".as_slice())); + assert!(sb.shutdown().await.unwrap().success()); + let spawn = |name: &str, job: &str| { + let ready = dir.path().join(name); + let tmp = ready.with_extension("tmp"); + let script = format!( + r#" +import os, signal, time +def publish(): + open({tmp:?}, "w").write(str(os.getpid())) + os.rename({tmp:?}, {ready:?}) + signal.pause() +if os.fork() == 0: + os.setsid() + {job} +while not os.path.exists({ready:?}): + time.sleep(0.01) +os._exit(7) +"# + ); + let mut sb = policy.clone(); + async move { + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(7)); + let job: i32 = std::fs::read_to_string(&ready).unwrap().parse().unwrap(); + (sb, job) + } + }; + + // A job whose group leader is still alive is reached by number. + let (mut sb, leader) = spawn("leader", "publish()").await; + let result = tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()) + .await + .unwrap(); + assert_eq!(result.unwrap().exit_status, sandlock_core::ExitStatus::Code(7)); + assert_eq!(unsafe { libc::kill(leader, 0) }, -1); + + // Once the leader is gone its number may be reused, so shutdown + // reports the job as unreachable instead of hanging. + let (mut sb, orphan) = + spawn("orphan", "publish() if os.fork() == 0 else os._exit(0)").await; + assert!(tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()) + .await + .unwrap() + .is_err()); + drop(sb); + assert_eq!(unsafe { libc::kill(orphan, 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..32b18279 100644 --- a/crates/sandlock-core/tests/integration/test_resource.rs +++ b/crates/sandlock-core/tests/integration/test_resource.rs @@ -492,7 +492,8 @@ async fn test_spawn_and_kill() { sb.start().unwrap(); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + sb.wait().await.unwrap(); + let result = sb.shutdown().await.unwrap(); assert!( matches!(result.exit_status, ExitStatus::Signal(_) | ExitStatus::Killed), "expected Signal or Killed, got {:?}", @@ -681,7 +682,8 @@ async fn test_pause_resume() { sb.resume().expect("resume should succeed"); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + sb.wait().await.unwrap(); + let result = sb.shutdown().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..41d50e0b 100644 --- a/crates/sandlock-core/tests/integration/test_restore.rs +++ b/crates/sandlock-core/tests/integration/test_restore.rs @@ -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 = sb2.wait().await; let _ = std::fs::remove_dir_all(&tmp); assert!( diff --git a/crates/sandlock-core/tests/integration/test_sandbox.rs b/crates/sandlock-core/tests/integration/test_sandbox.rs index fce2666d..74ce40c9 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 @@ -900,12 +900,12 @@ async fn test_cancelled_wait_keeps_the_capture_for_the_next_wait() { sb.spawn(&["sh", "-c", "printf hello; sleep 5"]).await.unwrap(); // The child sleeps well past this, so the first wait is cancelled mid-flight. - let first = tokio::time::timeout(std::time::Duration::from_millis(500), sb.wait()).await; + let first = tokio::time::timeout(std::time::Duration::from_millis(500), async { sb.wait().await?; sb.shutdown().await }).await; assert!(first.is_err(), "the child sleeps 5s: this wait must be cancelled, not complete"); 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(); @@ -945,7 +945,7 @@ fn test_capture_wait_does_not_park_blocking_pool_threads() { let mut quick = capture_policy().with_name("drain-quick"); - let idle_wait = idle.wait(); + let idle_wait = async { idle.wait().await?; idle.shutdown().await }; let quick_run = quick.run(&["echo", "hi"]); tokio::pin!(idle_wait); tokio::pin!(quick_run); diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 22d83f5c..015c8c33 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, }; @@ -879,8 +879,8 @@ 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. + * The handle remains valid; call `sandlock_handle_shutdown` to reap jobs and + * collect output, then `sandlock_handle_free` to release it. * * # Safety * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. @@ -888,26 +888,76 @@ 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. + * Pause all tracked live process groups. Returns 0 on success, -1 on error. * - * 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 with no concurrent handle operations. + */ +int sandlock_handle_pause(sandlock_handle_t *h); + +/** + * Resume all tracked live process groups. Returns 0 on success, -1 on error. * * # Safety - * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. + * `h` must be a valid handle with no concurrent handle operations. + */ +int sandlock_handle_resume(sandlock_handle_t *h); + +/** + * Wait for the main process, retaining supervision of background jobs. + * The returned result contains only exit status. Collect output and filesystem + * changes with `sandlock_handle_shutdown`. Free each result separately. + * Close caller-owned stdin and drain caller-owned output pipes while waiting. + * + * # Safety + * `h` must be a valid handle with no concurrent handle operations. */ sandlock_result_t *sandlock_handle_wait(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. + * Wait for the main process with a timeout in milliseconds (0 means unlimited). + * Returns 0 and stores a status-only result in `*out` once main exits, 1 if + * the timeout elapsed first, or -1 on error. A timeout leaves the session + * untouched: callers may wait again or shut it down. * * # Safety - * `h` must be a valid handle from `sandlock_create`. + * `h` must be a valid handle with no concurrent handle operations, and + * `out` must be a valid pointer. + */ +int sandlock_handle_wait_timeout(sandlock_handle_t *h, + uint64_t timeout_ms, + sandlock_result_t **out); + +/** + * Prepare to wait and return a new descriptor that becomes readable once the + * main process exits, or -1 on error. Poll it without holding the handle, so + * other threads can signal the session, then call `sandlock_handle_wait`, + * which no longer blocks. The caller closes the descriptor. Handles from + * `sandlock_create_for_run` are refused: wait on those natively. + * + * # Safety + * `h` must be a valid handle with no concurrent handle operations. + */ +int sandlock_handle_main_exit_fd(sandlock_handle_t *h); + +/** + * Terminate remaining jobs with SIGKILL, reap them, and release supervision. + * Returns the main status, captured output, and filesystem changes, or null + * on error. The handle remains valid until `sandlock_handle_free`. + * + * # Safety + * `h` must be a valid handle with no concurrent handle operations. + */ +sandlock_result_t *sandlock_handle_shutdown(sandlock_handle_t *h); + +/** + * Return a sorted snapshot of tracked live process group IDs as a JSON array. + * Free the returned string with `sandlock_string_free`. Returns null on error. + * + * # Safety + * `h` must be a valid handle with no concurrent handle operations. */ -sandlock_result_t *sandlock_handle_wait_timeout(sandlock_handle_t *h, uint64_t timeout_ms); +char *sandlock_handle_process_groups(const 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 fa0c91c4..3c9826bb 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1409,8 +1409,8 @@ pub unsafe extern "C" fn sandlock_handle_pid(h: *const sandlock_handle_t) -> i32 /// 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. +/// The handle remains valid; call `sandlock_handle_shutdown` to reap jobs and +/// collect output, then `sandlock_handle_free` to release it. /// /// # Safety /// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. @@ -1426,62 +1426,139 @@ pub unsafe extern "C" fn sandlock_handle_kill(h: *mut sandlock_handle_t) -> c_in 0 } -/// Wait for the sandbox to exit. Returns a result handle with stdout/stderr. +/// Pause all tracked live process groups. Returns 0 on success, -1 on error. /// -/// 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 with no concurrent handle operations. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_pause(h: *mut sandlock_handle_t) -> c_int { + if h.is_null() { return -1; } + match (*h).sandbox.pause() { Ok(()) => 0, Err(_) => -1 } +} + +/// Resume all tracked live process groups. Returns 0 on success, -1 on error. /// /// # Safety -/// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. +/// `h` must be a valid handle with no concurrent handle operations. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_resume(h: *mut sandlock_handle_t) -> c_int { + if h.is_null() { return -1; } + match (*h).sandbox.resume() { Ok(()) => 0, Err(_) => -1 } +} + +/// Wait for the main process, retaining supervision of background jobs. +/// The returned result contains only exit status. Collect output and filesystem +/// changes with `sandlock_handle_shutdown`. Free each result separately. +/// Close caller-owned stdin and drain caller-owned output pipes while waiting. +/// +/// # Safety +/// `h` must be a valid handle with no concurrent handle operations. #[no_mangle] pub unsafe extern "C" fn sandlock_handle_wait(h: *mut sandlock_handle_t) -> *mut sandlock_result_t { + let mut out = ptr::null_mut(); + sandlock_handle_wait_timeout(h, 0, &mut out); + out +} + +/// Wait for the main process with a timeout in milliseconds (0 means unlimited). +/// Returns 0 and stores a status-only result in `*out` once main exits, 1 if +/// the timeout elapsed first, or -1 on error. A timeout leaves the session +/// untouched: callers may wait again or shut it down. +/// +/// # Safety +/// `h` must be a valid handle with no concurrent handle operations, and +/// `out` must be a valid pointer. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_wait_timeout( + h: *mut sandlock_handle_t, + timeout_ms: u64, + out: *mut *mut sandlock_result_t, +) -> c_int { + if h.is_null() || out.is_null() { + return -1; + } + *out = ptr::null_mut(); + let h = &mut *h; + let status = block_on_runtime(&h.runtime, async { + if timeout_ms == 0 { + h.sandbox.wait().await.map(Some) + } else { + match tokio::time::timeout(Duration::from_millis(timeout_ms), h.sandbox.wait()).await { + Ok(result) => result.map(Some), + Err(_) => Ok(None), + } + } + }); + match status { + Some(Ok(Some(exit_status))) => { + *out = 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, + } +} + +/// Prepare to wait and return a new descriptor that becomes readable once the +/// main process exits, or -1 on error. Poll it without holding the handle, so +/// other threads can signal the session, then call `sandlock_handle_wait`, +/// which no longer blocks. The caller closes the descriptor. Handles from +/// `sandlock_create_for_run` are refused: wait on those natively. +/// +/// # Safety +/// `h` must be a valid handle with no concurrent handle operations. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_main_exit_fd(h: *mut sandlock_handle_t) -> c_int { if h.is_null() { - return ptr::null_mut(); + return -1; } let h = &mut *h; - match block_on_runtime(&h.runtime, h.sandbox.wait()) { - Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), - _ => ptr::null_mut(), + // A current_thread runtime runs the supervisor only inside block_on, so + // a caller blocked on the descriptor would starve it and never return. + if h.runtime.handle().runtime_flavor() == tokio::runtime::RuntimeFlavor::CurrentThread { + return -1; + } + // Starting the output drains spawns tasks on the handle's runtime. + let _runtime = h.runtime.enter(); + match h.sandbox.main_exit_fd() { + Ok(fd) => std::os::fd::IntoRawFd::into_raw_fd(fd), + Err(_) => -1, } } -/// 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. +/// Terminate remaining jobs with SIGKILL, reap them, and release supervision. +/// Returns the main status, captured output, and filesystem changes, or null +/// on error. The handle remains valid until `sandlock_handle_free`. /// /// # Safety -/// `h` must be a valid handle from `sandlock_create`. +/// `h` must be a valid handle with no concurrent handle operations. #[no_mangle] -pub unsafe extern "C" fn sandlock_handle_wait_timeout( - h: *mut sandlock_handle_t, - timeout_ms: u64, -) -> *mut sandlock_result_t { +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, h.sandbox.wait()) { - Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), - _ => ptr::null_mut(), - }; + 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(), } +} - let dur = Duration::from_millis(timeout_ms); - match block_on_runtime(&h.runtime, async { - tokio::time::timeout(dur, h.sandbox.wait()).await - }) { - 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 })) - } +/// Return a sorted snapshot of tracked live process group IDs as a JSON array. +/// Free the returned string with `sandlock_string_free`. Returns null on error. +/// +/// # Safety +/// `h` must be a valid handle with no concurrent handle operations. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_process_groups(h: *const sandlock_handle_t) -> *mut c_char { + if h.is_null() { + return ptr::null_mut(); + } + match (*h).sandbox.process_groups() { + Ok(groups) => CString::new(serde_json::to_string(&groups).unwrap()).unwrap().into_raw(), + Err(_) => ptr::null_mut(), } } @@ -1639,7 +1716,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, } @@ -2608,7 +2685,7 @@ 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, _ => -1, } } diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs index 562d7ce0..ee0e238f 100644 --- a/crates/sandlock-ffi/tests/changes.rs +++ b/crates/sandlock-ffi/tests/changes.rs @@ -10,7 +10,7 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_entry_kind_t, sandlock_entry_t, sandlock_handle_free, - sandlock_handle_wait, + sandlock_handle_shutdown, 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 +94,8 @@ 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) }; + unsafe { sandlock_result_free(sandlock_handle_wait(h)) }; + let r = unsafe { sandlock_handle_shutdown(h) }; assert!(unsafe { sandlock_result_success(r) }); let n = unsafe { sandlock_result_changes_len(r) }; diff --git a/crates/sandlock-ffi/tests/defer.rs b/crates/sandlock-ffi/tests/defer.rs index 381c1c45..50f9a4fa 100644 --- a/crates/sandlock-ffi/tests/defer.rs +++ b/crates/sandlock-ffi/tests/defer.rs @@ -10,7 +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_handle_pending, sandlock_handle_upper_dir, sandlock_handle_shutdown, sandlock_handle_wait, 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, @@ -57,7 +57,8 @@ 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) }; + unsafe { sandlock_result_free(sandlock_handle_wait(h)) }; + let r = unsafe { sandlock_handle_shutdown(h) }; assert!(!r.is_null(), "wait failed"); assert!(unsafe { sandlock_result_success(r) }); @@ -131,7 +132,8 @@ 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) }; + unsafe { sandlock_result_free(sandlock_handle_wait(h)) }; + let r = unsafe { sandlock_handle_shutdown(h) }; assert!(unsafe { sandlock_result_success(r) }); assert_eq!(unsafe { sandlock_result_changes_len(r) }, 1); unsafe { sandlock_result_free(r) }; diff --git a/crates/sandlock-ffi/tests/popen.rs b/crates/sandlock-ffi/tests/popen.rs index a0740c4e..83be4ba1 100644 --- a/crates/sandlock-ffi/tests/popen.rs +++ b/crates/sandlock-ffi/tests/popen.rs @@ -410,3 +410,65 @@ 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" ); } + +#[test] +fn wait_retains_background_session_until_shutdown() { + use sandlock_ffi::*; + let policy = build_policy(); + let (_owned, av) = argv(&["sh", "-c", "sleep 60 & echo background; exit 7"]); + unsafe { + let h = sandlock_create(policy, ptr::null(), av.as_ptr(), av.len() as c_uint); + assert!(!h.is_null()); + assert_eq!(sandlock_start(h), 0); + for _ in 0..2 { + let mut status = ptr::null_mut(); + assert_eq!(sandlock_handle_wait_timeout(h, 5000, &mut status), 0); + assert_eq!(sandlock_result_exit_code(status), 7); + let mut len = 0; + sandlock_result_stdout_bytes(status, &mut len); + assert_eq!(len, 0); + sandlock_result_free(status); + } + let groups = sandlock_handle_process_groups(h); + assert!(!groups.is_null()); + let groups: Vec = { + let parsed = serde_json::from_slice(std::ffi::CStr::from_ptr(groups).to_bytes()).unwrap(); + sandlock_string_free(groups); + parsed + }; + assert!(!groups.is_empty()); + let result = sandlock_handle_shutdown(h); + assert!(!result.is_null()); + assert_eq!(sandlock_result_exit_code(result), 7); + let mut len = 0; + let bytes = sandlock_result_stdout_bytes(result, &mut len); + assert_eq!(std::slice::from_raw_parts(bytes, len), b"background\n"); + sandlock_result_free(result); + let groups = sandlock_handle_process_groups(h); + assert_eq!(std::ffi::CStr::from_ptr(groups).to_bytes(), b"[]"); + sandlock_string_free(groups); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} + +#[test] +fn wait_timeout_leaves_main_alive_for_a_later_wait() { + use sandlock_ffi::*; + let policy = build_policy(); + let (_owned, av) = argv(&["sh", "-c", "sleep 0.2; exit 9"]); + unsafe { + let h = sandlock_create(policy, ptr::null(), av.as_ptr(), av.len() as c_uint); + assert!(!h.is_null()); + assert_eq!(sandlock_start(h), 0); + let mut status = ptr::null_mut(); + assert_eq!(sandlock_handle_wait_timeout(h, 1, &mut status), 1); + assert!(status.is_null()); + assert_eq!(sandlock_handle_wait_timeout(h, 5000, &mut status), 0); + assert_eq!(sandlock_result_exit_code(status), 9); + sandlock_result_free(status); + sandlock_result_free(sandlock_handle_shutdown(h)); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} diff --git a/crates/sandlock-ffi/tests/restore.rs b/crates/sandlock-ffi/tests/restore.rs index 2542174b..3a0c5100 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_handle_shutdown, sandlock_handle_wait, 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,8 @@ 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) }; + unsafe { sandlock_result_free(sandlock_handle_wait(h)) }; + let r = unsafe { sandlock_handle_shutdown(h) }; if !r.is_null() { unsafe { sandlock_result_free(r) }; } @@ -197,7 +198,8 @@ 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); + sandlock_result_free(sandlock_handle_wait(h2)); + let r2 = sandlock_handle_shutdown(h2); if !r2.is_null() { sandlock_result_free(r2); } 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 } 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 diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index 8b580a4d..374183e2 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -259,7 +259,7 @@ fields on `Sandbox`. | `clean_env` | `clean_env` | `bool` | `False` | When `True`, start with a minimal environment (`PATH`, `HOME`, `USER`, `TERM`, `LANG`) instead of inheriting the parent's. | | `no_coredump` | `no_coredump` | `bool` | `False` | Apply `prctl(PR_SET_DUMPABLE, 0)`. Disables core dumps and restricts `/proc/` access from other processes. Breaks `gdb`, `strace`, and `perf`. | | `no_huge_pages` | `no_huge_pages` | `bool` | `False` | Disable transparent huge pages via `prctl(PR_SET_THP_DISABLE)`. | -| `no_supervisor` | `no_supervisor` | `bool` | `False` | Skip the seccomp user-notification supervisor. The sandbox runs with Landlock + a kernel-only deny filter, without IP allowlisting, resource limits, COW, chroot mediation, `/proc` virtualization, or custom handlers. Required when nesting inside another sandlock (the kernel only allows one `SECCOMP_FILTER_FLAG_NEW_LISTENER` per task). | +| `no_supervisor` | `no_supervisor` | `bool` | `False` | Skip the seccomp user-notification supervisor. The sandbox runs with Landlock + a kernel-only deny filter, without IP allowlisting, resource limits, COW, chroot mediation, `/proc` virtualization, or custom handlers. Required at one level when nesting sandlock, inner or outer (the kernel allows only one `SECCOMP_FILTER_FLAG_NEW_LISTENER` per filter chain). | ## Variable expansion @@ -480,6 +480,61 @@ The two `allow_degraded` calls let the sandbox build on Linux kernels below 6.12, where the v6 IPC scopes are unavailable. On a kernel that does support them, the scopes remain enforced. +## Session lifecycle + +A sandbox session owns the main process launched by the caller and its +background jobs. The main process exiting does not end the session: +remaining jobs continue under the configured supervision until shutdown. + +The operations below describe the shared lifecycle. Method spelling and +return types vary by binding. + +| Operation | Behavior | +| :-------- | :------- | +| Create and start | Create a confined main process, then release it to execute the command. Spawn combines these steps. | +| Wait | Wait for the main process and return its exit status, retaining the session and background supervision. Repeated waits return the same main status. | +| Inspect process groups | Return a sorted snapshot of tracked live process group IDs. Membership can change immediately after inspection. | +| Pause, resume, kill | Send SIGSTOP, SIGCONT, or SIGKILL to tracked live process groups. Sending a signal does not collect the session's final result. | +| Shutdown | Send SIGKILL to remaining jobs, reap the session's children, release supervision, and return the main status, captured output, and filesystem changes. | +| Run | Start a command, wait for its main process, then shut down the session and return the complete result. | + +For a command that launches a background service, wait can report the +launcher's exit while the service remains available. Keep the session +alive while using the service, then shut it down. Run ends the session +as soon as the main process finishes, terminating any remaining jobs. +Shutdown can also be called before the main process exits to terminate +the whole session; it does not perform a graceful termination sequence. + +Where a wait timeout is supported, expiration leaves the session alive. +The caller can wait again or shut down. A run timeout terminates the +session and reports a timeout result. + +### Output and filesystem changes + +Wait returns the main status without waiting for output EOF: background +jobs may still hold output pipes open and write to them. Captured output +continues draining and is collected at shutdown. With streaming stdio, +caller-owned output streams must be drained as needed to prevent a full +pipe from blocking the workload; their bytes are not included in the +collected result. + +COW writes remain staged while the session is active, including writes +made by background jobs after the main process exits. Shutdown collects +the change set and applies the configured branch action after the jobs +have exited. The main process's status selects `on_exit` or `on_error`. +With `DEFER`, the branch becomes pending at session completion and stays +available for an explicit commit or abort. + +Releasing the owning session handle also terminates and reaps its jobs. +Use explicit shutdown when the caller needs the complete result. + +Jobs are terminated through their process groups, which requires Linux +6.9 or later. On older kernels, shutdown fails if any job outlives the +main process, and releasing the handle leaves those jobs running +without supervision. With `no_supervisor`, a job that moves to a new +process group or session is not tracked; shutdown releases it instead +of terminating it, still confined by Landlock and the deny filter. + ## Enumerations ### `BranchAction` @@ -547,7 +602,7 @@ parse_ports([80, "443", "8000-8005"]) so a dynamic override can never erase it. 3. **Seccomp COW with `workdir`.** When `workdir` is set, the seccomp-based COW path intercepts writes under `workdir` and stages - them in an upper layer, committed or aborted on exit per `on_exit` / + them in an upper layer, committed or aborted at session completion per `on_exit` / `on_error`. Staging covers regular files, directories, and symlinks. A FIFO, socket, or device node under `workdir` is a kernel object: opening it or changing its metadata goes to the kernel under the diff --git a/go/README.md b/go/README.md index b9b7c8bd..09cf7c41 100644 --- a/go/README.md +++ b/go/README.md @@ -139,8 +139,8 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) `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 + interrupt a blocked `Wait`. `Wait` returns an `ExitStatus` and retains the session. + 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. @@ -260,14 +260,16 @@ sb := &sandlock.Sandbox{ ```go func (p *Process) Pid() int -func (p *Process) Wait() (*Result, error) +func (p *Process) Wait() (*ExitStatus, error) +func (p *Process) Shutdown() (*Result, error) +func (p *Process) ProcessGroups() ([]int, error) func (p *Process) Pause() error // SIGSTOP to the 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 -// BranchActionDefer only: after Wait, the change set stays on the Process. +// 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) @@ -279,6 +281,12 @@ p.Stdout // *os.File p.Stderr // *os.File ``` +`Wait` reports the main process's status while background jobs remain supervised. +`ProcessGroups` returns a sorted snapshot of tracked live groups. `Shutdown` +terminates remaining jobs with SIGKILL, reaps them, and returns captured output +and filesystem changes. `Run` performs both steps automatically. Use `Close` +when no result is needed; it also closes caller-owned streams. + ### Confine the current process ```go diff --git a/go/sandbox.go b/go/sandbox.go index 28d6c2fa..5d48bba7 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -321,6 +321,14 @@ const ( ReasonTimeout ExitReason = 3 ) +// ExitStatus describes the main process without ending its sandbox session. +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..ca172236 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -10,9 +10,22 @@ package sandlock // // The C declarations come from the cbindgen-generated header, so the cgo // prototypes stay in lock-step with crates/sandlock-ffi automatically. +#include +#include #include +#include #include "sandlock.h" +// Go's runtime signals interrupt poll on cgo threads, so retry on EINTR. +static int sandlock_go_await_readable(int fd) { + struct pollfd p = { .fd = fd, .events = POLLIN }; + int r; + do { + r = poll(&p, 1, -1); + } while (r < 0 && errno == EINTR); + return r; +} + extern int32_t goPolicyCallback(sandlock_event_t *event, sandlock_ctx_t *ctx, void *user_data); extern void goPolicyDrop(void *user_data); */ @@ -731,13 +744,24 @@ 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_timeout(h, timeoutMs(ctx), &status) + if rc < 0 { + C.sandlock_handle_free(h) + return nil, fmt.Errorf("sandlock: wait failed") + } + timedOut := rc == 1 + 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 timedOut { + res.ExitCode, res.Reason, res.Signal, res.Success = -1, ReasonTimeout, -1, false + } return res, nil } @@ -852,21 +876,19 @@ 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. -// -// 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. +// Process owns a sandbox session started by Spawn or Popen. Wait reports the +// main process's status; Shutdown terminates remaining jobs and collects output. +// Close releases the session and caller-owned streams without returning output. +// Handle operations are serialized, including signals during a blocked Wait. 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 bool // a Wait reserves the session between bounded native calls + closeRequested bool + status *ExitStatus + result *Result + 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 @@ -980,7 +1002,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 +1013,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 +1024,119 @@ 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 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) { +// Wait returns the main process's status, retaining background supervision. +// It closes piped Stdin; caller-owned output must be drained concurrently if +// needed. Repeated waits return the cached status. Kill can interrupt a wait. +func (p *Process) Wait() (*ExitStatus, error) { p.mu.Lock() - if p.h == nil || p.waiting { + if p.waiting { + p.mu.Unlock() + return nil, ErrNotRunning + } + if p.status != nil { + status := *p.status + p.mu.Unlock() + return &status, nil + } + if p.h == nil { p.mu.Unlock() return nil, ErrNotRunning } - h := p.h - stdin := p.Stdin p.waiting = true + if p.Stdin != nil { + _ = p.Stdin.Close() + } 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() + defer func() { + p.mu.Lock() + defer p.mu.Unlock() + p.waiting = false + if p.closeRequested { + C.sandlock_handle_free(p.h) + p.h = nil + runtime.SetFinalizer(p, nil) + } + }() + p.mu.Lock() + fd := C.sandlock_handle_main_exit_fd(p.h) + p.mu.Unlock() + if fd < 0 { + return nil, fmt.Errorf("sandlock: wait failed") } + // Block without the lock so Kill and Close can reach the handle meanwhile. + ready := C.sandlock_go_await_readable(fd) + C.close(fd) + if ready < 0 { + return nil, fmt.Errorf("sandlock: wait failed") + } + p.mu.Lock() + defer p.mu.Unlock() + r := C.sandlock_handle_wait(p.h) + if r == nil { + return nil, fmt.Errorf("sandlock: wait failed") + } + status := 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 = &status + copy := status + return ©, nil +} - r := C.sandlock_handle_wait(h) - +// Shutdown sends SIGKILL to remaining jobs, reaps them, and collects output and +// filesystem changes. Caller-owned output streams stay open. A deferred branch +// stays available for Commit or Abort. Repeated calls return the cached result. +func (p *Process) Shutdown() (*Result, error) { p.mu.Lock() defer p.mu.Unlock() - p.waiting = false - // 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 p.waiting { + return nil, ErrNotRunning + } + 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) + p.status = &ExitStatus{ExitCode: p.result.ExitCode, Reason: p.result.Reason, + Signal: p.result.Signal, Success: p.result.Success} + 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 +} + +// ProcessGroups returns a sorted snapshot of tracked live process group IDs. +func (p *Process) ProcessGroups() ([]int, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.h == nil { + return nil, ErrNotRunning } - res := readResult(r) - C.sandlock_result_free(r) - return res, nil + 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 +1197,35 @@ 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) + case syscall.SIGKILL: + 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 the tracked live process groups. func (p *Process) Pause() error { return p.signal(syscall.SIGSTOP) } -// Resume sends SIGCONT to the sandbox process group. +// Resume sends SIGCONT to the tracked live process groups. 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 the tracked live process groups. It is idempotent. +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 @@ -1164,9 +1233,7 @@ func (p *Process) Kill() error { func (p *Process) Ports() (map[int]int, error) { p.mu.Lock() 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 { return map[int]int{}, nil } c := C.sandlock_handle_port_mappings(p.h) @@ -1206,10 +1273,9 @@ func (p *Process) Close() error { 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) + p.closeRequested = true + if C.sandlock_handle_kill(p.h) != 0 { + return fmt.Errorf("sandlock: kill failed") } return nil } diff --git a/go/sandlock_linux_test.go b/go/sandlock_linux_test.go index d1c26137..3f0d461b 100644 --- a/go/sandlock_linux_test.go +++ b/go/sandlock_linux_test.go @@ -393,7 +393,13 @@ func TestDeferHoldsTheBranchForTheProcess(t *testing.T) { t.Fatalf("Spawn: %v", err) } defer p.Close() - res, err := p.Wait() + if _, err := p.Wait(); err != nil { + t.Fatal(err) + } + if p.Pending() { + t.Fatal("wait must not settle the branch") + } + res, err := p.Shutdown() if err != nil { t.Fatalf("Wait: %v", err) } @@ -401,7 +407,7 @@ func TestDeferHoldsTheBranchForTheProcess(t *testing.T) { 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 +447,9 @@ func TestDeferAbortDiscards(t *testing.T) { if _, err := p.Wait(); err != nil { t.Fatalf("Wait: %v", err) } + if _, err := p.Shutdown(); err != nil { + t.Fatal(err) + } if err := p.Abort(); err != nil { t.Fatalf("Abort: %v", err) } @@ -556,7 +565,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..7f4e772a --- /dev/null +++ b/go/session_linux_test.go @@ -0,0 +1,99 @@ +//go:build linux + +package sandlock_test + +import ( + "context" + "fmt" + "github.com/multikernel/sandlock/go" + "runtime" + "testing" + "time" +) + +func TestSessionWaitRetainsBackgroundJobs(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sh", "-c", "sleep 60 & echo retained; exit 7") + if err != nil { + t.Fatal(err) + } + defer p.Close() + status, err := p.Wait() + if err != nil || status.ExitCode != 7 { + t.Fatalf("wait: %+v, %v", status, err) + } + again, err := p.Wait() + if err != nil || *again != *status { + t.Fatalf("repeat wait: %+v, %v", again, err) + } + groups, err := p.ProcessGroups() + if err != nil || len(groups) == 0 { + t.Fatalf("groups: %v, %v", groups, err) + } + result, err := p.Shutdown() + if err != nil || result.ExitCode != 7 || string(result.Stdout) != "retained\n" { + t.Fatalf("shutdown: %+v, %v", result, err) + } +} + +func TestSessionCloseInterruptsWait(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sleep", "60") + if err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { _, err := p.Wait(); done <- err }() + time.Sleep(100 * time.Millisecond) + if err := p.Close(); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("close did not release wait") + } +} + +func TestSessionRunTimeoutStillCleansUp(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + result, err := sb.Run(ctx, "sleep", "60") + if err != nil || result.Reason != sandlock.ReasonTimeout || result.Success { + t.Fatalf("run: %+v, %v", result, err) + } +} + +func TestSessionFinalizerCleansJobsAfterMainExit(t *testing.T) { + requireLandlock(t) + child := func() int { + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Popen(sandlock.Stdio{Stdout: sandlock.StdioPiped}, "sh", "-c", "sleep 60 & echo $!") + if err != nil { + t.Fatal(err) + } + var pid int + if _, err := fmt.Fscanln(p.Stdout, &pid); err != nil { + t.Fatal(err) + } + if status, err := p.Wait(); err != nil || !status.Success { + t.Fatalf("wait: %+v, %v", status, err) + } + return pid + }() + deadline := time.Now().Add(5 * time.Second) + for !procDead(child) { + runtime.GC() + if time.Now().After(deadline) { + t.Fatal("finalizer left background job alive") + } + time.Sleep(20 * time.Millisecond) + } +} diff --git a/python/README.md b/python/README.md index edf37a3a..3ae7f7a6 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,34 @@ 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 while background jobs remain supervised. The status +contains `success`, `exit_code`, `reason`, and `signal`. Repeated waits return +the same status. A timeout raises `TimeoutError` and leaves the session alive. + +#### `sandbox.process_groups() -> list[int]` + +Return a sorted snapshot of tracked live process group IDs. + +#### `sandbox.shutdown() -> Result` + +Terminate remaining jobs with SIGKILL, reap them, collect captured output, and +apply the configured filesystem branch action. `run()` waits and shuts down +automatically; its timeout still terminates the session. A sandbox remains +busy after `wait()` until shutdown or context exit. + +```python +with sandbox: + sandbox.spawn(["sh", "-c", "sleep 60 &"]) + status = sandbox.wait() + groups = sandbox.process_groups() + result = sandbox.shutdown() +``` + +A `Process` returned by `popen()` exposes the same wait, process-group, and +shutdown methods. Its piped output remains caller-owned. Exiting its context +terminates the session and closes its streams. #### Inspecting and deferring COW changes diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index e4fd45bb..044517a5 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, ExitStatus, ExitReason, 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..a1dc8d15 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -316,8 +316,19 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_handle_wait.restype = _c_result_p _lib.sandlock_handle_wait.argtypes = [_c_handle_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_wait_timeout.restype = ctypes.c_int +_lib.sandlock_handle_wait_timeout.argtypes = [ + _c_handle_p, ctypes.c_uint64, ctypes.POINTER(_c_result_p)] + +_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_pause", "sandlock_handle_resume", "sandlock_handle_kill", + "sandlock_handle_main_exit_fd"): + 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] @@ -907,6 +918,15 @@ class ExitReason(IntEnum): """Killed by sandlock because it exceeded its timeout.""" +@dataclass(frozen=True) +class ExitStatus: + """Main-process status returned by wait(), without ending the session.""" + success: bool + exit_code: int + reason: ExitReason + signal: int = -1 + + @dataclass class Result: """Result of a sandboxed command.""" diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 2e139313..578d0646 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -3,15 +3,17 @@ A Sandbox holds both the configuration (policy fields) and the runtime state for executing commands. Configuration fields are set at construction -time; runtime state (``_native``, ``_handle``) is initialized lazily. +time; runtime state (``_native``, ``_session``) is initialized lazily. """ from __future__ import annotations import inspect import itertools +import math import os import re +import threading import weakref from dataclasses import dataclass, field from enum import Enum, IntEnum @@ -528,7 +530,7 @@ def __post_init__(self): self.on_error = BranchAction(self.on_error) # Runtime state — not dataclass fields, not serialized self._native = None # _NativePolicy created lazily on first use - self._handle = None # live sandbox handle during start()/run() + self._session = _Session(None) # the create()/start() lifecycle handle self._pending = None # handle kept after a DEFER run until commit()/abort() self._process = None # weakref to the live popen() Process; it OWNS its # own handle (see `_popen_process`), this is only a @@ -582,15 +584,13 @@ def _popen_process(self): def _live_handle(self): """The handle owning the currently running child, from either source: - the ``create``/``start``/``run``/``spawn`` lifecycle (``self._handle``) or + the ``create``/``start``/``spawn`` lifecycle (``self._session``) or a :meth:`popen` :class:`Process` (which owns its handle). Returns ``None`` when nothing is running. This is the single source of truth for "busy".""" - if self._handle is not None: - return self._handle + if self._session.handle is not None: + return self._session.handle proc = self._popen_process() - if proc is not None and proc._handle is not None: - return proc._handle - return None + return proc._session.handle if proc is not None else None def _check_not_running(self) -> None: """Raise if any child is live, so a second lifecycle call can't leak the @@ -606,36 +606,29 @@ def _reject_if_popen(self) -> None: must not touch a handle the ``Process`` owns — freeing or signalling it behind the ``Process``'s back is exactly the aliasing this split avoids.""" proc = self._popen_process() - if proc is not None and proc._handle is not None: + if proc is not None and proc._session.handle is not None: raise RuntimeError( "this sandbox is running a process started with popen(); manage it " "through the returned Process (proc.wait() / proc.kill()), not the " "sandbox" ) + def __del__(self): + # A running wait() keeps self reachable, so there is none to interrupt. + try: + self._session.free() + self._release_pending() + except Exception: + pass + 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 - if self._handle is not None: - from ._sdk import _lib - try: - _lib.sandlock_handle_free(self._handle) - except Exception: - pass - self._handle = None + if proc is not None: + proc.__exit__(exc_type, exc_val, exc_tb) + self._session.close(self._session.free) # Freeing an undecided branch preserves it, same as KEEP. self._release_pending() return False @@ -651,17 +644,14 @@ def pid(self) -> int | None: 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. + # reading it can't race a concurrent shutdown() 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._session.peek(lambda h: _lib.sandlock_handle_pid(h) or None) @property def is_running(self) -> bool: - """True if a process is currently running in this sandbox (lifecycle or - a live :meth:`popen` :class:`Process`).""" + """True while a session is retained, including after the main exits.""" return self._live_handle() is not None @property @@ -722,7 +712,7 @@ def _release_pending(self) -> None: self._pending = None def _park_or_free(self, handle) -> None: - """After a wait: keep a handle whose branch is deferred, free any other.""" + """Retain a deferred branch after shutdown; otherwise release the handle.""" from ._sdk import _lib if _lib.sandlock_handle_pending(handle): self._pending = handle @@ -748,7 +738,8 @@ 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 + import ctypes + from ._sdk import _c_result_p, _lib, _make_argv, Result, ExitReason self._check_not_running() @@ -760,46 +751,36 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): # The one-shot run path immediately drives wait on this same Python # thread, so it can use the FFI current-thread runtime and avoid # eager Tokio worker-thread creation. - self._handle = _lib.sandlock_create_for_run( + handle = _lib.sandlock_create_for_run( native.ptr, _encode(resolved_name), argv, argc, ) - if not self._handle: + if not handle: return Result(success=False, exit_code=-1, error="sandlock_create failed") - if _lib.sandlock_start(self._handle) != 0: - _lib.sandlock_handle_free(self._handle) - self._handle = None + if _lib.sandlock_start(handle) != 0: + _lib.sandlock_handle_free(handle) return Result(success=False, exit_code=-1, error="sandlock_start failed") - 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) + # This handle's runtime advances only inside native calls, so + # wait natively; no other thread shares it. + status_p = _c_result_p() + rc = _lib.sandlock_handle_wait_timeout( + handle, 0 if timeout is None else max(1, math.ceil(timeout * 1000)), + ctypes.byref(status_p)) + if rc < 0: + raise RuntimeError("sandlock_handle_wait failed") + _lib.sandlock_result_free(status_p) + result_p = _lib.sandlock_handle_shutdown(handle) 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, - ) + result = _take_result(result_p) + if rc == 1: + result.exit_code, result.success, result.reason, result.signal = ( + -1, False, ExitReason.TIMEOUT, -1) + return result def run_with_handlers( self, @@ -1009,11 +990,12 @@ def create(self, cmd: Sequence[str]) -> None: argv, argc = _make_argv(list(cmd)) resolved_name = self._resolve_name() - self._handle = _lib.sandlock_create( + handle = _lib.sandlock_create( native.ptr, _encode(resolved_name), argv, argc, ) - if not self._handle: + if not handle: raise RuntimeError("sandlock_create failed") + self._session = _Session(handle) def start(self) -> None: """Release a previously ``create()``d child to ``execve`` the @@ -1024,12 +1006,12 @@ 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") + session = self._session + with session.lock: + if session.call(_lib.sandlock_start, exclusive=True, + missing="sandbox has not been created") != 0: + session.free() + 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 +1026,24 @@ 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, retaining supervision of background jobs. - 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). + A timeout raises TimeoutError and leaves the session alive. Call + shutdown() to terminate remaining jobs and collect output and changes. """ - 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") + return self._session.wait(timeout) - handle, self._handle = self._handle, None - try: - result_p = _lib.sandlock_handle_wait_timeout(handle, 0) - 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) + def process_groups(self) -> list[int]: + """Return a sorted snapshot of tracked live process group IDs.""" + self._reject_if_popen() + return self._session.call(_read_process_groups) - return Result( - success=bool(success), - exit_code=exit_code, - reason=reason, - signal=signal, - stdout=stdout, - stderr=stderr, - changes=changes, - ) + def shutdown(self) -> "Result": + """SIGKILL remaining jobs, reap them, and collect output and changes.""" + self._reject_if_popen() + return self._session.shutdown(self._park_or_free) def popen( self, @@ -1140,7 +1099,7 @@ def popen( fd_out = ctypes.c_int(-1) fd_err = ctypes.c_int(-1) # The handle is owned by the returned Process, not the Sandbox: it is - # never stored in self._handle. self._process (set below) is the busy + # never stored in self._session. self._process (set below) is the busy # marker (`_live_handle`), so a stale Process can never alias a later # child through the sandbox. handle = _lib.sandlock_popen( @@ -1288,46 +1247,22 @@ 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: + from ._sdk import _lib self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - os.killpg(pid, signal.SIGSTOP) + self._session.signal(getattr(_lib, name)) - def resume(self) -> None: - """Send SIGCONT to the sandbox process group. + def pause(self) -> None: + """Send SIGSTOP to the tracked live process groups.""" + 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 the tracked live process groups.""" + 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 - ``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 + """Send SIGKILL to the tracked live process groups.""" + self._signal("sandlock_handle_kill") def ports(self) -> dict[int, int]: """Return current port mappings {virtual_port: real_port}. @@ -1340,11 +1275,9 @@ 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) + return self._session.peek(_read_port_mappings, {}) def checkpoint( self, @@ -1373,9 +1306,9 @@ 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) + ptr = self._session.call( + _lib.sandlock_handle_checkpoint, exclusive=True, + missing="sandbox is not running (use start() or run() first)") if not ptr: raise RuntimeError("checkpoint capture failed") cp = Checkpoint(ptr) @@ -1423,9 +1356,9 @@ def restore_interactive(self, cp: "Checkpoint") -> None: ) if not handle: raise RuntimeError("checkpoint restore failed") - self._handle = handle + self._session = _Session(handle) # 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) @@ -1535,7 +1468,7 @@ class Process: ``.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 + remains supervised until :meth:`shutdown` is called, or the ``Process`` context manager exits. Prefer the context manager, which closes the streams and reaps the child even on error:: @@ -1549,21 +1482,14 @@ class Process: def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, stderr_fd: int): import os - import threading 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 # 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__). - self._handle = None + self._session = _Session(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, # so a Process that is closed/exited never leaks the pipe ends. Wrap the @@ -1590,147 +1516,60 @@ 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 only after every stream has been wrapped successfully. self._pid = _lib.sandlock_handle_pid(handle) or -1 - self._handle = handle + self._session = _Session(handle) @property def pid(self) -> int | None: - """The child PID while running, else ``None`` (after :meth:`wait`).""" - with self._lock: - return self._pid if self._handle is not None and self._pid > 0 else None + """The child PID while running, else ``None`` (after :meth:`shutdown`).""" + return self._session.peek(lambda _h: self._pid if 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: - if self._handle is None or self._waiting: - return {} - return _read_port_mappings(self._handle) + ``port_remap``).""" + return self._session.peek(_read_port_mappings, {}) 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: - 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 - - def wait(self, timeout: float | None = None) -> "Result": - """Wait for the child to exit and return its :class:`Result`. - - 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. + """SIGKILL tracked live groups; usable from another thread during wait.""" + from ._sdk import _lib + self._session.signal(_lib.sandlock_handle_kill, missing_ok=True) - The returned ``Result`` carries no ``stdout``/``stderr`` (those were - streamed to you as fds — read them off ``.stdout``/``.stderr``). + def wait(self, timeout: float | None = None) -> "ExitStatus": + """Wait for the main process while retaining the session. - 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. + Closes piped stdin. Drain caller-owned output concurrently if needed. + A timeout raises TimeoutError without terminating jobs. Repeated waits + return the cached main status. """ - 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: - 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: - 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. if self.stdin is not None and not self.stdin.closed: try: self.stdin.close() except OSError: pass + return self._session.wait(timeout) + + def process_groups(self) -> list[int]: + """Return a sorted snapshot of tracked live process group IDs.""" + return self._session.call(_read_process_groups) + def shutdown(self) -> "Result": + """Terminate and reap remaining jobs, returning the complete result. + + Caller-owned output streams remain open. A repeated call returns the + cached result; a deferred branch remains available on the Sandbox. + """ 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 self._session.shutdown(self._sandbox._park_or_free) finally: - with self._lock: - 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: + if self._session.handle is None and 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" - ) - 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. - if self._handle is not None: - try: - self.kill() - except Exception: - pass - try: - self.wait() - except Exception: - pass + self._session.close(self.shutdown) for stream in (self.stdin, self.stdout, self.stderr): if stream is not None and not stream.closed: try: @@ -1739,25 +1578,166 @@ 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 + # the missing shutdown()/`with` is visible. __del__ must never raise, and may # run during interpreter shutdown when imports/globals are gone — so this # is strictly best-effort inside a broad guard. try: - if getattr(self, "_handle", None) is None: + session = getattr(self, "_session", None) + if session is None or session.handle is None: return import warnings warnings.warn( - "Process was not waited on or used as a context manager; " + "Process was not shut down or used as a context manager; " "reaping the confined child. Use `with sandbox.popen(...)` or " - "call wait().", + "call shutdown().", ResourceWarning, stacklevel=2, ) - self.kill() - self.wait() + self.shutdown() except Exception: pass + + +class _Session: + """A native handle shared between threads. The lock serializes handle + calls, which the FFI requires, but wait() blocks outside it so another + thread can still kill the session.""" + + def __init__(self, handle): + self.handle = handle + self.lock = threading.RLock() + self._idle = threading.Condition(self.lock) + self._waiting = False + self.status = None + self.result = None + + def call(self, fn, *args, exclusive=False, missing="sandbox is not running"): + with self.lock: + if exclusive and self._waiting: + raise RuntimeError("lifecycle operation already in progress") + if self.handle is None: + raise RuntimeError(missing) + return fn(self.handle, *args) + + def peek(self, fn, default=None): + with self.lock: + return default if self.handle is None else fn(self.handle) + + def signal(self, fn, missing_ok=False): + with self.lock: + if self.handle is None and missing_ok: + return + if self.call(fn) != 0: + raise RuntimeError("sandbox signal failed") + + def wait(self, timeout): + with self.lock: + if self.status is not None: + return self.status + handle = self.call(lambda h: h, exclusive=True) + self._waiting = True + try: + self.status = _wait_status(handle, timeout, self.lock) + return self.status + finally: + with self.lock: + self._waiting = False + self._idle.notify_all() + + def shutdown(self, release): + """Shut down under the lock, then hand the handle to ``release``.""" + from ._sdk import _lib, ExitStatus + with self.lock: + if self.result is None: + handle = self.call(lambda h: h, exclusive=True) + try: + self.result = _take_result(_lib.sandlock_handle_shutdown(handle)) + finally: + self.handle = None + release(handle) + r = self.result + self.status = ExitStatus(r.success, r.exit_code, r.reason, r.signal) + return self.result + + def close(self, finish): + """Kill an in-flight wait so it returns, then ``finish`` the session.""" + from ._sdk import _lib + with self.lock: + if self.handle is not None and self._waiting: + _lib.sandlock_handle_kill(self.handle) + while self._waiting: + self._idle.wait() + if self.handle is not None: + finish() + + def free(self): + from ._sdk import _lib + with self.lock: + if self.handle is not None: + _lib.sandlock_handle_free(self.handle) + self.handle = None + + +def _wait_status(handle, timeout, lock): + import math + import os + import select + from ._sdk import _lib, ExitStatus, ExitReason + with lock: + fd = _lib.sandlock_handle_main_exit_fd(handle) + if fd < 0: + raise RuntimeError("sandlock_handle_main_exit_fd failed") + # Block outside the lock so other threads can signal the session meanwhile. + try: + poller = select.poll() + poller.register(fd, select.POLLIN) + if not poller.poll(None if timeout is None else math.ceil(max(0, timeout) * 1000)): + raise TimeoutError("main process has not exited") + finally: + os.close(fd) + with lock: + ptr = _lib.sandlock_handle_wait(handle) + if not ptr: + raise RuntimeError("sandlock_handle_wait failed") + try: + return ExitStatus(bool(_lib.sandlock_result_success(ptr)), + _lib.sandlock_result_exit_code(ptr), + ExitReason(_lib.sandlock_result_reason(ptr)), + _lib.sandlock_result_signal(ptr)) + finally: + _lib.sandlock_result_free(ptr) + + +def _read_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 _take_result(ptr): + from ._sdk import _lib, _read_result_bytes, _read_result_changes, Result, ExitReason + if not ptr: + raise RuntimeError("sandbox 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) diff --git a/python/tests/test_checkpoint.py b/python/tests/test_checkpoint.py index a1c856b3..cee2a8c4 100644 --- a/python/tests/test_checkpoint.py +++ b/python/tests/test_checkpoint.py @@ -13,7 +13,6 @@ import pytest from sandlock import Sandbox, Checkpoint, SkippedFd -from sandlock._sdk import _encode, _lib, _make_argv _PYTHON_READABLE = list(dict.fromkeys([ @@ -32,20 +31,9 @@ def _policy(**overrides): def running_sandbox(): """A sandbox with a long-running process for checkpoint tests.""" sb = _policy() - argv, argc = _make_argv(["sleep", "60"]) - native = sb._ensure_native() - sb._handle = _lib.sandlock_create( - native.ptr, - _encode(sb._resolve_name()), - argv, - argc, - ) - assert sb._handle, "create failed" - assert _lib.sandlock_start(sb._handle) == 0, "start failed" + sb.spawn(["sleep", "60"]) yield sb - if sb._handle: - _lib.sandlock_handle_free(sb._handle) - sb._handle = None + sb._session.free() class TestCheckpointCapture: @@ -261,9 +249,14 @@ def test_restore_resumes_counter(self, tmp_dir): counter.write_text("0\n") sb2 = _policy(**policy_kw) + sb2.spawn(["sh", "-c", "exit 7"]) + assert sb2.wait().exit_code == 7 + sb2.shutdown() assert sb2.restore_skipped == [] ret = sb2.restore_interactive(cp) assert ret is None + with pytest.raises(TimeoutError): + sb2.wait(timeout=0.01) skipped = sb2.restore_skipped assert isinstance(skipped, list) for s in skipped: diff --git a/python/tests/test_lifecycle.py b/python/tests/test_lifecycle.py index 423c0aa7..71c7bf1e 100644 --- a/python/tests/test_lifecycle.py +++ b/python/tests/test_lifecycle.py @@ -9,7 +9,6 @@ import pytest from sandlock import Sandbox, ExitReason -from sandlock._sdk import _lib _BIN_READABLE = ["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc"] @@ -70,7 +69,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().success + result = sb.shutdown() assert result.exit_code == 0 assert b"hello" in result.stdout @@ -102,7 +102,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() + assert sb.wait().success + result = sb.shutdown() assert result.exit_code == 0 assert b"two-step" in result.stdout @@ -123,8 +124,7 @@ def test_create_then_discard_reaps_child(self): assert _proc_state(pid) == "S" # Free the handle without start() — Rust Drop should SIGKILL + waitpid. - _lib.sandlock_handle_free(sb._handle) - sb._handle = None + sb._session.free() # After Drop, /proc/ must be gone. If it's still there as 'Z', # the reap is missing. diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py index 0283d87e..cd8633e0 100644 --- a/python/tests/test_popen.py +++ b/python/tests/test_popen.py @@ -149,7 +149,7 @@ 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 + second = proc.wait() # cached status, session remains alive assert first is second assert first.exit_code == 0 @@ -181,7 +181,7 @@ def test_popen_rejects_second_process_on_same_sandbox(): proc.wait() -def test_wait_timeout_kills_and_returns_nonsuccess(): +def test_wait_timeout_leaves_session_alive(): # 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 @@ -191,7 +191,9 @@ def test_wait_timeout_kills_and_returns_nonsuccess(): box = {} def _run(): - box["result"] = proc.wait(timeout=0.5) + with pytest.raises(TimeoutError): + proc.wait(timeout=0.5) + box["timed_out"] = True t = threading.Thread(target=_run, daemon=True) t.start() @@ -201,7 +203,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 box["timed_out"] + assert proc.process_groups() + proc.shutdown() def test_wait_without_timeout_completes_for_quick_child(): @@ -256,7 +260,8 @@ 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.wait() + p1.shutdown() p2 = sandbox.popen(["sleep", "100"]) try: @@ -267,7 +272,7 @@ def test_stale_process_does_not_alias_reused_sandbox(): assert p2.pid is not None, "p2 must still be alive" finally: p2.kill() - p2.wait() + p2.shutdown() assert not sandbox.is_running @@ -300,7 +305,7 @@ def test_sandbox_pid_and_ports_delegate_to_popen_process(): assert sandbox.ports() == proc.ports() # both {} without port_remap finally: proc.kill() - proc.wait() + proc.shutdown() assert sandbox.pid is None assert sandbox.ports() == {} @@ -314,7 +319,9 @@ def test_wait_timeout_zero_does_not_wait_forever(): box = {} def _run(): - box["result"] = proc.wait(timeout=0) + with pytest.raises(TimeoutError): + proc.wait(timeout=0) + box["timed_out"] = True t = threading.Thread(target=_run, daemon=True) t.start() @@ -324,7 +331,8 @@ def _run(): t.join(timeout=5) pytest.fail("wait(timeout=0) hung — 0 collapsed to wait-forever") - assert not box["result"].success + assert box["timed_out"] + 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..6c9fe9f0 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 + sb.shutdown() assert sb.pid is None def test_pause_not_running_raises(self): @@ -1008,7 +1010,8 @@ 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 p.wait().success + result = p.shutdown() assert result.success assert p.pending p.commit() diff --git a/python/tests/test_session.py b/python/tests/test_session.py new file mode 100644 index 00000000..1e604b4c --- /dev/null +++ b/python/tests/test_session.py @@ -0,0 +1,159 @@ +"""Session lifetime is independent of the main process's exit.""" +import os + +import pytest + +from sandlock import Sandbox, ExitStatus, ExitReason, StdioMode, BranchAction + + +def policy(**kwargs): + return Sandbox(fs_readable=["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc", "/dev"], **kwargs) + + +def test_wait_retains_jobs_and_shutdown_collects_output(): + with policy() as sb: + sb.spawn(["sh", "-c", "sleep 60 & echo retained; exit 7"]) + status = sb.wait(timeout=5) + assert isinstance(status, ExitStatus) + assert status.exit_code == 7 + assert sb.wait() == status + assert sb.is_running + assert sb.process_groups() + with pytest.raises(RuntimeError, match="already running"): + sb.spawn(["true"]) + result = sb.shutdown() + assert result.exit_code == 7 + assert result.stdout == b"retained\n" + assert not sb.is_running + assert sb.run(["true"]).success + + +def test_wait_timeout_retains_main_for_retry(): + with policy() as sb: + sb.spawn(["sh", "-c", "sleep 0.2; exit 9"]) + with pytest.raises(TimeoutError): + sb.wait(timeout=0) + assert sb.process_groups() + assert sb.wait(timeout=5).exit_code == 9 + assert sb.shutdown().exit_code == 9 + + +def test_handle_stays_usable_while_another_thread_waits(): + import threading + import time + with policy() as sb: + sb.spawn(["sleep", "60"]) + statuses = [] + waiter = threading.Thread(target=lambda: statuses.append(sb.wait())) + waiter.start() + time.sleep(0.1) + assert sb.process_groups() + sb.kill() + waiter.join(timeout=5) + assert not waiter.is_alive() + assert statuses[0].reason == ExitReason.KILLED + sb.shutdown() + + +def test_popen_context_cleans_background_jobs_after_wait(): + with policy().popen(["sh", "-c", "sleep 60 & echo $!"], stdout=StdioMode.PIPED) as proc: + child = int(proc.stdout.readline()) + assert proc.wait(timeout=5).success + assert proc.process_groups() + os.kill(child, 0) + with pytest.raises(ProcessLookupError): + os.kill(child, 0) + + +def test_abandoned_sandbox_ends_background_jobs(tmp_path): + import gc + pidfile = tmp_path / "job" + sb = policy(fs_writable=[str(tmp_path)]) + sb.spawn(["sh", "-c", f"sleep 60 & echo $! > {pidfile}"]) + assert sb.wait(timeout=5).success + child = int(pidfile.read_text()) + os.kill(child, 0) + del sb + gc.collect() + with pytest.raises(ProcessLookupError): + os.kill(child, 0) + + +def test_shutdown_defers_cow_until_jobs_finish(tmp_path): + with policy(fs_writable=[str(tmp_path)], workdir=str(tmp_path), on_exit=BranchAction.DEFER) as sb: + sb.spawn(["sh", "-c", f"echo data > {tmp_path}/out"]) + assert sb.wait(timeout=5).success + assert not sb.pending + assert not (tmp_path / "out").exists() + result = sb.shutdown() + assert result.changes + assert sb.pending + sb.commit() + assert (tmp_path / "out").read_text() == "data\n" + + +def test_run_timeout_terminates_session(): + with policy() as sb: + result = sb.run(["sleep", "60"], timeout=0.01) + assert result.reason == ExitReason.TIMEOUT + assert not result.success + assert not sb.is_running + +@pytest.mark.parametrize("streaming", [False, True]) +def test_context_exit_interrupts_wait_and_releases_session(streaming): + import threading + import time + sb = policy() + if streaming: + owner = sb.popen(["sleep", "60"]) + else: + sb.spawn(["sleep", "60"]) + owner = sb + outcomes = [] + worker = threading.Thread(target=lambda: outcomes.append(owner.wait()), daemon=True) + worker.start() + time.sleep(0.1) + owner.__exit__(None, None, None) + worker.join(5) + assert not worker.is_alive() + assert outcomes and not outcomes[0].success + assert not sb.is_running + assert sb.run(["true"]).success + + +def test_wait_rejects_conflicting_start_and_checkpoint(): + import threading + import time + with policy() as sb: + sb.spawn(["sleep", "60"]) + worker = threading.Thread(target=sb.wait, daemon=True) + worker.start() + time.sleep(0.1) + try: + for op in (sb.start, sb.checkpoint): + with pytest.raises(RuntimeError, match="already in progress"): + op() + finally: + sb.kill() + worker.join(5) + assert not worker.is_alive() + + +def test_kill_in_fresh_interpreter(): + import ctypes + from sandlock._sdk import _lib + assert _lib.sandlock_handle_kill.argtypes == [ctypes.c_void_p] + import subprocess + import sys + result = subprocess.run( + [sys.executable, "-c", """ +from sandlock import Sandbox +with Sandbox(fs_readable=["/usr", "/lib", "/lib64", "/bin", "/etc"]) as sb: + sb.spawn(["sleep", "60"]) + sb.kill() + assert not sb.wait().success + sb.shutdown() +"""], + capture_output=True, timeout=10, + ) + assert result.returncode == 0, result.stderr.decode()