diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 95668ca1..4822a894 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -617,12 +617,9 @@ pub async fn run(args: LearnArgs) -> Result<()> { eprintln!("sandlock learn: observing {cmd_str} ..."); - // Use the three-step lifecycle (create/start/wait) so we can get the child - // PID from sandbox.pid() and sample /proc/ for resource peaks. let mut sandbox = policy; sandbox.create_interactive(&cmd_refs).await .map_err(|e| anyhow!("sandbox error: {e}"))?; - let child_pid = sandbox.pid().expect("child pid after create") as u32; sandbox.start() .map_err(|e| anyhow!("sandbox error: {e}"))?; @@ -632,25 +629,24 @@ 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) => { - // Timeout: kill the child, drain the supervisor, write a partial profile. + // The shutdown below kills the session; the profile is partial. eprintln!("sandlock learn: timeout after {secs}s, killing process"); - unsafe { libc::kill(child_pid as i32, libc::SIGKILL); } - // Drain without timeout so the supervisor releases its resources cleanly. - let _ = sandbox.wait().await; true } } } 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..fad0ab8c 100644 --- a/crates/sandlock-cli/src/main.rs +++ b/crates/sandlock-cli/src/main.rs @@ -285,23 +285,35 @@ 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) => ( - info.mode.unwrap_or_else(|| "running".to_string()), - query_ports(name), - ), - 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()), + // The registry's main pid outlives main. A pidfd opened before + // the supervisor vouches main is alive names main, and /proc + // is trusted only if main is still alive after reading it. + let pid = sandlock_core::control::sandbox_pids(name).map(|p| p.child); + let main_fd = pid.as_ref().ok().and_then(|&pid| pidfd_open(pid)); + let (status, ports, pid, uptime, cmd) = match (sandlock_core::control::sandbox_info(name), pid) { + (Ok(info), Ok(pid)) if !info.main_exited => { + let uptime = proc_uptime(pid); + let cmd = proc_cmdline(pid); + if main_fd.as_ref().is_some_and(|fd| !pidfd_exited(fd)) { + ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), + pid.to_string(), + uptime.unwrap_or_else(|| "?".to_string()), + cmd.unwrap_or_else(|| "?".to_string()), + ) + } else { + main_exited_row(name, info) + } + } + (Ok(info), _) if info.main_exited => main_exited_row(name, info), + (info, pid) => ( + if info.is_ok() { "running" } else { "unresponsive" }.to_string(), + "?".to_string(), + 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 +370,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 => { @@ -956,6 +949,36 @@ fn validate_cli_name(name: &str) -> Result<(), String> { /// Query the control socket for the virtual→real port map, returning a /// compact display string or `"-"` when the socket is missing (e.g. /// `--no-supervisor`). +fn main_exited_row( + name: &str, + info: sandlock_core::control::SandboxInfo, +) -> (String, String, String, String, String) { + let cmd = match info.live_groups { + 0 => "(main exited, idle)".to_string(), + 1 => "(main exited, 1 group)".to_string(), + n => format!("(main exited, {n} groups)"), + }; + ( + info.mode.unwrap_or_else(|| "running".to_string()), + query_ports(name), + "-".to_string(), + "-".to_string(), + cmd, + ) +} + +fn pidfd_open(pid: i32) -> Option { + use std::os::fd::FromRawFd; + let fd = unsafe { libc::syscall(libc::SYS_pidfd_open, pid, 0) }; + (fd >= 0).then(|| unsafe { std::os::fd::OwnedFd::from_raw_fd(fd as i32) }) +} + +fn pidfd_exited(fd: &std::os::fd::OwnedFd) -> bool { + use std::os::fd::AsRawFd; + let mut poll = libc::pollfd { fd: fd.as_raw_fd(), events: libc::POLLIN, revents: 0 }; + unsafe { libc::poll(&mut poll, 1, 0) != 0 } +} + fn query_ports(name: &str) -> String { use sandlock_core::control::send_control_request; match send_control_request(name, "ports", serde_json::Value::Object(Default::default())) { diff --git a/crates/sandlock-cli/tests/cli_test.rs b/crates/sandlock-cli/tests/cli_test.rs index 06411b0a..f48e7574 100644 --- a/crates/sandlock-cli/tests/cli_test.rs +++ b/crates/sandlock-cli/tests/cli_test.rs @@ -671,6 +671,29 @@ fn test_kill_stops_sandbox() { } } +#[test] +fn test_kill_stops_sandbox_without_supervisor() { + let name = format!("test-kill-nosup-{}", std::process::id()); + let mut args = sandlock_sleep_args(&name); + args.insert(1, "--no-supervisor".into()); + let mut child = std::process::Command::new(env!("CARGO_BIN_EXE_sandlock")) + .args(&args) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()) + .spawn() + .expect("spawn sandlock"); + if let Err(e) = wait_for_sandbox(&mut child, &name) { + panic!("{}", e); + } + let out = sandlock_bin().args(["kill", &name]).output().expect("sandlock kill"); + assert!( + out.status.success(), + "kill should succeed without a supervisor: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + assert!(!child.wait().expect("wait for killed child").success()); +} + #[test] fn test_kill_nonexistent_sandbox() { let out = sandlock_bin() diff --git a/crates/sandlock-cli/tests/learn_test.rs b/crates/sandlock-cli/tests/learn_test.rs index 5b1966d5..a566981f 100644 --- a/crates/sandlock-cli/tests/learn_test.rs +++ b/crates/sandlock-cli/tests/learn_test.rs @@ -1151,3 +1151,27 @@ fn test_learn_merge_preserves_home_variable() { assert!(!reads.iter().any(|r| r.starts_with(&format!("{home}/.config"))), "merge added a machine-specific duplicate of ${{HOME}}/.config: {reads:?}"); } + +/// A timeout ends the whole session, background jobs included, and still +/// writes what was observed before it. +#[test] +fn test_learn_timeout_ends_the_session() { + // A sleep length no other process uses, so the job can be found afterwards. + let marker = format!("{}.{}", 3000 + std::process::id() % 1000, std::process::id()); + let cmd = format!("cat /etc/hostname; sleep {marker} & sleep 60"); + let started = std::time::Instant::now(); + let output = sandlock_bin() + .args(["learn", "--timeout", "1", "--", "sh", "-c", &cmd]) + .output() + .expect("failed to run sandlock learn"); + assert!(started.elapsed() < std::time::Duration::from_secs(30), "learn ignored its timeout"); + let stdout = String::from_utf8_lossy(&output.stdout); + let read_line = stdout.lines().find(|l| l.starts_with("read = [")).unwrap_or(""); + assert!( + read_line.contains("/etc/hostname"), + "expected a partial profile with /etc/hostname, got: {stdout}; stderr={}", + String::from_utf8_lossy(&output.stderr), + ); + let survivors = Command::new("pgrep").args(["-f", &format!("sleep {marker}")]).output().unwrap(); + assert!(survivors.stdout.is_empty(), "the background job outlived the timeout"); +} diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 7a2b5a22..48542f4a 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -65,6 +65,15 @@ impl PipePair { } } +/// Written on the exec pipe right before `execve`, which closes the pipe on +/// success; `EXEC_FAILED` follows if `execve` returns. +pub(crate) const EXEC_REACHED: u8 = b'e'; +pub(crate) const EXEC_FAILED: u8 = b'f'; + +fn mark_exec(exec_w: RawFd, mark: u8) { + unsafe { libc::write(exec_w, [mark].as_ptr().cast(), 1) }; +} + // ============================================================ // Pipe I/O helpers // ============================================================ @@ -237,10 +246,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 +296,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. @@ -305,6 +309,14 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { use std::io::Write; + // Created here rather than before fork so that no sibling forked by the + // parent inherits the write end and holds back its EOF. + let mut exec_fds = [0i32; 2]; + if unsafe { libc::pipe2(exec_fds.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + fail!("exec pipe"); + } + let [exec_r, exec_w] = exec_fds; + // 1. Interactive runs only: if stdin is a terminal, become the // foreground process group so interactive shells can read from the // TTY. Captured/piped runs must not: the embedding process keeps @@ -319,19 +331,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; @@ -527,6 +526,10 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { std::mem::forget(notif_fd); } + if let Err(e) = write_u32_fd(pipes.notif_w.as_raw_fd(), exec_r as u32) { + fail!(format!("write exec pipe fd: {}", e)); + } + // 10. Wait for parent to signal ready match read_u32_fd(pipes.ready_r.as_raw_fd()) { Ok(_) => {} @@ -538,6 +541,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { if keep_fd >= 0 { fds_to_keep.push(keep_fd); } + fds_to_keep.push(exec_w); close_fds_above(2, &fds_to_keep); // 13. Apply environment @@ -607,6 +611,7 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { // supervisor, so `run`'s code is already mapped; running it directly // avoids an execve that Landlock would otherwise have to authorize. set_proc_name(name); + unsafe { libc::close(exec_w) }; run(); unsafe { libc::_exit(0) }; } @@ -628,6 +633,8 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { .chain(std::iter::once(std::ptr::null())) .collect(); + mark_exec(exec_w, EXEC_REACHED); + if let Some(fd) = exec_fd { extern "C" { static environ: *const *const libc::c_char; @@ -662,8 +669,10 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { unsafe { libc::execvp(argv_ptrs[0], argv_ptrs.as_ptr()) }; } - // If we get here, exec failed - fail!(format!("exec '{}'", cmd[0].to_string_lossy())); + let exec_err = io::Error::last_os_error(); + mark_exec(exec_w, EXEC_FAILED); + let _ = write!(std::io::stderr(), "sandlock child: exec '{}': {}\n", cmd[0].to_string_lossy(), exec_err); + unsafe { libc::_exit(127) }; } // ============================================================ diff --git a/crates/sandlock-core/src/control.rs b/crates/sandlock-core/src/control.rs index 9601a8e7..d62bc525 100644 --- a/crates/sandlock-core/src/control.rs +++ b/crates/sandlock-core/src/control.rs @@ -169,6 +169,12 @@ pub(crate) fn bind_control_socket(name: &str) -> std::io::Result #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct SandboxInfo { pub mode: Option, + /// Main's pid in the registry is free for reuse once this is set. + #[serde(default)] + pub main_exited: bool, + /// Process groups of the session that still have members. + #[serde(default)] + pub live_groups: usize, } /// Spawn the control-loop task. `ctx` is `None` for sandboxes without a @@ -177,6 +183,7 @@ pub struct SandboxInfo { pub(crate) fn spawn_control_loop( sockets: ControlSocket, ctx: Option>, + controller: crate::Controller, sandbox: Sandbox, info: SandboxInfo, ) -> tokio::task::JoinHandle<()> { @@ -185,7 +192,7 @@ pub(crate) fn spawn_control_loop( let sandbox = Arc::new(tokio::sync::Mutex::new(sandbox)); tokio::spawn(async move { let ControlSocket { control, claim: _claim } = sockets; - control_loop(control, ctx, sandbox, info, unsafe { libc::getuid() }).await; + control_loop(control, ctx, controller, sandbox, info, unsafe { libc::getuid() }).await; }) } @@ -271,6 +278,7 @@ impl tokio::io::AsyncWrite for ControlStream { async fn control_loop( listener: ControlFd, ctx: Option>, + controller: crate::Controller, sandbox: Arc>, info: SandboxInfo, my_uid: u32, @@ -289,7 +297,7 @@ async fn control_loop( let Ok(stream) = AsyncFd::new(stream).map(ControlStream) else { continue }; let _ = tokio::time::timeout( std::time::Duration::from_secs(5), - serve_one(stream, ctx.as_ref(), &sandbox, &info), + serve_one(stream, ctx.as_ref(), &controller, &sandbox, &info), ) .await; } @@ -321,6 +329,7 @@ pub struct ControlResponse { async fn serve_one( stream: ControlStream, ctx: Option<&Arc>, + controller: &crate::Controller, sandbox: &Arc>, info: &SandboxInfo, ) { @@ -367,10 +376,10 @@ async fn serve_one( } match req.verb.as_str() { - "info" => handle_info(&mut stream, info).await, + "info" => handle_info(&mut stream, info, controller).await, "config" => handle_config(&mut stream, ctx, sandbox).await, "ports" => handle_ports(&mut stream, ctx).await, - "kill" => handle_kill(&mut stream, ctx).await, + "kill" => handle_kill(&mut stream, controller).await, _ => { let resp = ControlResponse { v: 1, @@ -383,8 +392,13 @@ async fn serve_one( } } -async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo) { - let resp = match serde_json::to_value(info) { +async fn handle_info(stream: &mut ControlStream, info: &SandboxInfo, controller: &crate::Controller) { + let info = SandboxInfo { + main_exited: controller.main_exited(), + live_groups: controller.process_groups().map_or(0, |groups| groups.len()), + ..info.clone() + }; + let resp = match serde_json::to_value(&info) { Ok(data) => ControlResponse { v: 1, ok: true, data: Some(data), err: None }, Err(e) => ControlResponse { v: 1, @@ -437,29 +451,10 @@ async fn handle_config( /// Only the supervisor knows the groups a process has moved into, so /// `sandlock kill` asks here before it signals the child's own group. -async fn handle_kill(stream: &mut ControlStream, ctx: Option<&Arc>) { - if let Some(ctx) = ctx { - ctx.policy_fn.lock().await.cancellation.cancel(); - } - let resp = match ctx.map(|ctx| ctx.groups.signal(libc::SIGKILL)) { - Some(Ok(groups)) => ControlResponse { - v: 1, - ok: true, - data: Some(serde_json::json!({ "groups": groups })), - err: None, - }, - Some(Err(e)) => ControlResponse { - v: 1, - ok: false, - data: None, - err: Some(format!("signal process groups: {}", e)), - }, - None => ControlResponse { - v: 1, - ok: false, - data: None, - err: Some("no supervisor".into()), - }, +async fn handle_kill(stream: &mut ControlStream, controller: &crate::Controller) { + let resp = match controller.kill() { + Ok(()) => ControlResponse { v: 1, ok: true, data: None, err: None }, + Err(e) => ControlResponse { v: 1, ok: false, data: None, err: Some(e.to_string()) }, }; let _ = write_response(stream, &resp).await; } @@ -616,10 +611,10 @@ 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. + /// Main, whose pid is free for reuse once it has exited. pub child: i32, pub supervisor: i32, } @@ -654,18 +649,62 @@ fn send_control_request_as( args: serde_json::Value, my_uid: u32, ) -> Result { - use std::io::{Read, Write}; + let (stream, _) = connect_control(name, my_uid)?; + // A short timeout so a wedged supervisor does not block the CLI forever. + exchange(name, stream, verb, args, std::time::Duration::from_secs(2)).map_err(|e| e.message) +} - let (mut stream, _) = connect_control(name, my_uid)?; +/// Kill through the supervisor's group table. Only a supervisor that accepts +/// the connection and then never answers is killed instead, and the reaper +/// then kills main's group; a closed connection means it is tearing down. +pub fn kill_sandbox(name: &str) -> Result<(), String> { + let (stream, cred) = connect_control(name, unsafe { libc::getuid() })?; + // Opened while connected, so it names the peer SO_PEERCRED vouched for. + let supervisor = crate::sys::syscall::pidfd_open(cred.pid as u32, 0) + .map_err(|e| format!("pidfd_open supervisor: {e}"))?; + let args = serde_json::Value::Object(Default::default()); + // Longer than the server's per-client cap, so waiting in its queue is not + // mistaken for a wedge. + match exchange(name, stream, "kill", args, std::time::Duration::from_secs(8)) { + Ok(resp) if resp.ok => Ok(()), + Ok(resp) => Err(resp.err.unwrap_or_else(|| "kill failed".into())), + Err(e) if e.timed_out => crate::sys::syscall::pidfd_send_signal(&supervisor, libc::SIGKILL) + .map_err(|e| format!("kill unresponsive supervisor: {e}")), + Err(e) => Err(e.message), + } +} + +struct ExchangeError { + timed_out: bool, + message: String, +} + +impl ExchangeError { + fn other(message: String) -> Self { + Self { timed_out: false, message } + } + + fn read(name: &str, e: std::io::Error) -> Self { + let timed_out = matches!(e.kind(), std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut); + Self { timed_out, message: unresponsive(name, e) } + } +} + +fn exchange( + name: &str, + mut stream: UnixStream, + verb: &str, + args: serde_json::Value, + timeout: std::time::Duration, +) -> Result { + use std::io::{Read, Write}; - // Set a 2-second timeout on reads so a wedged supervisor does not - // block the CLI forever. stream - .set_read_timeout(Some(std::time::Duration::from_secs(2))) - .map_err(|e| format!("set_read_timeout: {}", e))?; + .set_read_timeout(Some(timeout)) + .map_err(|e| ExchangeError::other(format!("set_read_timeout: {}", e)))?; stream - .set_write_timeout(Some(std::time::Duration::from_secs(2))) - .map_err(|e| format!("set_write_timeout: {}", e))?; + .set_write_timeout(Some(timeout)) + .map_err(|e| ExchangeError::other(format!("set_write_timeout: {}", e)))?; let req = serde_json::json!({ "v": 1, @@ -673,24 +712,24 @@ fn send_control_request_as( "args": args, }); let body = serde_json::to_vec(&req) - .map_err(|e| format!("serialize request: {}", e))?; + .map_err(|e| ExchangeError::other(format!("serialize request: {}", e)))?; let len = (body.len() as u32).to_be_bytes(); - stream.write_all(&len).map_err(|e| format!("write len: {}", e))?; - stream.write_all(&body).map_err(|e| format!("write body: {}", e))?; + stream.write_all(&len).map_err(|e| ExchangeError::other(format!("write len: {}", e)))?; + stream.write_all(&body).map_err(|e| ExchangeError::other(format!("write body: {}", e)))?; // Read response. let mut len_buf = [0u8; 4]; - stream.read_exact(&mut len_buf).map_err(|e| unresponsive(name, e))?; + stream.read_exact(&mut len_buf).map_err(|e| ExchangeError::read(name, e))?; let resp_len = u32::from_be_bytes(len_buf) as usize; if resp_len > 65536 { - return Err("response too large".to_string()); + return Err(ExchangeError::other("response too large".to_string())); } let mut resp_body = vec![0u8; resp_len]; - stream.read_exact(&mut resp_body).map_err(|e| unresponsive(name, e))?; + stream.read_exact(&mut resp_body).map_err(|e| ExchangeError::read(name, e))?; serde_json::from_slice(&resp_body) - .map_err(|e| format!("parse response: {}", e)) + .map_err(|e| ExchangeError::other(format!("parse response: {}", e))) } /// Ask a sandbox for its mode. @@ -806,7 +845,7 @@ mod tests { } fn info() -> SandboxInfo { - SandboxInfo { mode: Some("test".into()) } + SandboxInfo { mode: Some("test".into()), main_exited: false, live_groups: 0 } } /// Bind a listener for `name`, run the control loop on it with @@ -816,7 +855,13 @@ mod tests { let sandbox = Arc::new(tokio::sync::Mutex::new(test_sandbox())); tokio::spawn(async move { let _claim = sockets.claim; - control_loop(sockets.control, None, sandbox, info(), expected_uid).await; + let controller = crate::Controller { + groups: Arc::new(crate::pgroup::ProcessGroups::new()), + stop: Default::default(), + main_exit: crate::controller::MainExit::new(None), + pid: 0, + }; + control_loop(sockets.control, None, controller, sandbox, info(), expected_uid).await; }) } diff --git a/crates/sandlock-core/src/controller.rs b/crates/sandlock-core/src/controller.rs new file mode 100644 index 00000000..8deeaf9c --- /dev/null +++ b/crates/sandlock-core/src/controller.rs @@ -0,0 +1,111 @@ +//! Acts on a running session while `Sandbox::wait()` holds it mutably. + +use std::os::fd::{AsRawFd, OwnedFd}; +use std::sync::{Arc, Mutex}; + +use crate::error::{SandboxRuntimeError, SandlockError}; +use crate::result::ExitStatus; + +#[derive(Clone)] +pub struct Controller { + pub(crate) groups: Arc, + pub(crate) stop: tokio_util::sync::CancellationToken, + pub(crate) main_exit: Arc, + pub(crate) pid: i32, +} + +impl Controller { + pub fn kill(&self) -> Result<(), SandlockError> { + self.signal(libc::SIGKILL).map(|_| ()) + } + + pub fn pause(&self) -> Result<(), SandlockError> { + self.signal_live(libc::SIGSTOP) + } + + pub fn resume(&self) -> Result<(), SandlockError> { + self.signal_live(libc::SIGCONT) + } + + /// Sorted pgids that still have members; may change as soon as it returns. + pub fn process_groups(&self) -> Result, SandlockError> { + Ok(self.groups.live().map_err(SandboxRuntimeError::Io)?) + } + + pub fn main_exited(&self) -> bool { + self.main_exit.get().is_some() + } + + /// Main's pid until it exits; afterwards the number may name another process. + pub fn pid(&self) -> Option { + (!self.main_exited()).then_some(self.pid) + } + + /// Signal every group, returning how many had members left. + pub fn signal(&self, sig: i32) -> Result { + if sig == libc::SIGKILL { + self.stop.cancel(); + } + Ok(self.groups.signal(sig).map_err(SandboxRuntimeError::Io)?) + } + + fn signal_live(&self, sig: i32) -> Result<(), SandlockError> { + if self.signal(sig)? == 0 { + return Err(SandboxRuntimeError::Io(std::io::Error::from_raw_os_error(libc::ESRCH)).into()); + } + Ok(()) + } +} + +/// Main's exit status, read straight from the reaper's status pipe so that no +/// background task, and no particular runtime, has to be alive to learn it. +pub(crate) struct MainExit { + pipe: Option, + status: Mutex>, +} + +impl MainExit { + /// `pipe` carries main's raw wait status as four bytes; without one, the + /// owner publishes the status through [`MainExit::set`]. + pub(crate) fn new(pipe: Option) -> Arc { + if let Some(fd) = &pipe { + unsafe { libc::fcntl(fd.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK) }; + } + Arc::new(MainExit { pipe, status: Mutex::new(None) }) + } + + pub(crate) fn get(&self) -> Option { + let mut status = self.status.lock().unwrap(); + if status.is_none() { + if let Some(fd) = &self.pipe { + let mut raw = [0u8; 4]; + let n = unsafe { libc::read(fd.as_raw_fd(), raw.as_mut_ptr().cast(), raw.len()) }; + *status = match n { + 4 => Some(crate::sandbox::sandbox_wait_status_to_exit(i32::from_le_bytes(raw))), + // The write is atomic, so anything else but EAGAIN means + // the reaper died without reporting main. + n if n < 0 && std::io::Error::last_os_error().kind() == std::io::ErrorKind::WouldBlock => None, + _ => Some(ExitStatus::Killed), + }; + } + } + status.clone() + } + + pub(crate) fn set(&self, exit: ExitStatus) { + self.status.lock().unwrap().get_or_insert(exit); + } + + pub(crate) async fn wait(&self) -> std::io::Result { + let Some(fd) = &self.pipe else { + return Err(std::io::Error::other("main's exit has no status pipe")); + }; + let ready = tokio::io::unix::AsyncFd::with_interest(fd.as_raw_fd(), tokio::io::Interest::READABLE)?; + loop { + if let Some(exit) = self.get() { + return Ok(exit); + } + ready.readable().await?.clear_ready(); + } + } +} diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index a8847558..7482fa6d 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; @@ -43,12 +44,14 @@ pub mod fork; pub(crate) mod ca_inject; pub(crate) mod chroot; pub mod control; +pub mod controller; #[cfg(feature = "http")] mod transparent_proxy; #[cfg(not(feature = "http"))] #[path = "transparent_proxy/disabled.rs"] mod transparent_proxy; +pub use controller::Controller; pub use error::SandlockError; pub use policy_fn::PolicyFn; pub use sys::structs::{SeccompData, SeccompNotif}; diff --git a/crates/sandlock-core/src/pgroup.rs b/crates/sandlock-core/src/pgroup.rs index 0edd30f9..93aaff1b 100644 --- a/crates/sandlock-core/src/pgroup.rs +++ b/crates/sandlock-core/src/pgroup.rs @@ -47,11 +47,32 @@ 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) } + /// Whether `pgid` was a group of this sandbox, members or not. + pub fn tracks(&self, pgid: i32) -> bool { + self.leaders.lock().map(|leaders| leaders.contains_key(&pgid)).unwrap_or(false) + } + + /// Sorted pgids that still have members. + pub fn live(&self) -> io::Result> { + let leaders = self.leaders.lock() + .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 +83,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 +109,23 @@ impl Default for ProcessGroups { } } -fn has_exited(pidfd: RawFd) -> bool { +/// Kernels before 6.9 refuse group signals by pidfd. While the key process +/// has not exited it holds the number, so no other group can have taken it. +/// After that only a probe finding no group at all is conclusive, since an +/// empty group can never regain members. +fn signal_group(pgid: i32, key: &OwnedFd, sig: i32) -> io::Result<()> { + let refused = match pidfd_signal_group(key, sig) { + Err(e) if e.raw_os_error() == Some(libc::EINVAL) => e, + result => return result, + }; + let reachable = !has_exited(key.as_raw_fd()); + if unsafe { libc::kill(-pgid, if reachable { sig } else { 0 }) } != 0 { + return Err(io::Error::last_os_error()); + } + if reachable { Ok(()) } else { Err(refused) } +} + +pub(crate) fn has_exited(pidfd: RawFd) -> bool { let mut pfd = libc::pollfd { fd: pidfd, events: libc::POLLIN, @@ -259,6 +296,69 @@ mod tests { reap(leader); } + /// Runs `f` in a child whose kernel appears to predate group pidfd signals. + fn on_old_kernel(f: impl FnOnce() -> bool) -> bool { + let pid = unsafe { libc::fork() }; + if pid == 0 { + let filter = [ + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 0 }, + libc::sock_filter { code: 0x15, jt: 0, jf: 3, k: libc::SYS_pidfd_send_signal as u32 }, + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 16 + 3 * 8 }, + libc::sock_filter { code: 0x45, jt: 0, jf: 1, k: 1 << 2 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ERRNO | libc::EINVAL as u32 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ALLOW }, + ]; + let prog = libc::sock_fprog { len: filter.len() as u16, filter: filter.as_ptr() as *mut _ }; + unsafe { + libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + libc::prctl(libc::PR_SET_SECCOMP, libc::SECCOMP_MODE_FILTER, &prog); + libc::_exit(if f() { 0 } else { 1 }); + } + } + let mut status = 0; + unsafe { libc::waitpid(pid, &mut status, 0) }; + libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0 + } + + #[test] + fn old_kernel_signals_a_group_whose_leader_lives() { + assert!(on_old_kernel(|| { + let (leader, member) = spawn_group(true); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + let reached = groups.live().unwrap() == [leader] && groups.signal(libc::SIGKILL).unwrap() == 1; + reap(leader); + wait_gone(member.unwrap()); + reached + })); + } + + #[test] + fn old_kernel_drops_an_empty_group_whose_leader_is_gone() { + assert!(on_old_kernel(|| { + let (leader, _) = spawn_group(false); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + unsafe { libc::kill(leader, libc::SIGKILL) }; + reap(leader); + groups.signal(libc::SIGKILL).unwrap() == 0 && groups.live().unwrap().is_empty() + })); + } + + #[test] + fn old_kernel_reports_a_populated_group_whose_leader_is_gone() { + assert!(on_old_kernel(|| { + let (leader, member) = spawn_group(true); + let groups = ProcessGroups::new(); + groups.track(leader, pidfd_open(leader as u32, 0).unwrap()); + unsafe { libc::kill(leader, libc::SIGKILL) }; + reap(leader); + let refused = groups.signal(libc::SIGKILL).map_err(|e| e.raw_os_error()) == Err(Some(libc::EINVAL)); + unsafe { libc::kill(member.unwrap(), libc::SIGKILL) }; + refused + })); + } + #[test] fn owns_rejects_an_unrecorded_group() { let groups = ProcessGroups::new(); 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..845d0d16 --- /dev/null +++ b/crates/sandlock-core/src/reaper.rs @@ -0,0 +1,370 @@ +//! Main's parent and the session's subreaper, outside the sandbox. + +use std::io; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; + +pub(crate) struct Reaper { + pub pid: i32, + pub pidfd: OwnedFd, + status: Option, + finish: Option, +} + +impl Reaper { + /// Carries main's raw wait status as four little-endian bytes, then EOF. + pub fn take_status(&mut self) -> Option { + self.status.take() + } + + /// Exit after reaping what has exited; jobs still alive are released. + /// A byte rather than EOF: children parked before exec may hold a copy. + pub fn finish(&mut self) { + if let Some(fd) = self.finish.take() { + unsafe { send(fd.as_raw_fd(), &[0]) }; + } + } + + #[cfg(test)] + pub fn exited(&self) -> bool { + crate::pgroup::has_exited(self.pidfd.as_raw_fd()) + } + + pub async fn join(&self) { + if let Ok(fd) = tokio::io::unix::AsyncFd::new(self.pidfd.as_raw_fd()) { + let _ = fd.readable().await; + } + reap(self.pid); + } + + pub fn join_blocking(&self) { + reap(self.pid); + } +} + +/// Group R is named after the reaper's pid: while the reaper lives no other +/// process can hold that number, so signalling R by number stays exact on +/// every kernel, even after main has been reaped. +pub(crate) fn fork() -> io::Result<(i32, Option)> { + let (status_r, status_w) = pipe()?; + let (finish_r, finish_w) = pipe()?; + let owner = unsafe { libc::getpid() }; + let owner_pgid = unsafe { libc::getpgrp() }; + let reaper = crate::control::fork_without_control_fds(); + if reaper < 0 { + return Err(io::Error::last_os_error()); + } + if reaper == 0 { + unsafe { + let mut all = std::mem::zeroed(); + let mut previous_mask = std::mem::zeroed(); + libc::sigfillset(&mut all); + libc::sigprocmask(libc::SIG_SETMASK, &all, &mut previous_mask); + let mut previous_chld = std::mem::zeroed(); + let mut default_chld: libc::sigaction = std::mem::zeroed(); + default_chld.sa_sigaction = libc::SIG_DFL; + libc::sigaction(libc::SIGCHLD, &default_chld, &mut previous_chld); + // A pidfd of the owning process, not PDEATHSIG: that fires when + // the forking thread exits and would end the session with it. + let owner_fd = libc::syscall(libc::SYS_pidfd_open, owner, 0) as i32; + let mut chld = std::mem::zeroed(); + libc::sigemptyset(&mut chld); + libc::sigaddset(&mut chld, libc::SIGCHLD); + let signal_fd = libc::signalfd(-1, &chld, libc::SFD_CLOEXEC | libc::SFD_NONBLOCK); + if owner_fd < 0 + || signal_fd < 0 + || libc::getppid() != owner + || libc::prctl(libc::PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) != 0 + || libc::setpgid(0, 0) != 0 + { + libc::_exit(127); + } + let me = libc::getpid(); + let main = libc::fork(); + if main < 0 { + libc::_exit(127); + } + if main == 0 { + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) != 0 || libc::getppid() != me { + libc::_exit(127); + } + libc::sigaction(libc::SIGCHLD, &previous_chld, std::ptr::null_mut()); + libc::sigprocmask(libc::SIG_SETMASK, &previous_mask, std::ptr::null_mut()); + libc::close(owner_fd); + libc::close(signal_fd); + drop((status_r, status_w, finish_r, finish_w)); + return Ok((0, None)); + } + // Out of R, so a sweep of R never takes the reaper with it. + if libc::setpgid(0, owner_pgid) != 0 { + libc::kill(main, libc::SIGKILL); + libc::_exit(127); + } + close_all_but([status_w.as_raw_fd(), finish_r.as_raw_fd(), signal_fd, owner_fd]); + if !send(status_w.as_raw_fd(), &main.to_le_bytes()) { + libc::kill(main, libc::SIGKILL); + } + supervise(main, me, status_w.as_raw_fd(), finish_r.as_raw_fd(), signal_fd, owner_fd); + } + } + drop(status_w); + drop(finish_r); + let main = loop { + match crate::context::read_u32_fd(status_r.as_raw_fd()) { + Err(e) if e.kind() == io::ErrorKind::Interrupted => continue, + result => break result, + } + }; + let pidfd = crate::sys::syscall::pidfd_open(reaper as u32, 0); + match (main, pidfd) { + (Ok(main), Ok(pidfd)) if main > 0 => Ok(( + main as i32, + Some(Reaper { pid: reaper, pidfd, status: Some(status_r), finish: Some(finish_w) }), + )), + (main, pidfd) => { + unsafe { libc::kill(reaper, libc::SIGKILL) }; + reap(reaper); + Err(main.err().or(pidfd.err()).unwrap_or_else(|| io::Error::other("session reaper failed to start"))) + } + } +} + +unsafe fn supervise(main: i32, group: i32, status_fd: i32, finish_fd: i32, signal_fd: i32, owner_fd: i32) -> ! { + let mut poll = [ + libc::pollfd { fd: finish_fd, events: libc::POLLIN, revents: 0 }, + libc::pollfd { fd: signal_fd, events: libc::POLLIN, revents: 0 }, + libc::pollfd { fd: owner_fd, events: libc::POLLIN, revents: 0 }, + ]; + loop { + loop { + // WNOWAIT: main's status is sent before its pid is freed, so no + // reader can see main alive once that number is reusable. + let mut info: libc::siginfo_t = std::mem::zeroed(); + if libc::waitid(libc::P_ALL, 0, &mut info, libc::WEXITED | libc::WNOHANG | libc::WNOWAIT | libc::__WALL) < 0 { + if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) { + continue; + } + // No child left means no session process left. + libc::_exit(0); + } + let pid = info.si_pid(); + if pid == 0 { + break; + } + if pid == main { + send(status_fd, &wait_status(&info).to_le_bytes()); + libc::close(status_fd); + } + while libc::waitpid(pid, std::ptr::null_mut(), libc::__WALL) < 0 + && io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) + {} + } + libc::poll(poll.as_mut_ptr(), 3, -1); + // Owner death also closes its end of the finish pipe, so it must be + // seen first; EOF alone likewise means nobody can finish or join. + if poll[2].revents != 0 || (poll[0].revents != 0 && !read_finish(finish_fd)) { + libc::kill(-group, libc::SIGKILL); + libc::_exit(0); + } + if poll[0].revents != 0 { + libc::_exit(0); + } + if poll[1].revents != 0 { + let mut info: libc::signalfd_siginfo = std::mem::zeroed(); + libc::read(signal_fd, (&mut info as *mut libc::signalfd_siginfo).cast(), std::mem::size_of_val(&info)); + } + } +} + +/// The raw wait status that `waitpid` would have returned for `info`. +unsafe fn wait_status(info: &libc::siginfo_t) -> i32 { + let status = info.si_status(); + match info.si_code { + libc::CLD_EXITED => (status & 0xff) << 8, + libc::CLD_DUMPED => status | 0x80, + _ => status, + } +} + +/// True for the finish byte, false for EOF. +unsafe fn read_finish(fd: i32) -> bool { + let mut byte = 0u8; + loop { + match libc::read(fd, (&mut byte as *mut u8).cast(), 1) { + 1 => return true, + -1 if io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) => {} + _ => return false, + } + } +} + +fn pipe() -> io::Result<(OwnedFd, OwnedFd)> { + let mut fds = [0; 2]; + if unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + return Err(io::Error::last_os_error()); + } + Ok(unsafe { (OwnedFd::from_raw_fd(fds[0]), OwnedFd::from_raw_fd(fds[1])) }) +} + +unsafe fn close_all_but(mut keep: [i32; 4]) { + keep.sort_unstable(); + let mut next = 0u32; + for kept in keep { + if kept as u32 > next { + libc::syscall(libc::SYS_close_range, next, kept as u32 - 1, 0u32); + } + next = kept as u32 + 1; + } + libc::syscall(libc::SYS_close_range, next, u32::MAX, 0u32); +} + +fn reap(pid: i32) { + while unsafe { libc::waitpid(pid, std::ptr::null_mut(), 0) } < 0 + && io::Error::last_os_error().raw_os_error() == Some(libc::EINTR) + {} +} + +unsafe fn send(fd: i32, bytes: &[u8]) -> bool { + loop { + if libc::write(fd, bytes.as_ptr().cast(), bytes.len()) == bytes.len() as isize { + return true; + } + if io::Error::last_os_error().raw_os_error() != Some(libc::EINTR) { + return false; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn read_status(fd: &OwnedFd) -> i32 { + let mut raw = [0u8; 4]; + assert_eq!(unsafe { libc::read(fd.as_raw_fd(), raw.as_mut_ptr().cast(), 4) }, 4); + i32::from_le_bytes(raw) + } + + #[tokio::test] + async fn relays_main_status_and_names_its_group() { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { libc::_exit(if libc::getpgrp() == libc::getppid() { 42 } else { 1 }) } + } + let mut reaper = reaper.unwrap(); + let status = read_status(&reaper.take_status().unwrap()); + assert_eq!(libc::WEXITSTATUS(status), 42); + assert_ne!(unsafe { libc::getpgid(reaper.pid) }, reaper.pid, "reaper left group R"); + reaper.join().await; + } + + #[tokio::test] + async fn outlives_main_until_orphans_are_reaped() { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + if libc::fork() == 0 { + libc::usleep(300_000); + libc::_exit(0); + } + libc::_exit(7); + } + } + let mut reaper = reaper.unwrap(); + assert_eq!(libc::WEXITSTATUS(read_status(&reaper.take_status().unwrap())), 7); + assert!(!reaper.exited()); + reaper.join().await; + } + + #[tokio::test] + async fn outlives_the_thread_that_created_it() { + let reaper = std::thread::spawn(|| { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + libc::pause(); + libc::_exit(0); + } + } + reaper + }) + .join() + .unwrap() + .unwrap(); + std::thread::sleep(std::time::Duration::from_millis(50)); + assert!(!reaper.exited()); + unsafe { libc::kill(-reaper.pid, libc::SIGKILL) }; + reaper.join().await; + } + + #[tokio::test] + async fn finish_releases_jobs_still_running() { + let mut pipe = [0; 2]; + assert_eq!(unsafe { libc::pipe(pipe.as_mut_ptr()) }, 0); + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + let job = libc::fork(); + if job == 0 { + libc::setsid(); + libc::pause(); + libc::_exit(0); + } + libc::write(pipe[1], (&job as *const i32).cast(), 4); + libc::_exit(0); + } + } + let mut job = 0i32; + assert_eq!(unsafe { libc::read(pipe[0], (&mut job as *mut i32).cast(), 4) }, 4); + unsafe { + libc::close(pipe[0]); + libc::close(pipe[1]); + } + let mut reaper = reaper.unwrap(); + read_status(&reaper.take_status().unwrap()); + reaper.finish(); + reaper.join().await; + assert_eq!(unsafe { libc::kill(job, libc::SIGKILL) }, 0); + } + + /// The owner is a re-executed copy of this test, so its death can be + /// observed here without running Rust code in a forked child. + #[test] + fn owner_death_kills_group_r() { + const OWNER: &str = "SANDLOCK_TEST_REAPER_OWNER"; + if std::env::var_os(OWNER).is_some() { + let (main, reaper) = fork().unwrap(); + if main == 0 { + unsafe { + if libc::fork() == 0 { + libc::pause(); + } + libc::pause(); + libc::_exit(0); + } + } + use std::io::Write; + let mut out = std::io::stdout(); + let _ = writeln!(out, "group={}", reaper.unwrap().pid); + let _ = out.flush(); + unsafe { libc::_exit(0) } + } + let mut owner = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["reaper::tests::owner_death_kills_group_r", "--exact", "--nocapture"]) + .env(OWNER, "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap(); + // Read only up to the report: concurrent forks in this test binary + // may hold the pipe's write end, so EOF can come late. + let group: i32 = std::io::BufRead::lines(std::io::BufReader::new(owner.stdout.take().unwrap())) + .find_map(|l| l.ok()?.strip_prefix("group=")?.parse().ok()) + .expect("owner reported group R"); + owner.wait().unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while unsafe { libc::kill(-group, 0) } == 0 { + assert!(std::time::Instant::now() < deadline, "group R outlived its owner"); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } +} diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 840d6e22..a20fa956 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -288,7 +288,13 @@ struct Runtime { state: RuntimeState, child_pid: Option, pidfd: Option, + reaper: Option, + main_exit: Arc, groups: Arc, + /// Cancelled by a kill, so a held policy_fn verdict cannot stall it. + stop: tokio_util::sync::CancellationToken, + /// Ends the drains once released jobs might hold the write ends forever. + drain_cut: tokio_util::sync::CancellationToken, notif_handle: Option>, policy_fn_worker: Option, throttle_handle: Option>, @@ -322,6 +328,8 @@ struct Runtime { on_bind: Option) + Send + Sync>>, handlers: Vec<(i64, Arc)>, ready_w: Option, + /// The child's exec pipe: EOF after `EXEC_REACHED` alone means it exec'd. + exec_r: Option, /// Set when this sandbox is a stage of a [`Transaction`](crate::transaction::Transaction) /// that shares one COW upper across all stages. When present, `do_create_stdio` /// reuses this `CowState` (instead of building its own branch), and neither @@ -356,6 +364,7 @@ enum RuntimeState { Created, Running, Paused, + MainExited(crate::result::ExitStatus), Stopped(crate::result::ExitStatus), } @@ -855,15 +864,17 @@ impl Sandbox { .or_else(|| self.name.as_deref()) } - /// Return the child PID if spawned. + /// Main's pid until it exits; afterwards the number may name another process. pub fn pid(&self) -> Option { - self.runtime.as_ref().and_then(|r| r.child_pid) + self.runtime.as_ref() + .filter(|rt| rt.main_exit.get().is_none()) + .and_then(|rt| rt.child_pid) } /// Return whether the child is currently running or paused. pub fn is_running(&self) -> bool { self.runtime.as_ref().map(|r| { - matches!(r.state, RuntimeState::Running | RuntimeState::Paused) + matches!(r.state, RuntimeState::Running | RuntimeState::Paused | RuntimeState::MainExited(_)) }).unwrap_or(false) } @@ -871,41 +882,44 @@ impl Sandbox { /// members left. Groups stay reachable after the main process has been /// reaped, so this can sweep what a `wait()` left behind. pub fn signal(&self, sig: i32) -> Result { - use crate::error::SandboxRuntimeError; - let rt = self.runtime.as_ref() - .filter(|rt| rt.child_pid.is_some()) - .ok_or(SandboxRuntimeError::NotRunning)?; - if sig == libc::SIGKILL { - if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } - } - Ok(rt.groups.signal(sig).map_err(SandboxRuntimeError::Io)?) + self.controller()?.signal(sig) } - fn signal_live_groups(&self, sig: i32) -> Result<(), crate::error::SandlockError> { - if self.signal(sig)? == 0 { - let gone = std::io::Error::from_raw_os_error(libc::ESRCH); - return Err(crate::error::SandboxRuntimeError::Io(gone).into()); - } - Ok(()) + /// A handle that acts on this session from another task or thread, + /// including while `wait()` holds the `Sandbox`. + pub fn controller(&self) -> Result { + let rt = self.runtime.as_ref() + .filter(|rt| rt.child_pid.is_some()) + .ok_or(crate::error::SandboxRuntimeError::NotRunning)?; + Ok(crate::Controller { + groups: Arc::clone(&rt.groups), + stop: rt.stop.clone(), + main_exit: Arc::clone(&rt.main_exit), + pid: rt.child_pid.unwrap_or(0), + }) } /// Send SIGSTOP to the sandbox's process groups. pub fn pause(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal_live_groups(libc::SIGSTOP)?; - self.rt_mut().state = RuntimeState::Paused; + self.controller()?.pause()?; + if !matches!(self.rt().state, RuntimeState::MainExited(_)) { + self.rt_mut().state = RuntimeState::Paused; + } Ok(()) } /// Send SIGCONT to the sandbox's process groups. pub fn resume(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal_live_groups(libc::SIGCONT)?; - self.rt_mut().state = RuntimeState::Running; + self.controller()?.resume()?; + if !matches!(self.rt().state, RuntimeState::MainExited(_)) { + self.rt_mut().state = RuntimeState::Running; + } Ok(()) } /// Send SIGKILL to the sandbox's process groups. pub fn kill(&mut self) -> Result<(), crate::error::SandlockError> { - self.signal(libc::SIGKILL).map(|_| ()) + self.controller()?.kill() } /// Set a callback invoked whenever a port bind is recorded. @@ -938,35 +952,18 @@ impl Sandbox { } } - /// Wait for the child process to exit. + /// Wait for the main process to exit; background jobs stay supervised + /// until [`Self::shutdown`]. Returns the same status on repeated calls. /// - /// Dropping the returned future does not cost the captured output: the pipe - /// drains belong to the `Sandbox`, so a caller that cancels this `wait()` - /// (a `timeout` or `select!` around it) and calls `wait()` again still gets - /// what the child wrote. That holds wherever the cancellation lands — - /// before the drains start, while the child is still running, or while the - /// final join is blocked because a descendant is still holding the write - /// end open past the child's own exit. - pub async fn wait(&mut self) -> Result { + /// Dropping the returned future costs nothing: the pipe drains belong to + /// the `Sandbox`, so a cancelled `wait()` can be followed by another + /// `wait()` or by `shutdown()`, which still collects the output. + pub async fn wait(&mut self) -> Result { use crate::error::SandboxRuntimeError; - use crate::result::RunResult; let pid = self.rt().child_pid.ok_or(SandboxRuntimeError::NotRunning)?; - - // Already reaped: hand back the same status again, plus whatever the - // drains collected. They outlive the `wait()` that started them, so a - // second call still reports the output of a first one that was - // cancelled — including one cancelled while joining them here, since a - // join only unparks a drain once it has completed. A `wait()` that ran - // to the end took them with it, and both streams are then `None`. - let stopped = match self.rt().state { - RuntimeState::Stopped(ref es) => Some(es.clone()), - _ => None, - }; - if let Some(exit_status) = stopped { - let changes = self.settle_branch().await; - let (stdout, stderr) = self.collect_pipe_drains().await; - return Ok(RunResult { exit_status, stdout, stderr, changes }); + if let RuntimeState::MainExited(ref s) | RuntimeState::Stopped(ref s) = self.rt().state { + return Ok(s.clone()); } // Deliver EOF to a piped stdin the caller never took: otherwise a child @@ -993,12 +990,12 @@ impl Sandbox { // `Process::take_stdout`/`take_stderr` is `None` here and stays theirs. if self.rt().stdout_drain.is_none() { if let Some(fd) = self.rt_mut()._stdout_read.take() { - self.rt_mut().stdout_drain = sandbox_spawn_pipe_drain(fd); + self.rt_mut().stdout_drain = sandbox_spawn_pipe_drain(fd, self.rt().drain_cut.clone()); } } if self.rt().stderr_drain.is_none() { if let Some(fd) = self.rt_mut()._stderr_read.take() { - self.rt_mut().stderr_drain = sandbox_spawn_pipe_drain(fd); + self.rt_mut().stderr_drain = sandbox_spawn_pipe_drain(fd, self.rt().drain_cut.clone()); } } @@ -1010,37 +1007,67 @@ impl Sandbox { // race the worker for fork events and hang it). Mirrors // `spawn_pid_watcher`. Falls back to a blocking `waitpid` only when no // pidfd is available (kernel without `pidfd_open`). - let exit_status = match self.rt_mut().pidfd.take() { - Some(pidfd) => wait_child_exit_via_pidfd(pidfd, pid).await, - None => wait_child_exit_blocking(pid).await, + let exit_status = if self.rt().reaper.is_some() { + let main_exit = Arc::clone(&self.rt().main_exit); + main_exit.wait().await.map_err(SandboxRuntimeError::Io)? + } else { + match self.rt_mut().pidfd.take() { + Some(pidfd) => wait_child_exit_via_pidfd(pidfd, pid).await, + 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); - - self.rt_mut().state = RuntimeState::Stopped(exit_status.clone()); + self.rt_mut().state = RuntimeState::MainExited(exit_status.clone()); + self.rt().main_exit.set(exit_status.clone()); if self.rt().tty_foreground_taken { - sandbox_restore_tty_foreground(pid); - self.rt_mut().tty_foreground_taken = false; + sandbox_restore_tty_foreground(&self.rt().groups); } + Ok(exit_status) + } + + /// Kill every remaining job, wait until the session has no process left, + /// then settle COW and collect the captured output. Safe to call again. + pub async fn shutdown(&mut self) -> Result { + use crate::error::SandboxRuntimeError; + use crate::result::RunResult; + + self.rt().child_pid.ok_or(SandboxRuntimeError::NotRunning)?; + if !matches!(self.rt().state, RuntimeState::Stopped(_)) { + let swept = self.controller()?.kill(); + let exit_status = self.wait().await?; + let no_supervisor = self.no_supervisor; + if let Some(mut reaper) = self.rt_mut().reaper.take() { + // Untracked jobs (no supervisor, or a group out of reach) + // would keep it alive; release them instead. + if no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join().await; + self.rt().drain_cut.cancel(); + } + // A job that outlived main may have taken the terminal back. + if self.rt().tty_foreground_taken { + sandbox_restore_tty_foreground(&self.rt().groups); + self.rt_mut().tty_foreground_taken = false; + } + self.rt_mut().state = RuntimeState::Stopped(exit_status); - let rt = self.rt_mut(); - rt.control_claim = None; - if let Some(h) = rt.notif_handle.take() { h.abort(); } - rt.policy_fn_worker = None; - if let Some(h) = rt.throttle_handle.take() { h.abort(); } - if let Some(h) = rt.loadavg_handle.take() { h.abort(); } - // Finish shutting down the old endpoint before wait() returns. - if let Some(h) = rt.control_handle.take() { - h.abort(); - let _ = h.await; + let rt = self.rt_mut(); + rt.control_claim = None; + if let Some(h) = rt.notif_handle.take() { h.abort(); } + rt.policy_fn_worker = None; + if let Some(h) = rt.throttle_handle.take() { h.abort(); } + if let Some(h) = rt.loadavg_handle.take() { h.abort(); } + // Finish shutting down the old endpoint before releasing the name. + if let Some(h) = rt.control_handle.take() { + h.abort(); + let _ = h.await; + } + swept?; } + let RuntimeState::Stopped(ref exit_status) = self.rt().state else { unreachable!() }; + let exit_status = exit_status.clone(); let changes = self.settle_branch().await; let (stdout, stderr) = self.collect_pipe_drains().await; - Ok(RunResult { exit_status, stdout, stderr, changes }) } @@ -1083,7 +1110,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() } } @@ -1343,43 +1370,34 @@ impl Sandbox { &self.restore_skipped } - /// Wait for the child to finish `execve`. Detected by `/proc//exe` - /// no longer matching `/proc/self/exe` (before execve the child still - /// shares the supervisor's binary). The kernel offers no direct event - /// for execve completion, so this polls every 1ms. Policy approvals add - /// their configured timeout to the usual five-second startup allowance; - /// unlimited approvals also disable the startup deadline. - async fn wait_until_exec(&self) -> Result<(), crate::error::SandlockError> { + /// Wait for the child to finish `execve`, which closes its exec pipe. + /// Policy approvals add their configured timeout to the usual five-second + /// startup allowance; unlimited approvals also disable the startup deadline. + async fn wait_until_exec(&mut self) -> Result<(), crate::error::SandlockError> { use crate::error::SandboxRuntimeError; - let pid = self.pid().ok_or(SandboxRuntimeError::NotRunning)?; - let Some(our_exe) = std::fs::read_link("/proc/self/exe").ok() else { - return Ok(()); - }; - let child_link = format!("/proc/{}/exe", pid); + use tokio::io::AsyncReadExt; + let exec_r = self.rt_mut().exec_r.take().ok_or(SandboxRuntimeError::NotRunning)?; + let mut exec_r = tokio::net::unix::pipe::Receiver::from_owned_fd(exec_r) + .map_err(SandboxRuntimeError::Io)?; let startup_allowance = Duration::from_secs(5); let timeout = if let Some(policy_fn) = &self.policy_fn { policy_fn.verdict_timeout.map(|timeout| timeout.saturating_add(startup_allowance)) } else { Some(startup_allowance) }; - let deadline = timeout.and_then(|timeout| std::time::Instant::now().checked_add(timeout)); - loop { - match std::fs::read_link(&child_link) { - Ok(child_exe) if child_exe != our_exe => return Ok(()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(SandboxRuntimeError::Child( - "child exited before exec() completed".into(), - ).into()); - } - _ => {} - } - if deadline.is_some_and(|deadline| std::time::Instant::now() >= deadline) { - return Err(SandboxRuntimeError::Child( - "child did not exec() within the startup timeout".into(), - ).into()); - } - tokio::time::sleep(std::time::Duration::from_millis(1)).await; + let mut marks = Vec::new(); + let read = exec_r.read_to_end(&mut marks); + match timeout { + Some(timeout) => tokio::time::timeout(timeout, read).await.map_err(|_| { + SandboxRuntimeError::Child("child did not exec() within the startup timeout".into()) + })?, + None => read.await, + } + .map_err(SandboxRuntimeError::Io)?; + if marks != [context::EXEC_REACHED] { + return Err(SandboxRuntimeError::Child("child exited before exec() completed".into()).into()); } + Ok(()) } /// Create with explicit stdin/stdout/stderr fd redirection. Child is @@ -1466,9 +1484,7 @@ impl Sandbox { #[cfg(feature = "checkpoint")] pub async fn checkpoint(&self) -> Result { use crate::error::{SandboxRuntimeError, SandlockError}; - let pid = self.runtime.as_ref() - .and_then(|rt| rt.child_pid) - .ok_or(SandlockError::Runtime(SandboxRuntimeError::NotRunning))?; + let pid = self.pid().ok_or(SandlockError::Runtime(SandboxRuntimeError::NotRunning))?; self.checkpoint_pid(pid).await } @@ -1515,7 +1531,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 +1542,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 +1562,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 +1582,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. @@ -1688,6 +1708,10 @@ impl Sandbox { }), child_pid: Some(clone_pid), pidfd: None, + reaper: None, + main_exit: crate::controller::MainExit::new(None), + stop: tokio_util::sync::CancellationToken::new(), + drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -1712,6 +1736,7 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + exec_r: None, #[cfg(feature = "cow")] shared_cow: None, tty_foreground_taken: false, @@ -1766,7 +1791,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 @@ -1793,6 +1819,10 @@ impl Sandbox { state: RuntimeState::Created, child_pid: None, pidfd: None, + reaper: None, + main_exit: crate::controller::MainExit::new(None), + stop: tokio_util::sync::CancellationToken::new(), + drain_cut: tokio_util::sync::CancellationToken::new(), groups: Arc::new(crate::pgroup::ProcessGroups::new()), notif_handle: None, policy_fn_worker: None, @@ -1819,6 +1849,7 @@ impl Sandbox { on_bind: None, handlers: Vec::new(), ready_w: None, + exec_r: None, #[cfg(feature = "cow")] shared_cow: None, tty_foreground_taken: false, @@ -2050,10 +2081,6 @@ impl Sandbox { None }; - // Capture our PID before fork so the child can detect parent death - // without assuming PID 1 is always init (wrong in containers). - let parent_pid = unsafe { libc::getpid() }; - // Interactive (fully inherited) stdio on a terminal: the child will // take the tty foreground group, and this process must take it back // once the child is reaped. @@ -2085,20 +2112,11 @@ impl Sandbox { }; let child_publisher = control_socket.as_ref().and_then(|s| s.claim.as_ref()).map(|c| c.child_publisher()); - let pid = crate::control::fork_without_control_fds(); - if pid < 0 { - return Err(SandboxRuntimeError::Fork(std::io::Error::last_os_error()).into()); - } + let (pid, reaper) = crate::reaper::fork().map_err(SandboxRuntimeError::Fork)?; if pid == 0 { // ===== CHILD PROCESS ===== - // Publish only after the process group exists, before confinement denies IPC. - if unsafe { libc::setpgid(0, 0) } != 0 { - use std::io::Write; - let err = std::io::Error::last_os_error(); - let _ = writeln!(std::io::stderr(), "sandlock child: setpgid: {err}"); - unsafe { libc::_exit(127) }; - } + // Publish before confinement denies IPC. if let Some(publisher) = child_publisher { if !publisher.publish() { unsafe { libc::_exit(127); } } } @@ -2172,7 +2190,6 @@ impl Sandbox { keep_fds: &gather_keep_fds, sandbox_name: Some(sandbox_name.as_str()), extra_syscalls: &extra_syscalls, - parent_pid, foreground, }); } @@ -2190,35 +2207,40 @@ impl Sandbox { // State remains `Created` until `do_start` writes ready_w to release // the child to execve. - let pidfd = match syscall::pidfd_open(pid as u32, 0) { - Ok(fd) => Some(fd), - Err(_) => None, - }; - // The child's first act is setpgid(0, 0), so its pid names its group. - if let Some(leader) = pidfd.as_ref().and_then(|pfd| pfd.try_clone().ok()) { - self.rt_mut().groups.track(pid, leader); - } + let mut reaper = reaper.expect("parent has a reaper"); + let status = reaper.take_status().expect("fresh reaper"); + let key = reaper.pidfd.try_clone().map_err(SandboxRuntimeError::Io)?; + self.rt_mut().groups.track(reaper.pid, key); + self.rt_mut().main_exit = crate::controller::MainExit::new(Some(status)); + self.rt_mut().reaper = Some(reaper); + let pidfd = syscall::pidfd_open(pid as u32, 0).ok(); let notif_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) .map_err(|e| SandboxRuntimeError::Child(format!("read notif fd from child: {}", e)))?; + let exec_fd_num = read_u32_fd(pipes.notif_r.as_raw_fd()) + .map_err(|e| SandboxRuntimeError::Child(format!("read exec pipe fd from child: {}", e)))?; + let take_child_fd = |num: u32, flags: i32| -> Result { + if let Some(ref pfd) = pidfd { + return syscall::pidfd_getfd(pfd, num as i32, 0) + .map_err(|e| SandboxRuntimeError::Child(format!("pidfd_getfd: {}", e))); + } + let cpath = CString::new(format!("/proc/{}/fd/{}", pid, num)).unwrap(); + let raw = unsafe { libc::open(cpath.as_ptr(), flags | libc::O_CLOEXEC) }; + if raw < 0 { + return Err(SandboxRuntimeError::Child(format!("failed to open child fd {} from /proc", num))); + } + Ok(unsafe { OwnedFd::from_raw_fd(raw) }) + }; + let exec_r = take_child_fd(exec_fd_num, libc::O_RDONLY)?; - let control_info = crate::control::SandboxInfo { mode: self.mode.clone() }; + let control_info = crate::control::SandboxInfo { mode: self.mode.clone(), main_exited: false, live_groups: 0 }; let is_nested_mode = notif_fd_num == 0; let notif_fd = if is_nested_mode { None - } else if let Some(ref pfd) = pidfd { - Some(syscall::pidfd_getfd(pfd, notif_fd_num as i32, 0) - .map_err(|e| SandboxRuntimeError::Child(format!("pidfd_getfd: {}", e)))?) } else { - let path = format!("/proc/{}/fd/{}", pid, notif_fd_num); - let cpath = CString::new(path).unwrap(); - let raw = unsafe { libc::open(cpath.as_ptr(), libc::O_RDWR) }; - if raw < 0 { - return Err(SandboxRuntimeError::Child("failed to open notif fd from /proc".into()).into()); - } - Some(unsafe { OwnedFd::from_raw_fd(raw) }) + Some(take_child_fd(notif_fd_num, libc::O_RDWR)?) }; if let Some(notif_fd) = notif_fd { @@ -2373,6 +2395,8 @@ impl Sandbox { policy_fn_state.verdict_timeout = policy_fn.verdict_timeout; policy_fn_state.cancellation = worker.cancellation(); policy_fn_state.event_tx = Some(worker.sender()); + let (stop, worker_cancel) = (self.rt().stop.clone(), worker.cancellation()); + tokio::spawn(async move { stop.cancelled().await; worker_cancel.cancel() }); self.rt_mut().policy_fn_worker = Some(worker); } @@ -2454,6 +2478,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, Some(control_ctx), + self.controller()?, sandbox_snapshot, control_info.clone(), )); @@ -2479,6 +2504,7 @@ impl Sandbox { self.rt_mut().control_handle = Some(crate::control::spawn_control_loop( sockets, None, + self.controller()?, self.clone(), control_info, )); @@ -2493,6 +2519,7 @@ impl Sandbox { self.rt_mut().pidfd = pidfd; self.rt_mut().ready_w = Some(pipes.ready_w); + self.rt_mut().exec_r = Some(exec_r); Ok(()) } @@ -2531,12 +2558,12 @@ impl Sandbox { /// stream opened with [`StdioMode::Piped`] (each available once); the caller /// reads/writes those while the process runs. Unlike `std::process::Child`, /// this *borrows* the originating [`Sandbox`] rather than owning the process: -/// the process is killed and reaped when that `Sandbox` is dropped, or eagerly -/// via [`Process::kill`]. +/// the session is killed and reaped when that `Sandbox` is dropped, or eagerly +/// via [`Process::shutdown`]. /// -/// A `Process` that is dropped without [`Process::wait`] leaves the child -/// running until the `Sandbox` is dropped — call `wait` (or `kill`) to end it. -#[must_use = "a Process is a live confined child; call wait() (or kill()) or it runs until the Sandbox is dropped"] +/// A `Process` dropped without [`Process::shutdown`] leaves the session +/// running until the `Sandbox` is dropped. +#[must_use = "a Process is a live confined session; call shutdown() or it runs until the Sandbox is dropped"] pub struct Process<'a> { sandbox: &'a mut Sandbox, } @@ -2566,8 +2593,7 @@ impl Process<'_> { self.sandbox.rt_mut()._stderr_read.take() } - /// The child PID, or `None` if not spawned. Remains `Some` after the child - /// exits (until the `Sandbox` is dropped). + /// Main's pid until it exits. pub fn pid(&self) -> Option { self.sandbox.pid() } @@ -2579,25 +2605,33 @@ impl Process<'_> { self.sandbox.kill() } - /// Wait for the child to exit. Any `Piped` stdout/stderr the caller did not - /// take is drained into the returned `RunResult`; taken streams are `None` - /// because the caller owns them. An untaken piped stdin is closed here so the - /// child sees EOF; a *taken* stdin the caller must close itself first (see - /// [`Process::take_stdin`]) or this blocks forever. - pub async fn wait(self) -> Result { + /// Wait for the main process; background jobs stay supervised. An untaken + /// piped stdin is closed here so the child sees EOF; a *taken* stdin the + /// caller must close itself first (see [`Process::take_stdin`]). + pub async fn wait(&mut self) -> Result { self.sandbox.wait().await } + + /// End the session. Any `Piped` stdout/stderr the caller did not take is + /// drained into the returned `RunResult`; taken streams are `None`. + pub async fn shutdown(self) -> Result { + self.sandbox.shutdown().await + } + + pub fn controller(&self) -> Result { + self.sandbox.controller() + } } /// Hand the terminal's foreground process group back to this process after -/// reaping an interactive child that took it. Restores only while the child's -/// group still owns the terminal, so a foreground the caller has since given -/// to someone else is left alone. SIGTTOU is blocked around `tcsetpgrp`: +/// reaping an interactive child that took it. Restores only while one of the +/// sandbox's groups holds the terminal, so a foreground the caller has since +/// given to someone else is left alone. SIGTTOU is blocked around `tcsetpgrp`: /// this process is a background group at that moment, and an unblocked /// SIGTTOU would stop it, which is the exact symptom being prevented. -fn sandbox_restore_tty_foreground(child_pid: i32) { +fn sandbox_restore_tty_foreground(groups: &crate::pgroup::ProcessGroups) { unsafe { - if libc::isatty(0) != 1 || libc::tcgetpgrp(0) != child_pid { + if libc::isatty(0) != 1 || !groups.tracks(libc::tcgetpgrp(0)) { return; } let mut block: libc::sigset_t = std::mem::zeroed(); @@ -2616,19 +2650,23 @@ fn sandbox_restore_tty_foreground(child_pid: i32) { impl Drop for Sandbox { fn drop(&mut self) { + let no_supervisor = self.no_supervisor; if let Some(ref mut rt) = self.runtime { if let Some(worker) = &rt.policy_fn_worker { worker.cancel(); } if let Some(pid) = rt.child_pid { - if matches!(rt.state, RuntimeState::Created | RuntimeState::Running | RuntimeState::Paused) { - let _ = rt.groups.signal(libc::SIGKILL); - // The waitpid below must not hang on a child whose group - // was never recorded. Unreaped, its pid is still ours. - unsafe { libc::kill(pid, libc::SIGKILL) }; - let mut status: i32 = 0; - unsafe { libc::waitpid(pid, &mut status, 0) }; + if matches!(rt.state, RuntimeState::Created | RuntimeState::Running | RuntimeState::Paused | RuntimeState::MainExited(_)) { + let swept = rt.groups.signal(libc::SIGKILL); + if let Some(mut reaper) = rt.reaper.take() { + if no_supervisor || swept.is_err() { reaper.finish(); } + reaper.join_blocking(); + } else { + // Unreaped, a clone's pid is still ours. + unsafe { libc::kill(pid, libc::SIGKILL) }; + unsafe { libc::waitpid(pid, std::ptr::null_mut(), 0) }; + } } if rt.tty_foreground_taken { - sandbox_restore_tty_foreground(pid); + sandbox_restore_tty_foreground(&rt.groups); rt.tty_foreground_taken = false; } } @@ -2863,7 +2901,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 +2912,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 }))) } @@ -2925,7 +2976,7 @@ fn sandbox_read_fd_to_end(fd: std::os::fd::OwnedFd) -> Vec { buf } -fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { +pub(crate) fn sandbox_wait_status_to_exit(status: i32) -> crate::result::ExitStatus { use crate::result::ExitStatus; if libc::WIFEXITED(status) { ExitStatus::Code(libc::WEXITSTATUS(status)) diff --git a/crates/sandlock-core/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_checkpoint.rs b/crates/sandlock-core/tests/integration/test_checkpoint.rs index 43237b91..9bfbdbd7 100644 --- a/crates/sandlock-core/tests/integration/test_checkpoint.rs +++ b/crates/sandlock-core/tests/integration/test_checkpoint.rs @@ -152,6 +152,21 @@ async fn test_checkpoint_process_info() { let _ = sb.wait().await; } +#[tokio::test] +async fn test_checkpoint_refused_after_main_exits() { + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc") + .build().unwrap(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + assert!(matches!( + sb.checkpoint().await, + Err(sandlock_core::SandlockError::Runtime(sandlock_core::error::SandboxRuntimeError::NotRunning)) + )); + sb.shutdown().await.unwrap(); +} + /// Test loading from a nonexistent directory fails. #[tokio::test] async fn test_checkpoint_load_nonexistent() { diff --git a/crates/sandlock-core/tests/integration/test_control.rs b/crates/sandlock-core/tests/integration/test_control.rs index a0eb34bf..481c24e2 100644 --- a/crates/sandlock-core/tests/integration/test_control.rs +++ b/crates/sandlock-core/tests/integration/test_control.rs @@ -419,7 +419,7 @@ fn test_control_no_supervisor() { let pids = sandlock_core::control::sandbox_pids(&name).expect("pids"); assert_eq!(pids.supervisor, child.id() as i32, "supervisor pid: {:?}", pids); - assert_eq!(unsafe { libc::getpgid(pids.child) }, pids.child, "child leads its group: {:?}", pids); + assert_ne!(unsafe { libc::getpgid(pids.child) }, pids.child, "main runs in the reaper's group: {:?}", pids); let info = sandlock_core::control::sandbox_info(&name).expect("info"); assert_eq!(info.mode, None); @@ -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,78 @@ async fn test_control_cancelled_creation_releases_its_name() { assert!(!sandlock_core::control::list_sandboxes().unwrap().contains(&name)); drop(cancelled); } + +/// A session can outlive its main process, whose pid is then free for +/// reuse; ps must stop reporting it instead of reading a recycled /proc entry. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_ps_hides_the_pid_of_an_exited_main() { + let name = format!("test-ctrl-main-exited-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/proc") + .fs_read("/dev") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + assert!(sandlock_core::control::sandbox_info(&name).expect("info").main_exited); + let out = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + let stdout = String::from_utf8_lossy(&out.stdout); + let row = stdout.lines().find(|l| l.starts_with(&name)).expect("ps lists the session"); + assert_eq!(row.split_whitespace().nth(1), Some("-"), "exited main has no pid: {row}"); + assert!(row.ends_with("(main exited, 1 group)"), "{row}"); + sb.shutdown().await.unwrap(); +} + +/// A session whose main exited alone stays listed until shutdown(); ps must +/// tell it apart from one with jobs still running. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_ps_reports_an_idle_session() { + let name = format!("test-ctrl-idle-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["true"]).await.unwrap(); + sb.wait().await.unwrap(); + let out = sandlock_bin().args(["ps"]).output().expect("sandlock ps"); + let stdout = String::from_utf8_lossy(&out.stdout); + let row = stdout.lines().find(|l| l.starts_with(&name)).expect("ps lists the session"); + assert!(row.ends_with("(main exited, idle)"), "{row}"); + sb.shutdown().await.unwrap(); +} + +/// A host in shutdown drops its control socket mid-request; kill must not +/// take the host down for it. +#[tokio::test(flavor = "multi_thread")] +async fn test_control_kill_does_not_kill_a_host_in_teardown() { + let name = format!("test-ctrl-teardown-{}", std::process::id()); + let mut sb = sandlock_core::Sandbox::builder() + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .build() + .unwrap() + .with_name(&name); + sb.spawn(&["sleep", "60"]).await.unwrap(); + let killer = std::thread::spawn({ + let name = name.clone(); + move || { + for _ in 0..50 { + let _ = sandlock_core::control::kill_sandbox(&name); + } + } + }); + sb.shutdown().await.unwrap(); + killer.join().unwrap(); + // Reaching this line means the host survived every kill attempt. +} diff --git a/crates/sandlock-core/tests/integration/test_fork.rs b/crates/sandlock-core/tests/integration/test_fork.rs index 363f9119..55fd2dac 100644 --- a/crates/sandlock-core/tests/integration/test_fork.rs +++ b/crates/sandlock-core/tests/integration/test_fork.rs @@ -29,7 +29,7 @@ async fn test_fork_basic() { // Each clone is a live Sandbox — wait for all for c in clones.iter_mut() { - let r = c.wait().await.unwrap(); + let r = async { c.wait().await?; c.shutdown().await }.await.unwrap(); assert!(r.success()); } @@ -61,7 +61,7 @@ async fn test_fork_cow_sharing() { }); let mut clones = sb.fork(3).await.unwrap(); - for c in clones.iter_mut() { let _ = c.wait().await; } + for c in clones.iter_mut() { let _ = async { c.wait().await?; c.shutdown().await }.await; } for i in 0..3u32 { let content = std::fs::read_to_string(out_dir.join(format!("{}", i))).unwrap_or_default(); @@ -87,7 +87,7 @@ async fn test_fork_clone_id_env() { }); let mut clones = sb.fork(3).await.unwrap(); - for c in clones.iter_mut() { let _ = c.wait().await; } + for c in clones.iter_mut() { let _ = async { c.wait().await?; c.shutdown().await }.await; } for i in 0..3u32 { let content = std::fs::read_to_string(out_dir.join(format!("{}", i))).unwrap_or_default(); @@ -142,7 +142,7 @@ async fn test_fork_clone_exit_status() { // All clones should have succeeded (already finished) for (i, c) in clones.iter_mut().enumerate() { - let r = c.wait().await.unwrap(); + let r = async { c.wait().await?; c.shutdown().await }.await.unwrap(); assert!(r.success(), "clone {} should succeed", i); } diff --git a/crates/sandlock-core/tests/integration/test_policy_fn.rs b/crates/sandlock-core/tests/integration/test_policy_fn.rs index b613f505..0ff2de66 100644 --- a/crates/sandlock-core/tests/integration/test_policy_fn.rs +++ b/crates/sandlock-core/tests/integration/test_policy_fn.rs @@ -881,7 +881,7 @@ async fn unlimited_approval_survives_old_five_second_limit() { .unwrap(); let mut sandbox = policy.clone(); sandbox.spawn(&["/bin/sleep", "0.1"]).await.unwrap(); - let result = sandbox.wait().await.unwrap(); + let result = async { sandbox.wait().await?; sandbox.shutdown().await }.await.unwrap(); assert!( result.success(), "unlimited approval must accept a late verdict" diff --git a/crates/sandlock-core/tests/integration/test_popen.rs b/crates/sandlock-core/tests/integration/test_popen.rs index ace53d22..e82729a9 100644 --- a/crates/sandlock-core/tests/integration/test_popen.rs +++ b/crates/sandlock-core/tests/integration/test_popen.rs @@ -44,7 +44,7 @@ async fn test_popen_pipe_roundtrip() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "hello\n"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); } @@ -62,7 +62,7 @@ async fn test_popen_stdout_only() { let mut out = String::new(); stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "hi\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Null stdout is actually wired to `/dev/null`, not left inherited. The child @@ -71,7 +71,7 @@ async fn test_popen_stdout_only() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_null_stdout_is_dev_null() { let mut sb = base().with_name("popen-null"); - let child = sb + let mut child = sb .popen( // Stat the *shell's* fd 1 via $$ — `$(...)` would rebind fd 1 for the // inner stat itself, so /proc/self/fd/1 there is the substitution pipe. @@ -82,7 +82,7 @@ async fn test_popen_null_stdout_is_dev_null() { ) .await .unwrap(); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success(), "fd 1 was not /dev/null (1:3): exit={:?}", res.exit_status); } @@ -100,7 +100,7 @@ async fn test_popen_stderr_piped() { let mut out = String::new(); stderr.read_to_string(&mut out).unwrap(); assert_eq!(out, "oops\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// A piped stdin the caller never takes must still reach EOF at `wait`, or a @@ -109,12 +109,12 @@ async fn test_popen_stderr_piped() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_untaken_stdin_reaches_eof() { let mut sb = base().with_name("popen-stdin-eof"); - let child = sb + let mut child = sb .popen(&["cat"], StdioMode::Piped, StdioMode::Null, StdioMode::Inherit) .await .unwrap(); // Deliberately do not take stdin. cat reads to EOF; wait must deliver it. - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success(), "cat did not exit — untaken stdin never EOF'd"); } @@ -178,7 +178,7 @@ async fn test_wait_kills_descendants_the_main_process_left_behind() { let bg_pid: i32 = line.trim().parse().expect("background pid"); assert!(unsafe { libc::kill(bg_pid, 0) } == 0, "background job should be alive"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); // A killed orphan stays a zombie until init/subreaper reaps it. @@ -225,18 +225,18 @@ async fn test_popen_all_three_piped() { stderr.read_to_string(&mut err).unwrap(); assert_eq!(out, "data\n"); assert_eq!(err, "err\n"); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Null stdin delivers immediate EOF: `cat` reading /dev/null exits 0 at once. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_null_stdin_is_eof() { let mut sb = base().with_name("popen-null-stdin"); - let child = sb + let mut child = sb .popen(&["cat"], StdioMode::Null, StdioMode::Null, StdioMode::Inherit) .await .unwrap(); - assert!(child.wait().await.unwrap().success()); + assert!(async { child.wait().await?; child.shutdown().await }.await.unwrap().success()); } /// Explicit Process::kill terminates the child; wait still collects a @@ -249,7 +249,7 @@ async fn test_popen_explicit_kill_then_wait() { .await .unwrap(); child.kill().expect("kill"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(!res.success(), "a killed process must not report success"); } @@ -258,11 +258,11 @@ async fn test_popen_explicit_kill_then_wait() { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_popen_rejects_second_spawn() { let mut sb = base().with_name("popen-twice"); - let first = sb + let mut first = sb .popen(&["echo", "first"], StdioMode::Inherit, StdioMode::Inherit, StdioMode::Inherit) .await .unwrap(); - first.wait().await.unwrap(); // consume the Process → releases the &mut borrow + async { first.wait().await?; first.shutdown().await }.await.unwrap(); // consume the Process → releases the &mut borrow let second = sb .popen(&["echo", "second"], StdioMode::Inherit, StdioMode::Inherit, StdioMode::Inherit) @@ -293,7 +293,7 @@ async fn test_popen_taken_stdout_is_not_drained_by_wait() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "out", "the taken stream must still carry the child's output"); - let res = child.wait().await.unwrap(); + let res = async { child.wait().await?; child.shutdown().await }.await.unwrap(); assert!(res.success()); assert!(res.stdout.is_none(), "a taken stdout must be None in the RunResult"); assert_eq!( diff --git a/crates/sandlock-core/tests/integration/test_process_groups.rs b/crates/sandlock-core/tests/integration/test_process_groups.rs index bdc3b6aa..7f7c003c 100644 --- a/crates/sandlock-core/tests/integration/test_process_groups.rs +++ b/crates/sandlock-core/tests/integration/test_process_groups.rs @@ -206,3 +206,165 @@ os.rename("{0}.tmp", "{0}") let leader: i32 = std::fs::read_to_string(&pid_file).unwrap().trim().parse().unwrap(); assert!(wait_gone(leader), "session leader {leader} outlived the main process"); } + +fn alive(pid: i32) -> bool { + unsafe { libc::kill(pid, 0) == 0 } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_wait_keeps_jobs_and_shutdown_ends_them() { + use std::io::BufRead; + let mut sb = policy(); + let mut proc = sb + .popen(&["sh", "-c", "sleep 60 & echo $!; exit 3"], sandlock_core::StdioMode::Inherit, sandlock_core::StdioMode::Piped, sandlock_core::StdioMode::Inherit) + .await + .unwrap(); + let mut job = String::new(); + std::io::BufReader::new(std::fs::File::from(proc.take_stdout().unwrap())).read_line(&mut job).unwrap(); + let job: i32 = job.trim().parse().unwrap(); + assert_eq!(proc.wait().await.unwrap(), sandlock_core::ExitStatus::Code(3)); + assert!(alive(job), "the job survives wait()"); + assert!(!proc.controller().unwrap().process_groups().unwrap().is_empty()); + assert_eq!(proc.shutdown().await.unwrap().exit_status, sandlock_core::ExitStatus::Code(3)); + assert!(wait_gone(job), "shutdown() ends the job"); +} + +#[tokio::test] +async fn test_shutdown_returns_output_while_a_job_holds_stdout() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & echo main-output"]).await.unwrap(); + sb.wait().await.unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().unwrap(); + assert_eq!(result.stdout.as_deref(), Some(b"main-output\n".as_slice())); +} + +/// The reaper collects a main that exits right after exec before spawn() +/// can see its new image; spawn() must still report success. +#[tokio::test] +async fn test_spawn_of_an_instantly_exiting_main() { + for _ in 0..50 { + let mut sb = policy(); + sb.spawn(&["true"]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(0)); + sb.shutdown().await.unwrap(); + } +} + +#[tokio::test] +async fn test_spawn_beside_a_parked_sibling() { + let mut parked = policy(); + parked.create(&["true"]).await.unwrap(); + let mut sb = policy(); + tokio::time::timeout(std::time::Duration::from_secs(5), sb.spawn(&["true"])).await.unwrap().unwrap(); + sb.wait().await.unwrap(); + sb.shutdown().await.unwrap(); +} + +#[tokio::test] +async fn test_pid_is_gone_once_main_exits() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + let controller = sb.controller().unwrap(); + sb.wait().await.unwrap(); + assert_eq!(sb.pid(), None); + assert_eq!(controller.pid(), None); + sb.shutdown().await.unwrap(); +} + +/// Main's status must not depend on a task of the runtime that created the +/// sandbox. +#[test] +fn test_wait_on_another_runtime() { + let runtime = || tokio::runtime::Builder::new_multi_thread().enable_all().build().unwrap(); + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin") + .no_supervisor(true) + .build() + .unwrap(); + let first = runtime(); + first.block_on(sb.spawn(&["sh", "-c", "sleep 0.2; exit 3"])).unwrap(); + drop(first); + let second = runtime(); + let status = second + .block_on(async { tokio::time::timeout(std::time::Duration::from_secs(5), sb.wait()).await }) + .expect("wait() hangs") + .unwrap(); + assert_eq!(status, sandlock_core::ExitStatus::Code(3)); + second.block_on(sb.shutdown()).unwrap(); +} + +#[tokio::test] +async fn test_spawn_reports_a_failed_exec() { + let mut sb = policy(); + assert!(sb.spawn(&["/nonexistent-sandlock-binary"]).await.is_err()); +} + +#[tokio::test] +async fn test_shutdown_ends_a_paused_session() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 60 & exit 0"]).await.unwrap(); + sb.wait().await.unwrap(); + sb.controller().unwrap().pause().unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_controller_kills_during_a_blocked_wait() { + let mut sb = policy(); + sb.spawn(&["sleep", "60"]).await.unwrap(); + let controller = sb.controller().unwrap(); + let killer = tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + controller.kill().unwrap(); + }); + let status = tokio::time::timeout(std::time::Duration::from_secs(5), sb.wait()).await.unwrap().unwrap(); + assert_ne!(status, sandlock_core::ExitStatus::Code(0)); + killer.await.unwrap(); + sb.shutdown().await.unwrap(); +} + +#[tokio::test] +async fn test_cancelled_wait_then_shutdown() { + let mut sb = policy(); + sb.spawn(&["sh", "-c", "sleep 0.3; echo done"]).await.unwrap(); + assert!(tokio::time::timeout(std::time::Duration::from_millis(50), sb.wait()).await.is_err()); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(0)); + assert_eq!(sb.shutdown().await.unwrap().stdout.as_deref(), Some(b"done\n".as_slice())); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_drop_after_wait_ends_jobs() { + let dir = tempfile::tempdir().unwrap(); + let pidfile = dir.path().join("job"); + let mut sb = Sandbox::builder() + .fs_read("/usr").fs_read("/lib").fs_read_if_exists("/lib64").fs_read("/bin").fs_read("/etc") + .fs_read("/proc").fs_read("/dev").fs_write(dir.path()) + .build() + .unwrap(); + sb.spawn(&["sh", "-c", &format!("sleep 60 & echo $! > {}", pidfile.display())]).await.unwrap(); + sb.wait().await.unwrap(); + let job: i32 = std::fs::read_to_string(&pidfile).unwrap().trim().parse().unwrap(); + assert!(alive(job)); + drop(sb); + assert!(wait_gone(job)); +} + +/// Dropping the sandbox after wait() must also reach jobs that left main's +/// group, not only those the reaper can kill by itself. +#[tokio::test(flavor = "multi_thread")] +async fn test_drop_after_wait_ends_jobs_in_other_groups() { + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("leader"); + let script = format!( + "import os, signal\npid = os.fork()\nif pid == 0:\n os.setsid()\n signal.pause()\nopen('{p}.tmp', 'w').write(str(pid))\nos.rename('{p}.tmp', '{p}')\n", + p = pid_file.display() + ); + let mut sb = policy(); + sb.fs_writable.push(dir.path().to_path_buf()); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let leader: i32 = std::fs::read_to_string(&pid_file).unwrap().trim().parse().unwrap(); + assert!(alive(leader), "the setsid job survives wait()"); + drop(sb); + assert!(wait_gone(leader), "Drop must end jobs in groups the sandbox created"); +} diff --git a/crates/sandlock-core/tests/integration/test_protection.rs b/crates/sandlock-core/tests/integration/test_protection.rs index d12ed2b9..22a6efb1 100644 --- a/crates/sandlock-core/tests/integration/test_protection.rs +++ b/crates/sandlock-core/tests/integration/test_protection.rs @@ -534,3 +534,97 @@ async fn degraded_policy_confines_and_runs_below_v6() { "degraded-mode confinement must still execute the child" ); } + +/// Kernels before 6.9 reject PIDFD_SIGNAL_PROCESS_GROUP. Re-run this test in +/// a child that emulates one, so the filter never touches the test process. +#[test] +fn shutdown_without_pidfd_group_signals() { + const CHILD: &str = "SANDLOCK_TEST_NO_PIDFD_GROUP_SIGNALS"; + if std::env::var_os(CHILD).is_none() { + let mut child = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["test_protection::shutdown_without_pidfd_group_signals", "--exact", "--nocapture"]) + .env(CHILD, "1") + .spawn() + .unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + if let Some(status) = child.try_wait().unwrap() { + assert!(status.success()); + return; + } + if std::time::Instant::now() >= deadline { + child.kill().unwrap(); + child.wait().unwrap(); + panic!("shutdown hung without pidfd group signals"); + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + + let filter = [ + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 0 }, + libc::sock_filter { code: 0x15, jt: 0, jf: 3, k: libc::SYS_pidfd_send_signal as u32 }, + libc::sock_filter { code: 0x20, jt: 0, jf: 0, k: 16 + 3 * 8 }, + libc::sock_filter { code: 0x45, jt: 0, jf: 1, k: 1 << 2 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ERRNO | libc::EINVAL as u32 }, + libc::sock_filter { code: 0x06, jt: 0, jf: 0, k: libc::SECCOMP_RET_ALLOW }, + ]; + let program = libc::sock_fprog { len: filter.len() as u16, filter: filter.as_ptr() as *mut _ }; + unsafe { + assert_eq!(libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0), 0); + assert_eq!(libc::prctl(libc::PR_SET_SECCOMP, libc::SECCOMP_MODE_FILTER, &program), 0); + } + + let dir = tempfile::tempdir().unwrap(); + let d = dir.path().display().to_string(); + let mut builder = sandlock_core::Sandbox::builder(); + for protection in Protection::all() { + builder = builder.allow_degraded(protection); + } + let policy = builder + .fs_read("/usr") + .fs_read("/lib") + .fs_read_if_exists("/lib64") + .fs_read("/bin") + .fs_read("/etc") + .fs_read("/proc") + .fs_read("/dev") + .fs_write(dir.path()) + .build() + .unwrap(); + let job = |name: &str| -> i32 { + std::fs::read_to_string(dir.path().join(name)).unwrap().trim().parse().unwrap() + }; + let publish = |name: &str| format!("open('{d}/{name}.tmp','w').write(str(os.getpid())); os.rename('{d}/{name}.tmp','{d}/{name}'); signal.pause()"); + let wait_for = |name: &str| format!("while not os.path.exists('{d}/{name}'): time.sleep(0.01)"); + let gone = |pid: i32| unsafe { libc::kill(pid, 0) } != 0; + + tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap().block_on(async { + // `cmd &` stays in main's group, which the live reaper pins. + let mut sb = policy.clone(); + sb.spawn(&["sh", "-c", &format!("sleep 60 & echo $! > {d}/r; exit 7")]).await.unwrap(); + assert_eq!(sb.wait().await.unwrap(), sandlock_core::ExitStatus::Code(7)); + let background = job("r"); + assert_eq!(sb.shutdown().await.unwrap().exit_status, sandlock_core::ExitStatus::Code(7)); + assert!(gone(background)); + + // A setsid job whose leader lives is reached by number. + let mut sb = policy.clone(); + let script = format!("import os,signal,time\nif os.fork()==0:\n os.setsid(); {}\n{}", publish("s"), wait_for("s")); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let leader = job("s"); + sb.shutdown().await.unwrap(); + assert!(gone(leader)); + + // A double-fork daemon outlived its leader: unreachable, so reported. + let mut sb = policy.clone(); + let script = format!("import os,signal,time\nif os.fork()==0:\n os.setsid()\n if os.fork()==0:\n {}\n os._exit(0)\n{}", publish("d"), wait_for("d")); + sb.spawn(&["python3", "-c", &script]).await.unwrap(); + sb.wait().await.unwrap(); + let daemon = job("d"); + assert!(tokio::time::timeout(std::time::Duration::from_secs(5), sb.shutdown()).await.unwrap().is_err()); + drop(sb); + assert_eq!(unsafe { libc::kill(daemon, libc::SIGKILL) }, 0); + }); +} diff --git a/crates/sandlock-core/tests/integration/test_resource.rs b/crates/sandlock-core/tests/integration/test_resource.rs index 8371d21d..63d74de2 100644 --- a/crates/sandlock-core/tests/integration/test_resource.rs +++ b/crates/sandlock-core/tests/integration/test_resource.rs @@ -492,7 +492,7 @@ async fn test_spawn_and_kill() { sb.start().unwrap(); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + let result = async { sb.wait().await?; sb.shutdown().await }.await.unwrap(); assert!( matches!(result.exit_status, ExitStatus::Signal(_) | ExitStatus::Killed), "expected Signal or Killed, got {:?}", @@ -681,7 +681,7 @@ async fn test_pause_resume() { sb.resume().expect("resume should succeed"); sb.kill().unwrap(); - let result = sb.wait().await.unwrap(); + let result = async { sb.wait().await?; sb.shutdown().await }.await.unwrap(); // Process should have been killed cleanly after pause/resume cycle assert!( diff --git a/crates/sandlock-core/tests/integration/test_restore.rs b/crates/sandlock-core/tests/integration/test_restore.rs index 7c28733a..54902dd1 100644 --- a/crates/sandlock-core/tests/integration/test_restore.rs +++ b/crates/sandlock-core/tests/integration/test_restore.rs @@ -138,7 +138,7 @@ async fn test_restore_glibc_vdso_program_resumes() { // Kill the original so only the restored process can advance the file. sb.kill().unwrap(); - let _ = sb.wait().await; + let _ = async { sb.wait().await?; sb.shutdown().await }.await; // Sentinel: prove the *restored* process (not a leftover original) is writing. std::fs::write(&counter, b"0\n").unwrap(); @@ -168,7 +168,7 @@ async fn test_restore_glibc_vdso_program_resumes() { // Clean up before asserting so a failure never leaks the child/files. let _ = sb2.kill(); - let exit = sb2.wait().await.map(|r| r.exit_status); + let exit = async { sb2.wait().await?; sb2.shutdown().await }.await.map(|r| r.exit_status); let _ = std::fs::remove_dir_all(&tmp); assert!( @@ -251,7 +251,7 @@ async fn test_restore_resumes_inside_a_chroot() { let cp = sb.checkpoint().await.unwrap(); let baseline = read_counter().expect("counter file should exist with a value"); sb.kill().unwrap(); - let _ = sb.wait().await; + let _ = async { sb.wait().await?; sb.shutdown().await }.await; std::fs::write(&counter, b"0\n").unwrap(); let mut sb2 = policy.clone().with_name("chroot-restore-dst"); @@ -267,7 +267,7 @@ async fn test_restore_resumes_inside_a_chroot() { tokio::time::sleep(std::time::Duration::from_millis(50)).await; } let _ = sb2.kill(); - let _ = sb2.wait().await; + let _ = async { sb2.wait().await?; sb2.shutdown().await }.await; let _ = std::fs::remove_dir_all(&rootfs); restored.expect("restore under chroot"); diff --git a/crates/sandlock-core/tests/integration/test_sandbox.rs b/crates/sandlock-core/tests/integration/test_sandbox.rs index fce2666d..0ba30a4d 100644 --- a/crates/sandlock-core/tests/integration/test_sandbox.rs +++ b/crates/sandlock-core/tests/integration/test_sandbox.rs @@ -180,7 +180,7 @@ async fn test_nested_sandbox() { // separately verifying that running Sandbox::run after another // Sandbox::run works (the seccomp filter stacks). outer_sb.kill().unwrap(); - let _ = outer_sb.wait().await; + let _ = async { outer_sb.wait().await?; outer_sb.shutdown().await }.await; // Sequential sandboxes: first sandbox applies Landlock + seccomp, // second sandbox from the same parent gets EBUSY on seccomp @@ -905,7 +905,7 @@ async fn test_cancelled_wait_keeps_the_capture_for_the_next_wait() { sb.kill().unwrap(); - let result = tokio::time::timeout(std::time::Duration::from_secs(30), sb.wait()) + let result = tokio::time::timeout(std::time::Duration::from_secs(30), async { sb.wait().await?; sb.shutdown().await }) .await .expect("the second wait() hung") .unwrap(); diff --git a/crates/sandlock-core/tests/integration/test_tty.rs b/crates/sandlock-core/tests/integration/test_tty.rs index 42368a98..790de024 100644 --- a/crates/sandlock-core/tests/integration/test_tty.rs +++ b/crates/sandlock-core/tests/integration/test_tty.rs @@ -148,7 +148,8 @@ fn interactive_run_hands_tty_foreground_to_child_while_alive() { Some(p) => p, None => return EXIT_RUN_FAILED, }; - if unsafe { libc::tcgetpgrp(0) } != child { + // The child runs in its reaper's group, not one it leads. + if unsafe { libc::tcgetpgrp(0) != libc::getpgid(child) } { return EXIT_FOREGROUND_NOT_TAKEN; } if policy.kill().is_err() { @@ -169,3 +170,50 @@ fn interactive_run_hands_tty_foreground_to_child_while_alive() { 13=child never got the tty foreground, 14=tty foreground not returned after reap)" ); } + +/// A job that outlives main can take the terminal back after wait() returned +/// it; shutdown() kills that job and must not leave the caller in the +/// background. +#[test] +fn shutdown_returns_tty_foreground_taken_by_a_job() { + if !std::path::Path::new("/usr/bin/python3").exists() { + return; + } + let code = run_in_pty_session(|| { + block_on(async { + let name = format!("tty-job-steal-{}", std::process::id()); + let mut policy = test_policy().with_name(&name); + let script = "import os, signal, time\n\ + signal.signal(signal.SIGTTOU, signal.SIG_IGN)\n\ + if os.fork() == 0:\n \ + time.sleep(0.3)\n \ + os.tcsetpgrp(0, os.getpgrp())\n \ + time.sleep(60)\n"; + if policy.spawn_interactive(&["python3", "-c", script]).await.is_err() { + return EXIT_RUN_FAILED; + } + if policy.wait().await.is_err() { + return EXIT_RUN_FAILED; + } + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while unsafe { libc::tcgetpgrp(0) == libc::getpgrp() } { + if std::time::Instant::now() > deadline { + return EXIT_FOREGROUND_NOT_TAKEN; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + if policy.shutdown().await.is_err() { + return EXIT_RUN_FAILED; + } + if unsafe { libc::tcgetpgrp(0) != libc::getpgrp() } { + return EXIT_FOREGROUND_NOT_RESTORED; + } + EXIT_OK + }) + }); + assert_eq!( + code, EXIT_OK, + "helper exit {code} (10=no tty foreground at start, 11=spawn/wait/shutdown failed, \ + 13=the job never took the tty foreground, 14=tty foreground not returned after shutdown)" + ); +} diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index 22d83f5c..1020be11 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -70,7 +70,7 @@ enum sandlock_exit_reason */ SANDLOCK_EXIT_REASON_KILLED = 2, /** - * Killed by sandlock because it exceeded its timeout. + * A run exceeded its timeout and was killed. */ SANDLOCK_EXIT_REASON_TIMEOUT = 3, }; @@ -877,10 +877,10 @@ int sandlock_start(sandlock_handle_t *h); int32_t sandlock_handle_pid(const sandlock_handle_t *h); /** - * Send SIGKILL to the handle's entire process group. Idempotent: a process - * that already exited is not an error. Returns 0 on success, -1 on error. - * The handle remains valid; call `sandlock_handle_wait` to collect the exit - * status and `sandlock_handle_free` to release it. + * Send SIGKILL to every process group of the session. Idempotent: a session + * whose processes already exited is not an error. Returns 0 on success, -1 + * on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * * # Safety * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. @@ -888,26 +888,59 @@ int32_t sandlock_handle_pid(const sandlock_handle_t *h); int sandlock_handle_kill(sandlock_handle_t *h); /** - * Wait for the sandbox to exit. Returns a result handle with stdout/stderr. + * Send SIGSTOP to every process group of the session. Returns 0 on success, + * -1 on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * - * For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any - * piped `out_stdout_fd`/`out_stderr_fd` *before* calling this, or a child that - * reads to EOF or fills a pipe buffer never exits and this blocks forever. + * # Safety + * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. + */ +int sandlock_handle_pause(sandlock_handle_t *h); + +/** + * Send SIGCONT to every process group of the session. Returns 0 on success, + * -1 on error. Safe to call while another thread is blocked in + * `sandlock_handle_wait`. * * # Safety * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. */ -sandlock_result_t *sandlock_handle_wait(sandlock_handle_t *h); +int sandlock_handle_resume(sandlock_handle_t *h); /** - * Wait for the sandbox to exit with a timeout in milliseconds. - * Returns a result handle, or null on error. On timeout the sandbox is - * killed and a result with `ExitStatus::Timeout` is returned. + * Sorted ids of the session's process groups that still have members, as a + * JSON array. Free with `sandlock_string_free`. Returns null on error. Safe + * to call while another thread is blocked in `sandlock_handle_wait`. * * # Safety - * `h` must be a valid handle from `sandlock_create`. + * `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. + */ +char *sandlock_handle_process_groups(const sandlock_handle_t *h); + +/** + * Wait for the main process; background jobs stay supervised. Returns 0 and + * a status-only result in `*status`, 1 if `timeout_ms` (0 waits forever) + * elapsed first and the session is untouched, or -1 on error. + * + * For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any + * piped `out_stdout_fd`/`out_stderr_fd` while waiting, or a child that reads + * to EOF or fills a pipe buffer never exits. + * + * # Safety + * `h` must be a valid handle, `status` a valid pointer, and no other call on + * `h` except kill, pause, resume and process_groups may run concurrently. + */ +int sandlock_handle_wait(sandlock_handle_t *h, uint64_t timeout_ms, sandlock_result_t **status); + +/** + * Kill what is left of the session, wait until it has no process, and return + * main's status with the captured output and filesystem changes. Returns + * null on error. + * + * # Safety + * `h` must be a valid handle, and no other call on `h` may run concurrently. */ -sandlock_result_t *sandlock_handle_wait_timeout(sandlock_handle_t *h, uint64_t timeout_ms); +sandlock_result_t *sandlock_handle_shutdown(sandlock_handle_t *h); /** * Get port mappings as a JSON string (e.g. `{"80":9001,"443":9002}`). diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index fa0c91c4..53ddc3ae 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -1155,6 +1155,8 @@ pub unsafe extern "C" fn sandlock_run( pub struct sandlock_handle_t { sandbox: Sandbox, runtime: tokio::runtime::Runtime, + // Beside the Sandbox so signals need no access to it during a wait. + controller: Option, } /// Shared entry-point prologue for the create/popen family: validate the @@ -1218,6 +1220,7 @@ unsafe fn sandlock_create_with_runtime( } Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) @@ -1373,6 +1376,7 @@ pub unsafe extern "C" fn sandlock_popen( write_or_close_fd(out_stderr_fd, err_fd.map(|f| f.into_raw_fd()).unwrap_or(-1)); Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) @@ -1404,84 +1408,131 @@ pub unsafe extern "C" fn sandlock_handle_pid(h: *const sandlock_handle_t) -> i32 if h.is_null() { return 0; } - (*h).sandbox.pid().unwrap_or(0) + handle_controller(h).and_then(|c| c.pid()).unwrap_or(0) } -/// Send SIGKILL to the handle's entire process group. Idempotent: a process -/// that already exited is not an error. Returns 0 on success, -1 on error. -/// The handle remains valid; call `sandlock_handle_wait` to collect the exit -/// status and `sandlock_handle_free` to release it. +/// Send SIGKILL to every process group of the session. Idempotent: a session +/// whose processes already exited is not an error. Returns 0 on success, -1 +/// on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// /// # Safety /// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. #[no_mangle] pub unsafe extern "C" fn sandlock_handle_kill(h: *mut sandlock_handle_t) -> c_int { - if h.is_null() { - return -1; - } - let h = &mut *h; - if h.sandbox.kill().is_err() { - return -1; + match handle_controller(h).map(|c| c.kill()) { + Some(Ok(())) => 0, + _ => -1, } - 0 } -/// Wait for the sandbox to exit. Returns a result handle with stdout/stderr. +/// Send SIGSTOP to every process group of the session. Returns 0 on success, +/// -1 on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// -/// For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any -/// piped `out_stdout_fd`/`out_stderr_fd` *before* calling this, or a child that -/// reads to EOF or fills a pipe buffer never exits and this blocks forever. +/// # Safety +/// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_pause(h: *mut sandlock_handle_t) -> c_int { + match handle_controller(h).map(|c| c.pause()) { + Some(Ok(())) => 0, + _ => -1, + } +} + +/// Send SIGCONT to every process group of the session. Returns 0 on success, +/// -1 on error. Safe to call while another thread is blocked in +/// `sandlock_handle_wait`. /// /// # Safety /// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. #[no_mangle] -pub unsafe extern "C" fn sandlock_handle_wait(h: *mut sandlock_handle_t) -> *mut sandlock_result_t { - if h.is_null() { - return ptr::null_mut(); +pub unsafe extern "C" fn sandlock_handle_resume(h: *mut sandlock_handle_t) -> c_int { + match handle_controller(h).map(|c| c.resume()) { + Some(Ok(())) => 0, + _ => -1, } - let h = &mut *h; - match block_on_runtime(&h.runtime, h.sandbox.wait()) { - Some(Ok(result)) => Box::into_raw(Box::new(sandlock_result_t { _private: result })), +} + +/// Sorted ids of the session's process groups that still have members, as a +/// JSON array. Free with `sandlock_string_free`. Returns null on error. Safe +/// to call while another thread is blocked in `sandlock_handle_wait`. +/// +/// # Safety +/// `h` must be a valid handle from `sandlock_create` / `sandlock_popen`. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_process_groups(h: *const sandlock_handle_t) -> *mut c_char { + match handle_controller(h).map(|c| c.process_groups()) { + Some(Ok(groups)) => CString::new(serde_json::to_string(&groups).unwrap_or_default()) + .map(CString::into_raw) + .unwrap_or(ptr::null_mut()), _ => ptr::null_mut(), } } -/// Wait for the sandbox to exit with a timeout in milliseconds. -/// Returns a result handle, or null on error. On timeout the sandbox is -/// killed and a result with `ExitStatus::Timeout` is returned. +unsafe fn handle_controller<'a>(h: *const sandlock_handle_t) -> Option<&'a sandlock_core::Controller> { + if h.is_null() { None } else { (*h).controller.as_ref() } +} + +/// Wait for the main process; background jobs stay supervised. Returns 0 and +/// a status-only result in `*status`, 1 if `timeout_ms` (0 waits forever) +/// elapsed first and the session is untouched, or -1 on error. +/// +/// For a `sandlock_popen` handle, close a piped `out_stdin_fd` and drain any +/// piped `out_stdout_fd`/`out_stderr_fd` while waiting, or a child that reads +/// to EOF or fills a pipe buffer never exits. /// /// # Safety -/// `h` must be a valid handle from `sandlock_create`. +/// `h` must be a valid handle, `status` a valid pointer, and no other call on +/// `h` except kill, pause, resume and process_groups may run concurrently. #[no_mangle] -pub unsafe extern "C" fn sandlock_handle_wait_timeout( +pub unsafe extern "C" fn sandlock_handle_wait( h: *mut sandlock_handle_t, timeout_ms: u64, -) -> *mut sandlock_result_t { + status: *mut *mut sandlock_result_t, +) -> c_int { + if h.is_null() || status.is_null() { + return -1; + } + *status = ptr::null_mut(); + let sandbox = &mut (*h).sandbox; + let outcome = block_on_runtime(&(*h).runtime, async { + if timeout_ms == 0 { + sandbox.wait().await.map(Some) + } else { + match tokio::time::timeout(Duration::from_millis(timeout_ms), sandbox.wait()).await { + Ok(r) => r.map(Some), + Err(_) => Ok(None), + } + } + }); + match outcome { + Some(Ok(Some(exit_status))) => { + *status = Box::into_raw(Box::new(sandlock_result_t { + _private: RunResult { exit_status, stdout: None, stderr: None, changes: Vec::new() }, + })); + 0 + } + Some(Ok(None)) => 1, + _ => -1, + } +} + +/// Kill what is left of the session, wait until it has no process, and return +/// main's status with the captured output and filesystem changes. Returns +/// null on error. +/// +/// # Safety +/// `h` must be a valid handle, and no other call on `h` may run concurrently. +#[no_mangle] +pub unsafe extern "C" fn sandlock_handle_shutdown(h: *mut sandlock_handle_t) -> *mut sandlock_result_t { if h.is_null() { return ptr::null_mut(); } let h = &mut *h; - - if timeout_ms == 0 { - // No timeout -- same as sandlock_handle_wait. - return match block_on_runtime(&h.runtime, h.sandbox.wait()) { - 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 })) - } + 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(), } } @@ -1639,7 +1690,7 @@ pub enum sandlock_exit_reason_t { Signaled = 1, /// Killed with no recoverable signal number. Killed = 2, - /// Killed by sandlock because it exceeded its timeout. + /// A run exceeded its timeout and was killed. Timeout = 3, } @@ -2608,7 +2659,8 @@ pub unsafe extern "C" fn sandlock_wait(sb: *mut Sandbox) -> c_int { let sb = &mut *sb; match with_runtime(|rt| rt.block_on(sb.wait())) { - Some(Ok(r)) => r.code().unwrap_or(-1), + Some(Ok(ExitStatus::Code(code))) => code, + Some(Ok(_)) => -1, _ => -1, } } @@ -2952,6 +3004,7 @@ pub unsafe extern "C" fn sandlock_restore_interactive( return ptr::null_mut(); } Box::into_raw(Box::new(sandlock_handle_t { + controller: sb.controller().ok(), sandbox: sb, runtime: rt, })) diff --git a/crates/sandlock-ffi/tests/changes.rs b/crates/sandlock-ffi/tests/changes.rs index 562d7ce0..09330a18 100644 --- a/crates/sandlock-ffi/tests/changes.rs +++ b/crates/sandlock-ffi/tests/changes.rs @@ -10,7 +10,6 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_entry_kind_t, sandlock_entry_t, sandlock_handle_free, - sandlock_handle_wait, sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_change_target, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, @@ -94,7 +93,7 @@ fn both_sides_of_every_change_are_readable() { unsafe { sandlock_sandbox_free(policy) }; assert!(!h.is_null()); assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(unsafe { sandlock_result_success(r) }); let n = unsafe { sandlock_result_changes_len(r) }; @@ -137,3 +136,11 @@ fn both_sides_of_every_change_are_readable() { unsafe { sandlock_result_free(r) }; unsafe { sandlock_handle_free(h) }; } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/defer.rs b/crates/sandlock-ffi/tests/defer.rs index 381c1c45..104adea1 100644 --- a/crates/sandlock-ffi/tests/defer.rs +++ b/crates/sandlock-ffi/tests/defer.rs @@ -10,8 +10,7 @@ use std::ptr; use sandlock_ffi::{ sandlock_create_for_run, sandlock_handle_abort, sandlock_handle_commit, sandlock_handle_free, - sandlock_handle_pending, sandlock_handle_upper_dir, sandlock_handle_wait, - sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_changes_len, + sandlock_handle_pending, sandlock_handle_upper_dir, sandlock_result_change_entry, sandlock_result_change_path, sandlock_result_changes_len, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_cwd, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_storage, sandlock_sandbox_builder_fs_write, @@ -57,7 +56,7 @@ fn run_deferred(workdir: &Path, storage: &Path) -> *mut sandlock_ffi::sandlock_h unsafe { sandlock_sandbox_free(policy) }; assert!(!h.is_null(), "create failed"); assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(!r.is_null(), "wait failed"); assert!(unsafe { sandlock_result_success(r) }); @@ -131,7 +130,7 @@ fn a_committing_handle_is_never_pending() { let h = unsafe { sandlock_create_for_run(policy, ptr::null(), av.as_ptr(), av.len() as c_uint) }; unsafe { sandlock_sandbox_free(policy) }; assert_eq!(unsafe { sandlock_start(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; assert!(unsafe { sandlock_result_success(r) }); assert_eq!(unsafe { sandlock_result_changes_len(r) }, 1); unsafe { sandlock_result_free(r) }; @@ -141,3 +140,11 @@ fn a_committing_handle_is_never_pending() { assert_eq!(std::fs::read_to_string(workdir.path().join("out.txt")).unwrap(), "hi\n"); unsafe { sandlock_handle_free(h) }; } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/popen.rs b/crates/sandlock-ffi/tests/popen.rs index a0740c4e..20e1a4df 100644 --- a/crates/sandlock-ffi/tests/popen.rs +++ b/crates/sandlock-ffi/tests/popen.rs @@ -11,7 +11,7 @@ use std::os::raw::{c_char, c_int, c_uint}; use std::ptr; use sandlock_ffi::{ - sandlock_handle_free, sandlock_handle_kill, sandlock_handle_wait, sandlock_popen, + sandlock_handle_free, sandlock_handle_kill, sandlock_popen, sandlock_result_exit_code, sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_new, sandlock_sandbox_free, sandlock_sandbox_t, @@ -76,7 +76,7 @@ fn popen_streams_stdout_and_collects_exit() { .unwrap(); assert_eq!(out, "ffi-hi\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert!(!res.is_null()); assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { @@ -120,7 +120,7 @@ fn popen_stdin_stdout_roundtrip() { stdout.read_to_string(&mut out).unwrap(); assert_eq!(out, "ping\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -153,7 +153,7 @@ fn popen_null_mode_yields_no_fd() { }; assert!(!h.is_null()); assert_eq!(fd_out, -1, "Null stdout yields no caller fd"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -194,7 +194,7 @@ fn popen_stderr_piped_through_ffi() { .unwrap(); assert_eq!(out, "err\n"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert_eq!(unsafe { sandlock_result_exit_code(res) }, 0); unsafe { sandlock_result_free(res); @@ -230,7 +230,7 @@ fn popen_kill_then_wait_then_free() { assert!(!h.is_null()); assert_eq!(unsafe { sandlock_handle_kill(h) }, 0, "kill should succeed"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; assert!(!res.is_null(), "wait must still return after kill"); assert!( !unsafe { sandlock_result_success(res) }, @@ -307,7 +307,7 @@ fn popen_kill_is_idempotent() { 0, "second kill must be idempotent" ); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; unsafe { sandlock_result_free(res); sandlock_handle_free(h); @@ -389,7 +389,7 @@ fn popen_piped_stream_with_null_out_ptr_is_closed_not_leaked() { ); assert_eq!(fd_in, -1, "stdin inherited → no fd"); assert_eq!(fd_err, -1, "stderr inherited → no fd"); - let res = unsafe { sandlock_handle_wait(h) }; + let res = unsafe { finish(h) }; unsafe { sandlock_result_free(res); sandlock_handle_free(h); @@ -410,3 +410,11 @@ fn popen_piped_stream_with_null_out_ptr_is_closed_not_leaked() { stream with a null out-pointer is leaking its fd instead of being closed" ); } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/restore.rs b/crates/sandlock-ffi/tests/restore.rs index 2542174b..f4840569 100644 --- a/crates/sandlock-ffi/tests/restore.rs +++ b/crates/sandlock-ffi/tests/restore.rs @@ -16,7 +16,7 @@ use sandlock_ffi::{ sandlock_checkpoint_free, sandlock_handle_checkpoint, sandlock_handle_free, sandlock_handle_kill, sandlock_handle_pid, sandlock_handle_restore_skipped_fd, sandlock_handle_restore_skipped_len, sandlock_handle_restore_skipped_path, - sandlock_handle_wait, sandlock_result_free, sandlock_restore_interactive, + sandlock_result_free, sandlock_restore_interactive, sandlock_sandbox_build, sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_fs_write, sandlock_sandbox_builder_new, sandlock_sandbox_free, sandlock_sandbox_t, sandlock_start, sandlock_string_free, @@ -149,7 +149,7 @@ fn restore_interactive_resumes_via_c_abi() { assert!(baseline > 2, "counter should have advanced, got {baseline}"); assert_eq!(unsafe { sandlock_handle_kill(h) }, 0); - let r = unsafe { sandlock_handle_wait(h) }; + let r = unsafe { finish(h) }; if !r.is_null() { unsafe { sandlock_result_free(r) }; } @@ -197,7 +197,7 @@ fn restore_interactive_resumes_via_c_abi() { // Clean up before asserting so a failure never leaks the child/files. unsafe { sandlock_handle_kill(h2); - let r2 = sandlock_handle_wait(h2); + let r2 = finish(h2); if !r2.is_null() { sandlock_result_free(r2); } @@ -225,3 +225,11 @@ fn restore_skipped_accessors_tolerate_null_handle() { assert_eq!(unsafe { sandlock_handle_restore_skipped_fd(ptr::null(), 0) }, -1); assert!(unsafe { sandlock_handle_restore_skipped_path(ptr::null(), 0) }.is_null()); } + +/// Wait for main, then end the session and return the full result. +unsafe fn finish(h: *mut sandlock_ffi::sandlock_handle_t) -> *mut sandlock_ffi::sandlock_result_t { + let mut status = std::ptr::null_mut(); + assert_eq!(sandlock_ffi::sandlock_handle_wait(h, 0, &mut status), 0); + sandlock_ffi::sandlock_result_free(status); + sandlock_ffi::sandlock_handle_shutdown(h) +} diff --git a/crates/sandlock-ffi/tests/session.rs b/crates/sandlock-ffi/tests/session.rs new file mode 100644 index 00000000..7035e44b --- /dev/null +++ b/crates/sandlock-ffi/tests/session.rs @@ -0,0 +1,81 @@ +//! Session lifetime through the C ABI: a wait that keeps the session, and +//! signals from another thread while that wait is blocked. + +use std::ffi::{CStr, CString}; +use std::os::raw::{c_char, c_int, c_uint}; +use std::ptr; + +use sandlock_ffi::{ + sandlock_handle_free, sandlock_handle_kill, sandlock_handle_process_groups, + sandlock_handle_shutdown, sandlock_handle_t, sandlock_handle_wait, sandlock_popen, + sandlock_result_free, sandlock_result_success, sandlock_sandbox_build, + sandlock_sandbox_builder_fs_read, sandlock_sandbox_builder_new, sandlock_sandbox_free, + sandlock_sandbox_t, sandlock_string_free, +}; + +const INHERIT: u32 = 0; + +fn build_policy() -> *mut sandlock_sandbox_t { + let mut b = sandlock_sandbox_builder_new(); + for p in ["/usr", "/lib", "/lib64", "/bin", "/etc", "/proc", "/dev"] { + if p == "/lib64" && !std::path::Path::new("/lib64").exists() { continue; } + let c = CString::new(p).unwrap(); + b = unsafe { sandlock_sandbox_builder_fs_read(b, c.as_ptr()) }; + } + let mut err: c_int = 0; + let policy = unsafe { sandlock_sandbox_build(b, &mut err, ptr::null_mut()) }; + assert_eq!(err, 0, "policy build failed"); + policy +} + +fn popen(policy: *mut sandlock_sandbox_t, cmd: &[&str]) -> *mut sandlock_handle_t { + let owned: Vec = cmd.iter().map(|s| CString::new(*s).unwrap()).collect(); + let av: Vec<*const c_char> = owned.iter().map(|c| c.as_ptr()).collect(); + let (mut fd_in, mut fd_out, mut fd_err): (c_int, c_int, c_int) = (-1, -1, -1); + let h = unsafe { + sandlock_popen(policy, ptr::null(), av.as_ptr(), av.len() as c_uint, INHERIT, INHERIT, INHERIT, + &mut fd_in, &mut fd_out, &mut fd_err) + }; + assert!(!h.is_null(), "sandlock_popen returned null"); + h +} + +#[test] +fn kill_reaches_a_blocked_wait() { + let policy = build_policy(); + let h = popen(policy, &["sleep", "60"]); + let addr = h as usize; + let killer = std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(100)); + unsafe { sandlock_handle_kill(addr as *mut sandlock_handle_t) } + }); + let mut status = ptr::null_mut(); + assert_eq!(unsafe { sandlock_handle_wait(h, 5000, &mut status) }, 0); + assert_eq!(killer.join().unwrap(), 0); + assert_eq!(unsafe { sandlock_result_success(status) }, false); + unsafe { + sandlock_result_free(status); + sandlock_result_free(sandlock_handle_shutdown(h)); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} + +#[test] +fn wait_timeout_leaves_the_session_alive() { + let policy = build_policy(); + let h = popen(policy, &["sleep", "60"]); + let mut status = ptr::null_mut(); + assert_eq!(unsafe { sandlock_handle_wait(h, 50, &mut status) }, 1); + assert!(status.is_null()); + let groups = unsafe { sandlock_handle_process_groups(h) }; + assert!(!groups.is_null()); + let listed = unsafe { CStr::from_ptr(groups) }.to_string_lossy().into_owned(); + assert_ne!(listed, "[]", "the session is still running"); + unsafe { + sandlock_string_free(groups); + sandlock_result_free(sandlock_handle_shutdown(h)); + sandlock_handle_free(h); + sandlock_sandbox_free(policy); + } +} diff --git a/crates/sandlock-oci/src/main.rs b/crates/sandlock-oci/src/main.rs index a7d60a06..56f03947 100644 --- a/crates/sandlock-oci/src/main.rs +++ b/crates/sandlock-oci/src/main.rs @@ -723,17 +723,13 @@ fn cmd_kill(id: &str, signal: &str, all: bool) -> Result<()> { let signum = parse_signal(signal)?; if all { - // For group-wide signals, ask the daemon to killpg so it uses the - // correct pgid (sandlock-init's pid, not the workload's). Fall back - // to a direct killpg if the daemon is unreachable (already exited). - let sent = supervisor::send_command( + // Only the daemon knows the session's groups: init does not lead + // one, so its pid names no group. A daemon that is gone took the + // session with it, since its reaper kills the session on owner death. + let _ = supervisor::send_command( id, supervisor::SupervisorCmd::Signal { signum }, ); - if sent.is_err() { - // Daemon gone: best-effort direct killpg on the recorded pid. - unsafe { libc::killpg(state.pid, signum) }; - } } else { let ret = unsafe { libc::kill(state.pid, signum) }; if ret < 0 { @@ -768,15 +764,10 @@ fn cmd_delete(id: &str, force: bool) -> Result<()> { } // If running with --force, ask the daemon to shutdown (it sends Shutdown to - // sandlock-init, which killpg's its group and exits). This correctly targets - // the process group even when state.pid is the workload (not the pgid). - // Fall back to a direct killpg if the daemon is already gone. + // sandlock-init, which killpg's its group and exits). A daemon that is + // already gone took the session with it. if state.status == Status::Running && state.pid > 0 && state.is_alive() { - let sent = supervisor::send_command(id, supervisor::SupervisorCmd::Shutdown); - if sent.is_err() { - // Daemon already gone: kill whatever we can reach. - unsafe { libc::killpg(state.pid, libc::SIGKILL) }; - } + let _ = supervisor::send_command(id, supervisor::SupervisorCmd::Shutdown); std::thread::sleep(std::time::Duration::from_millis(50)); } 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/go/README.md b/go/README.md index b9b7c8bd..9ae1009e 100644 --- a/go/README.md +++ b/go/README.md @@ -138,11 +138,13 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) (`Stdin`/`Stdout`/`Stderr`) you read/write while the child runs. The zero `Stdio` inherits all three (identical to Spawn). Close a piped `Stdin` before `Wait` (or let `Wait` close it for you) so a reader child sees EOF; drain a - piped `Stdout`/`Stderr` before `Wait`, or `Kill` from another goroutine to - interrupt a blocked `Wait`. `Wait` returns a `Result` with the exit status - only — a Popen'd process sends piped output to the `Stdout`/`Stderr` fields - (inherited/null streams go to the parent fd or `/dev/null`), so (unlike `Run`) - `Result.Stdout`/`Result.Stderr` are always empty. + piped `Stdout`/`Stderr` while waiting, or `Kill` from another goroutine to + interrupt a blocked `Wait`. `Wait` returns the main process's `ExitStatus` + and leaves its background jobs supervised; `Shutdown` ends the session and + returns a `Result`. A Popen'd process sends piped output to the + `Stdout`/`Stderr` fields (inherited/null streams go to the parent fd or + `/dev/null`), so (unlike `Run`) `Result.Stdout`/`Result.Stderr` are always + empty. ```go p, _ := sb.Popen(sandlock.Stdio{Stdin: sandlock.StdioPiped, Stdout: sandlock.StdioPiped}, "cat") @@ -150,7 +152,8 @@ defer p.Close() p.Stdin.Write([]byte("hi\n")) p.Stdin.Close() // EOF so cat exits out, _ := io.ReadAll(p.Stdout) // "hi\n" -res, _ := p.Wait() +st, _ := p.Wait() // main's ExitStatus +res, _ := p.Shutdown() // ends the session ``` ### Container images @@ -260,14 +263,19 @@ sb := &sandlock.Sandbox{ ```go func (p *Process) Pid() int -func (p *Process) Wait() (*Result, error) -func (p *Process) Pause() error // SIGSTOP to the process group +func (p *Process) Wait() (*ExitStatus, error) // main exited; background jobs keep running +func (p *Process) WaitTimeout(d time.Duration) (*ExitStatus, error) // ErrTimeout leaves the session +func (p *Process) Shutdown() (*Result, error) // kill what is left, reap it, collect the result +func (p *Process) ProcessGroups() ([]int, error) // groups that still have members +func (p *Process) Pause() error // SIGSTOP to every process group func (p *Process) Resume() error // SIGCONT func (p *Process) Kill() error // SIGKILL func (p *Process) Ports() (map[int]int, error) // virtual→real, with PortRemap -func (p *Process) Close() error // release the handle (kills if running), close piped streams +func (p *Process) Close() error // release the handle (ends the session), close piped streams -// BranchActionDefer only: after Wait, the change set stays on the Process. +// Kill, Pause and Resume work while another goroutine is blocked in Wait. + +// BranchActionDefer only: after Shutdown, the change set stays on the Process. func (p *Process) Pending() bool // exited under Defer and undecided func (p *Process) UpperDir() string // new bytes of added/modified files, laid out like Workdir func (p *Process) Commit() error // merge into Workdir (blocks up to 5s on a contended workdir) diff --git a/go/errors.go b/go/errors.go index 9d9684ea..22194df4 100644 --- a/go/errors.go +++ b/go/errors.go @@ -9,3 +9,7 @@ var ErrInvalidString = errors.New("sandlock: string contains NUL byte") // ErrNotRunning is returned by *Process lifecycle methods when no process is // currently running in the handle. var ErrNotRunning = errors.New("sandlock: process is not running") + +// ErrTimeout is returned by WaitTimeout when the main process is still +// running; the session is left untouched. +var ErrTimeout = errors.New("sandlock: main process has not exited") diff --git a/go/sandbox.go b/go/sandbox.go index 28d6c2fa..977bcda5 100644 --- a/go/sandbox.go +++ b/go/sandbox.go @@ -317,10 +317,18 @@ const ( ReasonSignaled ExitReason = 1 // ReasonKilled: killed with no recoverable signal number. ReasonKilled ExitReason = 2 - // ReasonTimeout: killed by sandlock because it exceeded its timeout. + // ReasonTimeout: a run exceeded its timeout and was killed. ReasonTimeout ExitReason = 3 ) +// ExitStatus describes the main process; its session may still be running. +type ExitStatus struct { + ExitCode int + Reason ExitReason + Signal int + Success bool +} + // Result is the outcome of a captured run. type Result struct { ExitCode int // process exit code, or -1 if terminated abnormally diff --git a/go/sandlock_linux.go b/go/sandlock_linux.go index 91ebe7e2..3f49b721 100644 --- a/go/sandlock_linux.go +++ b/go/sandlock_linux.go @@ -731,13 +731,23 @@ func (s *Sandbox) Run(ctx context.Context, cmd ...string) (*Result, error) { C.sandlock_handle_free(h) return nil, fmt.Errorf("sandlock: failed to start sandbox") } - r := C.sandlock_handle_wait_timeout(h, timeoutMs(ctx)) + var status *C.sandlock_result_t + rc := C.sandlock_handle_wait(h, timeoutMs(ctx), &status) + if rc < 0 { + C.sandlock_handle_free(h) + return nil, fmt.Errorf("sandlock: wait failed") + } + C.sandlock_result_free(status) + r := C.sandlock_handle_shutdown(h) C.sandlock_handle_free(h) if r == nil { - return nil, fmt.Errorf("sandlock: wait failed") + return nil, fmt.Errorf("sandlock: shutdown failed") } res := readResult(r) C.sandlock_result_free(r) + if rc == 1 { + res.ExitCode, res.Reason, res.Signal, res.Success = -1, ReasonTimeout, -1, false + } return res, nil } @@ -852,21 +862,23 @@ func SyscallNr(name string) (int, error) { return int(nr), nil } -// Process is a live sandboxed process started by Spawn. It supports PID -// inspection, pause/resume/kill via the process group, and Wait. A Process -// holds at most one running command; create separate Spawns for concurrency. +// Process is a sandbox session started by Spawn or Popen. Wait returns when +// the main process exits and leaves its background jobs supervised; Shutdown +// ends the session. A Process holds one session; create separate Spawns for +// concurrency. // -// The underlying FFI handle is not safe for concurrent access, so all handle -// operations are serialized. Pause/Resume/Kill act on the OS process group by -// PID and touch no handle state, so they remain usable while Wait blocks on -// the handle — that is how Kill interrupts a blocked Wait. Ports, by contrast, -// reads the handle and is reported as empty while a Wait is in flight. +// Handle operations are serialized by the mutex, except the native wait: +// native signals are safe beside it, so Kill, Pause and Resume reach a +// blocked Wait, while Shutdown and Close wait for it to return before they +// touch the handle. Ports is reported as empty while a Wait is in flight. type Process struct { - mu sync.Mutex - h *C.sandlock_handle_t - pid int - waiting bool // a Wait owns the handle; other handle ops must defer to it - pending bool // the run exited under BranchActionDefer; Commit/Abort own the handle now + mu sync.Mutex + h *C.sandlock_handle_t + waiting int // Waits in the native call + waitDone chan struct{} // closed when the last of them returns + status *ExitStatus // main's status once a Wait has seen it + result *Result // the session's result once Shutdown has run + pending bool // the run exited under BranchActionDefer; Commit/Abort own the handle now // Caller-owned stdio for a process started by Popen. Each is non-nil only // for a stream wired StdioPiped; it owns the pipe fd (closing the file @@ -905,10 +917,10 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error) { C.sandlock_handle_free(h) return nil, fmt.Errorf("sandlock: failed to start sandbox") } - p := &Process{h: h, pid: int(C.sandlock_handle_pid(h))} - // Last-resort cleanup if the caller drops the Process without Wait/Close: - // kill the child and release the handle so neither is leaked. Wait and - // Close clear this once they have done the cleanup themselves. + p := &Process{h: h} + // Last-resort cleanup if the caller drops the Process without + // Shutdown/Close: freeing the handle kills and reaps the session. + // Shutdown and Close clear this once they have done the cleanup. runtime.SetFinalizer(p, (*Process).finalize) return p, nil } @@ -931,9 +943,9 @@ func fdFile(fd C.int, name string) *os.File { // // Deadlock warning (as with os/exec pipes): a piped Stdin is yours — close it // before Wait, or a child that reads to EOF (e.g. cat) never exits and Wait -// blocks. Likewise drain a piped Stdout/Stderr before Wait, or a child that +// blocks. Likewise drain a piped Stdout/Stderr while waiting, or a child that // fills the pipe buffer blocks on write. As an escape hatch, Kill from another -// goroutine interrupts a blocked Wait; Close reaps the child and closes the +// goroutine interrupts a blocked Wait; Close ends the session and closes the // streams. Wait closes a still-open piped Stdin for you to deliver EOF. func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { if len(cmd) == 0 { @@ -970,7 +982,6 @@ func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { } p := &Process{ h: h, - pid: int(C.sandlock_handle_pid(h)), Stdin: fdFile(fdIn, "sandlock-stdin"), Stdout: fdFile(fdOut, "sandlock-stdout"), Stderr: fdFile(fdErr, "sandlock-stderr"), @@ -980,7 +991,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,73 +1002,159 @@ 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 } -// Pid returns the child process ID, or 0 if it is not available. +// Pid returns main's process ID until it exits, else 0: the number may then +// name another process. func (p *Process) Pid() int { p.mu.Lock() defer p.mu.Unlock() - return p.pid + if p.h == nil || p.pending { + return 0 + } + return int(C.sandlock_handle_pid(p.h)) } -// Wait blocks until the process exits, returns its Result, and releases the -// handle. After Wait the Process is no longer running. -// -// The Result carries the exit status only. Unlike Run, a Popen'd process sends -// any StdioPiped output to the caller through the Stdout/Stderr fields (and -// StdioInherit/StdioNull output to the inherited fd or /dev/null), so it is -// never captured into Result.Stdout/Result.Stderr — read piped bytes off -// p.Stdout/p.Stderr, not off the Result. -// -// The blocking native wait runs without holding the mutex so that Kill (and -// Pause/Resume), which signal the process group by PID, can run concurrently -// and interrupt it. The waiting flag reserves exclusive use of the handle for -// the duration, so no other handle operation aliases it. +// Wait blocks until the main process exits and returns its status. Background +// jobs stay supervised until Shutdown. Repeated calls return the same status. // // Wait closes a still-open piped Stdin to deliver EOF, so do not write to Stdin // from another goroutine while Wait runs — the write would race a closing fd. -func (p *Process) Wait() (*Result, error) { +func (p *Process) Wait() (*ExitStatus, error) { return p.wait(0) } + +// WaitTimeout is Wait with a deadline. It returns ErrTimeout if main is still +// running when d elapses, leaving the session untouched. +func (p *Process) WaitTimeout(d time.Duration) (*ExitStatus, error) { + ms := d.Milliseconds() + if ms < 1 { + ms = 1 + } + return p.wait(uint64(ms)) +} + +func (p *Process) wait(ms uint64) (*ExitStatus, error) { p.mu.Lock() - if p.h == nil || p.waiting { + if p.status != nil { + s := *p.status + p.mu.Unlock() + return &s, nil + } + if p.h == nil || p.pending { p.mu.Unlock() return nil, ErrNotRunning } h := p.h + if p.waiting == 0 { + p.waitDone = make(chan struct{}) + } + p.waiting++ stdin := p.Stdin - p.waiting = true p.mu.Unlock() - - // Deliver EOF to a still-open piped stdin so a reader child (e.g. cat) can - // exit; otherwise the native wait blocks forever. Harmless if already closed. if stdin != nil { _ = stdin.Close() } - r := C.sandlock_handle_wait(h) + var r *C.sandlock_result_t + rc := C.sandlock_handle_wait(h, C.uint64_t(ms), &r) p.mu.Lock() defer p.mu.Unlock() - p.waiting = false + p.waiting-- + if p.waiting == 0 { + close(p.waitDone) + } + switch rc { + case 0: + case 1: + return nil, ErrTimeout + default: + return nil, fmt.Errorf("sandlock: wait failed") + } + s := ExitStatus{ + ExitCode: int(C.sandlock_result_exit_code(r)), + Reason: ExitReason(C.sandlock_result_reason(r)), + Signal: int(C.sandlock_result_signal(r)), + Success: bool(C.sandlock_result_success(r)), + } + C.sandlock_result_free(r) + p.status = &s + out := s + return &out, nil +} + +// Shutdown kills whatever is left of the session, waits until it has no +// process, and returns the result. The Result carries no Stdout/Stderr for a +// Popen'd process: read those off p.Stdout/p.Stderr. A Wait blocked in +// another goroutine is ended first. A deferred branch stays available for +// Commit or Abort. Repeated calls return the same result. +func (p *Process) Shutdown() (*Result, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.result != nil { + return p.result, nil + } + if p.h == nil { + return nil, ErrNotRunning + } + if p.waiting > 0 { + C.sandlock_handle_kill(p.h) + p.drainWaits() + if p.result != nil { + return p.result, nil + } + if p.h == nil { + return nil, ErrNotRunning + } + } + r := C.sandlock_handle_shutdown(p.h) + if r == nil { + return nil, fmt.Errorf("sandlock: shutdown failed") + } + p.result = readResult(r) + C.sandlock_result_free(r) // A deferred branch keeps the handle alive until Commit/Abort/Close; the // finalizer stays armed so a leaked Process still preserves it. - if C.sandlock_handle_pending(h) != 0 { + if C.sandlock_handle_pending(p.h) != 0 { p.pending = true } else { - C.sandlock_handle_free(h) + C.sandlock_handle_free(p.h) p.h = nil runtime.SetFinalizer(p, nil) } - if r == nil { - return nil, fmt.Errorf("sandlock: wait failed") + return p.result, nil +} + +// drainWaits releases p.mu until no Wait is in flight; callers recheck state. +// A channel, not a Cond on p.mu, which would cycle into p and stall its finalizer. +func (p *Process) drainWaits() { + for p.waiting > 0 { + done := p.waitDone + p.mu.Unlock() + <-done + p.mu.Lock() } - res := readResult(r) - C.sandlock_result_free(r) - return res, nil +} + +// ProcessGroups returns the sorted ids of the session's process groups that +// still have members. +func (p *Process) ProcessGroups() ([]int, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.h == nil || p.pending { + return nil, ErrNotRunning + } + raw := C.sandlock_handle_process_groups(p.h) + if raw == nil { + return nil, fmt.Errorf("sandlock: process group inspection failed") + } + defer C.sandlock_string_free(raw) + var groups []int + if err := json.Unmarshal([]byte(C.GoString(raw)), &groups); err != nil { + return nil, err + } + return groups, nil } // Pending reports whether the process exited under BranchActionDefer and its @@ -1118,45 +1215,37 @@ func (p *Process) dispose(op func(*C.sandlock_handle_t) C.int, name string) erro func (p *Process) signal(sig syscall.Signal) error { p.mu.Lock() defer p.mu.Unlock() - if p.h == nil || p.pid <= 0 || p.pending { + if p.h == nil || p.pending { + if sig == syscall.SIGKILL { + return nil + } return ErrNotRunning } - // The sandbox child leads its own process group; signal the whole group. - return syscall.Kill(-p.pid, sig) + var rc C.int + switch sig { + case syscall.SIGSTOP: + rc = C.sandlock_handle_pause(p.h) + case syscall.SIGCONT: + rc = C.sandlock_handle_resume(p.h) + default: + rc = C.sandlock_handle_kill(p.h) + } + if rc != 0 { + return fmt.Errorf("sandlock: signal failed") + } + return nil } -// Pause sends SIGSTOP to the sandbox process group. +// Pause sends SIGSTOP to every process group of the session. func (p *Process) Pause() error { return p.signal(syscall.SIGSTOP) } -// Resume sends SIGCONT to the sandbox process group. +// Resume sends SIGCONT to every process group of the session. func (p *Process) Resume() error { return p.signal(syscall.SIGCONT) } -// Kill sends SIGKILL to the sandbox process group. It is idempotent: a process -// that already exited (ESRCH) or was already reaped by Wait (ErrNotRunning) is -// not an error — killing it is a no-op success, matching the FFI -// (sandlock_handle_kill returns 0) and the Python binding (silent no-op after -// wait()). This keeps the "Kill from another goroutine" escape hatch race-free: -// a Kill that fires just as Wait reaps the child still returns nil. -func (p *Process) Kill() error { - err := p.signal(syscall.SIGKILL) - if err == syscall.ESRCH { - return nil // child already exited (still unreaped) — no-op - } - if err == ErrNotRunning { - // signal() returns ErrNotRunning for two causes: the handle was already - // reaped by Wait (h == nil) — an idempotent no-op — or a live handle with - // no pid. Only the reaped case is a success; a missing pid is a genuine - // error we must surface (and must never turn into killpg(-0), which would - // signal our own process group). - p.mu.Lock() - reaped := p.h == nil - p.mu.Unlock() - if reaped { - return nil - } - } - return err -} +// Kill sends SIGKILL to every process group of the session. It is idempotent: +// a session that already ended is not an error, so a Kill racing Shutdown in +// another goroutine still returns nil. +func (p *Process) Kill() error { return p.signal(syscall.SIGKILL) } // Ports returns the current virtual-to-real TCP port mappings while the // process is running. It is non-empty only when PortRemap is enabled and at @@ -1166,7 +1255,7 @@ func (p *Process) Ports() (map[int]int, error) { defer p.mu.Unlock() // While a Wait holds the handle, reading port mappings would alias it; // report empty rather than touch the handle concurrently. - if p.h == nil || p.waiting { + if p.h == nil || p.waiting > 0 { return map[int]int{}, nil } c := C.sandlock_handle_port_mappings(p.h) @@ -1190,8 +1279,8 @@ func (p *Process) Ports() (map[int]int, error) { return out, nil } -// Close releases the process handle, killing the process if it is still -// running. It is safe to call multiple times. +// Close releases the process handle, ending the session if it is still +// running, without returning its result. It is safe to call multiple times. func (p *Process) Close() error { p.mu.Lock() defer p.mu.Unlock() @@ -1205,13 +1294,12 @@ func (p *Process) Close() error { if p.h == nil { return nil } - if p.waiting { - // A Wait owns the handle and will free it; just kill the process - // group by PID to unblock that Wait, without touching the handle. - if p.pid > 0 { - _ = syscall.Kill(-p.pid, syscall.SIGKILL) + if p.waiting > 0 { + C.sandlock_handle_kill(p.h) + p.drainWaits() + if p.h == nil { + return nil } - return nil } C.sandlock_handle_free(p.h) p.h = nil diff --git a/go/sandlock_linux_test.go b/go/sandlock_linux_test.go index d1c26137..e49b66af 100644 --- a/go/sandlock_linux_test.go +++ b/go/sandlock_linux_test.go @@ -393,15 +393,18 @@ func TestDeferHoldsTheBranchForTheProcess(t *testing.T) { t.Fatalf("Spawn: %v", err) } defer p.Close() - res, err := p.Wait() - if err != nil { + if _, err := p.Wait(); err != nil { t.Fatalf("Wait: %v", err) } + res, err := p.Shutdown() + if err != nil { + t.Fatalf("Shutdown: %v", err) + } if !res.Success || len(res.Changes) != 1 { t.Fatalf("unexpected result: %+v", res) } if !p.Pending() { - t.Fatal("a Defer run must be pending after Wait") + t.Fatal("a Defer run must be pending after Shutdown") } if _, statErr := os.Stat(dir + "/out.txt"); statErr == nil { t.Fatal("nothing may land before Commit") @@ -441,6 +444,9 @@ func TestDeferAbortDiscards(t *testing.T) { if _, err := p.Wait(); err != nil { t.Fatalf("Wait: %v", err) } + if _, err := p.Shutdown(); err != nil { + t.Fatalf("Shutdown: %v", err) + } if err := p.Abort(); err != nil { t.Fatalf("Abort: %v", err) } @@ -556,7 +562,7 @@ func TestPopenWaitClosesUnclosedStdin(t *testing.T) { // stdout concurrently so a full pipe can never be the reason cat blocks. go io.ReadAll(p.Stdout) - done := make(chan *sandlock.Result, 1) + done := make(chan *sandlock.ExitStatus, 1) go func() { res, _ := p.Wait() done <- res diff --git a/go/session_linux_test.go b/go/session_linux_test.go new file mode 100644 index 00000000..6a1396a4 --- /dev/null +++ b/go/session_linux_test.go @@ -0,0 +1,95 @@ +//go:build linux + +package sandlock_test + +import ( + "bufio" + "strconv" + "strings" + "syscall" + "testing" + "time" + + sandlock "github.com/multikernel/sandlock/go" +) + +func gone(pid int) bool { + deadline := time.Now().Add(5 * time.Second) + for syscall.Kill(pid, 0) == nil { + if time.Now().After(deadline) { + return false + } + time.Sleep(10 * time.Millisecond) + } + return true +} + +func TestWaitKeepsJobsUntilShutdown(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Popen(sandlock.Stdio{Stdout: sandlock.StdioPiped}, "sh", "-c", "sleep 60 & echo $!; exit 7") + if err != nil { + t.Fatalf("Popen: %v", err) + } + defer p.Close() + line, err := bufio.NewReader(p.Stdout).ReadString('\n') + if err != nil { + t.Fatalf("read job pid: %v", err) + } + job, _ := strconv.Atoi(strings.TrimSpace(line)) + st, err := p.Wait() + if err != nil || st.ExitCode != 7 { + t.Fatalf("Wait: %+v %v", st, err) + } + if syscall.Kill(job, 0) != nil { + t.Fatal("the job died with main") + } + if groups, err := p.ProcessGroups(); err != nil || len(groups) == 0 { + t.Fatalf("ProcessGroups after Wait: %v %v", groups, err) + } + if res, err := p.Shutdown(); err != nil || res.ExitCode != 7 { + t.Fatalf("Shutdown: %+v %v", res, err) + } + if !gone(job) { + t.Fatal("the job outlived Shutdown") + } +} + +func TestWaitTimeoutKeepsTheSession(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sleep", "60") + if err != nil { + t.Fatalf("Spawn: %v", err) + } + defer p.Close() + if _, err := p.WaitTimeout(50 * time.Millisecond); err != sandlock.ErrTimeout { + t.Fatalf("WaitTimeout: want ErrTimeout, got %v", err) + } + if groups, err := p.ProcessGroups(); err != nil || len(groups) == 0 { + t.Fatalf("a timed-out wait must leave the session: %v %v", groups, err) + } +} + +func TestShutdownEndsABlockedWait(t *testing.T) { + requireLandlock(t) + sb := &sandlock.Sandbox{FSReadable: rootfs} + p, err := sb.Spawn("sleep", "60") + if err != nil { + t.Fatalf("Spawn: %v", err) + } + done := make(chan error, 1) + go func() { + _, werr := p.Wait() + done <- werr + }() + time.Sleep(100 * time.Millisecond) + if _, err := p.Shutdown(); err != nil { + t.Fatalf("Shutdown: %v", err) + } + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("Shutdown did not end a blocked Wait") + } +} diff --git a/python/README.md b/python/README.md index edf37a3a..b13141bd 100644 --- a/python/README.md +++ b/python/README.md @@ -331,7 +331,7 @@ result = sandbox.run(["python3", "-c", "print(42)"], timeout=10.0) #### `sandbox.spawn(cmd) -> None` Spawn `cmd` without waiting. Use `pid`, `pause()`, `resume()`, `kill()`, -and `wait()` to manage the process lifecycle. +`wait()` and `shutdown()` to manage the session. Raises `RuntimeError` if a process is already running. @@ -353,9 +353,21 @@ Release a previously `create()`d child to `execve`. Raises `RuntimeError` if no child has been created. -#### `sandbox.wait() -> Result` +#### `sandbox.wait(timeout=None) -> ExitStatus` -Wait for the running process to finish and return its `Result`. +Wait for the main process and return its `ExitStatus`. Background jobs it +started keep running under supervision. A `timeout` raises `TimeoutError` +and leaves the session running. + +#### `sandbox.shutdown() -> Result` + +Kill whatever is left of the session, wait until it has no process, and +return the `Result` with captured output and filesystem changes. + +`kill()`, `pause()`, `resume()` and `process_groups()` work from any +thread, including while another thread is blocked in `wait()`. A +`popen()` `Process` has the same `wait()`, `shutdown()` and +`process_groups()`. #### Inspecting and deferring COW changes @@ -406,7 +418,7 @@ The sandbox name. #### `sandbox.pid -> int | None` -The child PID while running, `None` otherwise. +Main's PID until it exits, `None` otherwise. #### `sandbox.is_running -> bool` @@ -419,7 +431,7 @@ Only contains entries where port remapping occurred. Requires `port_remap=True`. #### `sandbox.pause()` / `sandbox.resume()` / `sandbox.kill()` -Send SIGSTOP / SIGCONT / SIGKILL to the sandbox process group. +Send SIGSTOP / SIGCONT / SIGKILL to every process group of the session. Raises `RuntimeError` if the sandbox is not running. #### `sandbox.checkpoint(save_fn=None) -> Checkpoint` diff --git a/python/src/sandlock/__init__.py b/python/src/sandlock/__init__.py index e4fd45bb..e79384b5 100644 --- a/python/src/sandlock/__init__.py +++ b/python/src/sandlock/__init__.py @@ -7,7 +7,7 @@ from ._version import __version__ from ._sdk import ( - Stage, Pipeline, Result, ExitReason, SyscallEvent, PolicyContext, Checkpoint, SkippedFd, + Stage, Pipeline, Result, ExitReason, ExitStatus, SyscallEvent, PolicyContext, Checkpoint, SkippedFd, NamedStage, Gather, GatherPipeline, Protection, landlock_abi_version, min_landlock_abi, confine, pull_image, @@ -46,6 +46,7 @@ "Pipeline", "Result", "ExitReason", + "ExitStatus", "SyscallEvent", "PolicyContext", "Checkpoint", diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index 01b973d8..e8ddb41b 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -313,11 +313,18 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_handle_pid.restype = ctypes.c_int _lib.sandlock_handle_pid.argtypes = [_c_handle_p] -_lib.sandlock_handle_wait.restype = _c_result_p -_lib.sandlock_handle_wait.argtypes = [_c_handle_p] +_lib.sandlock_handle_wait.restype = ctypes.c_int +_lib.sandlock_handle_wait.argtypes = [_c_handle_p, ctypes.c_uint64, ctypes.POINTER(_c_result_p)] -_lib.sandlock_handle_wait_timeout.restype = _c_result_p -_lib.sandlock_handle_wait_timeout.argtypes = [_c_handle_p, ctypes.c_uint64] +_lib.sandlock_handle_shutdown.restype = _c_result_p +_lib.sandlock_handle_shutdown.argtypes = [_c_handle_p] + +_lib.sandlock_handle_process_groups.restype = ctypes.c_void_p +_lib.sandlock_handle_process_groups.argtypes = [_c_handle_p] + +for _name in ("sandlock_handle_kill", "sandlock_handle_pause", "sandlock_handle_resume"): + getattr(_lib, _name).restype = ctypes.c_int + getattr(_lib, _name).argtypes = [_c_handle_p] _lib.sandlock_handle_free.restype = None _lib.sandlock_handle_free.argtypes = [_c_handle_p] @@ -904,7 +911,16 @@ class ExitReason(IntEnum): KILLED = 2 """Killed with no recoverable signal number.""" TIMEOUT = 3 - """Killed by sandlock because it exceeded its timeout.""" + """A run exceeded its timeout and was killed.""" + + +@dataclass(frozen=True) +class ExitStatus: + """Main-process status from ``wait()``; the session may still be running.""" + success: bool + exit_code: int + reason: ExitReason + signal: int = -1 @dataclass diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index 2e139313..de97a70b 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -12,6 +12,7 @@ import itertools import os import re +import threading import weakref from dataclasses import dataclass, field from enum import Enum, IntEnum @@ -534,6 +535,11 @@ def __post_init__(self): # own handle (see `_popen_process`), this is only a # non-owning busy marker self._restore_skipped = [] # SkippedFd entries from the last restore + # Every native call on _handle except a wait runs under this lock, so + # none can use a handle that shutdown() is freeing; waits only count + # themselves in and out, so a signal can reach a blocked wait. + self._idle = threading.Condition() + self._waiting = 0 @staticmethod def _next_name() -> str: @@ -617,25 +623,14 @@ def __enter__(self): return self def __exit__(self, exc_type, exc_val, exc_tb): - # Reap a still-running popen() Process (it owns its own handle) so leaving - # the sandbox context never strands a confined child. proc = self._popen_process() if proc is not None and proc._handle is not None: - try: - proc.kill() - except Exception: - pass - try: - proc.wait() - except Exception: - pass + proc.__exit__(exc_type, exc_val, exc_tb) if self._handle is not None: - from ._sdk import _lib try: - _lib.sandlock_handle_free(self._handle) + self.shutdown() except Exception: pass - self._handle = None # Freeing an undecided branch preserves it, same as KEEP. self._release_pending() return False @@ -646,17 +641,15 @@ def __exit__(self, exc_type, exc_val, exc_tb): @property def pid(self) -> int | None: - """Child PID while running, None otherwise. Reflects a running child from - either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" + """Main's PID until it exits, None otherwise. Reflects a running child + from either the sandbox lifecycle or a live :meth:`popen` :class:`Process`.""" proc = self._popen_process() if proc is not None: - # The Process owns the handle; its pid is lock-guarded and cached, so - # reading it can't race a concurrent wait() freeing the handle. return proc.pid - if self._handle is None: - return None - from ._sdk import _lib - return _lib.sandlock_handle_pid(self._handle) or None + with self._idle: + if self._handle is None: + return None + return _main_pid(self._handle) @property def is_running(self) -> bool: @@ -748,7 +741,7 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): killed and a timeout result is returned if exceeded. None means no timeout. """ - from ._sdk import _lib, _make_argv, _read_result_bytes, _read_result_changes, Result, ExitReason + from ._sdk import _lib, _make_argv, Result, ExitReason self._check_not_running() @@ -772,34 +765,19 @@ def run(self, cmd: Sequence[str], timeout: float | None = None): handle, self._handle = self._handle, None try: - # None -> wait forever (0). A finite timeout clamps up to 1ms so - # timeout=0 / sub-ms don't collapse to 0 (= wait forever). - timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 - result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) + try: + _wait(handle, timeout) + timed_out = False + except TimeoutError: + timed_out = True + result = _take_result(_lib.sandlock_handle_shutdown(handle)) + except RuntimeError as e: + return Result(success=False, exit_code=-1, error=str(e)) finally: self._park_or_free(handle) - - if not result_p: - return Result(success=False, exit_code=-1, error="sandlock_handle_wait failed") - - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - stdout = _read_result_bytes(result_p, _lib.sandlock_result_stdout_bytes) - stderr = _read_result_bytes(result_p, _lib.sandlock_result_stderr_bytes) - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) - - return Result( - success=bool(success), - exit_code=exit_code, - reason=reason, - signal=signal, - stdout=stdout, - stderr=stderr, - changes=changes, - ) + if timed_out: + result.exit_code, result.success, result.reason, result.signal = -1, False, ExitReason.TIMEOUT, -1 + return result def run_with_handlers( self, @@ -1024,12 +1002,13 @@ def start(self) -> None: """ from ._sdk import _lib - if self._handle is None: - raise RuntimeError("sandbox has not been created") - if _lib.sandlock_start(self._handle) != 0: - _lib.sandlock_handle_free(self._handle) - self._handle = None - raise RuntimeError("sandlock_start failed") + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox has not been created") + if _lib.sandlock_start(self._handle) != 0: + _lib.sandlock_handle_free(self._handle) + self._handle = None + raise RuntimeError("sandlock_start failed") def spawn(self, cmd: Sequence[str]) -> None: """Spawn ``cmd`` in the sandbox without waiting for it to finish. @@ -1044,47 +1023,57 @@ def spawn(self, cmd: Sequence[str]) -> None: self.create(cmd) self.start() - def wait(self): - """Wait for the running process to finish and return its Result. + def wait(self, timeout: float | None = None) -> "ExitStatus": + """Wait for the main process and return its :class:`ExitStatus`. + + Background jobs stay supervised until :meth:`shutdown`. A timeout + raises ``TimeoutError`` and leaves the session running. Repeated + calls return the same status. Raises: - RuntimeError: If the sandbox is not running, or if it is running a - :meth:`popen` :class:`Process` (wait on that Process instead — - freeing its handle here would break it). + RuntimeError: If the sandbox is not running, or a :meth:`popen` + :class:`Process` owns the child (wait on that Process instead). """ - from ._sdk import _lib, _read_result_bytes, _read_result_changes, Result, ExitReason - self._reject_if_popen() - if self._handle is None: - raise RuntimeError("sandbox is not running") - - handle, self._handle = self._handle, None + with self._idle: + handle = self._handle + if handle is None: + raise RuntimeError("sandbox is not running") + self._waiting += 1 try: - result_p = _lib.sandlock_handle_wait_timeout(handle, 0) + return _wait(handle, timeout) finally: - self._park_or_free(handle) - - if not result_p: - return Result(success=False, exit_code=-1, error="sandlock_handle_wait failed") - - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - stdout = _read_result_bytes(result_p, _lib.sandlock_result_stdout_bytes) - stderr = _read_result_bytes(result_p, _lib.sandlock_result_stderr_bytes) - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) + with self._idle: + self._waiting -= 1 + self._idle.notify_all() + + def shutdown(self) -> "Result": + """Kill what is left of the session, wait until it has no process, and + return the :class:`Result` with captured output and changes. A wait + blocked on another thread is ended first.""" + from ._sdk import _lib + self._reject_if_popen() + with self._idle: + handle = self._handle + if handle is None: + raise RuntimeError("sandbox is not running") + if self._waiting: + _send_signal(handle, "sandlock_handle_kill") + while self._waiting: + self._idle.wait() + self._handle = None + try: + return _take_result(_lib.sandlock_handle_shutdown(handle)) + finally: + self._park_or_free(handle) - return Result( - success=bool(success), - exit_code=exit_code, - reason=reason, - signal=signal, - stdout=stdout, - stderr=stderr, - changes=changes, - ) + def process_groups(self) -> list[int]: + """Sorted ids of the session's process groups that still have members.""" + self._reject_if_popen() + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running") + return _process_groups(self._handle) def popen( self, @@ -1288,46 +1277,28 @@ def reduce(self, cmd: list[str], fork_result: "ForkResult") -> "Result": # Lifecycle methods # ------------------------------------------------------------------ - def pause(self) -> None: - """Send SIGSTOP to the sandbox process group. - - Raises RuntimeError if a :meth:`popen` Process owns the child (manage it - through the Process).""" - import signal + def _signal(self, name: str) -> None: self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - os.killpg(pid, signal.SIGSTOP) + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running") + _send_signal(self._handle, name) - def resume(self) -> None: - """Send SIGCONT to the sandbox process group. + def pause(self) -> None: + """Send SIGSTOP to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child.""" + self._signal("sandlock_handle_pause") - Raises RuntimeError if a :meth:`popen` Process owns the child.""" - import signal - self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - os.killpg(pid, signal.SIGCONT) + def resume(self) -> None: + """Send SIGCONT to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child.""" + self._signal("sandlock_handle_resume") def kill(self) -> None: - """Send SIGKILL to the sandbox process group. - - Raises RuntimeError if a :meth:`popen` Process owns the child (call + """Send SIGKILL to every process group of the session. Raises + RuntimeError if a :meth:`popen` Process owns the child (call ``proc.kill()`` instead).""" - import signal - self._reject_if_popen() - pid = self.pid - if pid is None: - raise RuntimeError("sandbox is not running") - import os - try: - os.killpg(pid, signal.SIGKILL) - except ProcessLookupError: - pass + self._signal("sandlock_handle_kill") def ports(self) -> dict[int, int]: """Return current port mappings {virtual_port: real_port}. @@ -1340,11 +1311,13 @@ def ports(self) -> dict[int, int]: proc = self._popen_process() if proc is not None: # The Process owns the handle; its ports() reads it under the lock so - # this can't race a concurrent wait() freeing it. + # this can't race a concurrent shutdown() freeing it. return proc.ports() - if self._handle is None: - return {} - return _read_port_mappings(self._handle) + with self._idle: + # The wait holds the native sandbox; only signals may run beside it. + if self._handle is None or self._waiting: + return {} + return _read_port_mappings(self._handle) def checkpoint( self, @@ -1373,9 +1346,12 @@ def checkpoint( # would fight the streaming Process for it, so reject rather than reach # past it — consistent with wait/pause/resume/kill. self._reject_if_popen() - if self._handle is None: - raise RuntimeError("sandbox is not running (use start() or run() first)") - ptr = _lib.sandlock_handle_checkpoint(self._handle) + with self._idle: + if self._handle is None: + raise RuntimeError("sandbox is not running (use start() or run() first)") + if self._waiting: + raise RuntimeError("cannot checkpoint while a wait() is in progress") + ptr = _lib.sandlock_handle_checkpoint(self._handle) if not ptr: raise RuntimeError("checkpoint capture failed") cp = Checkpoint(ptr) @@ -1425,7 +1401,7 @@ def restore_interactive(self, cp: "Checkpoint") -> None: raise RuntimeError("checkpoint restore failed") self._handle = 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) @@ -1534,17 +1510,17 @@ class Process: :meth:`Sandbox.popen`. ``.stdin`` / ``.stdout`` / ``.stderr`` are unbuffered binary file objects - for streams opened with :attr:`StdioMode.PIPED`, else ``None``. The process - keeps running until :meth:`wait` (or :meth:`kill`) is called, or the - ``Process`` context manager exits. Prefer the context manager, which closes - the streams and reaps the child even on error:: + for streams opened with :attr:`StdioMode.PIPED`, else ``None``. The session + keeps running until :meth:`shutdown` is called or the ``Process`` context + manager exits. Prefer the context manager, which closes the streams and + reaps the session even on error:: with sandbox.popen(["cat"], stdin=StdioMode.PIPED, stdout=StdioMode.PIPED) as proc: proc.stdin.write(b"hi\\n") proc.stdin.close() # EOF so cat exits data = proc.stdout.read() - result = proc.wait() + status = proc.wait() """ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, stderr_fd: int): @@ -1553,16 +1529,14 @@ def __init__(self, sandbox: "Sandbox", handle, stdin_fd: int, stdout_fd: int, st from ._sdk import _lib self._sandbox = sandbox - # _lock guards _handle (and the _waiting reservation) so kill()/pid on - # one thread can't observe or act on a handle wait() is freeing on - # another — the "kill from another thread while wait() blocks" pattern. - self._lock = threading.Lock() - self._waiting = False - # The handle and pid are set only AFTER the fds are wrapped below: if an + # As on Sandbox: native calls other than a wait run under the lock, and + # waits only count themselves in and out. + self._idle = threading.Condition() + self._waiting = 0 + # The handle is set only AFTER the fds are wrapped below: if an # fdopen raises, this Process never takes ownership, so popen()'s caller # frees the handle exactly once (no double free via __del__). self._handle = None - self._pid = -1 self._result = None self.stdin = self.stdout = self.stderr = None # os.fdopen takes ownership of each fd: closing the stream closes the fd, @@ -1590,145 +1564,101 @@ 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. - self._pid = _lib.sandlock_handle_pid(handle) or -1 + # Take ownership now that construction can no longer fail. self._handle = 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 + """Main's PID until it exits, else ``None``.""" + with self._idle: + if self._handle is None: + return None + return _main_pid(self._handle) def ports(self) -> dict[int, int]: """Current virtual→real port mappings while running, else ``{}`` (needs - ``port_remap``). Reads the handle under the lock, and reports ``{}`` - while a :meth:`wait` holds it, so it can't race the wait's free.""" - with self._lock: + ``port_remap``); ``{}`` too while a :meth:`wait` holds the session.""" + with self._idle: if self._handle is None or self._waiting: return {} return _read_port_mappings(self._handle) def kill(self) -> None: - """SIGKILL the child's entire process group by pid (like the sandbox and - the Go binding), so a kill from another thread is safe while :meth:`wait` - blocks on the handle. Idempotent: a child that already exited is not an - error, and after :meth:`wait` has reaped it this is a no-op. - - Raises: - RuntimeError: If the process has no pid (never a valid popen child). - OSError: If the kill genuinely fails (e.g. EPERM) — a - ``ProcessLookupError`` (already-exited group) is swallowed. - """ - import os - import signal - - with self._lock: + """SIGKILL every process group of the session; safe from another thread + while :meth:`wait` blocks. A no-op after :meth:`shutdown`.""" + with self._idle: + if self._handle is not None: + _send_signal(self._handle, "sandlock_handle_kill") + + def process_groups(self) -> list[int]: + """Sorted ids of the session's process groups that still have members.""" + with self._idle: if self._handle is None: - return # already reaped by wait() — no-op - if self._pid <= 0: - raise RuntimeError("process has no pid") - pid = self._pid - try: - os.killpg(pid, signal.SIGKILL) - except ProcessLookupError: - pass # the group already exited — idempotent no-op + raise RuntimeError("process is not running") + return _process_groups(self._handle) - def wait(self, timeout: float | None = None) -> "Result": - """Wait for the child to exit and return its :class:`Result`. + def wait(self, timeout: float | None = None) -> "ExitStatus": + """Wait for the main process and return its :class:`ExitStatus`. A still-open piped ``stdin`` is closed first so a child that reads to EOF - can exit; piped ``stdout``/``stderr`` stay open for the caller to finish - reading. Frees the underlying handle; a second call returns the cached - result. - - The returned ``Result`` carries no ``stdout``/``stderr`` (those were - streamed to you as fds — read them off ``.stdout``/``.stderr``). - - Args: - timeout: Maximum seconds to wait. On timeout the child is killed and - a non-success ``Result`` is returned (same semantics as - :meth:`Sandbox.run`'s ``timeout``) — the wait never hangs. ``None`` - (default) waits indefinitely. Note: without a ``timeout``, a piped - ``stdout``/``stderr`` you have not drained can block the child on a - full pipe and hang the wait forever; pass a ``timeout`` or drain first. + can exit. Background jobs stay supervised until :meth:`shutdown`. A + timeout raises ``TimeoutError`` and leaves the session running. """ - from ._sdk import _lib, _read_result_changes, Result, ExitReason - - # Reserve the handle under the lock so a concurrent kill()/pid sees a - # consistent state, then run the blocking wait WITHOUT the lock so kill() - # (which signals by pid, not through the handle) can interrupt it. - with self._lock: + from ._sdk import ExitStatus + with self._idle: handle = self._handle if handle is None: if self._result is not None: - return self._result + return ExitStatus(self._result.success, self._result.exit_code, + self._result.reason, self._result.signal) raise RuntimeError("process is not running") - if self._waiting: - raise RuntimeError("wait already in progress") - self._waiting = True - - # Deliver EOF to a still-open piped stdin so a reader child can exit and - # the wait below does not block forever. + self._waiting += 1 if self.stdin is not None and not self.stdin.closed: try: self.stdin.close() except OSError: pass - try: - # None -> wait forever (the FFI treats 0 as "no timeout"). A finite - # timeout maps to milliseconds, clamped up to 1ms so timeout=0 and - # sub-millisecond values don't collapse to 0 and wait forever — the - # opposite of the caller's intent. - timeout_ms = max(1, int(timeout * 1000)) if timeout is not None else 0 - result_p = _lib.sandlock_handle_wait_timeout(handle, timeout_ms) + return _wait(handle, timeout) finally: - with self._lock: + with self._idle: + self._waiting -= 1 + self._idle.notify_all() + + def shutdown(self) -> "Result": + """Kill what is left of the session, wait until it has no process, and + return its :class:`Result`. The result carries no ``stdout``/``stderr``: + those were streamed to you. A repeated call returns the same result.""" + from ._sdk import _lib + with self._idle: + handle = self._handle + if handle is None: + if self._result is not None: + return self._result + raise RuntimeError("process is not running") + if self._waiting: + _send_signal(handle, "sandlock_handle_kill") + while self._waiting: + self._idle.wait() + self._handle = None + try: + self._result = _take_result(_lib.sandlock_handle_shutdown(handle)) + finally: self._sandbox._park_or_free(handle) - self._handle = None - self._waiting = False - # Release the sandbox's busy marker so it can be reused. Guard that it - # still points at us (the weakref may already be dead during __del__). - if self._sandbox._popen_process() is self: - self._sandbox._process = None - - if not result_p: - self._result = Result( - success=False, exit_code=-1, error="sandlock_handle_wait failed" - ) + # Guard: the weakref may already be dead during __del__. + if self._sandbox._popen_process() is self: + self._sandbox._process = None return self._result - exit_code = _lib.sandlock_result_exit_code(result_p) - success = _lib.sandlock_result_success(result_p) - reason = ExitReason(_lib.sandlock_result_reason(result_p)) - signal = _lib.sandlock_result_signal(result_p) - # stdout/stderr were handed to the caller as fds, so the RunResult holds - # none — read them off the streams, not the Result. - changes = _read_result_changes(result_p) - _lib.sandlock_result_free(result_p) - self._result = Result( - success=bool(success), exit_code=exit_code, reason=reason, signal=signal, - changes=changes, - ) - return self._result - def __enter__(self) -> "Process": return self def __exit__(self, exc_type, exc_val, exc_tb) -> None: - # Terminate and reap a still-running child so no confined process is left - # behind, then close every stream we own. + # End the session so no confined process is left behind, then close + # every stream we own. if self._handle is not None: try: - self.kill() - except Exception: - pass - try: - self.wait() + self.shutdown() except Exception: pass for stream in (self.stdin, self.stdout, self.stderr): @@ -1739,7 +1669,7 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> None: pass def __del__(self): - # Safety net for a Process dropped without wait()/context: a live handle + # Safety net for a Process dropped without shutdown()/context: a live handle # would otherwise leak the confined child + its runtime and wedge the # Sandbox in "already running". Reap it, warning like subprocess.Popen so # the missing wait()/`with` is visible. __del__ must never raise, and may @@ -1751,13 +1681,76 @@ def __del__(self): import warnings warnings.warn( - "Process was not waited on or used as a context manager; " - "reaping the confined child. Use `with sandbox.popen(...)` or " - "call wait().", + "Process was not shut down or used as a context manager; " + "reaping the confined session. Use `with sandbox.popen(...)` or " + "call shutdown().", ResourceWarning, stacklevel=2, ) - self.kill() - self.wait() + self.shutdown() except Exception: pass + + +def _wait(handle, timeout): + import ctypes + import math + from ._sdk import _c_result_p, _lib, ExitReason, ExitStatus + ms = 0 if timeout is None else max(1, math.ceil(timeout * 1000)) + ptr = _c_result_p() + rc = _lib.sandlock_handle_wait(handle, ms, ctypes.byref(ptr)) + if rc == 1: + raise TimeoutError("main process has not exited") + if rc != 0: + raise RuntimeError("sandlock_handle_wait failed") + try: + return ExitStatus( + success=bool(_lib.sandlock_result_success(ptr)), + exit_code=_lib.sandlock_result_exit_code(ptr), + reason=ExitReason(_lib.sandlock_result_reason(ptr)), + signal=_lib.sandlock_result_signal(ptr), + ) + finally: + _lib.sandlock_result_free(ptr) + + +def _take_result(ptr): + from ._sdk import _lib, _read_result_bytes, _read_result_changes, Result, ExitReason + if not ptr: + raise RuntimeError("sandlock_handle_shutdown failed") + try: + return Result( + success=bool(_lib.sandlock_result_success(ptr)), + exit_code=_lib.sandlock_result_exit_code(ptr), + reason=ExitReason(_lib.sandlock_result_reason(ptr)), + signal=_lib.sandlock_result_signal(ptr), + stdout=_read_result_bytes(ptr, _lib.sandlock_result_stdout_bytes), + stderr=_read_result_bytes(ptr, _lib.sandlock_result_stderr_bytes), + changes=_read_result_changes(ptr), + ) + finally: + _lib.sandlock_result_free(ptr) + + +def _process_groups(handle): + import ctypes + import json + from ._sdk import _lib + ptr = _lib.sandlock_handle_process_groups(handle) + if not ptr: + raise RuntimeError("process group inspection failed") + try: + return json.loads(ctypes.string_at(ptr)) + finally: + _lib.sandlock_string_free(ctypes.cast(ptr, ctypes.c_char_p)) + + +def _main_pid(handle): + from ._sdk import _lib + return _lib.sandlock_handle_pid(handle) or None + + +def _send_signal(handle, name): + from ._sdk import _lib + if getattr(_lib, name)(handle) != 0: + raise RuntimeError("sandbox signal failed") diff --git a/python/tests/test_lifecycle.py b/python/tests/test_lifecycle.py index 423c0aa7..16f19819 100644 --- a/python/tests/test_lifecycle.py +++ b/python/tests/test_lifecycle.py @@ -70,7 +70,8 @@ class TestSpawn: def test_spawn_then_wait_returns_result(self): with _policy() as sb: sb.spawn(["sh", "-c", "echo hello; exit 0"]) - result = sb.wait() + assert sb.wait().exit_code == 0 + result = sb.shutdown() assert result.exit_code == 0 assert b"hello" in result.stdout @@ -102,7 +103,8 @@ def test_create_then_start_runs_command(self): with _policy() as sb: sb.create(["sh", "-c", "echo two-step"]) sb.start() - result = sb.wait() + sb.wait() + result = sb.shutdown() assert result.exit_code == 0 assert b"two-step" in result.stdout diff --git a/python/tests/test_popen.py b/python/tests/test_popen.py index 0283d87e..f125fa53 100644 --- a/python/tests/test_popen.py +++ b/python/tests/test_popen.py @@ -149,9 +149,10 @@ def test_wait_is_idempotent_and_cached(): proc = _policy().popen(["echo", "hi"], stdout=StdioMode.PIPED) assert proc.stdout.read() == b"hi\n" first = proc.wait() - second = proc.wait() # handle already freed → cached result, no double-free - assert first is second + second = proc.wait() + assert first == second assert first.exit_code == 0 + assert proc.shutdown() is proc.shutdown() def test_popen_unknown_stdio_mode_raises(): @@ -181,17 +182,19 @@ def test_popen_rejects_second_process_on_same_sandbox(): proc.wait() -def test_wait_timeout_kills_and_returns_nonsuccess(): - # A child that never exits within the timeout: wait(timeout) must RETURN (not - # hang), killing the child and yielding a non-success result — parity with - # Sandbox.run(timeout=...). Run under a watchdog so a regression (ignored - # timeout → hang) surfaces as a failure, not a stuck job. +def test_wait_timeout_raises_and_keeps_the_session(): + # A child that never exits within the timeout: wait(timeout) must return + # promptly by raising TimeoutError and leave the session running. Run under + # a watchdog so an ignored timeout surfaces as a failure, not a stuck job. proc = _policy().popen(["sleep", "100"]) box = {} def _run(): - box["result"] = proc.wait(timeout=0.5) + try: + proc.wait(timeout=0.5) + except TimeoutError as e: + box["result"] = e t = threading.Thread(target=_run, daemon=True) t.start() @@ -201,7 +204,9 @@ def _run(): t.join(timeout=5) pytest.fail("wait(timeout) did not return — timeout was ignored") - assert not box["result"].success, "a timed-out (killed) process is not success" + assert isinstance(box["result"], TimeoutError) + assert proc.pid is not None, "a timeout leaves the session running" + proc.shutdown() def test_wait_without_timeout_completes_for_quick_child(): @@ -256,18 +261,17 @@ def test_stale_process_does_not_alias_reused_sandbox(): sandbox = _policy() p1 = sandbox.popen(["echo", "one"], stdout=StdioMode.PIPED) assert p1.stdout.read() == b"one\n" - p1.wait() # frees p1's handle, releases the sandbox + p1.shutdown() # frees p1's handle, releases the sandbox p2 = sandbox.popen(["sleep", "100"]) try: assert sandbox.is_running p1.kill() # no-op on the reaped p1 — must NOT kill p2 - p1.wait() # cached result — must NOT reap p2 + p1.wait() # cached status — must NOT reap p2 assert sandbox.is_running, "stale p1 must not have reaped p2" assert p2.pid is not None, "p2 must still be alive" finally: - p2.kill() - p2.wait() + p2.shutdown() assert not sandbox.is_running @@ -299,22 +303,24 @@ def test_sandbox_pid_and_ports_delegate_to_popen_process(): assert sandbox.pid == proc.pid assert sandbox.ports() == proc.ports() # both {} without port_remap finally: - proc.kill() - proc.wait() + proc.shutdown() assert sandbox.pid is None assert sandbox.ports() == {} def test_wait_timeout_zero_does_not_wait_forever(): # timeout=0 (and sub-millisecond values) must not collapse to "wait forever"; - # they return promptly with a non-success (killed) result. Watchdog so a - # regression surfaces as a failure, not a hang. + # they raise TimeoutError promptly. Watchdog so a regression surfaces as a + # failure, not a hang. proc = _policy().popen(["sleep", "100"]) box = {} def _run(): - box["result"] = proc.wait(timeout=0) + try: + proc.wait(timeout=0) + except TimeoutError as e: + box["result"] = e t = threading.Thread(target=_run, daemon=True) t.start() @@ -324,7 +330,8 @@ def _run(): t.join(timeout=5) pytest.fail("wait(timeout=0) hung — 0 collapsed to wait-forever") - assert not box["result"].success + assert isinstance(box["result"], TimeoutError) + proc.shutdown() def test_abandoned_process_is_reaped_by_del(): diff --git a/python/tests/test_sandbox.py b/python/tests/test_sandbox.py index e2b5624b..631126ac 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 is None, "an exited main's pid may already be reused" + sb.shutdown() assert sb.pid is None def test_pause_not_running_raises(self): @@ -1008,8 +1010,9 @@ def test_spawn_wait_defers_too(self, tmp_path): workdir.mkdir() p = _policy(fs_writable=[str(workdir)], workdir=str(workdir), on_exit=BranchAction.DEFER) p.spawn(["sh", "-c", f"echo hi > {workdir}/out.txt"]) - result = p.wait() - assert result.success + assert p.wait().success + assert not p.pending, "the branch settles at shutdown, not at main's exit" + assert p.shutdown().success assert p.pending p.commit() assert (workdir / "out.txt").read_text() == "hi\n" diff --git a/python/tests/test_session.py b/python/tests/test_session.py new file mode 100644 index 00000000..1f471220 --- /dev/null +++ b/python/tests/test_session.py @@ -0,0 +1,88 @@ +"""wait() returns with the main process; shutdown() ends the session.""" +import os +import threading +import time + +import pytest + +from sandlock import Sandbox, ExitStatus, ExitReason, StdioMode + + +def policy(**kw): + readable = ["/usr", "/lib", "/bin", "/etc", "/proc", "/dev"] + if os.path.exists("/lib64"): + readable.append("/lib64") + return Sandbox(fs_readable=readable, **kw) + + +def alive(pid): + try: + os.kill(pid, 0) + return True + except ProcessLookupError: + return False + + +def gone(pid, seconds=5): + deadline = time.monotonic() + seconds + while alive(pid): + if time.monotonic() > deadline: + return False + time.sleep(0.01) + return True + + +def test_wait_keeps_jobs_until_shutdown(): + with policy().popen(["sh", "-c", "sleep 60 & echo $!; exit 7"], stdout=StdioMode.PIPED) as proc: + job = int(proc.stdout.readline()) + status = proc.wait(timeout=5) + assert isinstance(status, ExitStatus) and status.exit_code == 7 + assert alive(job) and proc.process_groups() + assert proc.shutdown().exit_code == 7 + assert gone(job) + + +def test_wait_timeout_keeps_the_session(): + sb = policy() + sb.spawn(["sleep", "60"]) + with pytest.raises(TimeoutError): + sb.wait(timeout=0.05) + assert sb.process_groups() + sb.shutdown() + + +def test_kill_from_another_thread_ends_a_blocked_wait(): + sb = policy() + sb.spawn(["sleep", "60"]) + threading.Timer(0.1, sb.kill).start() + status = sb.wait(timeout=5) + assert not status.success + assert status.reason in (ExitReason.KILLED, ExitReason.SIGNALED) + sb.shutdown() + + +def test_shutdown_during_a_wait_ends_it(): + sb = policy() + sb.spawn(["sleep", "60"]) + waiter = threading.Thread(target=sb.wait) + waiter.start() + time.sleep(0.1) + sb.shutdown() + waiter.join(timeout=5) + assert not waiter.is_alive() + + +def test_context_exit_ends_jobs_left_after_wait(tmp_path): + pidfile = tmp_path / "job" + with policy(fs_writable=[str(tmp_path)]) as sb: + sb.spawn(["sh", "-c", f"sleep 60 & echo $! > {pidfile}"]) + sb.wait(timeout=5) + job = int(pidfile.read_text()) + assert alive(job) + assert gone(job) + + +def test_run_timeout_reports_timeout(): + result = policy().run(["sleep", "60"], timeout=0.2) + assert result.reason == ExitReason.TIMEOUT + assert not result.success