test: clean up detached bwrap descendants

This commit is contained in:
Adam Perry
2026-07-10 00:42:30 +00:00
parent b50ea03384
commit 16ebe19d34
5 changed files with 443 additions and 49 deletions
+6 -1
View File
@@ -11,6 +11,7 @@ rust_library(
srcs = [
"src/lib.rs",
"src/lib_tests.rs",
"src/sandbox_init.rs",
],
crate_name = "bwrap_test_support",
crate_root = "src/lib.rs",
@@ -19,6 +20,7 @@ rust_library(
deps = [
"//codex-rs/utils/cargo-bin",
"@crates//:anyhow",
"@crates//:libc",
"@crates//:tempfile",
"@crates//:tokio",
],
@@ -33,7 +35,10 @@ rust_binary(
edition = "2024",
target_compatible_with = ["@platforms//os:linux"],
visibility = ["//visibility:private"],
deps = ["@crates//:anyhow"],
deps = [
"@crates//:anyhow",
"@crates//:libc",
],
)
rust_test(
@@ -14,10 +14,11 @@ use anyhow::Result;
/// Namespace classes whose topology is part of this harness's contract.
///
/// The outer bwrap isolates user, mount, IPC, and UTS, but deliberately inherits
/// PID plus writable procfs for nested bwrap setup and network for loopback.
/// Cgroup and time namespaces stay executor-owned; `*_for_children` entries are
/// alternate PID/time handles rather than separate namespace classes.
/// The outer bwrap isolates user, mount, PID, IPC, and UTS, but deliberately
/// inherits the VM's writable procfs for nested bwrap setup and its network for
/// loopback. Cgroup and time namespaces stay executor-owned; `*_for_children`
/// entries are alternate PID/time handles rather than separate namespace
/// classes.
const NAMESPACES: &[&str] = &["user", "mnt", "pid", "ipc", "uts", "net"];
fn main() -> Result<()> {
@@ -26,6 +27,8 @@ fn main() -> Result<()> {
let mode = args.next().context("missing bwrap smoke fixture mode")?;
match mode.to_str() {
Some("wait") => wait(),
Some("spawn-detached-descendant") => spawn_detached_descendant(),
Some("detached-wait") => detached_wait(),
Some("inspect") => inspect(),
Some("filesystem") => {
let tmp_dir = args.next().context("missing tmp directory")?;
@@ -58,6 +61,34 @@ fn wait() -> Result<()> {
}
}
fn spawn_detached_descendant() -> Result<()> {
let _descendant = Command::new(env::current_exe()?)
.arg("detached-wait")
.spawn()
.context("spawn detached descendant")?;
loop {
std::thread::park();
}
}
fn detached_wait() -> Result<()> {
// SAFETY: setsid has no memory-safety preconditions. This fixture is a
// forked child and therefore is not already a process-group leader.
if unsafe { libc::setsid() } == -1 {
return Err(std::io::Error::last_os_error()).context("detach descendant session");
}
let status = fs::read_to_string("/proc/self/status").context("read detached child status")?;
let vm_pid = status_field(&status, "NSpid")?
.split_whitespace()
.next()
.context("detached child status has no VM-visible PID")?;
println!("BWRAP_TEST_DETACHED_DESCENDANT_READY={vm_pid}");
std::io::stdout().flush()?;
loop {
std::thread::park();
}
}
fn inspect() -> Result<()> {
for (key, value) in inspection()? {
println!("{key}={value}");
@@ -156,8 +187,10 @@ fn filesystem(tmp_dir: &Path, workspace_dir: &Path, root_target: &Path) -> Resul
println!("{name}.value={}", fs::read_to_string(&writable_file)?);
}
let error = fs::write(root_target, "must not be written")
.expect_err("writing through the read-only root unexpectedly succeeded");
let error = match fs::write(root_target, "must not be written") {
Ok(()) => anyhow::bail!("writing through the read-only root unexpectedly succeeded"),
Err(error) => error,
};
println!(
"root.write_errno={}",
error
+183 -32
View File
@@ -4,13 +4,19 @@ compile_error!("bwrap_test_support can only run on Linux");
use std::ffi::OsString;
use std::fs;
use std::future::Future;
use std::io;
use std::io::Read;
use std::io::Write;
use std::os::fd::AsRawFd;
use std::os::unix::fs::PermissionsExt;
use std::os::unix::net::UnixStream;
use std::os::unix::process::CommandExt;
use std::path::Path;
use std::path::PathBuf;
use std::process::Command as StdCommand;
use std::process::Stdio;
use std::time::Duration;
use std::time::Instant;
use anyhow::Context;
use anyhow::Result;
@@ -20,6 +26,13 @@ use tokio::process::Child;
use tokio::process::ChildStdout;
use tokio::process::Command as TokioCommand;
use self::sandbox_init::BWRAP_CLEANUP_TIMEOUT;
use self::sandbox_init::SandboxInit;
const BWRAP_SETUP_TIMEOUT: Duration = Duration::from_secs(10);
mod sandbox_init;
/// Builds a command that runs inside the Linux integration-test namespace.
pub struct BwrapTestCommand {
executable: PathBuf,
@@ -27,17 +40,19 @@ pub struct BwrapTestCommand {
env: Vec<(OsString, OsString)>,
}
/// Owns a process running inside the Linux integration-test namespace.
/// Owns a process tree running inside a dedicated PID namespace.
///
/// Call [`Self::scope`] or [`Self::shutdown`] on every successful path. A
/// normal unguarded drop panics, while a drop during unwinding performs
/// blocking cleanup without introducing a second panic.
/// blocking cleanup without introducing a second panic. Teardown signals the
/// namespace init and waits for it, so detached descendants cannot escape.
pub struct BwrapTestProcess {
processes: Option<BwrapProcesses>,
}
struct BwrapProcesses {
child: Child,
sandbox_init: SandboxInit,
cleanup_complete: bool,
environment: TempDir,
}
@@ -109,14 +124,15 @@ impl BwrapTestCommand {
},
)?;
// Firecracker supplies the host PID boundary, so the outer wrapper
// deliberately shares the disposable VM's PID namespace and rebinds
// its procfs read-write. Nested unprivileged bwrap writes UID/GID maps
// through the caller's /proc before mounting a fresh procfs for its
// own PID namespace. Giving the outer wrapper a fresh procfs on this
// executor leaves locked child mounts that make the nested proc mount
// fail Linux's mount_too_revealing check; making the inherited procfs
// read-only instead makes UID/GID map setup fail.
// Firecracker supplies the host PID boundary. The outer wrapper adds a
// child PID namespace so terminating its init kills every descendant,
// but deliberately rebinds the VM's existing procfs read-write. Nested
// unprivileged bwrap writes UID/GID maps through the caller's /proc
// before mounting a fresh procfs for its own PID namespace. Giving the
// outer wrapper a fresh procfs on this executor leaves locked child
// mounts that make the nested proc mount fail Linux's
// mount_too_revealing check; making the inherited procfs read-only
// instead makes UID/GID map setup fail.
//
// A private directory backs /tmp so remote fixtures cannot alias files
// owned by the test runner. The Bazel workspace is the other deliberate
@@ -127,8 +143,25 @@ impl BwrapTestCommand {
// capabilities. Bubblewrap rejects a non-root caller that already has
// capabilities, while an unprivileged nested bwrap acquires the setup
// capabilities it needs inside the user namespace it creates.
let (mut info_reader, info_writer) =
UnixStream::pair().context("create bwrap info pipe")?;
let (block_reader, block_writer) =
UnixStream::pair().context("create bwrap startup gate")?;
info_reader
.set_read_timeout(Some(BWRAP_SETUP_TIMEOUT))
.context("set bwrap info timeout")?;
let info_writer_fd = info_writer.as_raw_fd();
let block_reader_fd = block_reader.as_raw_fd();
// Hold the sandbox at --block-fd before it forks the payload. This
// keeps the reported PID-namespace init alive until its pidfd is open;
// dropping block_writer below releases the sandbox with EOF.
let mut command = StdCommand::new(&runtime.bwrap);
command
.arg("--info-fd")
.arg(info_writer_fd.to_string())
.arg("--block-fd")
.arg(block_reader_fd.to_string())
.args([
"--new-session",
"--die-with-parent",
@@ -147,6 +180,7 @@ impl BwrapTestCommand {
.arg(&workspace)
.args([
"--unshare-user",
"--unshare-pid",
"--unshare-ipc",
"--unshare-uts",
"--uid",
@@ -168,15 +202,57 @@ impl BwrapTestCommand {
.stdout(Stdio::piped())
.stderr(Stdio::inherit());
// SAFETY: only async-signal-safe fcntl calls run between fork and exec.
// The parent retains both descriptors until spawn completes.
unsafe {
command.pre_exec(move || {
for fd in [info_writer_fd, block_reader_fd] {
let flags = libc::fcntl(fd, libc::F_GETFD);
if flags == -1
|| libc::fcntl(fd, libc::F_SETFD, flags & !libc::FD_CLOEXEC) == -1
{
return Err(io::Error::last_os_error());
}
}
Ok(())
});
}
let mut command = TokioCommand::from(command);
command.kill_on_drop(true);
let child = command
let mut child = command
.spawn()
.context("start process inside bwrap test environment")?;
// This binding is intentionally declared after child. On an early
// return it drops first, releasing the startup gate before
// kill_on_drop terminates the launcher. Failures before a pidfd is
// acquired ultimately remain contained by the disposable action VM.
let startup_gate = block_writer;
let launcher_pid = child.id().context("bwrap launcher omitted its PID")?;
drop(info_writer);
drop(block_reader);
let mut bwrap_info = String::new();
let startup_result = info_reader
.read_to_string(&mut bwrap_info)
.context("read bwrap child information")
.and_then(|_| SandboxInit::from_bwrap_info(&bwrap_info, launcher_pid));
drop(startup_gate);
let sandbox_init = match startup_result {
Ok(sandbox_init) => sandbox_init,
Err(error) => {
if let Err(kill_error) = child.start_kill() {
return Err(error.context(format!(
"bwrap startup cleanup could not kill its launcher: {kill_error}"
)));
}
return Err(error);
}
};
Ok(BwrapTestProcess {
processes: Some(BwrapProcesses {
child,
sandbox_init,
cleanup_complete: false,
environment,
}),
@@ -225,7 +301,7 @@ impl BwrapTestProcess {
}
}
/// Kills the isolated process and waits for bwrap to exit.
/// Kills the isolated PID namespace and waits for bwrap to exit.
pub async fn shutdown(mut self) -> Result<()> {
let Some(processes) = self.processes.as_mut() else {
anyhow::bail!("bwrap process guard is missing");
@@ -256,21 +332,57 @@ impl BwrapRuntimePaths {
impl BwrapProcesses {
async fn shutdown(&mut self) -> Result<()> {
let (kill_result, check_exit_status) = match self.child.try_wait() {
Ok(Some(_)) => (Ok(()), true),
Ok(None) => (
self.child
.start_kill()
.context("kill process running inside bwrap"),
false,
),
Err(error) => (Err(error).context("check bwrap process status"), false),
let child_status = self.child.try_wait();
let sandbox_kill_result = self
.sandbox_init
.start_kill()
.context("kill bwrap PID namespace init");
let (mut check_exit_status, status_check_result) = match child_status {
Ok(Some(_)) => (true, Ok(())),
Ok(None) => (false, Ok(())),
Err(error) => (false, Err(error).context("check bwrap process status")),
};
let wait_result = self
.child
let mut fallback_started = false;
let early_fallback_result = if sandbox_kill_result.is_err() {
check_exit_status = false;
fallback_started = true;
self.start_kill_launcher()
} else {
Ok(())
};
let sandbox_wait_result = self
.sandbox_init
.wait()
.await
.context("wait for process running inside bwrap")
.context("wait for bwrap PID namespace init");
let late_fallback_result = if sandbox_wait_result.is_err() && !fallback_started {
check_exit_status = false;
self.start_kill_launcher()
} else {
Ok(())
};
let mut timeout_kill_result = Ok(());
let launcher_timeout_result;
let launcher_wait_result =
match tokio::time::timeout(BWRAP_CLEANUP_TIMEOUT, self.child.wait()).await {
Ok(result) => {
launcher_timeout_result = Ok(());
result.context("wait for bwrap launcher")
}
Err(_) => {
launcher_timeout_result = Err(anyhow::anyhow!(
"timed out waiting for bwrap launcher after {BWRAP_CLEANUP_TIMEOUT:?}"
));
check_exit_status = false;
timeout_kill_result = self.start_kill_launcher();
match tokio::time::timeout(BWRAP_CLEANUP_TIMEOUT, self.child.wait()).await {
Ok(result) => result.context("wait for killed bwrap launcher"),
Err(_) => Err(anyhow::anyhow!(
"timed out waiting for killed bwrap launcher after {BWRAP_CLEANUP_TIMEOUT:?}"
)),
}
}
}
.and_then(|status| {
anyhow::ensure!(
!check_exit_status || status.success(),
@@ -282,8 +394,24 @@ impl BwrapProcesses {
// Every cleanup action has been attempted, so an individual error
// should not cause the blocking fallback to repeat them.
self.cleanup_complete = true;
kill_result?;
wait_result
status_check_result?;
sandbox_kill_result?;
early_fallback_result?;
sandbox_wait_result?;
late_fallback_result?;
launcher_timeout_result?;
timeout_kill_result?;
launcher_wait_result
}
fn start_kill_launcher(&mut self) -> Result<()> {
if let Err(kill_error) = self.child.start_kill() {
match self.child.try_wait() {
Ok(Some(_)) => return Ok(()),
Ok(None) | Err(_) => return Err(kill_error).context("kill bwrap launcher"),
}
}
Ok(())
}
fn shutdown_blocking(&mut self) {
@@ -291,25 +419,48 @@ impl BwrapProcesses {
"bwrap panic cleanup starting for environment {}",
self.environment.path().display()
));
if let Err(error) = self.child.start_kill() {
if let Err(error) = self.sandbox_init.start_kill() {
log_panic_cleanup(format_args!(
"bwrap panic cleanup could not kill its child: {error}"
"bwrap panic cleanup could not kill its PID namespace init: {error}"
));
}
if let Err(error) = self.start_kill_launcher() {
log_panic_cleanup(format_args!(
"bwrap panic cleanup could not kill its launcher: {error}"
));
}
log_panic_cleanup(format_args!("bwrap panic cleanup waiting for its child"));
log_panic_cleanup(format_args!(
"bwrap panic cleanup waiting for its PID namespace init"
));
if let Err(error) = self.sandbox_init.wait_blocking() {
log_panic_cleanup(format_args!(
"bwrap panic cleanup could not wait for its PID namespace init: {error}"
));
}
log_panic_cleanup(format_args!("bwrap panic cleanup waiting for its launcher"));
let deadline = Instant::now() + BWRAP_CLEANUP_TIMEOUT;
loop {
match self.child.try_wait() {
Ok(Some(status)) => {
log_panic_cleanup(format_args!(
"bwrap panic cleanup child exited with {status}"
"bwrap panic cleanup launcher exited with {status}"
));
break;
}
Ok(None) => std::thread::sleep(Duration::from_millis(10)),
Ok(None) => {
if Instant::now() >= deadline {
log_panic_cleanup(format_args!(
"bwrap panic cleanup timed out waiting for its launcher"
));
break;
}
std::thread::sleep(Duration::from_millis(10));
}
Err(error) => {
log_panic_cleanup(format_args!(
"bwrap panic cleanup could not wait for its child: {error}"
"bwrap panic cleanup could not wait for its launcher: {error}"
));
break;
}
+72 -10
View File
@@ -2,6 +2,9 @@ use std::any::Any;
use std::collections::BTreeMap;
use std::fs;
use std::future::Future;
use std::os::fd::AsRawFd;
use std::os::fd::FromRawFd;
use std::os::fd::OwnedFd;
use std::panic::AssertUnwindSafe;
use std::path::Path;
use std::path::PathBuf;
@@ -138,6 +141,61 @@ async fn async_teardown_disarms_drop_bomb_and_cleans_up() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn async_teardown_kills_descendants_in_detached_sessions() -> Result<()> {
let mut process = BwrapTestCommand::new(smoke_fixture()?)
.arg("spawn-detached-descendant")
.spawn()?;
let mut lines = BufReader::new(process.take_stdout()).lines();
let ready_line = timeout(Duration::from_secs(10), lines.next_line())
.await
.context("detached descendant did not become ready")??
.context("detached descendant exited before becoming ready")?;
let descendant_pid: libc::pid_t = ready_line
.strip_prefix("BWRAP_TEST_DETACHED_DESCENDANT_READY=")
.context("invalid detached descendant ready line")?
.parse()
.context("parse detached descendant PID")?;
// SAFETY: pidfd_open does not dereference userspace pointers. The returned
// descriptor is uniquely owned when the syscall succeeds.
let raw_pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, descendant_pid, 0) };
if raw_pidfd == -1 {
return Err(std::io::Error::last_os_error()).context("open detached descendant pidfd");
}
// SAFETY: a successful pidfd_open returns a new owned descriptor.
let descendant_pidfd = unsafe { OwnedFd::from_raw_fd(raw_pidfd as i32) };
process.shutdown().await?;
let next_line = match timeout(Duration::from_secs(10), lines.next_line()).await {
Ok(result) => result?,
Err(timeout_error) => {
// SAFETY: pidfd_send_signal only reads the supplied null siginfo
// pointer, and descendant_pidfd remains owned for the syscall.
let kill_result = unsafe {
libc::syscall(
libc::SYS_pidfd_send_signal,
descendant_pidfd.as_raw_fd(),
libc::SIGKILL,
std::ptr::null::<libc::siginfo_t>(),
0,
)
};
if kill_result == -1 {
let kill_error = std::io::Error::last_os_error();
return Err(anyhow::Error::new(timeout_error).context(format!(
"detached descendant kept stdout open after teardown; \
cleanup kill also failed: {kill_error}"
)));
}
return Err(anyhow::Error::new(timeout_error)
.context("detached descendant kept stdout open after teardown"));
}
};
assert_eq!(next_line, None);
Ok(())
}
#[tokio::test]
async fn scope_returns_value_and_cleans_up() -> Result<()> {
let process = waiting_smoke_process().await?;
@@ -179,13 +237,16 @@ async fn outer_namespace_topology_matches_the_exec_server_contract() -> Result<(
Some(&namespace_identity("user")?),
"outer user namespace should differ from its parent"
);
for namespace in ["pid", "net"] {
assert_eq!(
values.get(&format!("ns.{namespace}")),
Some(&namespace_identity(namespace)?),
"outer {namespace} namespace should deliberately match its parent"
);
}
assert_eq!(
values.get("ns.net"),
Some(&namespace_identity("net")?),
"outer network namespace should deliberately match its parent"
);
assert_ne!(
values.get("ns.pid"),
Some(&namespace_identity("pid")?),
"outer pid namespace should differ from its parent"
);
for namespace in ["mnt", "ipc", "uts"] {
assert_ne!(
values.get(&format!("ns.{namespace}")),
@@ -194,18 +255,19 @@ async fn outer_namespace_topology_matches_the_exec_server_contract() -> Result<(
);
}
if let Some(pid_one_namespace) = values.get("proc.1.pid_ns") {
assert_eq!(Some(pid_one_namespace), values.get("proc.self.pid_ns"));
assert_eq!(pid_one_namespace, &namespace_identity("pid")?);
} else {
assert!(
values.contains_key("proc.1.pid_ns.error"),
"outer procfs should either expose PID 1 or report why it is unreadable"
);
}
assert_eq!(
assert_ne!(
values.get("proc.self.pid_ns"),
Some(&namespace_identity("pid")?),
"outer /proc should deliberately expose the parent PID namespace"
"outer process should run in a private PID namespace"
);
assert_eq!(values.get("proc.self.pid_ns"), values.get("ns.pid"));
for name in ["overflowuid", "overflowgid"] {
let value = values
.get(&format!("proc.{name}"))
@@ -0,0 +1,143 @@
use std::fs;
use std::io;
use std::os::fd::AsRawFd;
use std::os::fd::FromRawFd;
use std::os::fd::OwnedFd;
use std::os::fd::RawFd;
use std::time::Duration;
use std::time::Instant;
use anyhow::Context;
use anyhow::Result;
pub(super) const BWRAP_CLEANUP_TIMEOUT: Duration = Duration::from_secs(10);
pub(super) struct SandboxInit {
pidfd: OwnedFd,
}
impl SandboxInit {
pub(super) fn from_bwrap_info(info: &str, launcher_pid: u32) -> Result<Self> {
let child_pid: libc::pid_t = info
.lines()
.find_map(|line| line.trim().strip_prefix("\"child-pid\":"))
.map(|value| value.trim().trim_end_matches(',').parse())
.transpose()
.context("parse bwrap child PID")?
.context("bwrap child information omitted child-pid")?;
anyhow::ensure!(
child_pid > 0,
"bwrap reported invalid child PID {child_pid}"
);
// SAFETY: pidfd_open does not dereference userspace pointers. The
// returned descriptor is uniquely owned when the syscall succeeds.
let raw_pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, child_pid, 0) };
if raw_pidfd == -1 {
return Err(io::Error::last_os_error()).context("open bwrap child pidfd");
}
// SAFETY: a successful pidfd_open returns a new owned descriptor.
let pidfd = unsafe { OwnedFd::from_raw_fd(raw_pidfd as i32) };
// Validate the numeric PID before retaining the pidfd. If setup failed
// and Linux already reused the PID, it cannot still name the launcher's
// direct child.
let status = fs::read_to_string(format!("/proc/{child_pid}/status"))
.context("read bwrap child status")?;
let parent_pid: u32 = status
.lines()
.find_map(|line| line.strip_prefix("PPid:"))
.map(str::trim)
.context("bwrap child status omitted PPid")?
.parse()
.context("parse bwrap child parent PID")?;
anyhow::ensure!(
parent_pid == launcher_pid,
"bwrap child PID {child_pid} belongs to parent {parent_pid}, not launcher {launcher_pid}"
);
Ok(Self { pidfd })
}
pub(super) fn start_kill(&self) -> io::Result<()> {
// SAFETY: pidfd_send_signal only reads the supplied null siginfo
// pointer, and pidfd remains owned by self for the syscall's duration.
let result = unsafe {
libc::syscall(
libc::SYS_pidfd_send_signal,
self.pidfd.as_raw_fd(),
libc::SIGKILL,
std::ptr::null::<libc::siginfo_t>(),
0,
)
};
if result == -1 {
let error = io::Error::last_os_error();
if error.raw_os_error() != Some(libc::ESRCH) {
return Err(error);
}
}
Ok(())
}
pub(super) async fn wait(&self) -> io::Result<()> {
let pidfd = self.pidfd.try_clone()?;
let wait = tokio::task::spawn_blocking(move || {
wait_for_pidfd_exit(pidfd.as_raw_fd(), BWRAP_CLEANUP_TIMEOUT)
});
match tokio::time::timeout(BWRAP_CLEANUP_TIMEOUT, wait).await {
Ok(result) => result.map_err(io::Error::other)?,
Err(_) => Err(io::Error::new(
io::ErrorKind::TimedOut,
"timed out scheduling wait for bwrap PID namespace init",
)),
}
}
pub(super) fn wait_blocking(&self) -> io::Result<()> {
wait_for_pidfd_exit(self.pidfd.as_raw_fd(), BWRAP_CLEANUP_TIMEOUT)
}
}
fn wait_for_pidfd_exit(pidfd: RawFd, timeout: Duration) -> io::Result<()> {
let deadline = Instant::now() + timeout;
let mut poll_fd = libc::pollfd {
fd: pidfd,
events: libc::POLLIN,
revents: 0,
};
loop {
let remaining = deadline.saturating_duration_since(Instant::now());
if remaining.is_zero() {
return Err(io::Error::new(
io::ErrorKind::TimedOut,
"timed out waiting for bwrap PID namespace init",
));
}
let timeout_millis = remaining.as_millis().clamp(1, i32::MAX as u128) as i32;
poll_fd.revents = 0;
// SAFETY: poll_fd points to one initialized pollfd for the duration of
// the call.
let result = unsafe {
libc::poll(&mut poll_fd, /*nfds*/ 1, timeout_millis)
};
if result == 0 {
return Err(io::Error::new(
io::ErrorKind::TimedOut,
"timed out waiting for bwrap PID namespace init",
));
}
if result == -1 {
let error = io::Error::last_os_error();
if error.kind() == io::ErrorKind::Interrupted {
continue;
}
return Err(error);
}
if poll_fd.revents & libc::POLLIN != 0 {
return Ok(());
}
return Err(io::Error::other(format!(
"unexpected pidfd poll events: {:#x}",
poll_fd.revents
)));
}
}