Skip to content

Commit db3c3d0

Browse files
committed
sandlock-oci: collapse container process group when the main process exits
Signed-off-by: Cong Wang <cwang@multikernel.io>
1 parent 9e89bf1 commit db3c3d0

3 files changed

Lines changed: 175 additions & 3 deletions

File tree

crates/sandlock-oci/src/supervisor.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ async fn serve_running(
391391
Some(w) => w,
392392
None => {
393393
// Cannot watch concurrently: just wait for exit (no serving).
394-
return exit_info_from(sandbox.wait().await);
394+
return reap_and_collapse(sandbox, child_pid).await;
395395
}
396396
};
397397
loop {
@@ -401,7 +401,7 @@ async fn serve_running(
401401
// way collect the status via the sandbox's own pidfd. We return
402402
// immediately, so there is no need to clear readiness.
403403
let _ = ready;
404-
return exit_info_from(sandbox.wait().await);
404+
return reap_and_collapse(sandbox, child_pid).await;
405405
}
406406
conn = listener.accept() => {
407407
match conn {
@@ -414,13 +414,34 @@ async fn serve_running(
414414
}
415415
}
416416
}
417-
Err(_) => return exit_info_from(sandbox.wait().await),
417+
Err(_) => return reap_and_collapse(sandbox, child_pid).await,
418418
}
419419
}
420420
}
421421
}
422422
}
423423

424+
/// Collect the main process's exit status, then collapse its process group.
425+
///
426+
/// sandlock uses no PID namespace, so when the container's main process exits
427+
/// the kernel does not tear down the processes it spawned (background children,
428+
/// and exec'd siblings sharing the group). Send SIGKILL to the whole group so
429+
/// nothing outlives the container with a now-dead supervisor. `child_pid` is the
430+
/// group's pgid (core does `setpgid(0, 0)` in the child); `killpg` reaches any
431+
/// remaining members and is a harmless `ESRCH` when the group is already empty.
432+
/// The `Shutdown` path does not call this because `sandbox.kill()` already
433+
/// SIGKILLs the same process group.
434+
async fn reap_and_collapse(
435+
sandbox: &mut sandlock_core::Sandbox,
436+
child_pid: i32,
437+
) -> Option<crate::state::ExitInfo> {
438+
let info = exit_info_from(sandbox.wait().await);
439+
if child_pid > 0 {
440+
unsafe { libc::killpg(child_pid, libc::SIGKILL) };
441+
}
442+
info
443+
}
444+
424445
/// Run the supervisor in the **current process** for an OCI `restore`.
425446
///
426447
/// Unlike [`run_supervisor`], the policy comes from the checkpoint image (the

crates/sandlock-oci/tests/integration.rs

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,4 +623,118 @@ fn which(prog: &str) -> bool {
623623
std::env::var_os("PATH").map_or(false, |paths| {
624624
std::env::split_paths(&paths).any(|d| d.join(prog).is_file())
625625
})
626+
}
627+
628+
/// Path to the prebuilt static `rootfs-helper` (compiled by sandlock-core's
629+
/// build.rs). It is a self-contained, busybox-style binary the chroot
630+
/// integration tests drop into a rootfs; building `sandlock-oci` pulls in
631+
/// `sandlock-core`, so the binary is available here too.
632+
fn rootfs_helper() -> std::path::PathBuf {
633+
std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../tests/rootfs-helper")
634+
}
635+
636+
/// Regression test for process-group collapse on container stop. sandlock has
637+
/// no PID namespace, so when the container's main process exits the supervisor
638+
/// must explicitly SIGKILL the process group; otherwise background children (or
639+
/// exec'd siblings) outlive the container with a dead supervisor.
640+
///
641+
/// The container's main process (`rootfs-helper spawn-loop`) forks a worker that
642+
/// advances `/child.cnt`, then `pause`s. We confirm the worker is running,
643+
/// `kill` the main process, and assert the worker stops advancing. Without the
644+
/// `reap_and_collapse` fix the orphaned worker keeps writing and this test fails.
645+
#[tokio::test(flavor = "multi_thread")]
646+
async fn oci_stop_collapses_process_group() {
647+
if sandlock_core::landlock_abi_version().is_err() {
648+
eprintln!("skipping: Landlock unavailable on this host");
649+
return;
650+
}
651+
let helper = rootfs_helper();
652+
if !helper.exists() {
653+
eprintln!("skipping: rootfs-helper not built (needs musl-gcc or cc -static)");
654+
return;
655+
}
656+
657+
let tmp = std::env::temp_dir().join(format!("sandlock-oci-pgroup-{}", std::process::id()));
658+
fs::create_dir_all(&tmp).unwrap();
659+
660+
// The container chroots to rootfs, so the worker's in-sandbox path
661+
// `/child.cnt` resolves to `rootfs/child.cnt` on the host. Drop the static
662+
// rootfs-helper into the rootfs and run its `spawn-loop` worker.
663+
let bundle = tmp.join("bundle");
664+
let rootfs = bundle.join("rootfs");
665+
fs::create_dir_all(&rootfs).unwrap();
666+
fs::copy(&helper, rootfs.join("rootfs-helper")).unwrap();
667+
{
668+
use std::os::unix::fs::PermissionsExt;
669+
fs::set_permissions(rootfs.join("rootfs-helper"), fs::Permissions::from_mode(0o755)).unwrap();
670+
}
671+
create_bundle(&bundle, &["/rootfs-helper", "spawn-loop", "/child.cnt"]);
672+
673+
let host_child = rootfs.join("child.cnt");
674+
let host_child_s = host_child.to_str().unwrap().to_string();
675+
let read_counter = |path: &str| -> Option<u64> {
676+
fs::read_to_string(path).ok().and_then(|s| s.trim().parse::<u64>().ok())
677+
};
678+
679+
let root = tempdir().unwrap();
680+
let root_s = root.path().to_str().unwrap().to_string();
681+
let id = "oci-pgroup-e2e";
682+
683+
// create (daemonizes a supervisor that inherits stdio; redirect + .status()).
684+
let create_log = tmp.join("create.log");
685+
let create_status = Command::new(oci_bin())
686+
.args(["--root", &root_s, "create", id, "-b", bundle.to_str().unwrap()])
687+
.stdout(std::process::Stdio::from(fs::File::create(&create_log).unwrap()))
688+
.stderr(std::process::Stdio::from(
689+
fs::OpenOptions::new().append(true).open(&create_log).unwrap(),
690+
))
691+
.status()
692+
.expect("run create");
693+
assert!(create_status.success(), "create failed: {}", fs::read_to_string(&create_log).unwrap_or_default());
694+
695+
let start_out = Command::new(oci_bin())
696+
.args(["--root", &root_s, "start", id])
697+
.output()
698+
.expect("run start");
699+
assert!(start_out.status.success(), "start failed: {}", String::from_utf8_lossy(&start_out.stderr));
700+
701+
// Wait until the forked worker is genuinely running.
702+
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
703+
let mut worker_running = false;
704+
while std::time::Instant::now() < deadline {
705+
if read_counter(&host_child_s).map(|v| v > 2).unwrap_or(false) {
706+
worker_running = true;
707+
break;
708+
}
709+
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
710+
}
711+
712+
// Kill ONLY the main process (default SIGTERM to state.pid, not the group).
713+
// The supervisor's group-collapse is what must take the worker down.
714+
let kill_out = Command::new(oci_bin())
715+
.args(["--root", &root_s, "kill", id, "SIGTERM"])
716+
.output()
717+
.expect("run kill");
718+
let kill_ok = kill_out.status.success();
719+
720+
// Give the supervisor time to observe the exit and collapse the group.
721+
tokio::time::sleep(std::time::Duration::from_millis(600)).await;
722+
let sample_a = read_counter(&host_child_s);
723+
tokio::time::sleep(std::time::Duration::from_millis(400)).await;
724+
let sample_b = read_counter(&host_child_s);
725+
726+
// clean up before asserting so a failure never leaks the worker.
727+
let _ = Command::new(oci_bin())
728+
.args(["--root", &root_s, "delete", id, "--force"])
729+
.output();
730+
let _ = fs::remove_dir_all(&tmp);
731+
732+
assert!(worker_running, "forked worker never started; create_log: {}", fs::read_to_string(&create_log).unwrap_or_default());
733+
assert!(kill_ok, "kill failed: {}", String::from_utf8_lossy(&kill_out.stderr));
734+
assert_eq!(
735+
sample_a, sample_b,
736+
"worker must stop advancing after the container's main process is killed \
737+
(process group was not collapsed); samples {:?} -> {:?}",
738+
sample_a, sample_b
739+
);
626740
}

tests/rootfs-helper.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
#include <string.h>
2020
#include <sys/stat.h>
2121
#include <sys/syscall.h>
22+
#include <sys/types.h>
2223
#include <sys/xattr.h>
24+
#include <time.h>
2325
#include <unistd.h>
2426

2527
/* ── echo ───────────────────────────────────────────────────── */
@@ -545,6 +547,40 @@ static int cmd_legacy_chmod(int argc, char **argv) {
545547
}
546548
#endif
547549

550+
/* ── spawn-loop (non-standard: fork a background worker, then pause) ──────── */
551+
/*
552+
* Models a container whose main process spawns a long-lived background worker.
553+
* The forked child opens <file> once and loops publishing an incrementing
554+
* counter (single fixed-width 21-byte overwrite, never truncating, so a reader
555+
* always sees a complete value); the parent blocks forever in pause(). Used by
556+
* the OCI process-group-collapse test: when the container's main process is
557+
* killed, the worker must be reaped with the group rather than left running.
558+
*/
559+
static int cmd_spawn_loop(int argc, char **argv) {
560+
if (argc < 1) { fprintf(stderr, "spawn-loop: missing file operand\n"); return 1; }
561+
const char *path = argv[0];
562+
pid_t pid = fork();
563+
if (pid < 0) { perror("spawn-loop: fork"); return 1; }
564+
if (pid == 0) {
565+
int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
566+
if (fd < 0) _exit(1);
567+
unsigned long i = 0;
568+
char buf[24];
569+
struct timespec t = { 0, 20000000 };
570+
for (;;) {
571+
i++;
572+
unsigned long v = i;
573+
for (int d = 19; d >= 0; d--) { buf[d] = '0' + (v % 10); v /= 10; }
574+
buf[20] = '\n';
575+
lseek(fd, 0, SEEK_SET);
576+
if (write(fd, buf, 21) < 0) _exit(1);
577+
nanosleep(&t, NULL);
578+
}
579+
}
580+
for (;;) pause();
581+
return 0;
582+
}
583+
548584
/* ── dispatch ───────────────────────────────────────────────── */
549585

550586
static int dispatch(const char *cmd, int argc, char **argv) {
@@ -572,6 +608,7 @@ static int dispatch(const char *cmd, int argc, char **argv) {
572608
if (strcmp(cmd, "setxattr") == 0) return cmd_setxattr(argc, argv);
573609
if (strcmp(cmd, "listxattr") == 0) return cmd_listxattr(argc, argv);
574610
if (strcmp(cmd, "fstat-fd") == 0) return cmd_fstat_fd(argc, argv);
611+
if (strcmp(cmd, "spawn-loop") == 0) return cmd_spawn_loop(argc, argv);
575612
if (strcmp(cmd, "true") == 0) return 0;
576613
if (strcmp(cmd, "false") == 0) return 1;
577614

0 commit comments

Comments
 (0)