diff --git a/crates/runner/src/lib.rs b/crates/runner/src/lib.rs index 41c54bc..fc95ad1 100644 --- a/crates/runner/src/lib.rs +++ b/crates/runner/src/lib.rs @@ -116,6 +116,8 @@ mod process; #[cfg(feature = "devguard")] pub mod registration; mod rollback; +#[cfg(target_os = "linux")] +mod sandbox_tree; mod socket; mod uds; mod watch; diff --git a/crates/runner/src/process.rs b/crates/runner/src/process.rs index c1405ab..62b6360 100644 --- a/crates/runner/src/process.rs +++ b/crates/runner/src/process.rs @@ -148,6 +148,8 @@ enum SessionIo { Pipe { child: Arc>, stdin: Arc>>, + /// Launched through the Linux command sandbox: killed with every process it started. + sandboxed: bool, }, Pty { session: Arc, @@ -833,6 +835,7 @@ impl InProcessRunner { io: SessionIo::Pipe { child: child.clone(), stdin: Arc::new(Mutex::new(stdin)), + sandboxed, }, output: output.clone(), completed_at: completed_at.clone(), @@ -882,7 +885,7 @@ impl InProcessRunner { return; } life.note_kill(KillIntent::Timeout); - let _ = ch.start_kill(); + let _ = kill_pipe_root(&mut ch, sandboxed); drop(life); after_kill(&on_kill); if let Ok(mut buf) = timeout_out.lock() { @@ -1286,9 +1289,10 @@ impl InProcessRunner { } /// Terminate every process of every workspace, as `terminate_process` does each one: a pipe - /// process alone, a PTY process with its process group. How many were still running that - /// nothing had asked to end yet, through a timeout or a termination. An independent Runner - /// does this when it stops (P1R-C01). + /// process alone, or with every process it started when the Linux command sandbox runs it, + /// and a PTY process with its process group. How many were still running that nothing had + /// asked to end yet, through a timeout or a termination. An independent Runner does this + /// when it stops (P1R-C01). pub fn terminate_all(&self) -> u32 { let mut map = self.inner.lock().expect("runner"); self.evict_completed(&mut map); @@ -1398,6 +1402,19 @@ fn exited_unreaped(child: &Child) -> bool { unsafe { info.si_pid() != 0 } } +/// Kill a pipe root that its reaper has not reaped. A root launched through the Linux command +/// sandbox ends with every process it started, also while its sandbox is still being set up +/// (#109); any other root is killed alone. +fn kill_pipe_root(child: &mut Child, sandboxed: bool) -> std::io::Result<()> { + #[cfg(target_os = "linux")] + if let (true, Some(root)) = (sandboxed, child.id()) { + crate::sandbox_tree::kill_descendants(root); + } + #[cfg(not(target_os = "linux"))] + let _ = sandboxed; + child.start_kill() +} + async fn join_pump(handle: Option>) { if let Some(handle) = handle { let _ = handle.await; @@ -1406,7 +1423,9 @@ async fn join_pump(handle: Option>) { fn request_kill(slot: &Slot) -> Result { match &slot.io { - SessionIo::Pipe { child, .. } => { + SessionIo::Pipe { + child, sandboxed, .. + } => { let mut child = child.lock().expect("child"); let mut life = slot.lifecycle.lock().expect("lifecycle"); // Only the reaper waits (see `reap_child`): an exited child is left to it. @@ -1414,8 +1433,7 @@ fn request_kill(slot: &Slot) -> Result { return Ok(false); } life.note_kill(KillIntent::Terminated); - child - .start_kill() + kill_pipe_root(&mut child, *sandboxed) .map_err(|err| ErrorBody::new(ErrorCode::InvalidPatch, err.to_string()))?; after_kill(&slot.on_kill); Ok(true) @@ -2340,6 +2358,80 @@ mod tests { assert!(eof, "SIGTERM on the helper must reap the sandbox tree"); } + /// A terminate that lands while the sandbox is still being set up ends it with every + /// process it started (#109). Killing the helper alone could then leave the sandbox's first + /// process running without its parent, and the command with it, holding the output open. + #[cfg(target_os = "linux")] + #[tokio::test] + async fn terminate_during_sandbox_setup_leaves_no_process_behind() { + use crate::sandbox_tree::tests::{left_running, running, sandbox_setting_up}; + if !crate::linux_sandbox_available() { + return; + } + let dir = tempdir().unwrap(); + let ws = workspace(dir.path()); + let runner = InProcessRunner::new(Arc::new(|_| {})); + let mut during_setup = 0; + for attempt in 0..40 { + if during_setup == 5 { + break; + } + let marker = format!("30.1094{attempt:02}"); + let sleep = ["/bin/sleep", marker.as_str()]; + let process_id = ProcessId(format!("proc-sandbox-setup-{attempt}")); + let root = runner + .exec( + &ws, + RunnerExecRequest::for_host( + sleep.iter().map(|arg| arg.to_string()).collect(), + process_id.clone(), + Profile::WorkspaceWrite, + ), + ) + .await + .unwrap() + .root + .expect("a pipe root") + .pid; + // Terminate once the sandbox's first process exists and has not started the command. + let started = Instant::now(); + let mut setting_up = false; + while !setting_up + && running(&sleep).is_empty() + && started.elapsed() < Duration::from_secs(5) + { + setting_up = sandbox_setting_up(root, &marker); + } + runner.kill_host(&process_id).unwrap(); + during_setup += usize::from(setting_up); + let mut eof = false; + for _ in 0..100 { + let result = runner + .read_process(RunnerReadProcess { + process_id: process_id.clone(), + cursor: 0, + }) + .await + .unwrap(); + if result.eof { + eof = true; + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + let left = left_running(&sleep); + assert!( + eof && left.is_empty(), + "attempt {attempt} (during setup: {setting_up}): output ended {eof}, \ + still running {left:?}" + ); + } + assert!( + during_setup > 0, + "no terminate landed while the sandbox was being set up" + ); + } + #[tokio::test] async fn pipe_child_keeps_only_its_standard_descriptors() { // A descriptor that another thread's creation window could leave inheritable (#79). diff --git a/crates/runner/src/sandbox_tree.rs b/crates/runner/src/sandbox_tree.rs new file mode 100644 index 0000000..33b59b7 --- /dev/null +++ b/crates/runner/src/sandbox_tree.rs @@ -0,0 +1,295 @@ +//! Ending a Linux sandbox launch with every process it started (#109). +//! +//! A sandboxed pipe process's root is the sandbox helper, which becomes bubblewrap, directly or +//! through a process of its own. The sandbox process that bubblewrap creates is the first +//! process of a new PID namespace, and it sets bubblewrap's parent-death signal +//! (`--die-with-parent`) only just before it runs the command. Killing the root alone while the +//! sandbox is being set up can therefore leave that process running without its parent, and +//! with it the command it then starts, which keeps the Runner's output pipes open until it ends +//! on its own. +//! +//! [`kill_descendants`] stops the root, then each process that it finds below a stopped one, +//! from the top down, and then kills every process it found, children before their parents. +//! A stopped process can neither start a process nor reap one, so each process ID found stays +//! that process's own until the process is killed. Only a parent that ignores SIGCHLD has its +//! children reaped by the kernel, and the kernel hands out a freed process ID again only once +//! its counter has gone round every other one. Killing a sandbox's first process ends its +//! PID namespace, so every process in the sandbox ends with it, also one the walk did not find. +//! SIGSTOP and SIGKILL reach that process from the Runner's PID namespace whatever signal +//! handlers it has set. The caller then kills the root, as it kills any other. +//! +//! The walk reads `/proc`. Where `/proc` does not show the root as this process's child, as when +//! it belongs to another PID namespace, the walk does nothing and the root is killed alone. + +use std::collections::HashSet; +use std::time::{Duration, Instant}; + +/// How long the walk waits for the processes it stops, in all. Past it, the walk goes on +/// without waiting, so a process that has not stopped yet may start one that it misses. +const STOP_BUDGET: Duration = Duration::from_millis(500); +/// The pause between two looks at processes that are stopping. +const STOP_POLL: Duration = Duration::from_micros(250); +/// The most levels of processes the walk goes down. +const MAX_DEPTH: usize = 64; + +/// Stop every process below `root`, the caller's child that its reaper has not reaped, from the +/// top down, then kill them, children before their parents. `root` itself is left stopped for +/// the caller to kill. +pub(crate) fn kill_descendants(root: u32) { + // `/proc` shows `root` as this process's child only when it is this PID namespace's. + if parent_of(root) != Some(std::process::id()) || !signal(root, libc::SIGSTOP) { + return; + } + let deadline = Instant::now() + STOP_BUDGET; + let mut found = vec![root]; + let mut known = HashSet::from([root]); + let mut stopping = vec![root]; + for _ in 0..MAX_DEPTH { + // Once the processes found so far have stopped, none can appear below them. + wait_stopped(stopping, deadline); + stopping = processes() + .into_iter() + .filter(|(pid, parent)| known.contains(parent) && !known.contains(pid)) + .map(|(pid, _)| pid) + .filter(|pid| signal(*pid, libc::SIGSTOP)) + .collect(); + if stopping.is_empty() { + break; + } + known.extend(stopping.iter().copied()); + found.extend(stopping.iter().copied()); + } + for pid in found[1..].iter().rev() { + signal(*pid, libc::SIGKILL); + } +} + +/// Wait until each of `pids` has stopped or exited, or is gone, or until `deadline`. +fn wait_stopped(mut pids: Vec, deadline: Instant) { + loop { + pids.retain(|pid| !stopped(*pid)); + if pids.is_empty() || Instant::now() >= deadline { + return; + } + std::thread::sleep(STOP_POLL); + } +} + +/// Whether every thread of process `pid` has stopped or exited, or the process is gone. A +/// process stops once each of its threads has. +fn stopped(pid: u32) -> bool { + let Ok(threads) = std::fs::read_dir(format!("/proc/{pid}/task")) else { + return true; + }; + threads.flatten().all(|thread| { + std::fs::read_to_string(thread.path().join("stat")) + .ok() + .and_then(|stat| state_and_parent(&stat)) + .is_none_or(|(state, _)| matches!(state, 'T' | 't' | 'Z' | 'X' | 'x')) + }) +} + +/// Every process that `/proc` lists, with its parent, from one read. +fn processes() -> Vec<(u32, u32)> { + let Ok(entries) = std::fs::read_dir("/proc") else { + return Vec::new(); + }; + entries + .flatten() + .filter_map(|entry| entry.file_name().to_str()?.parse::().ok()) + .filter_map(|pid| Some((pid, parent_of(pid)?))) + .collect() +} + +/// The parent of process `pid` as its `stat` states it, or `None` once the process is gone. +fn parent_of(pid: u32) -> Option { + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; + state_and_parent(&stat).map(|(_, parent)| parent) +} + +/// The state and the parent's process ID in a `stat` line: its 3rd and 4th fields, after the +/// command name in parentheses, which may itself hold spaces and parentheses. +fn state_and_parent(stat: &str) -> Option<(char, u32)> { + let mut fields = stat[stat.rfind(')')? + 1..].split_whitespace(); + let state = fields.next()?.chars().next()?; + let parent = fields.next()?.parse().ok()?; + Some((state, parent)) +} + +/// Send `signal` to process `pid`: never to a process group, to every process, or to this +/// process. Whether it was sent. +fn signal(pid: u32, signal: libc::c_int) -> bool { + if pid == std::process::id() { + return false; + } + match libc::pid_t::try_from(pid) { + // SAFETY: `kill` only sends a signal, here to the one process that `pid` names. + Ok(pid) if pid > 0 => unsafe { libc::kill(pid, signal) == 0 }, + _ => false, + } +} + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + use std::io::Read; + use std::process::{Child, Command, Stdio}; + + /// The processes other than this one whose argv is `argv`. + pub(crate) fn running(argv: &[&str]) -> Vec { + processes() + .into_iter() + .map(|(pid, _)| pid) + .filter(|pid| *pid != std::process::id()) + .filter(|pid| { + std::fs::read(format!("/proc/{pid}/cmdline")).is_ok_and(|cmdline| { + let args = cmdline + .split(|byte| *byte == 0) + .filter(|arg| !arg.is_empty()); + args.eq(argv.iter().map(|arg| arg.as_bytes())) + }) + }) + .collect() + } + + /// Wait up to two seconds for no process with argv `argv` to run, then kill any that still + /// does, so that a failed test leaves none behind. The ones that still ran. + pub(crate) fn left_running(argv: &[&str]) -> Vec { + let started = Instant::now(); + while !running(argv).is_empty() && started.elapsed() < Duration::from_secs(2) { + std::thread::sleep(Duration::from_millis(10)); + } + let left = running(argv); + for pid in &left { + signal(*pid, libc::SIGKILL); + } + left + } + + /// Whether the sandbox whose command has `marker` in its argv is being set up: its first + /// process, in a PID namespace below `root`'s, still runs bubblewrap, so it has not started + /// the command yet, and its parent is the bubblewrap that runs the command, not the one that + /// the helper's preflight runs first. + pub(crate) fn sandbox_setting_up(root: u32, marker: &str) -> bool { + let all = processes(); + let mut below = HashSet::from([root]); + loop { + let before = below.len(); + for (pid, parent) in &all { + if below.contains(parent) { + below.insert(*pid); + } + } + if below.len() == before { + break; + } + } + let read = |pid: u32, file: &str| std::fs::read(format!("/proc/{pid}/{file}")); + let bubblewrap = |pid: u32| read(pid, "comm").is_ok_and(|comm| comm == b"bwrap\n"); + // The `NSpid` line of `status` lists the process's ID in each PID namespace it is in. + let namespaces = |pid: u32| { + let status = String::from_utf8(read(pid, "status").ok()?).ok()?; + let line = status.lines().find(|line| line.starts_with("NSpid:"))?; + Some(line.split_whitespace().count() - 1) + }; + let runs_marker = |pid: u32| { + read(pid, "cmdline").is_ok_and(|cmdline| { + cmdline + .split(|byte| *byte == 0) + .any(|arg| arg == marker.as_bytes()) + }) + }; + let root_namespaces = namespaces(root); + all.iter().any(|(pid, parent)| { + *pid != root + && below.contains(pid) + && bubblewrap(*pid) + && namespaces(*pid) > root_namespaces + && bubblewrap(*parent) + && runs_marker(*parent) + }) + } + + /// `/bin/sh -c script` with its standard output piped, once a process with argv `argv` runs. + fn spawn_until_running(script: &str, argv: &[&str]) -> Child { + let child = Command::new("/bin/sh") + .args(["-c", script]) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + let started = Instant::now(); + while running(argv).is_empty() { + assert!( + started.elapsed() < Duration::from_secs(10), + "{argv:?} never ran" + ); + std::thread::sleep(Duration::from_millis(5)); + } + child + } + + #[test] + fn a_stat_line_names_its_state_and_parent() { + assert_eq!( + state_and_parent("42 (sleep) S 7 42 42 0 -1 4194304"), + Some(('S', 7)) + ); + // The command name may hold spaces and parentheses. + assert_eq!( + state_and_parent("42 (a) b (c) T 9 42 42 0 -1 4194304"), + Some(('T', 9)) + ); + assert_eq!(state_and_parent("42 (sleep"), None); + } + + #[test] + fn signal_never_reaches_a_group_or_this_process() { + assert!(!signal(0, 0)); + assert!(!signal(u32::MAX, 0)); + assert!(!signal(std::process::id(), 0)); + } + + /// A root whose processes outlive it when it is killed alone: a shell that starts a shell + /// that starts `sleep` in a session of its own, as bubblewrap's sandbox process runs, all of + /// them holding the root's standard output. + #[test] + fn every_process_below_the_root_ends_and_the_root_stays_stopped() { + let sleep = ["/bin/sleep", "30.1092"]; + let script = format!( + "/bin/sh -c 'setsid {} {} & wait' & wait", + sleep[0], sleep[1] + ); + let mut root = spawn_until_running(&script, &sleep); + kill_descendants(root.id()); + let stat = std::fs::read_to_string(format!("/proc/{}/stat", root.id())).unwrap(); + assert_eq!(state_and_parent(&stat).map(|(state, _)| state), Some('T')); + let left = left_running(&sleep); + root.kill().unwrap(); + root.wait().unwrap(); + assert_eq!(left, Vec::::new()); + // Nothing holds the root's output any more. + let mut output = Vec::new(); + root.stdout + .take() + .unwrap() + .read_to_end(&mut output) + .unwrap(); + } + + #[test] + fn a_process_that_is_not_this_ones_child_is_left_alone() { + let sleep = ["/bin/sleep", "30.1093"]; + let script = format!("{} {} & wait", sleep[0], sleep[1]); + let mut parent = spawn_until_running(&script, &sleep); + let grandchild = running(&sleep)[0]; + kill_descendants(grandchild); + let untouched = !stopped(grandchild); + signal(grandchild, libc::SIGKILL); + parent.kill().unwrap(); + parent.wait().unwrap(); + assert!( + untouched, + "the walk stopped a process that is not this one's child" + ); + } +} diff --git a/docs/ko/runner-isolation.md b/docs/ko/runner-isolation.md index 2cf6e44..f5310be 100644 --- a/docs/ko/runner-isolation.md +++ b/docs/ko/runner-isolation.md @@ -47,7 +47,7 @@ Runner는 소켓 디렉터리를 권한 0700으로 직접 만듭니다. 이미 | 게이트웨이가 Runner에 중지를 요청하거나, Runner가 SIGTERM이나 SIGINT를 받음 | 관리 worker는 요청을 거부하며 이 신호를 처리하지 않습니다 | Runner가 프로세스를 끝내고 최대 5초 동안 기다린 뒤 종료합니다 | | 프로세스가 timeout에 도달함 | 프로세스를 강제로 종료합니다 | 게이트웨이 연결 여부와 관계없이 원래 deadline에 프로세스를 강제로 종료합니다 | -프로세스를 끝낼 때 tty 프로세스는 그 프로세스 그룹에 신호를 보내지만, 파이프 프로세스는 그 프로세스에만 보냅니다. 파이프 프로세스에는 자기 프로세스 그룹이 없으므로, 그 프로세스가 시작한 프로세스는 중지할 때를 포함해 더 오래 남을 수 있습니다. +프로세스를 끝낼 때 tty 프로세스는 그 프로세스 그룹에 신호를 보내지만, 파이프 프로세스는 그 프로세스에만 보냅니다. 파이프 프로세스에는 자기 프로세스 그룹이 없으므로, 그 프로세스가 시작한 프로세스는 중지할 때를 포함해 더 오래 남을 수 있습니다. Linux 명령 샌드박스가 실행한 파이프 프로세스는 샌드박스를 설정하는 중에도 자신이 시작한 모든 프로세스와 함께 끝납니다. Runner가 샌드박스 도우미와 그 아래의 각 프로세스를 먼저 멈춘 뒤 모두 강제로 종료하기 때문입니다([#109](https://github.com/novelKR/CodeSpace/issues/109)). diff --git a/docs/runner-isolation.md b/docs/runner-isolation.md index 9a302a0..bdcf32d 100644 --- a/docs/runner-isolation.md +++ b/docs/runner-isolation.md @@ -44,7 +44,7 @@ A gateway does not start when its `--runner-lifecycle` differs from the Runner's | A gateway asks the Runner to stop, or the Runner receives SIGTERM or SIGINT | A managed worker refuses the request and does not handle these signals | The Runner ends its processes, waits up to 5 seconds for them, and exits | | A process reaches its timeout | The process is killed | The process is killed at its original deadline, whether or not a gateway is connected | -Ending a process signals a tty process's process group, but a pipe process alone: a pipe process has no process group of its own, so the processes it started may outlive it, a stop included. +Ending a process signals a tty process's process group, but a pipe process alone: a pipe process has no process group of its own, so the processes it started may outlive it, a stop included. Under the Linux command sandbox, a pipe process ends with every process it started, also while its sandbox is still being set up: the Runner first stops the sandbox helper and each process below it, then kills them all ([#109](https://github.com/novelKR/CodeSpace/issues/109)). diff --git a/docs/translations.json b/docs/translations.json index f284bde..578423d 100644 --- a/docs/translations.json +++ b/docs/translations.json @@ -730,8 +730,8 @@ "컨테이너-실험-구성과-검증", "호스트와-worker-실행" ], - "source_sha256": "d3e6eef123f6f141c921e35d38d95cb6a160ee8cde031f674b397f8f132bd6c6", - "translation_sha256": "584fc9708b41e85bbb437656d027844223c79ff8fa954a25abcd86c88a49fc4c" + "source_sha256": "b75bcfe64896dec48d60c355e208caa4e2360b79de5fc0751fed0d58cadeb46a", + "translation_sha256": "caa5b8c46cbf2268b435d2b4b54c9851ff82ff77f5ca356a2619d00a080b41a7" }, { "id": "error-codes",