From ab9b7cec780bbbab4338b1d31614995be05b7823 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 19:56:49 -0700 Subject: [PATCH 1/7] exec-relay: add the relay program and its args trailer Policy-checked execs need an argv the sandbox cannot change after the supervisor has judged it. Freezing every task with ptrace closes that window today, but it needs ptrace fork tracking to know every task, and ptrace is the one mechanism this project wants to keep out of the exec path. The relay is a freestanding static program with a fresh private address space: the supervisor will exec it in place of the approved program, and it execs the target with the argv carried in a trailer on its own image. This commit adds the program, its wire format with a Rust encoder and test decoder, and the build wiring; nothing uses it yet. Signed-off-by: Cong Wang --- crates/sandlock-core/build.rs | 32 +++ crates/sandlock-core/src/exec_relay/mod.rs | 189 +++++++++++++++ crates/sandlock-core/src/exec_relay/relay.c | 254 ++++++++++++++++++++ crates/sandlock-core/src/lib.rs | 1 + 4 files changed, 476 insertions(+) create mode 100644 crates/sandlock-core/src/exec_relay/mod.rs create mode 100644 crates/sandlock-core/src/exec_relay/relay.c diff --git a/crates/sandlock-core/build.rs b/crates/sandlock-core/build.rs index e23237fd..ac55d75e 100644 --- a/crates/sandlock-core/build.rs +++ b/crates/sandlock-core/build.rs @@ -102,6 +102,38 @@ fn main() { // Emit the path every run (rustc-env is not cached across build-script runs), // whether or not the binary was just (re)built. println!("cargo:rustc-env=RESTORE_STUB_PATH={}", stub_bin.display()); + + // exec-relay: the supervisor execs it in place of every policy-checked + // execve, so unlike the restore stub it is embedded into the crate and + // must build for every target; a missing compiler is a hard error. + let relay_src = manifest_dir.join("src/exec_relay/relay.c"); + let relay_bin = out_dir.join("exec-relay"); + let relay_ccs: &[&str] = if is_riscv64 && !host.starts_with("riscv64") { + &["riscv64-linux-gnu-gcc", "riscv64-unknown-linux-gnu-gcc"] + } else if target.starts_with("aarch64") && !host.starts_with("aarch64") { + &["aarch64-linux-gnu-gcc"] + } else { + &["cc"] + }; + if !build_static( + &relay_src, + &relay_bin, + relay_ccs, + &[ + "-static", + "-nostdlib", + "-no-pie", + "-fPIE", + "-O2", + "-ffreestanding", + "-fno-builtin", + "-fno-tree-loop-distribute-patterns", + "-fno-stack-protector", + ], + ) { + panic!("failed to compile exec-relay for {target}: no working C compiler"); + } + println!("cargo:rustc-env=EXEC_RELAY_PATH={}", relay_bin.display()); } /// Compile `src` to `bin` with the first working compiler in `ccs`, skipping the diff --git a/crates/sandlock-core/src/exec_relay/mod.rs b/crates/sandlock-core/src/exec_relay/mod.rs new file mode 100644 index 00000000..198b81fa --- /dev/null +++ b/crates/sandlock-core/src/exec_relay/mod.rs @@ -0,0 +1,189 @@ +//! Exec relay: argv safety for policy-checked execs without stopping any task. +//! +//! The supervisor reads argv from the child, judges it, and continues the +//! execve; the kernel then copies argv from the same memory, which sibling +//! threads and CLONE_VM peers can rewrite in between. Instead of freezing +//! those tasks, an approved execve is redirected to `relay.c`: the kernel runs +//! it from a sealed memfd, and it execs the target with the argv the policy +//! saw, read from a trailer on its own image. See `relay.c` for the child side. + +use std::io; + +/// The relay program, built by build.rs for the target and embedded so a +/// deployed library never depends on a file beside it. +pub(crate) const RELAY_ELF: &[u8] = include_bytes!(env!("EXEC_RELAY_PATH")); + +/// Marks the config block in `RELAY_ELF`; the same bytes as relay.c's magic. +const CONFIG_MAGIC: u64 = 0x5359_414c_4552_4c53; +const CONFIG_LEN: usize = 32; + +const TRAILER_MAGIC: u32 = 0x5245_4c41; +const TRAILER_VERSION: u32 = 1; +const HEADER_LEN: usize = 40; +pub(crate) const ARGS_MAX: usize = 2 << 20; +pub(crate) const ENTRIES_MAX: usize = 65536; + +/// How the relay reaches the target once it runs. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ArgsMode { + /// `execveat(dirfd, base)` after checking the directory's identity, so + /// swapping a path component after the policy looked has no effect. + Pinned = 0, + /// `execve(full_path)`: scripts, whose interpreter re-opens the path + /// anyway, and COW/chroot modes, whose exec handlers pin the target + /// themselves against the single-threaded relay. + ByPath = 1, +} + +/// Everything the relay needs, serialized as the trailer on its image. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ExecArgs { + pub mode: ArgsMode, + pub dir_dev: u64, + pub dir_ino: u64, + pub dir: Vec, + pub base: Vec, + pub full_path: Vec, + pub argv: Vec>, + pub envp: Vec>, +} + +impl ExecArgs { + pub(crate) fn encode(&self) -> io::Result> { + if self.argv.len() > ENTRIES_MAX || self.envp.len() > ENTRIES_MAX { + return Err(io::Error::from_raw_os_error(libc::E2BIG)); + } + let mut out = Vec::with_capacity(HEADER_LEN + 256); + for v in [TRAILER_MAGIC, TRAILER_VERSION, self.mode as u32, + self.argv.len() as u32, self.envp.len() as u32, 0] { + out.extend_from_slice(&v.to_ne_bytes()); + } + out.extend_from_slice(&self.dir_dev.to_ne_bytes()); + out.extend_from_slice(&self.dir_ino.to_ne_bytes()); + let strings = [&self.dir, &self.base, &self.full_path] + .into_iter() + .chain(self.argv.iter()) + .chain(self.envp.iter()); + for s in strings { + if s.contains(&0) { + return Err(io::Error::from_raw_os_error(libc::EINVAL)); + } + out.extend_from_slice(s); + out.push(0); + } + if out.len() > ARGS_MAX { + return Err(io::Error::from_raw_os_error(libc::E2BIG)); + } + Ok(out) + } + + /// Mirror of relay.c's reader, kept so the tests pin the wire format. + #[cfg(test)] + pub(crate) fn decode(bytes: &[u8]) -> Option { + if bytes.len() < HEADER_LEN { + return None; + } + let u32_at = |i: usize| u32::from_ne_bytes(bytes[i..i + 4].try_into().unwrap()); + let u64_at = |i: usize| u64::from_ne_bytes(bytes[i..i + 8].try_into().unwrap()); + if u32_at(0) != TRAILER_MAGIC || u32_at(4) != TRAILER_VERSION { + return None; + } + let mode = match u32_at(8) { + 0 => ArgsMode::Pinned, + 1 => ArgsMode::ByPath, + _ => return None, + }; + let (argc, envc) = (u32_at(12) as usize, u32_at(16) as usize); + let mut pos = HEADER_LEN; + let mut next = || { + let end = bytes[pos..].iter().position(|&b| b == 0)? + pos; + let s = bytes[pos..end].to_vec(); + pos = end + 1; + Some(s) + }; + let dir = next()?; + let base = next()?; + let full_path = next()?; + let argv = (0..argc).map(|_| next()).collect::>>()?; + let envp = (0..envc).map(|_| next()).collect::>>()?; + Some(Self { mode, dir_dev: u64_at(24), dir_ino: u64_at(32), dir, base, full_path, argv, envp }) + } +} + +/// The relay image for one exec: `RELAY_ELF` with the config block pointing +/// at the trailer appended after it, to be read back through `fd`. +pub(crate) fn build_image(args: &ExecArgs, fd: i32) -> io::Result> { + let trailer = args.encode()?; + let mut image = RELAY_ELF.to_vec(); + let at = image + .windows(8) + .position(|w| w == CONFIG_MAGIC.to_ne_bytes()) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "exec-relay image has no config block"))?; + let trailer_off = image.len() as u64; + let trailer_len = trailer.len() as u64; + let block = &mut image[at..at + CONFIG_LEN]; + block[8..12].copy_from_slice(&(fd as u32).to_ne_bytes()); + block[16..24].copy_from_slice(&trailer_off.to_ne_bytes()); + block[24..32].copy_from_slice(&trailer_len.to_ne_bytes()); + image.extend_from_slice(&trailer); + Ok(image) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample(mode: ArgsMode) -> ExecArgs { + ExecArgs { + mode, + dir_dev: 0x1234, + dir_ino: 0x5678, + dir: b"/usr/bin".to_vec(), + base: b"echo".to_vec(), + full_path: b"/usr/bin/echo".to_vec(), + argv: vec![b"echo".to_vec(), b"hello world".to_vec(), Vec::new()], + envp: vec![b"PATH=/usr/bin".to_vec()], + } + } + + #[test] + fn trailer_roundtrips_both_modes() { + for mode in [ArgsMode::Pinned, ArgsMode::ByPath] { + let args = sample(mode); + assert_eq!(ExecArgs::decode(&args.encode().unwrap()), Some(args)); + } + } + + #[test] + fn decode_rejects_bad_magic_and_truncation() { + let mut bytes = sample(ArgsMode::Pinned).encode().unwrap(); + assert!(ExecArgs::decode(&bytes[..bytes.len() - 1]).is_none()); + bytes[0] ^= 1; + assert!(ExecArgs::decode(&bytes).is_none()); + } + + #[test] + fn encode_rejects_embedded_nul_and_too_many_entries() { + let mut args = sample(ArgsMode::Pinned); + args.argv.push(b"a\0b".to_vec()); + assert_eq!(args.encode().unwrap_err().raw_os_error(), Some(libc::EINVAL)); + let mut args = sample(ArgsMode::Pinned); + args.envp = vec![Vec::new(); ENTRIES_MAX + 1]; + assert_eq!(args.encode().unwrap_err().raw_os_error(), Some(libc::E2BIG)); + } + + #[test] + fn build_image_patches_the_config_block_and_appends_the_trailer() { + let args = sample(ArgsMode::ByPath); + let image = build_image(&args, 1023).unwrap(); + assert_eq!(&image[..4], b"\x7fELF"); + let at = image.windows(8).position(|w| w == CONFIG_MAGIC.to_ne_bytes()).unwrap(); + let block = &image[at..at + CONFIG_LEN]; + assert_eq!(u32::from_ne_bytes(block[8..12].try_into().unwrap()), 1023); + let off = u64::from_ne_bytes(block[16..24].try_into().unwrap()) as usize; + let len = u64::from_ne_bytes(block[24..32].try_into().unwrap()) as usize; + assert_eq!(off, RELAY_ELF.len()); + assert_eq!(ExecArgs::decode(&image[off..off + len]), Some(args)); + assert_eq!(RELAY_ELF.windows(8).filter(|w| *w == CONFIG_MAGIC.to_ne_bytes()).count(), 1); + } +} diff --git a/crates/sandlock-core/src/exec_relay/relay.c b/crates/sandlock-core/src/exec_relay/relay.c new file mode 100644 index 00000000..34af0e57 --- /dev/null +++ b/crates/sandlock-core/src/exec_relay/relay.c @@ -0,0 +1,254 @@ +/* + * exec-relay: freestanding program the supervisor execs in place of an + * approved execve (x86_64, aarch64, riscv64). + * + * The supervisor cannot stop sandbox tasks from rewriting argv between its + * policy check and the kernel's copy, so instead of continuing the original + * execve it runs this program, which has a fresh private address space. The + * argv and envp the policy approved travel in a trailer appended to this + * program's own image, reachable through the fd the supervisor pinned; the + * relay execs the real target with exactly that. + * + * Exit code 127 on any failure, with one line on stderr. + */ +typedef unsigned long u64; +typedef unsigned int u32; +typedef long i64; +typedef unsigned char u8; + +#if defined(__x86_64__) +#define SYS_write 1 +#define SYS_close 3 +#define SYS_fstat 5 +#define SYS_pread64 17 +#define SYS_execve 59 +#define SYS_exit_group 231 +#define SYS_openat 257 +#define SYS_execveat 322 +#define O_DIRECTORY 0200000 +static i64 sc6(long n, u64 a, u64 b, u64 c, u64 d, u64 e, u64 f) { + i64 r; + register u64 r10 __asm__("r10") = d; + register u64 r8 __asm__("r8") = e; + register u64 r9 __asm__("r9") = f; + __asm__ volatile("syscall" : "=a"(r) + : "a"(n), "D"(a), "S"(b), "d"(c), "r"(r10), "r"(r8), "r"(r9) + : "rcx", "r11", "memory"); + return r; +} +#elif defined(__aarch64__) || (defined(__riscv) && __riscv_xlen == 64) +#define SYS_openat 56 +#define SYS_close 57 +#define SYS_write 64 +#define SYS_pread64 67 +#define SYS_fstat 80 +#define SYS_exit_group 94 +#define SYS_execve 221 +#define SYS_execveat 281 +#define O_DIRECTORY 040000 +#if defined(__aarch64__) +static i64 sc6(long n, u64 a, u64 b, u64 c, u64 d, u64 e, u64 f) { + register long x8 __asm__("x8") = n; + register u64 x0 __asm__("x0") = a; + register u64 x1 __asm__("x1") = b; + register u64 x2 __asm__("x2") = c; + register u64 x3 __asm__("x3") = d; + register u64 x4 __asm__("x4") = e; + register u64 x5 __asm__("x5") = f; + __asm__ volatile("svc 0" : "+r"(x0) + : "r"(x1), "r"(x2), "r"(x3), "r"(x4), "r"(x5), "r"(x8) + : "memory"); + return (i64)x0; +} +#else +static i64 sc6(long n, u64 a, u64 b, u64 c, u64 d, u64 e, u64 f) { + register long nr __asm__("a7") = n; + register u64 a0 __asm__("a0") = a; + register u64 a1 __asm__("a1") = b; + register u64 a2 __asm__("a2") = c; + register u64 a3 __asm__("a3") = d; + register u64 a4 __asm__("a4") = e; + register u64 a5 __asm__("a5") = f; + __asm__ volatile("ecall" : "+r"(a0) + : "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5), "r"(nr) + : "memory"); + return (i64)a0; +} +#endif +#else +#error "unsupported architecture" +#endif + +#define SC1(n,a) sc6(n,(u64)(a),0,0,0,0,0) +#define SC2(n,a,b) sc6(n,(u64)(a),(u64)(b),0,0,0,0) +#define SC3(n,a,b,c) sc6(n,(u64)(a),(u64)(b),(u64)(c),0,0,0) +#define SC4(n,a,b,c,d) sc6(n,(u64)(a),(u64)(b),(u64)(c),(u64)(d),0,0) +#define SC5(n,a,b,c,d,e) sc6(n,(u64)(a),(u64)(b),(u64)(c),(u64)(d),(u64)(e),0) + +#define O_RDONLY 0 +#define O_CLOEXEC 02000000 +#define O_PATH 010000000 +#define AT_FDCWD (-100) + +#define TRAILER_MAGIC 0x52454c41u +#define TRAILER_VERSION 1u +#define ARGS_MAX (2u << 20) +#define ENTRIES_MAX 65536u + +/* Patched by the supervisor in the image it builds for each exec; found in + * the ELF bytes by the magic. Volatile so the initial values are never folded. */ +struct config { + u64 magic; + u32 fd; + u32 reserved; + u64 trailer_off; + u64 trailer_len; +}; +__attribute__((used, section(".data"))) +static volatile struct config config = { 0x5359414c45524c53ULL, 0, 0, 0, 0 }; + +struct hdr { + u32 magic, version, mode, argc, envc, reserved; + u64 dir_dev, dir_ino; +}; + +static u8 args_buf[ARGS_MAX]; +static char *ptrs[2 * ENTRIES_MAX + 2]; + +/* GCC may still emit these for aggregate copies even with -fno-builtin. */ +__attribute__((used)) void *memset(void *s, int c, unsigned long n) { + u8 *p = s; + while (n--) *p++ = (u8)c; + return s; +} +__attribute__((used)) void *memcpy(void *d, const void *s, unsigned long n) { + u8 *dp = d; + const u8 *sp = s; + while (n--) *dp++ = *sp++; + return d; +} + +static unsigned long slen(const char *s) { + unsigned long n = 0; + while (s[n]) n++; + return n; +} + +static void put(const char *s) { + SC3(SYS_write, 2, s, slen(s)); +} + +static void put_num(u64 v) { + char b[24]; + int i = 23; + b[i] = 0; + if (v == 0) b[--i] = '0'; + while (v) { b[--i] = (char)('0' + v % 10); v /= 10; } + put(b + i); +} + +static void die(const char *what, i64 ret) { + put("sandlock exec-relay: "); + put(what); + if (ret < 0) { + put(": errno "); + put_num((u64)(-ret)); + } + put("\n"); + SC1(SYS_exit_group, 127); + for (;;) {} +} + +/* Advance past one NUL-terminated string inside [p, end); NULL if unterminated. */ +static const u8 *skip_str(const u8 *p, const u8 *end) { + while (p < end && *p) p++; + return p < end ? p + 1 : 0; +} + +__attribute__((used, noinline)) +static void relay_main(void) { + u64 len = config.trailer_len; + u64 off = config.trailer_off; + int fd = (int)config.fd; + if (len < sizeof(struct hdr) || len > ARGS_MAX) die("trailer size", 0); + + u64 got = 0; + while (got < len) { + i64 r = SC4(SYS_pread64, fd, args_buf + got, len - got, off + got); + if (r <= 0) die("read trailer", r); + got += (u64)r; + } + SC1(SYS_close, fd); + + struct hdr h; + memcpy(&h, args_buf, sizeof h); + if (h.magic != TRAILER_MAGIC || h.version != TRAILER_VERSION) die("trailer format", 0); + if (h.argc > ENTRIES_MAX || h.envc > ENTRIES_MAX) die("trailer entries", 0); + + const u8 *end = args_buf + len; + const u8 *p = args_buf + sizeof h; + const char *dir = (const char *)p; + if (!(p = skip_str(p, end))) die("trailer format", 0); + const char *base = (const char *)p; + if (!(p = skip_str(p, end))) die("trailer format", 0); + const char *full = (const char *)p; + if (!(p = skip_str(p, end))) die("trailer format", 0); + + u32 i; + char **argv = ptrs; + for (i = 0; i < h.argc; i++) { + argv[i] = (char *)p; + if (!(p = skip_str(p, end))) die("trailer format", 0); + } + argv[h.argc] = 0; + char **envp = ptrs + h.argc + 1; + for (i = 0; i < h.envc; i++) { + envp[i] = (char *)p; + if (!(p = skip_str(p, end))) die("trailer format", 0); + } + envp[h.envc] = 0; + + i64 r; + if (h.mode == 0) { + i64 dirfd = SC4(SYS_openat, AT_FDCWD, dir, O_PATH | O_DIRECTORY | O_CLOEXEC, 0); + if (dirfd < 0) die("open target directory", dirfd); + /* st_dev and st_ino are the first two u64 of struct stat on all three arches. */ + u64 st[18]; + r = SC2(SYS_fstat, dirfd, st); + if (r < 0) die("stat target directory", r); + if (st[0] != h.dir_dev || st[1] != h.dir_ino) die("target directory changed", 0); + r = SC5(SYS_execveat, dirfd, base, argv, envp, 0); + } else { + r = SC3(SYS_execve, full, argv, envp); + } + die("exec", r); +} + +#if defined(__x86_64__) +__asm__( + ".global _start\n" + "_start:\n" + " xor %rbp, %rbp\n" + " and $-16, %rsp\n" + " call relay_main\n" + " hlt\n" +); +#elif defined(__aarch64__) +__asm__( + ".global _start\n" + "_start:\n" + " mov x29, #0\n" + " mov x30, #0\n" + " bl relay_main\n" + " brk #0\n" +); +#else +__asm__( + ".global _start\n" + "_start:\n" + " li fp, 0\n" + " li ra, 0\n" + " call relay_main\n" + " ebreak\n" +); +#endif diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 2fa3cbfd..a150710b 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -22,6 +22,7 @@ pub(crate) mod cow; pub mod recovery; pub(crate) mod checkpoint; pub(crate) mod freeze; +pub(crate) mod exec_relay; pub mod netlink; pub(crate) mod procfs; pub(crate) mod port_remap; From 7a476decca89ae31cc7e8047e199c2c25b22a617 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 20:11:03 -0700 Subject: [PATCH 2/7] exec-relay: run approved execs through a pinned memfd relay With a policy_fn or an execve extra handler active, the argv the supervisor judged could be rewritten by a sibling thread or CLONE_VM peer before the kernel copied it. Instead of freezing every sandbox task with ptrace, an approved execve now runs the relay: its image and the judged argv go into a sealed memfd installed at a free fd just below the child's soft RLIMIT_NOFILE, the soft limit is pinned to that number so nothing in the sandbox can put another file there, and the child's path is rewritten to /dev/fd/K. The relay execs the target with exactly the judged argv, pinning the parent directory for ELF targets and going by path for scripts and under COW or chroot, where the existing exec handlers resolve the target against the now single-threaded relay. Targets the kernel would refuse fail the caller's execve with the same errno first, so execvp's PATH walk still works. The relay's own execve is recognised by the caller's exe inode and passes through without a second policy event; a clone sharing the fd table without CLONE_THREAD is refused because its own limit would not pin the fd. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/dispatch.rs | 2 +- crates/sandlock-core/src/exec_relay/mod.rs | 380 ++++++++++++++++++ crates/sandlock-core/src/resource.rs | 14 +- crates/sandlock-core/src/sandbox.rs | 3 +- crates/sandlock-core/src/seccomp/ctx.rs | 2 + crates/sandlock-core/src/seccomp/dispatch.rs | 41 +- crates/sandlock-core/src/seccomp/notif.rs | 88 +++- crates/sandlock-core/src/sys/structs.rs | 2 + crates/sandlock-core/tests/integration.rs | 3 + .../tests/integration/test_exec_relay.rs | 215 ++++++++++ crates/sandlock-ffi/tests/handler_smoke.rs | 12 +- tests/rootfs-helper.c | 36 ++ 12 files changed, 773 insertions(+), 25 deletions(-) create mode 100644 crates/sandlock-core/tests/integration/test_exec_relay.rs diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index e845ac99..dfe65bde 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -139,7 +139,7 @@ fn resolve_at_path_with_virtual( } } -fn map_cow_upper_path(cow: &SeccompCowBranch, path: &str) -> String { +pub(crate) fn map_cow_upper_path(cow: &SeccompCowBranch, path: &str) -> String { let path = PathBuf::from(path); if let Ok(rel) = path.strip_prefix(cow.upper_dir()) { return normalize_path(cow.workdir().join(rel)).to_string_lossy().into_owned(); diff --git a/crates/sandlock-core/src/exec_relay/mod.rs b/crates/sandlock-core/src/exec_relay/mod.rs index 198b81fa..c3d84015 100644 --- a/crates/sandlock-core/src/exec_relay/mod.rs +++ b/crates/sandlock-core/src/exec_relay/mod.rs @@ -6,8 +6,29 @@ //! those tasks, an approved execve is redirected to `relay.c`: the kernel runs //! it from a sealed memfd, and it execs the target with the argv the policy //! saw, read from a trailer on its own image. See `relay.c` for the child side. +//! +//! The memfd is installed at a free fd K just below the child's soft +//! RLIMIT_NOFILE and the soft limit is then set to K until the relay runs: +//! no dup2, open, F_DUPFD, SCM_RIGHTS or pidfd_getfd in the sandbox can +//! place a different file at K, so the sibling that could rewrite argv +//! cannot swap the program either. The child's path is rewritten in place +//! to `/dev/fd/K`, kept short because the bytes after a short path are +//! often the argv pointer array, which cannot move. +use std::collections::HashMap; +use std::ffi::OsStr; use std::io; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; +use std::os::unix::ffi::OsStrExt; +use std::os::unix::fs::MetadataExt; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +use crate::seccomp::ctx::SupervisorCtx; +use crate::seccomp::notif::{read_child_mem, read_exec_cstr, read_exec_ptr_array, rewrite_exec_path}; +use crate::seccomp::state::read_tgid_of_tid; +use crate::sys::structs::{SeccompNotif, SeccompNotifAddfd, SECCOMP_ADDFD_FLAG_SETFD, SECCOMP_IOCTL_NOTIF_ADDFD}; +use std::os::unix::io::RawFd; /// The relay program, built by build.rs for the target and embedded so a /// deployed library never depends on a file beside it. @@ -129,6 +150,359 @@ pub(crate) fn build_image(args: &ExecArgs, fd: i32) -> io::Result> { Ok(image) } +// ============================================================ +// Supervisor side +// ============================================================ + +/// One execve/execveat as the child issued it, read once from its memory. +/// This copy is what the policy judges and what the relay runs. +pub(crate) struct ExecRequest { + pub path: Vec, + pub argv: Vec>, + pub envp: Vec>, + pub path_ptr: u64, + pub argv_ptr: u64, + pub envp_ptr: u64, + /// The target as the policy event reports it: host path, or the virtual + /// path under chroot. + pub resolved: PathBuf, +} + +impl ExecRequest { + pub(crate) fn argv_strings(&self) -> Vec { + self.argv.iter().map(|a| String::from_utf8_lossy(a).into_owned()).collect() + } +} + +struct Hold { + memfd_ident: (u64, u64), + old_soft: u64, +} + +/// Per-tgid record of a relay exec in flight, from commit until the relay's +/// own execve arrives (or the process shows up again without it). +#[derive(Default)] +pub struct RelayState { + holds: Mutex>, +} + +pub(crate) enum Prepared { + /// The relay's own execve: already judged, let the exec handlers run it. + SecondExec, + /// A fresh application exec, judged next and relayed on allow. + First(PendingExec), +} + +pub(crate) struct PendingExec { + pub request: ExecRequest, + args: ExecArgs, + tgid: i32, +} + +fn errno_of(e: &io::Error) -> i32 { + e.raw_os_error().unwrap_or(libc::EIO) +} + +fn ident_of(path: &Path) -> Option<(u64, u64)> { + std::fs::metadata(path).ok().map(|m| (m.dev(), m.ino())) +} + +fn nofile_limits(pid: i32) -> io::Result<(u64, u64)> { + let mut old = libc::rlimit64 { rlim_cur: 0, rlim_max: 0 }; + let r = unsafe { libc::prlimit64(pid, libc::RLIMIT_NOFILE, std::ptr::null(), &mut old) }; + if r != 0 { + return Err(io::Error::last_os_error()); + } + Ok((old.rlim_cur, old.rlim_max)) +} + +fn set_soft_nofile(pid: i32, soft: u64, hard: u64) -> io::Result<()> { + let new = libc::rlimit64 { rlim_cur: soft, rlim_max: hard }; + let r = unsafe { libc::prlimit64(pid, libc::RLIMIT_NOFILE, &new, std::ptr::null_mut()) }; + if r != 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) +} + +/// Step one of a policy-checked exec: recognise the relay's own execve, or +/// read the request and resolve the target the way the kernel would, so a +/// target that cannot run fails the caller's execve with the kernel's errno +/// (execvp's PATH walk depends on ENOENT arriving here). +pub(crate) async fn prepare( + notif: &SeccompNotif, + notif_fd: RawFd, + ctx: &Arc, +) -> Result { + let pid = notif.pid as i32; + let tgid = read_tgid_of_tid(pid).unwrap_or(pid); + + let hold = ctx.exec_relay.holds.lock().unwrap().remove(&tgid); + if let Some(hold) = hold { + let ours = ident_of(Path::new(&format!("/proc/{pid}/exe"))) == Some(hold.memfd_ident); + if let Ok((_, hard)) = nofile_limits(tgid) { + let _ = set_soft_nofile(tgid, hold.old_soft.min(hard), hard); + } + if ours { + return Ok(Prepared::SecondExec); + } + } + + let request = read_request(notif, notif_fd)?; + let (request, args) = resolve_target(notif, request, ctx).await?; + Ok(Prepared::First(PendingExec { request, args, tgid })) +} + +fn read_request(notif: &SeccompNotif, notif_fd: RawFd) -> Result { + let nr = notif.data.nr as i64; + let a = ¬if.data.args; + let (path_ptr, argv_ptr, envp_ptr) = if nr == libc::SYS_execveat { + (a[1], a[2], a[3]) + } else { + (a[0], a[1], a[2]) + }; + let mut read = |addr: u64, len: usize| read_child_mem(notif_fd, notif.id, notif.pid, addr, len); + let path = read_exec_cstr(&mut read, path_ptr).map_err(|_| libc::EFAULT)?; + let mut strings = |base: u64| -> Result>, i32> { + let ptrs = read_exec_ptr_array(&mut read, base).map_err(|_| libc::EFAULT)?; + if ptrs.len() > ENTRIES_MAX { + return Err(libc::E2BIG); + } + ptrs.iter() + .map(|&p| read_exec_cstr(&mut read, p).map_err(|_| libc::E2BIG)) + .collect() + }; + let argv = strings(argv_ptr)?; + let envp = strings(envp_ptr)?; + Ok(ExecRequest { path, argv, envp, path_ptr, argv_ptr, envp_ptr, resolved: PathBuf::new() }) +} + +fn normalize(path: &Path) -> PathBuf { + let mut out = PathBuf::from("/"); + for c in path.components() { + match c { + std::path::Component::ParentDir => { out.pop(); } + std::path::Component::Normal(n) => out.push(n), + _ => {} + } + } + out +} + +/// The target as an absolute path in the child's view, from its cwd or the +/// execveat dirfd. +fn absolute_target(notif: &SeccompNotif, request: &ExecRequest, ctx: &SupervisorCtx) -> Result { + let nr = notif.data.nr as i64; + let (dirfd, flags) = if nr == libc::SYS_execveat { + (notif.data.args[0] as i64 as i32, notif.data.args[4] as i32) + } else { + (libc::AT_FDCWD, 0) + }; + let pid = notif.pid; + let rel = Path::new(OsStr::from_bytes(&request.path)); + if request.path.is_empty() { + if flags & libc::AT_EMPTY_PATH == 0 { + return Err(libc::ENOENT); + } + let target = std::fs::read_link(format!("/proc/{pid}/fd/{dirfd}")).map_err(|_| libc::EBADF)?; + if !target.is_absolute() { + return Err(libc::ENOENT); + } + return Ok(target); + } + if rel.is_absolute() { + return Ok(normalize(rel)); + } + let base = if dirfd == libc::AT_FDCWD { + ctx.processes + .virtual_cwd(pid as i32) + .or_else(|| std::fs::read_link(format!("/proc/{pid}/cwd")).ok()) + .ok_or(libc::ENOENT)? + } else { + std::fs::read_link(format!("/proc/{pid}/fd/{dirfd}")).map_err(|_| libc::EBADF)? + }; + Ok(normalize(&base.join(rel))) +} + +fn stat_errno(e: io::Error) -> i32 { + match e.raw_os_error() { + Some(n) if n == libc::ENOENT || n == libc::ENOTDIR || n == libc::ELOOP || n == libc::ENAMETOOLONG => n, + _ => libc::EACCES, + } +} + +/// Check the target exists and may run, and decide how the relay reaches it. +async fn resolve_target( + notif: &SeccompNotif, + mut request: ExecRequest, + ctx: &Arc, +) -> Result<(ExecRequest, ExecArgs), i32> { + let target = absolute_target(notif, &request, ctx)?; + let policy = &ctx.policy; + let bytes = |p: &Path| p.as_os_str().as_bytes().to_vec(); + let by_path = |host: &Path, reported: PathBuf, request: &mut ExecRequest| { + request.resolved = reported; + ExecArgs { + mode: ArgsMode::ByPath, + dir_dev: 0, + dir_ino: 0, + dir: Vec::new(), + base: Vec::new(), + full_path: bytes(host), + argv: request.argv.clone(), + envp: request.envp.clone(), + } + }; + + if let Some(root) = policy.chroot_root.as_deref() { + let host = crate::sandbox::resolve_sandbox_path_to_host(&target, Some(root), &policy.chroot_mounts); + std::fs::metadata(&host).map_err(stat_errno)?; + // The chroot exec handler resolves the virtual path itself when the + // relay execs it, so the relay hands it the path the child used. + let args = by_path(&target, target.clone(), &mut request); + return Ok((request, args)); + } + + let host = { + let st = ctx.cow.lock().await; + match st.branch.as_ref() { + Some(cow) if cow.has_changes() => { + let upper = crate::cow::dispatch::map_cow_upper_path(cow, &target.to_string_lossy()); + if cow.matches(&upper) { + match cow.handle_stat(&upper) { + Some(real) => Some(real), + None => return Err(libc::ENOENT), + } + } else { + None + } + } + _ => None, + } + }; + if let Some(real) = host { + std::fs::metadata(&real).map_err(stat_errno)?; + let args = by_path(&target, target.clone(), &mut request); + return Ok((request, args)); + } + + let meta = std::fs::metadata(&target).map_err(stat_errno)?; + if !meta.is_file() { + return Err(libc::EACCES); + } + let c_target = std::ffi::CString::new(bytes(&target)).map_err(|_| libc::EINVAL)?; + let executable = unsafe { libc::faccessat(libc::AT_FDCWD, c_target.as_ptr(), libc::X_OK, libc::AT_EACCESS) } == 0; + if !executable { + return Err(libc::EACCES); + } + let mut head = [0u8; 2]; + let is_script = std::fs::File::open(&target) + .and_then(|mut f| { use std::io::Read; f.read(&mut head) }) + .map(|n| n == 2 && &head == b"#!") + .unwrap_or(false); + if is_script { + let args = by_path(&target, target.clone(), &mut request); + return Ok((request, args)); + } + let dir = target.parent().ok_or(libc::ENOENT)?; + let (dir_dev, dir_ino) = ident_of(dir).ok_or(libc::ENOENT)?; + request.resolved = target.clone(); + let args = ExecArgs { + mode: ArgsMode::Pinned, + dir_dev, + dir_ino, + dir: bytes(dir), + base: bytes(Path::new(target.file_name().ok_or(libc::ENOENT)?)), + full_path: bytes(&target), + argv: request.argv.clone(), + envp: request.envp.clone(), + }; + Ok((request, args)) +} + +/// `/dev/fd` when the host has it (a symlink to /proc/self/fd), for the +/// shorter rewrite; else the procfs path. +fn fd_dir() -> &'static str { + static DIR: std::sync::OnceLock<&'static str> = std::sync::OnceLock::new(); + DIR.get_or_init(|| { + if std::fs::read_link("/dev/fd").is_ok() { "/dev/fd" } else { "/proc/self/fd" } + }) +} + +fn sealed_memfd(image: &[u8]) -> io::Result { + let fd = crate::sys::syscall::memfd_create( + "sandlock-exec-relay", + (libc::MFD_CLOEXEC | libc::MFD_ALLOW_SEALING) as u32, + )?; + { + use std::io::Write; + let mut file = std::mem::ManuallyDrop::new(unsafe { std::fs::File::from_raw_fd(fd.as_raw_fd()) }); + file.write_all(image)?; + } + let seals = libc::F_SEAL_SEAL | libc::F_SEAL_WRITE | libc::F_SEAL_GROW | libc::F_SEAL_SHRINK; + if unsafe { libc::fcntl(fd.as_raw_fd(), libc::F_ADD_SEALS, seals) } != 0 { + return Err(io::Error::last_os_error()); + } + Ok(fd) +} + +/// Step two, after the policy allowed: install the relay at a pinned fd and +/// point the child's execve at it. +pub(crate) fn commit(pending: PendingExec, notif: &SeccompNotif, notif_fd: RawFd, ctx: &Arc) -> Result<(), i32> { + let pid = notif.pid as i32; + let PendingExec { request, args, tgid } = pending; + let (soft, hard) = nofile_limits(tgid).map_err(|e| errno_of(&e))?; + // Seven digits keep "/dev/fd/K" within 16 bytes; a free slot just below + // the soft limit is almost never in use. + let top = soft.min(hard).min(10_000_000); + if top < 32 { + return Err(libc::EAGAIN); + } + let k = (top - 16..top) + .rev() + .find(|k| std::fs::symlink_metadata(format!("/proc/{pid}/fd/{k}")).is_err()) + .ok_or(libc::EAGAIN)?; + let k_link = format!("/proc/{pid}/fd/{k}"); + let image = build_image(&args, k as i32).map_err(|e| errno_of(&e))?; + let memfd = sealed_memfd(&image).map_err(|e| errno_of(&e))?; + let ident = std::fs::metadata(format!("/proc/self/fd/{}", memfd.as_raw_fd())) + .map(|m| (m.dev(), m.ino())) + .map_err(|e| errno_of(&e))?; + + let restore = || { let _ = set_soft_nofile(tgid, soft, hard); }; + let addfd = SeccompNotifAddfd { + id: notif.id, + flags: SECCOMP_ADDFD_FLAG_SETFD, + srcfd: memfd.as_raw_fd() as u32, + newfd: k as u32, + newfd_flags: 0, + }; + let installed = unsafe { libc::ioctl(notif_fd, SECCOMP_IOCTL_NOTIF_ADDFD as libc::Ioctl, &addfd as *const _) }; + if installed < 0 { + restore(); + return Err(libc::EAGAIN); + } + if let Err(e) = set_soft_nofile(tgid, k, hard) { + restore(); + return Err(errno_of(&e)); + } + // Nothing in the sandbox can change fd K from here on, so this check + // settles what the kernel will open. + if ident_of(Path::new(&k_link)) != Some(ident) { + restore(); + return Err(libc::EAGAIN); + } + let new_path = format!("{}/{k}\0", fd_dir()); + if rewrite_exec_path( + notif_fd, notif.id, notif.pid, request.path_ptr, request.argv_ptr, request.envp_ptr, new_path.as_bytes(), + ).is_err() { + restore(); + return Err(libc::EFAULT); + } + ctx.exec_relay.holds.lock().unwrap().insert(tgid, Hold { memfd_ident: ident, old_soft: soft }); + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -186,4 +560,10 @@ mod tests { assert_eq!(ExecArgs::decode(&image[off..off + len]), Some(args)); assert_eq!(RELAY_ELF.windows(8).filter(|w| *w == CONFIG_MAGIC.to_ne_bytes()).count(), 1); } + + #[test] + fn normalize_collapses_dots_and_parents() { + assert_eq!(normalize(Path::new("/usr/./bin/../bin/echo")), PathBuf::from("/usr/bin/echo")); + assert_eq!(normalize(Path::new("/../x")), PathBuf::from("/x")); + } } diff --git a/crates/sandlock-core/src/resource.rs b/crates/sandlock-core/src/resource.rs index 27835e13..8d3a317c 100644 --- a/crates/sandlock-core/src/resource.rs +++ b/crates/sandlock-core/src/resource.rs @@ -26,6 +26,7 @@ use crate::sys::structs::{ /// CLONE_THREAD flag — threads don't count toward process limit. const CLONE_THREAD: u64 = 0x0001_0000; +const CLONE_FILES: u64 = 0x0000_0400; /// MAP_ANONYMOUS flag: anonymous and writable private file mappings count. const MAP_ANONYMOUS: u64 = 0x20; @@ -86,11 +87,21 @@ pub(crate) async fn handle_fork( notif: &SeccompNotif, notif_fd: RawFd, ctx: &Arc, - _policy: &NotifPolicy, + policy: &NotifPolicy, ) -> NotifAction { let nr = notif.data.nr as i64; let args = ¬if.data.args; + // The exec relay pins its fd through the caller's RLIMIT_NOFILE, which a + // process sharing the fd table without sharing the limit could defeat. + if policy.argv_safety_required { + if let Some(flags) = clone_flags(notif, notif_fd) { + if flags & CLONE_FILES != 0 && flags & CLONE_THREAD == 0 { + return NotifAction::Errno(libc::EINVAL); + } + } + } + // Namespace flags are denied for clone (clone3's are caught by the // BPF arg filter; vfork takes no flags). if nr == libc::SYS_clone && (args[0] & CLONE_NS_FLAGS) != 0 { @@ -1031,6 +1042,7 @@ mod tests { chroot: Arc::new(Mutex::new(ChrootState::new())), netlink: Arc::new(NetlinkState::new()), processes: Arc::new(ProcessIndex::new()), + exec_relay: Default::default(), policy: Arc::new(fake_policy(argv_safety_required)), child_pidfd: None, notif_fd: -1, diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index a1d19946..07dba502 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -2227,6 +2227,7 @@ impl Sandbox { chroot: Arc::clone(&chroot_state), netlink: Arc::new(crate::netlink::NetlinkState::new()), processes: Arc::clone(&processes), + exec_relay: Default::default(), policy: Arc::new(notif_policy), child_pidfd: child_pidfd_raw, notif_fd: notif_raw_fd, @@ -2987,7 +2988,7 @@ fn parse_bind_ports(specs: &[String], label: &str) -> Result, SandboxEr /// existence can be checked before spawn. Honors `--fs-mount` (virtual:host) /// mappings (which take precedence) and chroot. Used to validate /// `--http-inject-ca` targets. -fn resolve_sandbox_path_to_host( +pub(crate) fn resolve_sandbox_path_to_host( child_path: &std::path::Path, chroot_root: Option<&std::path::Path>, mounts: &[(std::path::PathBuf, std::path::PathBuf)], diff --git a/crates/sandlock-core/src/seccomp/ctx.rs b/crates/sandlock-core/src/seccomp/ctx.rs index 78c90801..78ef41a6 100644 --- a/crates/sandlock-core/src/seccomp/ctx.rs +++ b/crates/sandlock-core/src/seccomp/ctx.rs @@ -33,6 +33,8 @@ pub struct SupervisorCtx { /// an internal RwLock, so handlers can query it synchronously /// without `.await`. pub processes: Arc, + /// Relay execs in flight, keyed by tgid. + pub exec_relay: Arc, /// Immutable policy — no lock needed. pub policy: Arc, /// pidfd for the child process (immutable after spawn). diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 651caa2a..02f9e195 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -76,6 +76,9 @@ pub trait Handler: Send + Sync + 'static { pub struct HandlerCtx { pub notif: SeccompNotif, pub notif_fd: RawFd, + /// The exec relay re-executing a program the policy already approved; + /// handlers observing application execs should ignore it. + pub relay_exec: bool, } // Blanket impl: any Fn(&HandlerCtx) -> Future is a Handler. @@ -236,10 +239,11 @@ impl DispatchTable { &self, notif: SeccompNotif, notif_fd: RawFd, + relay_exec: bool, ) -> NotifAction { let nr = notif.data.nr as i64; if let Some(chain) = self.chains.get(&nr) { - let handler_ctx = HandlerCtx { notif, notif_fd }; + let handler_ctx = HandlerCtx { notif, notif_fd, relay_exec }; for handler in &chain.handlers { let action = handler.handle(&handler_ctx).await; if !matches!(action, NotifAction::Continue) { @@ -836,10 +840,16 @@ fn register_chroot_handlers( crate::chroot::dispatch::handle_chroot_legacy_open)); } - // execve, execveat — unconditional return + // execve, execveat — unconditional return. Under argv safety the exec + // relay owns the application's exec; this handler runs on the relay's. for &nr in &[libc::SYS_execve, libc::SYS_execveat] { - table.register(nr, chroot_handler!(policy, - crate::chroot::dispatch::handle_chroot_exec)); + let inner = chroot_handler!(policy, crate::chroot::dispatch::handle_chroot_exec); + let relayed = policy.argv_safety_required; + table.register(nr, move |cx: &HandlerCtx| { + let skip = relayed && !cx.relay_exec; + let fut = inner(cx); + async move { if skip { NotifAction::Continue } else { fut.await } } + }); } // Modern write syscalls @@ -1061,8 +1071,16 @@ fn register_cow_handlers(table: &mut DispatchTable, ctx: &Arc) { table.register(libc::SYS_chdir, cow_call!(crate::cow::dispatch::handle_cow_chdir)); table.register(libc::SYS_getcwd, cow_call!(crate::cow::dispatch::handle_cow_getcwd)); + // Under argv safety the exec relay owns the application's exec; the COW + // handler runs on the relay's. for &nr in &[libc::SYS_execve, libc::SYS_execveat] { - table.register(nr, cow_call!(crate::cow::dispatch::handle_cow_exec)); + let inner = cow_call!(crate::cow::dispatch::handle_cow_exec); + let relayed = ctx.policy.argv_safety_required; + table.register(nr, move |cx: &HandlerCtx| { + let skip = relayed && !cx.relay_exec; + let fut = inner(cx); + async move { if skip { NotifAction::Continue } else { fut.await } } + }); } } @@ -1124,6 +1142,7 @@ mod handler_tests { chroot: Arc::new(Mutex::new(ChrootState::new())), netlink: Arc::new(NetlinkState::new()), processes: Arc::new(ProcessIndex::new()), + exec_relay: Default::default(), policy: Arc::new(NotifPolicy { max_memory_bytes: 0, max_processes: 0, @@ -1230,7 +1249,7 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1) + .dispatch(fake_notif(libc::SYS_openat as i32), -1, false) .await; assert!(matches!(action, NotifAction::Continue)); @@ -1283,7 +1302,7 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1) + .dispatch(fake_notif(libc::SYS_openat as i32), -1, false) .await; assert!(matches!(action, NotifAction::Continue)); @@ -1334,7 +1353,7 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1) + .dispatch(fake_notif(libc::SYS_openat as i32), -1, false) .await; match action { @@ -1375,7 +1394,7 @@ mod handler_tests { let _ctx = fake_supervisor_ctx(); let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1) + .dispatch(fake_notif(libc::SYS_openat as i32), -1, false) .await; assert!( @@ -1439,7 +1458,7 @@ mod handler_tests { let _sup = fake_supervisor_ctx(); let notif = fake_notif(libc::SYS_openat as i32); - let cx = HandlerCtx { notif, notif_fd: -1 }; + let cx = HandlerCtx { notif, notif_fd: -1, relay_exec: false }; let action = h.handle(&cx).await; assert!(matches!(action, NotifAction::Continue)); @@ -1487,7 +1506,7 @@ mod handler_tests { // Walker MUST hit the struct's handle() each time, accumulating // state on &self.calls. for _ in 0..3 { - let action = table.dispatch(notif, -1).await; + let action = table.dispatch(notif, -1, false).await; assert!(matches!(action, NotifAction::Continue)); } diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index b1b7f45a..91377e7b 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -1224,7 +1224,7 @@ struct ExecRewritePlan { /// Read a NULL-terminated pointer array (argv or envp) from child memory. /// Chunked at page boundaries because a single straddling read fails whole /// if any page is unmapped, and mappings are page-granular. -fn read_exec_ptr_array( +pub(crate) fn read_exec_ptr_array( read: &mut impl FnMut(u64, usize) -> Result, NotifError>, base: u64, ) -> Result, NotifError> { @@ -1278,7 +1278,7 @@ fn nul_free_run( /// Read a NUL-terminated string (NUL excluded) of at most /// `EXEC_MAX_ARG_STRLEN` bytes, chunked at page boundaries. -fn read_exec_cstr( +pub(crate) fn read_exec_cstr( read: &mut impl FnMut(u64, usize) -> Result, NotifError>, addr: u64, ) -> Result, NotifError> { @@ -1438,8 +1438,22 @@ pub(crate) fn rewrite_exec_path_to_fd( child_fd: i32, ) -> Result<(), NotifError> { let fd_path = format!("/proc/self/fd/{}\0", child_fd); + rewrite_exec_path(notif_fd, id, pid, path_ptr, argv_ptr, envp_ptr, fd_path.as_bytes()) +} + +/// Rewrite the child's exec path to `new_path` (NUL included), relocating +/// any argv/envp string the new bytes would overwrite. +pub(crate) fn rewrite_exec_path( + notif_fd: RawFd, + id: u64, + pid: u32, + path_ptr: u64, + argv_ptr: u64, + envp_ptr: u64, + new_path: &[u8], +) -> Result<(), NotifError> { let mut read = |addr: u64, len: usize| read_child_mem(notif_fd, id, pid, addr, len); - let plan = plan_exec_rewrite(&mut read, path_ptr, fd_path.as_bytes(), argv_ptr, envp_ptr)?; + let plan = plan_exec_rewrite(&mut read, path_ptr, new_path, argv_ptr, envp_ptr)?; write_child_mem_force(notif_fd, id, pid, path_ptr, &plan.buf)?; for (slot, new_ptr) in plan.patches { write_child_mem_force(notif_fd, id, pid, slot, &new_ptr.to_ne_bytes())?; @@ -1891,6 +1905,7 @@ async fn emit_policy_event( action: &NotifAction, policy_fn_state: &Arc>, notif_fd: RawFd, + exec: Option<&crate::exec_relay::ExecRequest>, ) -> Option { let pfs = policy_fn_state.lock().await; let tx = match pfs.event_tx.as_ref() { @@ -1934,7 +1949,10 @@ async fn emit_policy_event( let mut path2 = None; let mut flags = None; - if !denied && (nr == libc::SYS_execve || nr == libc::SYS_execveat) { + if let Some(req) = exec { + argv = Some(req.argv_strings()); + path = Some(req.resolved.clone()); + } else if !denied && (nr == libc::SYS_execve || nr == libc::SYS_execveat) { // execve(pathname, argv, envp): args[1] = argv ptr // execveat(dirfd, pathname, argv, ..): args[2] = argv ptr let argv_ptr = if nr == libc::SYS_execveat { @@ -2179,6 +2197,16 @@ async fn handle_notification( maybe_patch_vdso(notif.pid as i32, &mut pfs, policy); } + // Policy-checked execs take their own path: the relay carries the argv + // the policy judged, so nothing below applies to them. + { + let nr = notif.data.nr as i64; + if policy.argv_safety_required && (nr == libc::SYS_execve || nr == libc::SYS_execveat) { + handle_relay_exec(notif, ctx, dispatch_table, fd).await; + return; + } + } + // Check dynamic path denials before dispatch. The gated syscall set is // shared with the BPF notif list so enforcement scope cannot drift from // interception scope; see `fs_denied_path_syscalls` for what is gated @@ -2196,7 +2224,7 @@ async fn handle_notification( drop(pfs); // Let normal dispatch run first so /proc virtualization and // other handlers still win for their paths. - let action = dispatch_table.dispatch(notif, fd).await; + let action = dispatch_table.dispatch(notif, fd, false).await; // A bare `Continue` for openat/open is the racy window: the // supervisor's resolution said "not denied", but the kernel // re-resolves after Continue and a racing thread can swap a @@ -2216,7 +2244,7 @@ async fn handle_notification( } } } else { - dispatch_table.dispatch(notif, fd).await + dispatch_table.dispatch(notif, fd, false).await } }; @@ -2271,7 +2299,7 @@ async fn handle_notification( // Emit event to policy_fn callback if active. For execve, argv is // only populated after `exec_freeze` has stopped every possible // writer, and those tasks stay stopped until after NOTIF_SEND. - if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd).await { + if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd, None).await { use crate::policy_fn::Verdict; match verdict { Verdict::Deny => { action = NotifAction::Errno(libc::EPERM); } @@ -2377,6 +2405,52 @@ async fn handle_notification( } } +/// An execve under argv safety. The request is read once; that copy is what +/// the policy sees and what the relay runs. The relay's own execve comes +/// back through here already judged and only needs the exec handlers. +async fn handle_relay_exec( + notif: SeccompNotif, + ctx: &Arc, + dispatch_table: &super::dispatch::DispatchTable, + fd: RawFd, +) { + use crate::exec_relay::{self, Prepared}; + let pending = match exec_relay::prepare(¬if, fd, ctx).await { + Err(errno) => { + let _ = send_response(fd, notif.id, NotifAction::Errno(errno)); + return; + } + Ok(Prepared::SecondExec) => { + let mut action = dispatch_table.dispatch(notif, fd, true).await; + if matches!(action, NotifAction::Defer(_)) { + action = NotifAction::Errno(libc::EPERM); + } + let _ = send_response(fd, notif.id, action); + return; + } + Ok(Prepared::First(pending)) => pending, + }; + + let mut action = dispatch_table.dispatch(notif, fd, false).await; + if matches!(action, NotifAction::Defer(_)) { + action = NotifAction::Errno(libc::EPERM); + } + if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd, Some(&pending.request)).await { + use crate::policy_fn::Verdict; + match verdict { + Verdict::Deny => action = NotifAction::Errno(libc::EPERM), + Verdict::DenyWith(errno) => action = NotifAction::Errno(errno), + Verdict::Audit | Verdict::Allow => {} + } + } + if matches!(action, NotifAction::Continue) { + if let Err(errno) = exec_relay::commit(pending, ¬if, fd, ctx) { + action = NotifAction::Errno(errno); + } + } + let _ = send_response(fd, notif.id, action); +} + // ============================================================ // Main supervisor loop // ============================================================ diff --git a/crates/sandlock-core/src/sys/structs.rs b/crates/sandlock-core/src/sys/structs.rs index f5fa4729..5d18291d 100644 --- a/crates/sandlock-core/src/sys/structs.rs +++ b/crates/sandlock-core/src/sys/structs.rs @@ -154,6 +154,8 @@ pub const SECCOMP_RET_ERRNO: u32 = 0x0005_0000; pub const SECCOMP_RET_KILL_PROCESS: u32 = 0x8000_0000; pub const SECCOMP_USER_NOTIF_FLAG_CONTINUE: u32 = 1; pub const SECCOMP_USER_NOTIF_FD_SYNC_WAKE_UP: u32 = 1; +/// Install the fd at `newfd`, replacing whatever is there (Linux 5.9+). +pub const SECCOMP_ADDFD_FLAG_SETFD: u32 = 1 << 0; /// Atomically install the fd and respond to the syscall (Linux 5.14+). pub const SECCOMP_ADDFD_FLAG_SEND: u32 = 1 << 1; diff --git a/crates/sandlock-core/tests/integration.rs b/crates/sandlock-core/tests/integration.rs index 5e9387a0..ef6c0a5f 100644 --- a/crates/sandlock-core/tests/integration.rs +++ b/crates/sandlock-core/tests/integration.rs @@ -78,3 +78,6 @@ mod test_popen; #[path = "integration/test_tty.rs"] mod test_tty; + +#[path = "integration/test_exec_relay.rs"] +mod test_exec_relay; diff --git a/crates/sandlock-core/tests/integration/test_exec_relay.rs b/crates/sandlock-core/tests/integration/test_exec_relay.rs new file mode 100644 index 00000000..75a201f3 --- /dev/null +++ b/crates/sandlock-core/tests/integration/test_exec_relay.rs @@ -0,0 +1,215 @@ +//! The exec relay carries policy-checked execs: what the policy saw is what +//! runs, whatever the sandbox does to argv memory in the meantime, and the +//! program that finally runs cannot tell it was relayed. + +use sandlock_core::policy_fn::Verdict; +use sandlock_core::Sandbox; +use std::path::PathBuf; +use std::sync::{Arc, Mutex}; + +fn helper_binary() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../tests/rootfs-helper") + .canonicalize() + .expect("tests/rootfs-helper is built by build.rs") +} + +fn scratch_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("sl-relay-{}-{}", std::process::id(), name)); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn base_policy() -> sandlock_core::SandboxBuilder { + 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("/tmp") +} + +fn stdout_of(r: &sandlock_core::result::RunResult) -> String { + String::from_utf8_lossy(r.stdout.as_deref().unwrap_or(b"")).trim().to_string() +} + +fn stderr_of(r: &sandlock_core::result::RunResult) -> String { + String::from_utf8_lossy(r.stderr.as_deref().unwrap_or(b"")).trim().to_string() +} + +/// A sibling thread flips argv[1] between "allowed" and "blocked" while the +/// process execs. Whatever the policy judged is what must run: "blocked" is +/// denied, and anything that prints must be the word the policy recorded. +#[tokio::test] +async fn race_cannot_change_what_runs() { + let helper = helper_binary(); + let helper_dir = helper.parent().unwrap().to_path_buf(); + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + let policy = base_policy() + .fs_read(&helper_dir) + .policy_fn(move |event, _ctx| { + if event.syscall == "execve" { + if let Some(argv) = &event.argv { + if argv.first().map(|a| a == "echo").unwrap_or(false) { + seen_cb.lock().unwrap().push(argv.get(1).cloned().unwrap_or_default()); + if event.argv_contains("blocked") { + return Verdict::Deny; + } + } + } + } + Verdict::Allow + }) + .build() + .unwrap(); + + let helper_s = helper.to_str().unwrap(); + let mut printed = 0; + for _ in 0..40 { + seen.lock().unwrap().clear(); + let r = policy.clone().run(&[helper_s, "argv-race"]).await.unwrap(); + let out = stdout_of(&r); + assert!(!out.contains("blocked"), "a denied argv ran: {out:?}"); + let judged = seen.lock().unwrap().last().cloned(); + if !out.is_empty() { + printed += 1; + assert_eq!(Some(out.clone()), judged, "what ran differs from what the policy judged"); + } else { + assert_eq!(judged.as_deref(), Some("blocked"), "silent run was not a deny: {}", stderr_of(&r)); + } + } + assert!(printed > 0, "the allowed word never ran in 40 attempts"); +} + +#[tokio::test] +async fn script_sees_its_own_path_as_dollar_zero() { + let dir = scratch_dir("script"); + let script = dir.join("show-name.sh"); + std::fs::write(&script, "#!/bin/sh\necho \"$0\"\n").unwrap(); + std::fs::set_permissions(&script, std::os::unix::fs::PermissionsExt::from_mode(0o755)).unwrap(); + let policy = base_policy().fs_read(&dir).policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let r = policy.clone().run(&[script.to_str().unwrap()]).await.unwrap(); + assert!(r.success(), "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), script.to_str().unwrap()); + let _ = std::fs::remove_dir_all(&dir); +} + +#[tokio::test] +async fn argv0_is_preserved_for_elf() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let r = policy + .clone() + .run(&["python3", "-c", "import os; os.execv('/bin/sh', ['custom0', '-c', 'echo $0'])"]) + .await + .unwrap(); + assert!(r.success(), "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), "custom0"); +} + +#[tokio::test] +async fn comm_is_the_program_name() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let r = policy.clone().run(&["sh", "-c", "cat /proc/self/comm"]).await.unwrap(); + assert!(r.success(), "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), "cat"); +} + +/// execvp walks PATH on ENOENT, so a missing target must fail the caller's +/// execve itself rather than run a relay that fails later. +#[tokio::test] +async fn missing_binary_returns_enoent_to_the_caller() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let script = "import os\ntry:\n os.execv('/nonexistent/x', ['x'])\nexcept FileNotFoundError:\n print('ENOENT')\n"; + let r = policy.clone().run(&["python3", "-c", script]).await.unwrap(); + assert_eq!(stdout_of(&r), "ENOENT", "stderr: {}", stderr_of(&r)); +} + +#[tokio::test] +async fn non_executable_file_returns_eacces_to_the_caller() { + let dir = scratch_dir("noexec"); + let file = dir.join("plain"); + std::fs::write(&file, "not a program").unwrap(); + std::fs::set_permissions(&file, std::os::unix::fs::PermissionsExt::from_mode(0o644)).unwrap(); + let policy = base_policy().fs_read(&dir).policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let script = format!( + "import os\ntry:\n os.execv('{}', ['plain'])\nexcept PermissionError:\n print('EACCES')\n", + file.display() + ); + let r = policy.clone().run(&["python3", "-c", &script]).await.unwrap(); + assert_eq!(stdout_of(&r), "EACCES", "stderr: {}", stderr_of(&r)); + let _ = std::fs::remove_dir_all(&dir); +} + +/// The relay's own execve is not an application exec and must not reach +/// the callback a second time. +#[tokio::test] +async fn one_policy_event_per_exec() { + let count = Arc::new(Mutex::new(0usize)); + let count_cb = count.clone(); + let policy = base_policy() + .policy_fn(move |event, _ctx| { + if event.syscall == "execve" { + *count_cb.lock().unwrap() += 1; + } + Verdict::Allow + }) + .build() + .unwrap(); + let r = policy.clone().run(&["/bin/true"]).await.unwrap(); + assert!(r.success()); + assert_eq!(*count.lock().unwrap(), 1); +} + +#[tokio::test] +async fn threads_spawning_subprocesses_all_succeed() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let script = concat!( + "import subprocess, threading\n", + "ok = []\n", + "def work():\n", + " for _ in range(10):\n", + " ok.append(subprocess.run(['/bin/true']).returncode == 0)\n", + "ts = [threading.Thread(target=work) for _ in range(8)]\n", + "[t.start() for t in ts]; [t.join() for t in ts]\n", + "print('SPAWNS', sum(ok))\n", + ); + let r = tokio::time::timeout(std::time::Duration::from_secs(60), policy.clone().run(&["python3", "-c", script])) + .await + .expect("threaded spawns must not hang") + .unwrap(); + assert_eq!(stdout_of(&r), "SPAWNS 80", "stderr: {}", stderr_of(&r)); +} + +/// The relay borrows the soft NOFILE limit to pin its fd; the program that +/// finally runs must see the limit it would have had. +#[tokio::test] +async fn soft_nofile_limit_is_restored_for_the_program() { + let outside = std::process::Command::new("sh").args(["-c", "ulimit -Sn"]).output().unwrap(); + let outside = String::from_utf8_lossy(&outside.stdout).trim().to_string(); + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let r = policy.clone().run(&["sh", "-c", "ulimit -Sn"]).await.unwrap(); + assert_eq!(stdout_of(&r), outside); +} + +/// A process sharing its fd table with another process could repopulate the +/// pinned fd number, so that clone shape is refused under an argv policy. +#[tokio::test] +async fn clone_files_without_thread_is_rejected() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let script = concat!( + "import ctypes, os, platform\n", + "libc = ctypes.CDLL(None, use_errno=True)\n", + "CLONE_FILES = 0x400; SIGCHLD = 17\n", + "nr = 56 if platform.machine() == 'x86_64' else 220\n", + "r = libc.syscall(nr, CLONE_FILES | SIGCHLD, 0, 0, 0, 0)\n", + "if r == 0: os._exit(0)\n", + "print('EINVAL' if r < 0 and ctypes.get_errno() == 22 else 'RET %d' % r)\n", + ); + let r = policy.clone().run(&["python3", "-c", script]).await.unwrap(); + assert_eq!(stdout_of(&r), "EINVAL", "stderr: {}", stderr_of(&r)); +} diff --git a/crates/sandlock-ffi/tests/handler_smoke.rs b/crates/sandlock-ffi/tests/handler_smoke.rs index c8b43d25..fed73c3d 100644 --- a/crates/sandlock-ffi/tests/handler_smoke.rs +++ b/crates/sandlock-ffi/tests/handler_smoke.rs @@ -291,9 +291,10 @@ fn fake_ctx() -> HandlerCtx { arch: 0xC000003E, instruction_pointer: 0, args: [0; 6], - }, + }, }, notif_fd: -1, + relay_exec: false, } } @@ -356,9 +357,10 @@ fn fake_ctx_with_isolated_child() -> (HandlerCtx, std::process::Child) { arch: 0xC000003E, instruction_pointer: 0, args: [0; 6], - }, + }, }, notif_fd: -1, + relay_exec: false, }; (ctx, child) } @@ -835,9 +837,10 @@ async fn ffi_handler_translates_kill_zero_pgid_substitutes_child_pgid() { arch: 0xC000_003E, instruction_pointer: 0, args: [0; 6], - }, + }, }, notif_fd: -1, + relay_exec: false, }; let action = h.handle(&cx).await; @@ -872,9 +875,10 @@ fn fake_ctx_with_pid(pid: u32) -> HandlerCtx { arch: 0xC000_003E, instruction_pointer: 0, args: [0; 6], - }, + }, }, notif_fd: -1, + relay_exec: false, } } diff --git a/tests/rootfs-helper.c b/tests/rootfs-helper.c index 1163dce8..e6099850 100644 --- a/tests/rootfs-helper.c +++ b/tests/rootfs-helper.c @@ -12,6 +12,7 @@ */ #define _GNU_SOURCE #include +#include #include #include #include @@ -851,10 +852,45 @@ static int cmd_write_fd_link(int argc, char **argv) { return 0; } +/* ── argv-race (argv policy TOCTOU probe) ─────────────────────────────────── */ +/* A sibling thread keeps swapping the word argv[1] points at between + * "allowed" and "blocked" while the main thread execs /bin/echo with it. + * Both words are eight bytes including the NUL, stored as one aligned + * 64-bit write, so the kernel never sees a torn value. */ +static union { char s[16]; unsigned long long w[2]; } race_buf __attribute__((aligned(8))); + +static int race_flipper(void *arg) { + unsigned long long allowed, blocked; + (void)arg; + memcpy(&allowed, "allowed", 8); + memcpy(&blocked, "blocked", 8); + for (;;) { + *(volatile unsigned long long *)&race_buf.w[0] = blocked; + *(volatile unsigned long long *)&race_buf.w[0] = allowed; + } + return 0; +} + +static int cmd_argv_race(int argc, char **argv) { + (void)argc; (void)argv; + size_t sz = 64 * 1024; + char *stack = malloc(sz); + if (!stack) { perror("argv-race: malloc"); return 3; } + memcpy(race_buf.s, "allowed", 8); + int flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM; + if (clone(race_flipper, stack + sz, flags, NULL) < 0) { perror("argv-race: clone"); return 3; } + usleep(200); + char *args[] = { "echo", race_buf.s, NULL }; + execv("/bin/echo", args); + fprintf(stderr, "EXEC_FAILED %d\n", errno); + return 3; +} + /* ── dispatch ───────────────────────────────────────────────── */ static int dispatch(const char *cmd, int argc, char **argv) { if (strcmp(cmd, "chdir") == 0) return cmd_chdir(argc, argv); + if (strcmp(cmd, "argv-race") == 0) return cmd_argv_race(argc, argv); if (strcmp(cmd, "fchdir") == 0) return cmd_fchdir(argc, argv); if (strcmp(cmd, "openat2") == 0) return cmd_openat2(argc, argv); if (strcmp(cmd, "chdir-self") == 0) return cmd_chdir_self(argc, argv); From 3edd5e38306ce82ba8f310c1d6f10248e3f49743 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 20:23:47 -0700 Subject: [PATCH 3/7] core: drop the argv freeze and ptrace fork tracking The exec relay carries the judged argv itself, so nothing needs to stop sandbox tasks around an execve, and nothing needs to know every child at creation time. Remove freeze.rs, the one-shot ptrace fork-event tracking in resource.rs, and the fork(2) interception that existed only to feed it. Fork counting for the process limit stays. ptrace now appears only in checkpoint capture, which has no other way to read a task's registers. Signed-off-by: Cong Wang --- crates/sandlock-core/src/context/tests.rs | 7 +- crates/sandlock-core/src/freeze.rs | 597 ------------------ crates/sandlock-core/src/lib.rs | 1 - crates/sandlock-core/src/policy_fn.rs | 18 +- crates/sandlock-core/src/resource.rs | 582 +---------------- crates/sandlock-core/src/seccomp/notif.rs | 125 +--- crates/sandlock-core/src/seccomp_plan.rs | 10 - .../tests/integration/test_policy_fn.rs | 12 +- 8 files changed, 20 insertions(+), 1332 deletions(-) delete mode 100644 crates/sandlock-core/src/freeze.rs diff --git a/crates/sandlock-core/src/context/tests.rs b/crates/sandlock-core/src/context/tests.rs index 507fd06c..ec3df232 100644 --- a/crates/sandlock-core/src/context/tests.rs +++ b/crates/sandlock-core/src/context/tests.rs @@ -58,15 +58,18 @@ fn test_notif_syscalls_always_has_clone() { } } +/// Bare fork(2) stays out of the filter even under policy_fn: the exec +/// relay needs no fork-time child registration, so hot fork loops keep +/// bypassing the supervisor. #[test] -fn test_notif_syscalls_fork_gated_on_policy_fn() { +fn test_notif_syscalls_fork_not_intercepted_under_policy_fn() { let Some(fork) = arch::sys_fork() else { return }; let policy = Sandbox::builder() .policy_fn(|_event, _ctx| crate::policy_fn::Verdict::Allow) .build() .unwrap(); let nrs = notif_syscalls(&policy, None); - assert!(nrs.contains(&(fork as u32))); + assert!(!nrs.contains(&(fork as u32))); } #[test] diff --git a/crates/sandlock-core/src/freeze.rs b/crates/sandlock-core/src/freeze.rs deleted file mode 100644 index d1d57649..00000000 --- a/crates/sandlock-core/src/freeze.rs +++ /dev/null @@ -1,597 +0,0 @@ -//! Freeze sandbox threads of an execve caller before exposing argv. -//! -//! # Why -//! -//! Per `seccomp_unotify(2)`, after the supervisor responds with -//! `Continue`, the kernel re-reads the syscall's user-memory pointers -//! before executing the syscall. For execve, that means the kernel -//! re-reads `pathname` and the argv array from child memory. Any task -//! that can write to that memory in the window between the supervisor's -//! inspection and the kernel's re-read can defeat the decision -//! `policy_fn` made on the values it saw. -//! -//! Two distinct task classes can write that memory: -//! 1. Sibling threads of the calling tid (same TGID; share `mm_struct` -//! by definition). -//! 2. Peer processes in other TGIDs that alias the same pages via -//! `MAP_SHARED` mappings (memfd, SysV shm, shared file mmap), or -//! that share the calling task's `mm_struct` via -//! `clone(CLONE_VM)` without `CLONE_THREAD`. -//! -//! `freeze_sandbox_for_execve` closes both classes. When `policy_fn` -//! is active, every fork-like syscall is traced for one ptrace -//! fork/clone/vfork event and the child is registered in -//! `ProcessIndex` before it can run user code. The exec freeze can -//! therefore enumerate every tracked TGID, walk `/proc//task`, -//! and `PTRACE_SEIZE` + `PTRACE_INTERRUPT` every TID that could mutate -//! argv. -//! -//! # Sibling vs peer cleanup -//! -//! Sibling threads (same TGID as the caller) are killed by the kernel -//! during execve's `de_thread` step when execve is allowed, so the -//! supervisor does not detach them on the allow path — their ptrace -//! state is reaped along with the threads. If the policy callback -//! denies execve after argv inspection, the supervisor detaches both -//! siblings and peers because `de_thread` will not run. -//! -//! Peer threads (different TGID) survive execve. The supervisor must -//! `PTRACE_DETACH` them after `NOTIF_SEND` so they can resume normal -//! execution. The freeze function returns the peer TID list for that -//! purpose; siblings are not returned because they need no follow-up. -//! -//! # Failure modes (strict) -//! -//! The freeze is an invariant: if the supervisor exposed argv to -//! `policy_fn` and the callback returned Allow, the kernel must re-read -//! the same memory the supervisor inspected. We refuse to silently -//! degrade — if the freeze cannot be established, the supervisor -//! denies the execve with `EPERM` rather than letting it proceed -//! without TOCTOU protection. -//! -//! - `PTRACE_SEIZE` returns `ESRCH` for a sibling that exited between -//! enumeration and seize. Treated as success: there is no thread to -//! race. -//! - Any other ptrace failure (YAMA `ptrace_scope` >= 2 outside the -//! parent chain, another tracer attached, kernel resource limits) -//! produces an error; siblings already frozen during the partial -//! attempt are detached so they resume normally; the caller fails -//! the syscall closed. - -use std::collections::HashSet; -use std::fs; -use std::io; - -/// Read the `State:` field from `/proc//status`. Returns the -/// single-character state code (`R`, `S`, `D`, `T`, `t`, `Z`, `X`) -/// or `None` if the file or line is unreadable. -fn read_task_state(tid: i32) -> Option { - let status = fs::read_to_string(format!("/proc/{}/status", tid)).ok()?; - let line = status.lines().find(|l| l.starts_with("State:"))?; - // Format is "State:\t ()" — find the first non-space - // character after the colon. - line.split_whitespace().nth(1).and_then(|s| s.chars().next()) -} - -/// What `seize_and_interrupt` did with one tid. -#[derive(Debug, PartialEq, Eq)] -enum SeizeOutcome { - /// Confirmed ptrace-stopped; must be detached later. - Frozen, - /// No attachment exists (already exited, or held in an uninterruptible - /// kernel wait without ever being seized): nothing to release. - NotNeeded, - /// Seized with the interrupt queued, but the task entered an - /// uninterruptible kernel wait before stopping. It cannot run user code - /// (so it cannot mutate argv), and it will trap into ptrace-stop the - /// moment its wait clears. The caller must reap and detach it AFTER the - /// execve response is sent — for the vfork parent, the wait clears only - /// once that very execve resolves. - PendingStop, -} - -/// `PTRACE_SEIZE` + `PTRACE_INTERRUPT` a single tid and reap the confirmed -/// ptrace-stop without ever blocking unboundedly. -/// -/// # Why the reap must be bounded -/// -/// A task in `TASK_UNINTERRUPTIBLE` (`State: D`) — most commonly the vfork -/// parent of the execve caller, suspended in `kernel_clone` until its child -/// execs — cannot enter ptrace-stop until its kernel wait clears. For vfork -/// specifically, the wait won't clear until we send Continue, but we can't -/// send Continue while we're blocked in `waitpid` for that exact task: an -/// unbounded waitpid here deadlocks the whole supervisor. The pre-check on -/// `/proc//status` catches a task already parked in `D`, but it RACES -/// the tracee — the vfork parent can pass the check runnable and park -/// before the interrupt lands (seen in the wild under CPU load). So after -/// arming the interrupt the reap polls with `WNOHANG`, and a task observed -/// in `D` is handed back as [`SeizeOutcome::PendingStop`] instead of being -/// waited for. -/// -/// On a partial-progress failure (PTRACE_SEIZE succeeded but -/// PTRACE_INTERRUPT did not), the function detaches itself before -/// returning so the caller doesn't have to track partial state. -fn seize_and_interrupt(tid: i32) -> io::Result { - // Fast path: the kernel is already holding this task; it cannot mutate - // user memory and does not need an attachment. - if read_task_state(tid) == Some('D') { - return Ok(SeizeOutcome::NotNeeded); - } - - let ret = unsafe { - libc::ptrace(libc::PTRACE_SEIZE, tid, 0, 0) - }; - if ret < 0 { - let err = io::Error::last_os_error(); - if err.raw_os_error() == Some(libc::ESRCH) { - return Ok(SeizeOutcome::NotNeeded); // already exited - } - return Err(err); - } - // PTRACE_SEIZE succeeded; from here, any error path must DETACH - // before returning so we don't leave the task traced-but-running. - - let ret = unsafe { - libc::ptrace(libc::PTRACE_INTERRUPT, tid, 0, 0) - }; - if ret < 0 { - let err = io::Error::last_os_error(); - let _ = unsafe { libc::ptrace(libc::PTRACE_DETACH, tid, 0, 0) }; - if err.raw_os_error() == Some(libc::ESRCH) { - return Ok(SeizeOutcome::NotNeeded); - } - return Err(err); - } - - // Bounded reap. A runnable task stops within a scheduling quantum; the - // budget only exists so a starved box still converges. `__WALL` because - // siblings are threads, which waitpid(2) ignores by default. - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(1); - loop { - let mut status: i32 = 0; - let r = unsafe { libc::waitpid(tid, &mut status, libc::__WALL | libc::WNOHANG) }; - if r == tid { - if libc::WIFEXITED(status) || libc::WIFSIGNALED(status) { - return Ok(SeizeOutcome::NotNeeded); - } - if libc::WIFSTOPPED(status) { - return Ok(SeizeOutcome::Frozen); - } - } else if r < 0 { - let e = io::Error::last_os_error(); - if e.raw_os_error() != Some(libc::EINTR) { - // Reaped elsewhere or gone: nothing left to hold. - return Ok(SeizeOutcome::NotNeeded); - } - } - if read_task_state(tid) == Some('D') { - return Ok(SeizeOutcome::PendingStop); - } - if std::time::Instant::now() >= deadline { - let _ = unsafe { libc::ptrace(libc::PTRACE_DETACH, tid, 0, 0) }; - return Err(io::Error::new( - io::ErrorKind::TimedOut, - format!("tid {tid} did not enter ptrace-stop within the freeze budget"), - )); - } - std::thread::sleep(std::time::Duration::from_millis(1)); - } -} - -/// Detach a previously-frozen task. Used to roll back partial -/// progress when a later task refuses to be frozen, and to release -/// peer tasks after the kernel has re-read execve argv. -fn detach(tid: i32) { - let _ = unsafe { libc::ptrace(libc::PTRACE_DETACH, tid, 0, 0) }; -} - -/// Enumerate every TID in a TGID via `/proc//task/`. Linux -/// resolves `/proc//task` to the same directory, so this -/// works whether `tgid` is the leader's PID or any TID in the group. -fn list_threads_of_tgid(tgid: i32) -> io::Result> { - let dir = fs::read_dir(format!("/proc/{}/task", tgid))?; - let mut tids = Vec::new(); - for entry in dir { - let entry = match entry { - Ok(e) => e, - Err(_) => continue, - }; - let name = entry.file_name(); - let name_str = match name.to_str() { - Some(s) => s, - None => continue, - }; - if let Ok(tid) = name_str.parse::() { - tids.push(tid); - } - } - Ok(tids) -} - -/// Read the TGID containing `tid`, as an `io::Result` so a missing or -/// unparseable value aborts the freeze instead of silently narrowing it -/// to one task. -fn read_tgid_of_tid(tid: i32) -> io::Result { - crate::seccomp::state::read_tgid_of_tid(tid).ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidData, - "no usable Tgid: line in /proc//status", - ) - }) -} - -/// Outcome of a sandbox-wide freeze. -#[derive(Debug, Default)] -pub(crate) struct SandboxFreeze { - /// Sibling TIDs in the caller's TGID. These die in `de_thread` if - /// execve is allowed, but must be detached if execve is denied - /// after `policy_fn` inspected argv. - pub sibling_tids: Vec, - /// TIDs in *other* TGIDs that were ptrace-stopped. These survive - /// execve and must be detached so they can resume normal - /// execution. - pub peer_tids: Vec, - /// TIDs seized with a queued interrupt that had entered an - /// uninterruptible kernel wait before stopping (the vfork parent racing - /// the freeze). Kernel-held for the duration of the freeze window; must - /// be reaped with [`reap_pending`] after the execve response is sent. - pub pending_tids: Vec, -} - -/// A freeze that could not be completed. Carries any tasks that were left -/// with a queued interrupt and could not be released during rollback (they -/// had not entered ptrace-stop yet); the caller must [`reap_pending`] them -/// after sending its deny response, for the same reason the freeze itself -/// could not wait for them. -#[derive(Debug)] -pub(crate) struct FreezeError { - pub error: io::Error, - pub pending_tids: Vec, -} - -impl std::fmt::Display for FreezeError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.error.fmt(f) - } -} - -/// Freeze every sandbox thread that could mutate execve argv before -/// the supervisor reads it for `policy_fn` and before the kernel -/// re-reads it. -/// -/// Walks every TGID in `processes`, enumerates each TGID's threads via -/// `/proc//task/`, and `PTRACE_SEIZE` + `PTRACE_INTERRUPT`s -/// every TID except `caller_tid`. Sibling threads of `caller_tid` and -/// peer threads in other TGIDs are both covered. `processes` is -/// complete for `policy_fn` runs because fork-like syscalls are tracked -/// before new children can run. -/// -/// Strict semantics: if any task refuses to be frozen, every -/// already-frozen task is detached and the error is propagated. The -/// caller is expected to deny the execve with `EPERM`, preserving the -/// invariant that exposed argv is always TOCTOU-safe. -/// -/// On success, returns the sibling and peer TIDs that were frozen. The -/// caller detaches peers after an allowed execve, or detaches all TIDs -/// after a denied execve. -pub(crate) fn freeze_sandbox_for_execve( - processes: &crate::seccomp::state::ProcessIndex, - caller_tid: i32, -) -> Result { - let no_pending = |error| FreezeError { error, pending_tids: Vec::new() }; - let caller_tgid = read_tgid_of_tid(caller_tid).map_err(no_pending)?; - let mut tgids: HashSet = processes.tgids_snapshot(); - tgids.insert(caller_tgid); - - let mut sibling_tids: Vec = Vec::new(); - let mut peer_tids: Vec = Vec::new(); - let mut pending_tids: Vec = Vec::new(); - - for tgid in &tgids { - // /proc//task may disappear if the TGID exited between - // snapshot and walk — that's fine, no threads to freeze. - let tids = match list_threads_of_tgid(*tgid) { - Ok(t) => t, - Err(_) => continue, - }; - for tid in tids { - if tid == caller_tid { - continue; - } - match seize_and_interrupt(tid) { - Ok(SeizeOutcome::Frozen) => { - if *tgid == caller_tgid { - sibling_tids.push(tid); - } else { - peer_tids.push(tid); - } - } - Ok(SeizeOutcome::PendingStop) => pending_tids.push(tid), - Ok(SeizeOutcome::NotNeeded) => continue, - Err(e) => { - // Roll back: detach every task we already froze - // (siblings + peers) so they resume normally. Pending - // tasks cannot be detached until they stop, which - // requires the caller's response to go out first — - // hand them back through the error. - for t in &sibling_tids { - detach(*t); - } - for t in &peer_tids { - detach(*t); - } - return Err(FreezeError { error: e, pending_tids }); - } - } - } - } - - Ok(SandboxFreeze { - sibling_tids, - peer_tids, - pending_tids, - }) -} - -/// Reap and detach tasks that were seized with a queued interrupt while in -/// an uninterruptible kernel wait. Called strictly AFTER the execve -/// response is sent: for the vfork parent the wait clears as soon as that -/// execve resolves, so the queued interrupt fires promptly. Bounded so an -/// unrelated long uninterruptible wait cannot stall the supervisor loop; -/// on expiry the attachment is abandoned loudly rather than silently. -pub(crate) fn reap_pending(pending: &[i32]) { - for &tid in pending { - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); - loop { - let mut status: i32 = 0; - let r = unsafe { libc::waitpid(tid, &mut status, libc::__WALL | libc::WNOHANG) }; - if r == tid { - if libc::WIFEXITED(status) || libc::WIFSIGNALED(status) { - break; - } - if libc::WIFSTOPPED(status) { - detach(tid); - break; - } - } else if r < 0 { - let e = io::Error::last_os_error(); - if e.raw_os_error() != Some(libc::EINTR) { - break; // reaped elsewhere or gone - } - } - if std::time::Instant::now() >= deadline { - eprintln!( - "sandlock: tid {tid} never left its kernel wait; \ - abandoning its ptrace attachment" - ); - break; - } - std::thread::sleep(std::time::Duration::from_millis(1)); - } - } -} - -/// Detach peer TIDs after the kernel has re-read execve argv. Errors -/// are ignored: a peer that already exited returns ESRCH, which is -/// harmless. -pub(crate) fn detach_peers(peer_tids: &[i32]) { - for tid in peer_tids { - detach(*tid); - } -} - -/// Detach every task in a freeze after execve was denied or the -/// notification response could not be sent. -pub(crate) fn detach_all(freeze: &SandboxFreeze) { - for tid in &freeze.sibling_tids { - detach(*tid); - } - for tid in &freeze.peer_tids { - detach(*tid); - } -} - -/// Helper called from the dispatch hot path. Returns true if the -/// notification is for an execve-class syscall whose Continue response -/// requires freezing siblings. -pub(crate) fn requires_freeze_on_continue(syscall_nr: i64) -> bool { - syscall_nr == libc::SYS_execve || syscall_nr == libc::SYS_execveat -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::seccomp::state::ProcessIndex; - - #[test] - fn list_threads_of_tgid_includes_self() { - // Our own /proc/self/task always exists and always contains - // at least our own tid. - let our_tid = unsafe { libc::syscall(libc::SYS_gettid) } as i32; - let tids = list_threads_of_tgid(our_tid).unwrap(); - assert!(tids.contains(&our_tid)); - } - - #[test] - fn requires_freeze_only_for_exec() { - assert!(requires_freeze_on_continue(libc::SYS_execve)); - assert!(requires_freeze_on_continue(libc::SYS_execveat)); - assert!(!requires_freeze_on_continue(libc::SYS_openat)); - assert!(!requires_freeze_on_continue(libc::SYS_connect)); - } - - /// Regression test for the cross-process TOCTOU concern raised on - /// issue #27 (Changaco): a peer process in the sandbox — different - /// TGID, possibly aliasing argv pages via shared memory — must also - /// be frozen before the kernel re-reads execve argv. Sibling-thread - /// freeze alone does not cover this. In real policy_fn runs, - /// fork-like syscall tracking registers peer processes before they - /// can run; this unit test mirrors that completed registration. - /// - /// # Why we spawn a separate "caller" process - /// - /// In production, `freeze_sandbox_for_execve` runs in the supervisor - /// process and `caller_tid` is the sandboxed child's tid — i.e. the - /// supervisor and the execve caller are in *different* TGIDs, and - /// every TID the freeze walks is a descendant of the supervisor. - /// Under YAMA `ptrace_scope=1` (the Ubuntu/Debian default), that - /// descendant relationship is exactly what makes PTRACE_SEIZE - /// permitted without any privilege. - /// - /// If this test instead used the test thread's own tid as - /// `caller_tid`, `caller_tgid` would be the cargo test binary's - /// TGID, the freeze would walk the test binary's sibling threads - /// (libtest workers, runtime helpers), and PTRACE_SEIZE would be - /// rejected with EPERM by YAMA — sibling threads are not - /// descendants of each other. That would force the test to require - /// privileges sandlock itself does not require. So we spawn a - /// dedicated "caller" sleep to play the sandboxed-process role, - /// matching production topology. - #[test] - fn freeze_sandbox_includes_peer_process() { - use std::process::{Command, Stdio}; - - // The "execve caller" — stands in for the sandboxed process. - // Its tid is a descendant of the test process (the parent), so - // ptracing into its TGID is YAMA-allowed under ptrace_scope=1. - let mut caller = Command::new("/bin/sleep") - .arg("60") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn caller sleep"); - let caller_tid = caller.id() as i32; - - let mut peer = Command::new("/bin/sleep") - .arg("60") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn peer sleep"); - let peer_pid = peer.id() as i32; - - // Give both children a moment to actually be running. - std::thread::sleep(std::time::Duration::from_millis(50)); - - let processes = ProcessIndex::new(); - processes - .register(peer_pid) - .expect("register peer in ProcessIndex"); - - let outcome = freeze_sandbox_for_execve(&processes, caller_tid) - .expect("freeze_sandbox_for_execve"); - - // Peer's TID is its own TGID (single-threaded sleep), and it's - // a different TGID from the execve caller, so it should be in peer_tids. - assert!( - outcome.peer_tids.contains(&peer_pid), - "peer pid {} should be in peer_tids: {:?}", - peer_pid, - outcome.peer_tids - ); - - // Verify the peer is actually ptrace-stopped via /proc. - let status = std::fs::read_to_string(format!("/proc/{}/status", peer_pid)) - .expect("read peer status"); - let state_line = status - .lines() - .find(|l| l.starts_with("State:")) - .expect("State: line"); - assert!( - state_line.contains("t (tracing stop)") || state_line.contains("T (stopped)"), - "peer should be ptrace-stopped, got: {}", - state_line - ); - - // Cleanup: detach the peer so it can resume and be killed. - detach_peers(&outcome.peer_tids); - let _ = peer.kill(); - let _ = peer.wait(); - let _ = caller.kill(); - let _ = caller.wait(); - } - - /// Re-executed by `freeze_sandbox_tolerates_thread_tids_in_index` as a - /// multi-threaded peer; a no-op in a normal test run. - #[test] - fn freeze_helper_multithreaded_child() { - if std::env::var_os("SANDLOCK_FREEZE_HELPER").is_none() { - return; - } - std::thread::spawn(|| loop { - std::thread::sleep(std::time::Duration::from_secs(60)); - }); - loop { - std::thread::sleep(std::time::Duration::from_secs(60)); - } - } - - /// Issue #212: the index is keyed by notifying tid, so one thread group - /// can appear under several entries, and re-seizing a thread the freeze - /// already holds fails with EPERM. - #[test] - fn freeze_sandbox_tolerates_thread_tids_in_index() { - use std::process::{Command, Stdio}; - - let mut caller = Command::new("/bin/sleep") - .arg("60") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn caller sleep"); - let caller_tid = caller.id() as i32; - - let mut peer = Command::new(std::env::current_exe().unwrap()) - .args([ - "--exact", - "freeze::tests::freeze_helper_multithreaded_child", - "--test-threads=1", - ]) - .env("SANDLOCK_FREEZE_HELPER", "1") - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .expect("spawn multi-threaded peer"); - let peer_pid = peer.id() as i32; - - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - let tids = loop { - let tids = list_threads_of_tgid(peer_pid).unwrap_or_default(); - if tids.len() >= 2 { - break tids; - } - assert!(std::time::Instant::now() < deadline, "peer never became multi-threaded"); - std::thread::sleep(std::time::Duration::from_millis(10)); - }; - - let processes = ProcessIndex::new(); - for tid in &tids { - processes.register(*tid).expect("register peer tid"); - } - - let outcome = freeze_sandbox_for_execve(&processes, caller_tid); - if let Ok(freeze) = &outcome { - // Detach before killing: a traced thread that dies stays a - // zombie until its tracer reaps it, and that would wedge wait(). - detach_peers(&freeze.peer_tids); - } - let _ = peer.kill(); - let _ = caller.kill(); - let _ = peer.wait(); - let _ = caller.wait(); - - let outcome = outcome.expect("freeze with duplicate thread-group entries"); - for tid in &tids { - assert!( - outcome.peer_tids.contains(tid), - "peer tid {} missing from {:?}", - tid, - outcome.peer_tids - ); - } - } -} diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index a150710b..002dff30 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -21,7 +21,6 @@ pub(crate) mod time; pub(crate) mod cow; pub mod recovery; pub(crate) mod checkpoint; -pub(crate) mod freeze; pub(crate) mod exec_relay; pub mod netlink; pub(crate) mod procfs; diff --git a/crates/sandlock-core/src/policy_fn.rs b/crates/sandlock-core/src/policy_fn.rs index 7257ea20..d49ba25b 100644 --- a/crates/sandlock-core/src/policy_fn.rs +++ b/crates/sandlock-core/src/policy_fn.rs @@ -63,19 +63,11 @@ pub enum SyscallCategory { /// (`fs_read` / `fs_write` / `fs_deny`); see issue #27. /// /// `argv` *is* exposed for `execve`/`execveat` and is TOCTOU-safe by -/// construction: with `policy_fn` active, fork-like syscalls are traced -/// for one ptrace creation event, so children are registered in -/// `ProcessIndex` before they can run user code. Before the supervisor -/// exposes `argv` to `policy_fn` or returns `Continue` for an execve, it -/// then `PTRACE_SEIZE`+`PTRACE_INTERRUPT`s every task that could write -/// the memory — both sibling threads of the calling tid (same TGID, share -/// `mm_struct`) and peer threads in other TGIDs that may alias argv -/// pages via `MAP_SHARED` mappings or share `mm_struct` via -/// `clone(CLONE_VM)`. The kernel's post-Continue re-read therefore -/// sees the same memory the supervisor inspected. Siblings are killed -/// by the kernel during execve's `de_thread` step; peer threads are -/// detached after `NOTIF_SEND` and resume normally. See -/// `crate::freeze`. +/// construction: the supervisor reads it once, and an allowed execve is +/// redirected to the exec relay, a static program that execs the target +/// with exactly that copy. Whatever a sibling thread or CLONE_VM peer +/// writes to the original memory afterwards changes nothing. See +/// `crate::exec_relay`. /// /// Network fields (`host`, `port`) are TOCTOU-safe because the /// supervisor performs `connect`/`sendto`/`bind` on-behalf via diff --git a/crates/sandlock-core/src/resource.rs b/crates/sandlock-core/src/resource.rs index 8d3a317c..621b9ba6 100644 --- a/crates/sandlock-core/src/resource.rs +++ b/crates/sandlock-core/src/resource.rs @@ -12,7 +12,6 @@ // the suspended calling thread's saved registers, which a sibling // thread cannot mutate. -use std::io; use std::os::unix::io::RawFd; use std::sync::Arc; use tokio::sync::Mutex; @@ -41,7 +40,7 @@ const MAP_ANONYMOUS: u64 = 0x20; /// TOCTOU note: the `clone3` read is from racy user memory — a sibling /// thread could mutate the struct between this read and the kernel's /// re-read after `Continue`. Callers use this only for resource -/// accounting (`proc_count`, fork-event tracking gate), never as a +/// accounting (`proc_count`, the CLONE_FILES gate), never as a /// security boundary, so a misread can throttle incorrectly but cannot /// bypass any kernel-enforced deny. fn clone_flags(notif: &SeccompNotif, notif_fd: RawFd) -> Option { @@ -80,9 +79,7 @@ fn is_thread_create(notif: &SeccompNotif, notif_fd: RawFd) -> bool { /// /// Note: `notif.pid` here is the *parent* (the task issuing /// fork/clone/vfork). The kernel hasn't run the syscall yet, so we don't -/// know the child's pid yet. When `policy_fn` is active, the supervisor -/// wraps the eventual `Continue` in one-shot ptrace fork-event tracking -/// and registers the new child before it can run user code. +/// know the child's pid; it registers itself on its first notification. pub(crate) async fn handle_fork( notif: &SeccompNotif, notif_fd: RawFd, @@ -150,11 +147,6 @@ pub(crate) async fn handle_fork( /// run, so handlers can rely on `ProcessIndex::key_for(notif.pid)` /// returning a fresh PidKey. /// -/// With `policy_fn` active, fork-like syscalls additionally register -/// new child processes at creation time via ptrace fork events, before -/// the child can run user code. Without `policy_fn`, lazy registration -/// is enough because no argv-based security decision is exposed. -/// /// The fast path is a single `RwLock` read: if the pid is already /// tracked, we trust the entry. PID-identity correctness comes from /// the per-child pidfd watcher — a process can't issue notifications @@ -199,48 +191,6 @@ pub(crate) async fn register_child_if_new(ctx: &Arc, pid: i32) { let _ = register_pid_if_new(ctx, pid); } -/// Command sent to the per-trace ptrace worker after `prepare` returns. -enum TraceCmd { - /// The seccomp `Continue` has been sent; resume and capture the fork event. - Proceed, - /// Tear down without proceeding (e.g. `send_response` failed). - Abort, -} - -/// Handle to a one-shot ptrace fork-tracking session. -/// -/// ptrace *commands* (`PTRACE_SEIZE`, `GETEVENTMSG`, `DETACH`, …) are -/// per-tracer-thread — issuing one from a thread other than the one that -/// `SEIZE`d fails with `ESRCH`. (Only `waitpid` may be called cross-thread.) -/// So the whole command sequence — SEIZE, the post-`Continue` event wait, and -/// the final `PTRACE_DETACH` — runs inside one `spawn_blocking` worker -/// (`process_creation_worker`) pinned to a single thread. This handle only -/// carries the channels driving that worker plus the tracee tid (used by -/// `finish` to wake the worker's blocking wait on the failed-fork path); it -/// owns no ptrace state, so dropping it never issues a cross-thread ptrace op. -pub(crate) struct ProcessCreationTrace { - cmd_tx: std::sync::mpsc::SyncSender, - join: Option>>, - /// The traced (forking) task's tid — `finish`'s watchdog signals it. - caller_tid: i32, - /// True once `finish`/`abort` has sent a command; gates the Drop fallback. - signaled: bool, -} - -impl Drop for ProcessCreationTrace { - fn drop(&mut self) { - // If neither `finish` nor `abort` ran (early return / panic between - // `prepare` and `finish`), the worker is blocked waiting for a command. - // Tell it to abort so it detaches the tracee on its own thread and - // exits, rather than leaking a blocked blocking-pool thread. - if !self.signaled { - let _ = self.cmd_tx.send(TraceCmd::Abort); - } - // The dropped `join` handle detaches the worker task; it runs to - // completion (performing the ptrace detach on its owning thread). - } -} - fn is_process_creation_notif(notif: &SeccompNotif) -> bool { crate::arch::fork_like_syscalls().contains(&(notif.data.nr as i64)) } @@ -256,300 +206,6 @@ pub(crate) fn fork_counted_on_continue(notif: &SeccompNotif, notif_fd: RawFd) -> is_process_creation_notif(notif) && !is_thread_create(notif, notif_fd) } -/// True when this notification can create a new task that must be in -/// `ProcessIndex` before it can race a later execve argv decision. -pub(crate) fn requires_process_creation_tracking( - notif: &SeccompNotif, - notif_fd: RawFd, - policy: &NotifPolicy, -) -> bool { - policy.argv_safety_required && fork_counted_on_continue(notif, notif_fd) -} - -/// Arm ptrace fork-event tracking on the syscall's calling task. -/// -/// The caller is parked in the seccomp user-notification wait when this -/// runs. Crucially, the tracee **cannot reach a ptrace-stop until the -/// supervisor sends `Continue`** — so we must not `PTRACE_INTERRUPT`+wait -/// here (that deadlocks). Instead `prepare` only performs `PTRACE_SEIZE` -/// (which does not stop the tracee) on a dedicated worker thread, then -/// returns once SEIZE is confirmed. The worker parks until `finish` (called -/// after `Continue`) tells it to proceed, at which point it does the -/// `INTERRUPT` + event loop + detach — all on that same thread, as ptrace -/// requires. -pub(crate) async fn prepare_process_creation_tracking( - ctx: &Arc, - caller_tid: i32, -) -> io::Result { - let ctx = Arc::clone(ctx); - // SEIZE result, reported back as an errno so `io::Error` need not cross - // the channel (it is not `Clone`/`Send`-friendly to reconstruct). - let (attached_tx, attached_rx) = tokio::sync::oneshot::channel::>(); - // Capacity 1: `finish`/`abort`/Drop send exactly one command; the send is - // non-blocking and the worker is always waiting to receive it. - let (cmd_tx, cmd_rx) = std::sync::mpsc::sync_channel::(1); - - let join = tokio::task::spawn_blocking(move || { - process_creation_worker(caller_tid, ctx, attached_tx, cmd_rx) - }); - - match attached_rx.await { - Ok(Ok(())) => Ok(ProcessCreationTrace { cmd_tx, join: Some(join), caller_tid, signaled: false }), - Ok(Err(errno)) => { - let _ = join.await; - Err(io::Error::from_raw_os_error(errno)) - } - Err(_) => { - // Worker dropped the sender without reporting (panic). Reap it. - let _ = join.await; - Err(io::Error::new( - io::ErrorKind::Other, - "process-creation worker exited before SEIZE", - )) - } - } -} - -/// Owns the entire ptrace lifecycle for one fork-tracking session on a single -/// thread. SEIZE happens before `Continue`; the `INTERRUPT` + event loop + -/// detach happen after, once `cmd_rx` delivers `Proceed`. -fn process_creation_worker( - caller_tid: i32, - ctx: Arc, - attached_tx: tokio::sync::oneshot::Sender>, - cmd_rx: std::sync::mpsc::Receiver, -) -> io::Result { - // SEIZE (does NOT stop the tracee) before `Continue`, so the child is born - // traced/stopped once the fork runs. Because SEIZE itself never blocks on - // a stop, it is safe against the seccomp-notify wait the tracee sits in. - let opts = (libc::PTRACE_O_TRACEFORK - | libc::PTRACE_O_TRACEVFORK - | libc::PTRACE_O_TRACECLONE - | libc::PTRACE_O_TRACESYSGOOD) as libc::c_ulong; - let ret = unsafe { libc::ptrace(libc::PTRACE_SEIZE, caller_tid, 0, opts) }; - if ret < 0 { - let errno = io::Error::last_os_error().raw_os_error().unwrap_or(libc::EPERM); - let _ = attached_tx.send(Err(errno)); - return Err(io::Error::from_raw_os_error(errno)); - } - let _ = attached_tx.send(Ok(())); - - // Park until the orchestration confirms `Continue` was sent (Proceed) or - // asks us to tear down (Abort). - match cmd_rx.recv() { - Ok(TraceCmd::Proceed) => {} - Ok(TraceCmd::Abort) | Err(_) => { - detach_traced(caller_tid); - return Ok(false); - } - } - - // After `Continue`, watch for the fork-creation event (no INTERRUPT — see - // `run_creation_event_loop`). - let result = run_creation_event_loop(caller_tid, &ctx); - detach_traced(caller_tid); - result -} - -fn detach_traced(tid: i32) { - let _ = unsafe { libc::ptrace(libc::PTRACE_DETACH, tid, 0, 0) }; -} - -fn wait_for_ptrace_stop(tid: i32) -> io::Result { - let mut status: libc::c_int = 0; - loop { - let ret = unsafe { libc::waitpid(tid, &mut status, libc::__WALL) }; - if ret < 0 { - let err = io::Error::last_os_error(); - if err.raw_os_error() == Some(libc::EINTR) { - continue; - } - return Err(err); - } - break; - } - - if !libc::WIFSTOPPED(status) { - return Err(io::Error::new( - io::ErrorKind::Other, - format!("unexpected ptrace wait status: {status:#x}"), - )); - } - Ok(status) -} - - -#[cfg(test)] -static CHILD_REGISTERED_HOOK: std::sync::Mutex< - Option>, -> = std::sync::Mutex::new(None); - -#[cfg(test)] -fn child_registered_for_test(child_pid: i32) { - if let Ok(guard) = CHILD_REGISTERED_HOOK.lock() { - if let Some(hook) = guard.as_ref() { - hook(child_pid); - } - } -} - -/// Signal `finish`'s watchdog sends to the tracee to wake this blocking wait -/// when a fork created no child (a failed fork emits no ptrace event). SIGURG -/// is effectively unused by normal programs and ignored by default, so it is a -/// safe wake poke that we recognise and swallow. -const FORK_WATCHDOG_SIGNAL: libc::c_int = libc::SIGURG; - -/// Watch the SEIZE'd parent for the fork-creation event after `Continue`. -/// -/// Resolves to `Ok(true)` when the fork created a child (registered before it -/// can run user code) or `Ok(false)` when the fork-like syscall created none. -/// The caller (`process_creation_worker`) detaches the tracee afterward. -/// -/// We request only fork events (PTRACE_O_TRACEFORK family), not syscall -/// tracing. A *successful* fork therefore stops the parent at -/// `PTRACE_EVENT_{FORK,VFORK,CLONE}` synchronously with the fork — with both -/// parent and child born stopped, so the child cannot run user code while we -/// register it. A *failed* fork produces no ptrace stop at all, so this would -/// block forever; `finish` bounds it by sending [`FORK_WATCHDOG_SIGNAL`] to the -/// tracee after a deadline, which we observe here as a signal-delivery-stop and -/// treat as "no child". (We do **not** `PTRACE_INTERRUPT` to force a stop — -/// that races the fork and is unreliable; and we do not busy-poll.) -fn run_creation_event_loop(caller_tid: i32, ctx: &Arc) -> io::Result { - loop { - let mut status: libc::c_int = 0; - let r = unsafe { libc::waitpid(caller_tid, &mut status, libc::__WALL) }; - if r < 0 { - let e = io::Error::last_os_error(); - if e.raw_os_error() == Some(libc::EINTR) { - continue; - } - return Err(e); - } - if libc::WIFEXITED(status) || libc::WIFSIGNALED(status) { - // Tracee exited / was killed out from under us: no child to track. - return Ok(false); - } - if !libc::WIFSTOPPED(status) { - continue; - } - - let event = (status >> 16) & 0xffff; - if event == libc::PTRACE_EVENT_FORK - || event == libc::PTRACE_EVENT_VFORK - || event == libc::PTRACE_EVENT_CLONE - { - return handle_fork_event(caller_tid, ctx); - } - - let stopsig = libc::WSTOPSIG(status); - if stopsig == FORK_WATCHDOG_SIGNAL { - // `finish`'s watchdog fired: the fork-like syscall created no child - // (it returned without a fork event, e.g. EAGAIN/ENOMEM). Swallow - // the wake signal — the worker detaches the tracee next. - return Ok(false); - } - - // Some other signal-delivery-stop in the window: forward the pending - // signal and keep waiting for the fork event. - let inject = if stopsig == libc::SIGTRAP { 0 } else { stopsig as libc::c_ulong }; - ptrace_cont(caller_tid, inject)?; - } -} - -/// Resume `tid` with `sig` injected (0 for none), failing on error. -fn ptrace_cont(tid: i32, sig: libc::c_ulong) -> io::Result<()> { - let ret = unsafe { libc::ptrace(libc::PTRACE_CONT, tid, 0, sig) }; - if ret < 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) -} - -/// On a `PTRACE_EVENT_{FORK,VFORK,CLONE}`: read the new child's pid, register -/// it in `ProcessIndex` (so the execve argv-freeze can enumerate it), then -/// detach the child so it can run. Runs on the worker thread. -fn handle_fork_event(caller_tid: i32, ctx: &Arc) -> io::Result { - let mut child_pid: libc::c_ulong = 0; - let ret = unsafe { - libc::ptrace( - libc::PTRACE_GETEVENTMSG, - caller_tid, - 0, - &mut child_pid, - ) - }; - if ret < 0 { - return Err(io::Error::last_os_error()); - } - - let child_pid = child_pid as i32; - if !register_pid_if_new(ctx, child_pid) { - let _ = unsafe { libc::kill(child_pid, libc::SIGKILL) }; - detach_traced(child_pid); - return Err(io::Error::new( - io::ErrorKind::Other, - format!("failed to register new child pid {child_pid}"), - )); - } - #[cfg(test)] - child_registered_for_test(child_pid); - - // The child is born stopped under PTRACE_O_TRACEFORK; wait for its - // birth-stop, then detach so it can run. Result ignored: a racing exit is - // possible and detach is harmless either way. The caller (parent) is - // detached by `process_creation_worker`. - let _ = wait_for_ptrace_stop(child_pid); - detach_traced(child_pid); - Ok(true) -} - -/// Complete one-shot process-creation tracking after `Continue`. -/// -/// Signals the worker (started in `prepare`) to proceed, then awaits its -/// result. All ptrace work happens on the worker's single thread; this only -/// drives it and bounds the failed-fork case. -pub(crate) async fn finish_process_creation_tracking( - mut trace: ProcessCreationTrace, -) -> io::Result { - /// Upper bound on how long to wait for the fork event. The event is - /// delivered synchronously with the fork (sub-millisecond), so this only - /// elapses for a fork that created no child (e.g. EAGAIN/ENOMEM). - const FORK_EVENT_DEADLINE: std::time::Duration = std::time::Duration::from_secs(2); - - trace.signaled = true; - let caller_tid = trace.caller_tid; - // Send is non-blocking (capacity-1 channel, single sender) — the worker is - // parked waiting to receive, then blocks in `waitpid` for the fork event. - let _ = trace.cmd_tx.send(TraceCmd::Proceed); - let mut join = trace.join.take().expect("join handle present until finish/abort"); - - let join_err = - |e| io::Error::new(io::ErrorKind::Other, format!("spawn_blocking join: {e}")); - - // Race the worker against a watchdog. The worker's `waitpid` is blocking, so - // a *failed* fork (no ptrace event) would hang it forever; on the deadline - // we poke the tracee so its `waitpid` returns and the worker reports "no - // child". `kill` does not need the tracer thread, so this is safe from here. - tokio::select! { - res = &mut join => res.map_err(join_err)?, - _ = tokio::time::sleep(FORK_EVENT_DEADLINE) => { - unsafe { libc::kill(caller_tid, FORK_WATCHDOG_SIGNAL); } - join.await.map_err(join_err)? - } - } -} - -/// Tear down a tracking session whose `Continue` was never sent (e.g. -/// `send_response` failed). Signals the worker to abort; it detaches the -/// tracee on its own thread. -pub(crate) async fn abort_process_creation_tracking(mut trace: ProcessCreationTrace) { - trace.signaled = true; - let _ = trace.cmd_tx.send(TraceCmd::Abort); - if let Some(join) = trace.join.take() { - let _ = join.await; - } -} - /// Handle wait4/waitid notifications — decrement the concurrent process count. /// /// Only blocking waits reach the supervisor (WNOHANG/WNOWAIT calls are @@ -977,16 +633,6 @@ mod tests { TimeRandomState, }; use crate::sys::structs::{SeccompData, SeccompNotif}; - use std::ptr; - - const GO: isize = 0; - const CHILD_RAN: isize = 1; - const REGISTERED_BEFORE_RUN: isize = 2; - const REGISTERED_PID: isize = 3; - const DONE: isize = 4; - const FORK_FAILED: isize = 5; - const FLAGS_LEN: usize = 4096; - fn fake_notif(nr: i64, arg0: u64) -> SeccompNotif { SeccompNotif { id: 0, @@ -1165,228 +811,4 @@ mod tests { unsafe { libc::munmap(writable as *mut _, 16 << 20) }; } - #[test] - fn process_creation_tracking_predicates_follow_argv_safety_gate() { - let no_argv_safety = fake_policy(false); - let argv_safety = fake_policy(true); - let clone_proc = fake_notif(libc::SYS_clone, 0); - let clone_thread = fake_notif(libc::SYS_clone, CLONE_THREAD); - let clone3 = fake_notif(libc::SYS_clone3, 0); - let openat = fake_notif(libc::SYS_openat, 0); - - // notif_fd = -1: clone3's user-memory read fails (id_valid), - // which fail-safes to "not a thread" → counted as process. - // Matches the synthetic clone3 notif's expected accounting. - let fd = -1; - - assert!(fork_counted_on_continue(&clone_proc, fd)); - assert!(!fork_counted_on_continue(&clone_thread, fd)); - assert!(fork_counted_on_continue(&clone3, fd)); - assert!(!fork_counted_on_continue(&openat, fd)); - - assert!(!requires_process_creation_tracking(&clone_proc, fd, &no_argv_safety)); - assert!(requires_process_creation_tracking(&clone_proc, fd, &argv_safety)); - assert!(!requires_process_creation_tracking(&clone_thread, fd, &argv_safety)); - assert!(requires_process_creation_tracking(&clone3, fd, &argv_safety)); - assert!(!requires_process_creation_tracking(&openat, fd, &argv_safety)); - - if let Some(fork_nr) = crate::arch::sys_fork() { - let fork = fake_notif(fork_nr, 0); - assert!(fork_counted_on_continue(&fork, fd)); - assert!(requires_process_creation_tracking(&fork, fd, &argv_safety)); - } - if let Some(vfork_nr) = crate::arch::sys_vfork() { - let vfork = fake_notif(vfork_nr, 0); - assert!(fork_counted_on_continue(&vfork, fd)); - assert!(requires_process_creation_tracking(&vfork, fd, &argv_safety)); - } - } - - struct SharedFlags { - ptr: *mut i32, - } - - impl SharedFlags { - fn new() -> Self { - let ptr = unsafe { - libc::mmap( - ptr::null_mut(), - FLAGS_LEN, - libc::PROT_READ | libc::PROT_WRITE, - libc::MAP_SHARED | libc::MAP_ANONYMOUS, - -1, - 0, - ) - }; - assert_ne!(ptr, libc::MAP_FAILED, "mmap shared flags"); - Self { - ptr: ptr.cast::(), - } - } - - fn read(&self, slot: isize) -> i32 { - unsafe { ptr::read_volatile(self.ptr.offset(slot)) } - } - - fn write(&self, slot: isize, value: i32) { - unsafe { ptr::write_volatile(self.ptr.offset(slot), value) }; - } - - fn addr(&self) -> usize { - self.ptr as usize - } - } - - impl Drop for SharedFlags { - fn drop(&mut self) { - unsafe { - libc::munmap(self.ptr.cast(), FLAGS_LEN); - } - } - } - - struct HookReset; - - impl Drop for HookReset { - fn drop(&mut self) { - if let Ok(mut hook) = CHILD_REGISTERED_HOOK.lock() { - *hook = None; - } - } - } - - struct CallerGuard { - pid: i32, - flags_addr: usize, - } - - impl CallerGuard { - fn new(pid: i32, flags: &SharedFlags) -> Self { - Self { - pid, - flags_addr: flags.addr(), - } - } - - fn disarm(&mut self) { - self.pid = 0; - } - } - - impl Drop for CallerGuard { - fn drop(&mut self) { - if self.pid <= 0 { - return; - } - let flags = self.flags_addr as *mut i32; - unsafe { - ptr::write_volatile(flags.offset(GO), 1); - ptr::write_volatile(flags.offset(DONE), 1); - libc::kill(self.pid, libc::SIGKILL); - let mut status = 0; - let _ = libc::waitpid(self.pid, &mut status, 0); - } - } - } - - #[cfg(any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "riscv64"))] - unsafe fn caller_wait_then_fork(flags: *mut i32) -> ! { - while ptr::read_volatile(flags.offset(GO)) == 0 { - core::hint::spin_loop(); - } - - // x86_64 has a real fork(2) syscall; generic-ABI arches (aarch64, riscv64) - // have none, so glibc fork() emulates it via clone(SIGCHLD). Either way the - // kernel reports a PTRACE_EVENT_{FORK,CLONE}, which is what we track. - #[cfg(target_arch = "x86_64")] - let pid = libc::syscall(libc::SYS_fork) as i32; - #[cfg(not(target_arch = "x86_64"))] - let pid = libc::fork(); - if pid == 0 { - ptr::write_volatile(flags.offset(CHILD_RAN), 1); - while ptr::read_volatile(flags.offset(DONE)) == 0 { - core::hint::spin_loop(); - } - libc::_exit(0); - } - if pid > 0 { - let mut status = 0; - let _ = libc::waitpid(pid, &mut status, 0); - libc::_exit(0); - } - - ptr::write_volatile(flags.offset(FORK_FAILED), 1); - libc::_exit(1); - } - - #[cfg(any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "riscv64"))] - #[test] - fn process_creation_tracking_registers_child_before_user_code_runs() { - let flags = SharedFlags::new(); - let flags_addr = flags.addr(); - - let caller = unsafe { libc::fork() }; - assert!(caller >= 0, "fork caller"); - if caller == 0 { - unsafe { caller_wait_then_fork(flags.ptr) }; - } - let mut caller_guard = CallerGuard::new(caller, &flags); - - let _hook_reset = HookReset; - { - let mut hook = CHILD_REGISTERED_HOOK.lock().expect("hook lock"); - *hook = Some(Box::new(move |child_pid| { - let flags = flags_addr as *mut i32; - unsafe { - let child_ran = ptr::read_volatile(flags.offset(CHILD_RAN)); - ptr::write_volatile(flags.offset(REGISTERED_PID), child_pid); - ptr::write_volatile( - flags.offset(REGISTERED_BEFORE_RUN), - if child_ran == 0 { 1 } else { -1 }, - ); - } - })); - } - - let ctx = fake_supervisor_ctx(true); - let rt = tokio::runtime::Builder::new_current_thread() - // `enable_all` (not just io): `finish_process_creation_tracking` - // arms a `tokio::time` watchdog, which needs the time driver. - .enable_all() - .build() - .expect("tokio runtime"); - let trace = match rt.block_on(prepare_process_creation_tracking(&ctx, caller)) { - Ok(trace) => trace, - Err(e) if matches!(e.raw_os_error(), Some(libc::EPERM | libc::EACCES)) => { - eprintln!("skipping ptrace fork-event test: ptrace denied: {e}"); - return; - } - Err(e) => panic!("prepare process-creation tracking: {e}"), - }; - - flags.write(GO, 1); - let created = rt - .block_on(finish_process_creation_tracking(trace)) - .expect("finish process-creation tracking"); - assert!(created, "fork/clone should produce a ptrace process-creation event"); - - let registered_pid = flags.read(REGISTERED_PID); - assert!(registered_pid > 0, "child pid should be captured by hook"); - assert!( - ctx.processes.contains(registered_pid), - "child should be registered in ProcessIndex" - ); - assert_eq!( - flags.read(REGISTERED_BEFORE_RUN), - 1, - "child should still be ptrace-stopped when registered" - ); - - flags.write(DONE, 1); - let mut status = 0; - let waited = unsafe { libc::waitpid(caller, &mut status, 0) }; - assert_eq!(waited, caller, "wait caller"); - assert_eq!(flags.read(FORK_FAILED), 0, "fork in caller failed"); - caller_guard.disarm(); - } } diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index 91377e7b..1c8d6714 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -2248,57 +2248,10 @@ async fn handle_notification( } }; - let nr = notif.data.nr as i64; let fork_counted = matches!(action, NotifAction::Continue) && crate::resource::fork_counted_on_continue(¬if, fd); - // TOCTOU-close for execve (issue #27): freeze every sandbox task - // that could mutate argv before policy_fn reads argv and before the - // kernel re-reads it after Continue. This covers two writer classes: - // 1. Sibling threads of the calling tid (same TGID, share mm). - // 2. Peer processes in other TGIDs that alias argv pages via - // MAP_SHARED mappings or share mm via clone(CLONE_VM). - // - // The freeze enumerates ProcessIndex. With policy_fn active, that - // index is complete: fork-like syscalls are traced at creation time - // below, before new children can run user code. - // - // Strict on failure: if we cannot establish the freeze, we cannot - // safely expose argv or allow execve, so we deny with EPERM. - let mut exec_freeze = None; - if matches!(action, NotifAction::Continue) - && policy.argv_safety_required - && crate::freeze::requires_freeze_on_continue(nr) - { - match crate::freeze::freeze_sandbox_for_execve( - &ctx.processes, - notif.pid as i32, - ) { - Ok(outcome) => { - exec_freeze = Some(outcome); - } - Err(e) => { - eprintln!( - "sandlock: argv-safety freeze failed for pid {}: {} \ - — denying execve to preserve TOCTOU invariant", - notif.pid, e - ); - action = NotifAction::Errno(libc::EPERM); - // Rollback could not release tasks that had not entered - // ptrace-stop yet; carry them to the post-send reap. - if !e.pending_tids.is_empty() { - exec_freeze = Some(crate::freeze::SandboxFreeze { - pending_tids: e.pending_tids, - ..Default::default() - }); - } - } - } - } - - // Emit event to policy_fn callback if active. For execve, argv is - // only populated after `exec_freeze` has stopped every possible - // writer, and those tasks stay stopped until after NOTIF_SEND. + // Emit event to policy_fn callback if active. if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd, None).await { use crate::policy_fn::Verdict; match verdict { @@ -2313,49 +2266,11 @@ async fn handle_notification( crate::resource::rollback_fork_count(&ctx.resource).await; } - // With policy_fn active, fork-like syscalls are traced for exactly - // one ptrace event so ProcessIndex becomes complete before the new - // child can run user code. That closes the race where a peer - // process could exist without ever having produced a notification. - let mut creation_trace = None; - if matches!(action, NotifAction::Continue) - && crate::resource::requires_process_creation_tracking(¬if, fd, policy) - { - match crate::resource::prepare_process_creation_tracking(ctx, notif.pid as i32).await { - Ok(trace) => { - creation_trace = Some(trace); - } - Err(e) => { - eprintln!( - "sandlock: process-creation tracking failed for pid {}: {} \ - — denying fork-like syscall to preserve argv TOCTOU invariant", - notif.pid, e - ); - if fork_counted { - crate::resource::rollback_fork_count(&ctx.resource).await; - } - action = NotifAction::Errno(libc::EPERM); - } - } - } - // Deferred response: run the handler's future on a worker task so the // single supervisor loop is not blocked waiting for slow work (a network // round-trip, a blocking syscall). The trapped child stays parked in the // syscall; the worker sends the real response later, keyed by notif.id. - // - // Deferral is refused on syscalls whose Continue path requires the - // execve argv-safety freeze or fork creation-tracking: sending the - // response off-loop would skip that TOCTOU-closing work. (When `action` - // is Defer it is not Continue, so `exec_freeze`/`creation_trace` above - // are already None — there is nothing to unwind here.) if let NotifAction::Defer(deferred) = action { - if crate::freeze::requires_freeze_on_continue(nr) - || crate::resource::requires_process_creation_tracking(¬if, fd, policy) - { - let _ = send_response(fd, notif.id, NotifAction::Errno(libc::EPERM)); - return; - } match Arc::clone(defer_sem).try_acquire_owned() { Ok(permit) => spawn_deferred(fd, notif.id, deferred, permit), // Too many deferrals in flight: fail fast with EAGAIN rather than @@ -2367,42 +2282,8 @@ async fn handle_notification( return; } - // Ignore error — child may have exited between recv and response. - let exec_continued = exec_freeze.is_some() && matches!(action, NotifAction::Continue); - let send_result = send_response(fd, notif.id, action); - - if let Some(trace) = creation_trace { - if send_result.is_ok() { - match crate::resource::finish_process_creation_tracking(trace).await { - Ok(true) => {} - Ok(false) => { - crate::resource::rollback_fork_count(&ctx.resource).await; - } - Err(e) => { - crate::resource::rollback_fork_count(&ctx.resource).await; - eprintln!( - "sandlock: process-creation tracking completion failed for pid {}: {}", - notif.pid, e - ); - } - } - } else { - crate::resource::rollback_fork_count(&ctx.resource).await; - crate::resource::abort_process_creation_tracking(trace).await; - } - } - - if let Some(freeze) = exec_freeze { - if exec_continued && send_result.is_ok() { - crate::freeze::detach_peers(&freeze.peer_tids); - } else { - crate::freeze::detach_all(&freeze); - } - // Now that the response is out, the kernel wait holding any pending - // task (the vfork parent waiting on this very execve) can clear; - // reap the queued interrupts and detach. - crate::freeze::reap_pending(&freeze.pending_tids); - } + // Ignore error: the child may have exited between recv and response. + let _ = send_response(fd, notif.id, action); } /// An execve under argv safety. The request is read once; that copy is what diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index 520756d3..fce7645b 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -361,16 +361,6 @@ pub(crate) fn notif_syscalls_resolved(resolved: &ResolvedSandbox) -> Vec { let mut nrs = SyscallList::with(BASE_NOTIF_SYSCALLS); nrs.push_optional(arch::sys_vfork()); - // Bare fork(2) carries none of the namespace/process-limit risk of - // clone/clone3 and was historically left out of the BPF filter so - // hot fork-loops (COW map-reduce) bypass the supervisor entirely. - // It only needs interception when argv safety is required, so the - // supervisor can register the new child via ptrace fork events before - // user code can mutate argv observed by policy_fn or exec handlers. - if features.argv_safety_required { - nrs.push_optional(arch::sys_fork()); - } - if features.memory_limit { nrs.extend(MEMORY_NOTIF_SYSCALLS); // shmget is in notif only when SysV IPC is allowed. The BPF diff --git a/crates/sandlock-core/tests/integration/test_policy_fn.rs b/crates/sandlock-core/tests/integration/test_policy_fn.rs index 855615e4..4e88483f 100644 --- a/crates/sandlock-core/tests/integration/test_policy_fn.rs +++ b/crates/sandlock-core/tests/integration/test_policy_fn.rs @@ -194,7 +194,7 @@ async fn test_policy_fn_passthrough() { assert!(count > 0, "callback should have been called at least once, got {}", count); } -/// Test execve events include argv (TOCTOU-safe via sibling freeze). +/// Test execve events include argv (what the relay then runs). #[tokio::test] async fn test_policy_fn_execve_argv() { let argvs: Arc>>> = Arc::new(Mutex::new(Vec::new())); @@ -222,9 +222,8 @@ async fn test_policy_fn_execve_argv() { assert!(has_python, "argv should contain python3, got: {:?}", *captured); } -/// Test argv_contains-based denial. The supervisor freezes sibling -/// threads of the calling tid before Continue, so the policy_fn's -/// argv inspection binds to what the kernel will run. +/// Test argv_contains-based denial. The exec relay runs exactly the argv +/// the policy inspected, so the verdict binds to what will run. #[tokio::test] async fn test_policy_fn_deny_by_argv() { let policy = base_policy() @@ -552,9 +551,8 @@ async fn test_policy_fn_restrict_max_processes_enforced() { } /// Regression: a workload that forks under an active policy_fn must not -/// deadlock the supervisor's fork-event ptrace tracking. Fork many times in one -/// run and require it to complete (bounded so a regression fails instead of -/// hanging the suite forever). +/// deadlock the supervisor. Fork many times in one run and require it to +/// complete (bounded so a regression fails instead of hanging the suite). #[tokio::test] async fn test_policy_fn_fork_does_not_deadlock() { let many_forks = concat!( From f5312645d7b9489b91c65942d79dc23d9a79cbb0 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 21:23:37 -0700 Subject: [PATCH 4/7] exec-relay: cover chroot and COW execs Under chroot and COW the relay execs by the child's own path and leaves resolution to the existing exec handlers, which now run against the single-threaded relay. Pin both with tests: a chroot rootfs whose argv reaches the policy and whose program runs, and a script the sandbox wrote into the COW branch and then executed. Signed-off-by: Cong Wang --- .../tests/integration/test_exec_relay.rs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/crates/sandlock-core/tests/integration/test_exec_relay.rs b/crates/sandlock-core/tests/integration/test_exec_relay.rs index 75a201f3..e6d68073 100644 --- a/crates/sandlock-core/tests/integration/test_exec_relay.rs +++ b/crates/sandlock-core/tests/integration/test_exec_relay.rs @@ -213,3 +213,76 @@ async fn clone_files_without_thread_is_rejected() { let r = policy.clone().run(&["python3", "-c", script]).await.unwrap(); assert_eq!(stdout_of(&r), "EINVAL", "stderr: {}", stderr_of(&r)); } + +/// Under chroot the relay hands the child's virtual path to the chroot exec +/// handler, which resolves it against the now single-threaded relay. +#[tokio::test] +async fn chroot_exec_reports_argv_and_runs() { + let helper = helper_binary(); + let rootfs = scratch_dir("rootfs"); + for dir in ["usr/bin", "etc", "proc", "dev", "tmp"] { + std::fs::create_dir_all(rootfs.join(dir)).unwrap(); + } + let dest = rootfs.join("usr/bin/rootfs-helper"); + std::fs::copy(&helper, &dest).unwrap(); + std::fs::set_permissions(&dest, std::os::unix::fs::PermissionsExt::from_mode(0o755)).unwrap(); + std::os::unix::fs::symlink("usr/bin", rootfs.join("bin")).unwrap(); + + let seen: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + let policy = Sandbox::builder() + .chroot(&rootfs) + .fs_read("/usr") + .fs_read("/bin") + .fs_read("/proc") + .fs_read("/dev") + .policy_fn(move |event, _ctx| { + if event.syscall == "execve" { + if let Some(argv) = &event.argv { + seen_cb.lock().unwrap().push(argv.clone()); + } + } + Verdict::Allow + }) + .build() + .unwrap(); + let r = policy.clone().run(&["/bin/rootfs-helper", "echo", "chroot-relay-ok"]).await.unwrap(); + assert!(r.success(), "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), "chroot-relay-ok"); + let seen = seen.lock().unwrap(); + assert!(seen.iter().any(|a| a.iter().any(|s| s == "chroot-relay-ok")), "argv seen: {seen:?}"); + let _ = std::fs::remove_dir_all(&rootfs); +} + +/// A script the sandbox itself wrote lives only in the COW branch; the COW +/// exec handler serves it to the relay's exec. +#[tokio::test] +async fn cow_exec_of_a_file_written_in_the_sandbox() { + let workdir = scratch_dir("cow"); + let script = workdir.join("made-inside.sh"); + let seen: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let seen_cb = seen.clone(); + let policy = base_policy() + .fs_write(&workdir) + .workdir(&workdir) + .policy_fn(move |event, _ctx| { + if event.syscall == "execve" { + if let Some(argv) = &event.argv { + seen_cb.lock().unwrap().push(argv.clone()); + } + } + Verdict::Allow + }) + .build() + .unwrap(); + let cmd = format!( + "printf '#!/bin/sh\\necho cow-relay-ok $1\\n' > {s} && chmod +x {s} && {s} from-cow", + s = script.display() + ); + let r = policy.clone().run(&["sh", "-c", &cmd]).await.unwrap(); + assert!(r.success(), "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), "cow-relay-ok from-cow"); + let seen = seen.lock().unwrap(); + assert!(seen.iter().any(|a| a.iter().any(|s| s == "from-cow")), "argv seen: {seen:?}"); + let _ = std::fs::remove_dir_all(&workdir); +} From f30dbf9accfca220a6ff7633e924b3051ac23a92 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 21:24:41 -0700 Subject: [PATCH 5/7] docs: describe the exec relay in place of the argv freeze The freeze and fork-event tracking are gone; say what now keeps argv TOCTOU-safe and which deferral rule remains. Signed-off-by: Cong Wang --- README.md | 16 +++++++--------- docs/extension-handlers.md | 9 +++++---- docs/sandbox-reference.md | 4 ++-- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e458a902..b5e2ad3f 100644 --- a/README.md +++ b/README.md @@ -385,15 +385,13 @@ positive int = deny with errno, `"audit"`/`-2` = allow + flag. > belongs in static Landlock rules (`fs_readable` / `fs_writable` / > `fs_denied`) — kernel-enforced and TOCTOU-immune. Use > `ctx.deny_path()` for runtime additions. -> - **`event.argv` is exposed and TOCTOU-safe.** Before exposing -> `argv` to `policy_fn` or returning `Continue` for an -> `execve`, the supervisor freezes every task in `ProcessIndex`, -> including peer processes that may alias argv through shared memory. -> With `policy_fn` active, fork-like syscalls are traced for one -> ptrace creation event, so children are registered in `ProcessIndex` -> before they can run user code. If the freeze or creation tracking -> cannot be established (e.g., YAMA blocks ptrace), the syscall is -> denied with `EPERM`; the safety invariant is never silently relaxed. +> - **`event.argv` is exposed and TOCTOU-safe.** The supervisor reads +> `argv` once and, on `Continue`, redirects the `execve` to a small +> static relay program delivered in a sealed memfd at an fd number the +> sandbox cannot repopulate. The relay execs the target with exactly the +> argv the policy saw, so a sibling thread or `CLONE_VM` peer rewriting +> the original memory changes nothing. No task is stopped and no ptrace +> is involved; the one visible cost is an extra `execve` per spawn. **Context methods:** - `ctx.restrict_network(ips)` / `ctx.grant_network(ips)` — network control diff --git a/docs/extension-handlers.md b/docs/extension-handlers.md index cb207241..56376ac6 100644 --- a/docs/extension-handlers.md +++ b/docs/extension-handlers.md @@ -512,10 +512,11 @@ Contract: - **Terminal decision.** `Defer` is non-`Continue`, so it short-circuits the handler chain exactly like `Errno`/`ReturnValue`: later handlers on the same syscall do not run. A deferring handler decides the outcome. -- **No deferral on freeze/fork syscalls.** Deferral is refused (with `EPERM`) on - `execve`/`execveat` and fork-creating syscalls, because moving the response off-loop would skip - the argv-safety freeze (see [issue #27][i27]) and process creation-tracking that those paths - require before `Continue`. +- **No deferral on policy-checked execs.** Deferral is refused (with `EPERM`) on + `execve`/`execveat` while a `policy_fn` or an exec-bound handler is active, because the + exec relay that keeps `argv` TOCTOU-safe (see [issue #27][i27]) must rewrite the exec on the + supervisor loop before `Continue`. Such handlers also see the relay's own re-exec of an + approved program, flagged by `HandlerCtx::relay_exec`. - **Bounded fan-out.** At most `DEFER_MAX_INFLIGHT` deferred futures run concurrently; beyond that, further deferrals fail fast with `EAGAIN` rather than queuing. The cap also bounds the resources workers hold (memfds, sockets). diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index fb38124c..a95284b4 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -551,5 +551,5 @@ parse_ports([80, "443", "8000-8005"]) user-memory pointers after `Continue`. Path-based control belongs in static Landlock rules (`fs_readable`, `fs_writable`, `fs_denied`) or in `ctx.deny_path()` for runtime additions. - `event.argv` is exposed and TOCTOU-safe; the supervisor freezes - peer tasks before exposing it. + `event.argv` is exposed and TOCTOU-safe: an allowed `execve` runs + through a relay that execs the target with the argv the policy saw. From c4de622b1f3b72ace70bfb33bf4e29c6c3a4a3da Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 11 Sep 2026 21:37:05 -0700 Subject: [PATCH 6/7] exec-relay: hide the relay's own syscalls from policy events Between its two execs the relay opens the target's directory, and that open reached policy_fn as an event from the application's pid. sandlock learn reads /proc//exe and maps on the first event after an exec to pin the real binary, so it recorded the memfd instead and produced profiles that could not run the program. A running relay is mechanism, not application behaviour: its notifications are still dispatched but never emitted as events. Signed-off-by: Cong Wang --- crates/sandlock-core/src/exec_relay/mod.rs | 17 +++++++++++++++++ crates/sandlock-core/src/seccomp/notif.rs | 12 ++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/sandlock-core/src/exec_relay/mod.rs b/crates/sandlock-core/src/exec_relay/mod.rs index c3d84015..bfdcd35d 100644 --- a/crates/sandlock-core/src/exec_relay/mod.rs +++ b/crates/sandlock-core/src/exec_relay/mod.rs @@ -186,6 +186,23 @@ pub struct RelayState { holds: Mutex>, } +impl RelayState { + /// Whether `pid` is a running relay: a hold exists for its process and its + /// exe is the memfd. Syscalls the relay makes between its two execs are + /// mechanism, not application behaviour, and must not reach policy_fn. + pub(crate) fn is_relay_task(&self, pid: i32) -> bool { + let holds = self.holds.lock().unwrap(); + if holds.is_empty() { + return false; + } + let tgid = read_tgid_of_tid(pid).unwrap_or(pid); + match holds.get(&tgid) { + Some(hold) => ident_of(Path::new(&format!("/proc/{pid}/exe"))) == Some(hold.memfd_ident), + None => false, + } + } +} + pub(crate) enum Prepared { /// The relay's own execve: already judged, let the exec handlers run it. SecondExec, diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index 1c8d6714..866a01a7 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -2251,8 +2251,16 @@ async fn handle_notification( let fork_counted = matches!(action, NotifAction::Continue) && crate::resource::fork_counted_on_continue(¬if, fd); - // Emit event to policy_fn callback if active. - if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd, None).await { + // Emit event to policy_fn callback if active. A running exec relay's own + // syscalls are hidden: observers would otherwise record the memfd as + // the program that ran. + let relay_internal = policy.argv_safety_required && ctx.exec_relay.is_relay_task(notif.pid as i32); + let verdict = if relay_internal { + None + } else { + emit_policy_event(¬if, &action, &ctx.policy_fn, fd, None).await + }; + if let Some(verdict) = verdict { use crate::policy_fn::Verdict; match verdict { Verdict::Deny => { action = NotifAction::Errno(libc::EPERM); } From 33e083ab775d2d590de099d5860463e25f941ca0 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Sat, 12 Sep 2026 19:44:33 -0700 Subject: [PATCH 7/7] exec-relay: fence the pin fd against dup2/dup3 swap The relay pins its memfd at a fd K just below the child's soft RLIMIT_NOFILE and rewrites the exec to /dev/fd/K. A sibling thread or a CLONE_VM peer could replace K with another file between the supervisor verifying it and the kernel opening it, running an unapproved program with an attacker-chosen argv. The earlier defense lowered the soft limit to fence K, which a task could undo by raising the limit back, so it also needed a prlimit/setrlimit trap and a racy clone gate for CLONE_FILES peers that carry their own limit. Of every fd-creating syscall, only dup2 and dup3 can force a file onto an already-occupied descriptor; open, F_DUPFD, SCM_RIGHTS and pidfd_getfd all take the lowest free number and cannot land on an occupied K. So trap dup2 and dup3 under argv safety and, while an exec is in flight, refuse one whose newfd is a pinned K. newfd is a register argument, not child memory, so the check cannot be raced, and keying on the fd rather than a per-process limit also covers a CLONE_FILES peer sharing the fd table. The hold that arms the refusal is recorded before the install, so K is never live but unguarded. This drops the soft-limit lowering and restore, the prlimit64/setrlimit trap, and the racy clone3 CLONE_FILES gate. The program that runs keeps its original NOFILE limits untouched, and dup2/dup3 outside an exec pass through. Signed-off-by: Cong Wang --- crates/sandlock-core/src/arch.rs | 1 + crates/sandlock-core/src/exec_relay/mod.rs | 131 +++++++++++++----- crates/sandlock-core/src/resource.rs | 17 +-- crates/sandlock-core/src/seccomp/dispatch.rs | 4 +- crates/sandlock-core/src/seccomp/notif.rs | 13 ++ crates/sandlock-core/src/seccomp_plan.rs | 10 ++ .../tests/integration/test_exec_relay.rs | 37 +++-- 7 files changed, 148 insertions(+), 65 deletions(-) diff --git a/crates/sandlock-core/src/arch.rs b/crates/sandlock-core/src/arch.rs index 228e02bd..f0300766 100644 --- a/crates/sandlock-core/src/arch.rs +++ b/crates/sandlock-core/src/arch.rs @@ -75,6 +75,7 @@ legacy_syscall!(sys_chown, "chown"); legacy_syscall!(sys_lchown, "lchown"); legacy_syscall!(sys_vfork, "vfork"); legacy_syscall!(sys_fork, "fork"); +legacy_syscall!(sys_dup2, "dup2"); /// `renameat` syscall number on this architecture, or `None` where the ABI /// omits it. Unlike the legacy syscalls above it survived into the generic diff --git a/crates/sandlock-core/src/exec_relay/mod.rs b/crates/sandlock-core/src/exec_relay/mod.rs index bfdcd35d..18850a12 100644 --- a/crates/sandlock-core/src/exec_relay/mod.rs +++ b/crates/sandlock-core/src/exec_relay/mod.rs @@ -8,12 +8,21 @@ //! saw, read from a trailer on its own image. See `relay.c` for the child side. //! //! The memfd is installed at a free fd K just below the child's soft -//! RLIMIT_NOFILE and the soft limit is then set to K until the relay runs: -//! no dup2, open, F_DUPFD, SCM_RIGHTS or pidfd_getfd in the sandbox can -//! place a different file at K, so the sibling that could rewrite argv -//! cannot swap the program either. The child's path is rewritten in place -//! to `/dev/fd/K`, kept short because the bytes after a short path are -//! often the argv pointer array, which cannot move. +//! RLIMIT_NOFILE and the child's path is rewritten in place to `/dev/fd/K`, +//! kept short because the bytes after a short path are often the argv pointer +//! array, which cannot move. +//! +//! K must stay the relay memfd from the install until the kernel opens it for +//! the exec. Of every fd-creating syscall, only `dup2` and `dup3` can force a +//! file onto an *already-occupied* descriptor; `open`, `F_DUPFD`, SCM_RIGHTS +//! and `pidfd_getfd` all take the lowest free number and cannot land on K. +//! So `dup2`/`dup3` are trapped under argv safety and, while an exec is in +//! flight, one whose `newfd` is a pinned K is refused (`guard_dup`). Their +//! `newfd` is a register argument, not child memory, so the check cannot be +//! raced, and it is keyed on the fd rather than a per-process limit, so it +//! also covers a `CLONE_FILES` peer that shares the fd table with its own +//! rlimit. The hold that arms the refusal is recorded before the install, so +//! there is no window in which K is live but unguarded. use std::collections::HashMap; use std::ffi::OsStr; @@ -176,7 +185,8 @@ impl ExecRequest { struct Hold { memfd_ident: (u64, u64), - old_soft: u64, + /// The pinned fd. A `dup2`/`dup3` onto it is refused while the hold stands. + k: i32, } /// Per-tgid record of a relay exec in flight, from commit until the relay's @@ -201,8 +211,38 @@ impl RelayState { None => false, } } + + /// Whether `newfd` is the pinned fd of any exec currently in flight, i.e. + /// a `dup2`/`dup3` onto it would swap the relay memfd out. See `guard_dup`. + fn is_pinned_fd(&self, newfd: i32) -> bool { + let holds = self.holds.lock().unwrap(); + !holds.is_empty() && holds.values().any(|h| h.k == newfd) + } } +/// Decision for a trapped `dup2`/`dup3`: refuse it when `newfd` is a fd pinned +/// by an in-flight exec, so the sandbox cannot replace the relay memfd between +/// the supervisor installing it and the kernel opening it. `newfd` is a +/// register argument (dup2(oldfd, newfd) / dup3(oldfd, newfd, flags)), not +/// child memory, so this cannot be raced; keying on the fd rather than a +/// per-process rlimit also covers a `CLONE_FILES` peer sharing the fd table. +pub(crate) fn guard_dup(notif: &SeccompNotif, relay: &RelayState) -> Result<(), i32> { + let newfd = notif.data.args[1] as i64; + if (0..=i32::MAX as i64).contains(&newfd) && relay.is_pinned_fd(newfd as i32) { + return Err(libc::EPERM); + } + Ok(()) +} + +/// Decision for a trapped `prlimit64`/`setrlimit`: refuse an `RLIMIT_NOFILE` +/// change to a tgid whose exec is in flight, so the sandbox cannot raise the +/// soft limit back and place another file at the pinned fd; otherwise let the +/// kernel run it. `resource` and the `new_limit` presence come from register +/// args, not child memory, so this decision cannot be raced. +/// +/// The syscalls are trapped for the whole run (seccomp cannot arm a filter for +/// just the exec window), so the common case — no exec in flight — returns on a +/// single lock-guarded emptiness check, before touching args or `/proc`. pub(crate) enum Prepared { /// The relay's own execve: already judged, let the exec handlers run it. SecondExec, @@ -224,22 +264,13 @@ fn ident_of(path: &Path) -> Option<(u64, u64)> { std::fs::metadata(path).ok().map(|m| (m.dev(), m.ino())) } -fn nofile_limits(pid: i32) -> io::Result<(u64, u64)> { +fn nofile_soft(pid: i32) -> io::Result { let mut old = libc::rlimit64 { rlim_cur: 0, rlim_max: 0 }; let r = unsafe { libc::prlimit64(pid, libc::RLIMIT_NOFILE, std::ptr::null(), &mut old) }; if r != 0 { return Err(io::Error::last_os_error()); } - Ok((old.rlim_cur, old.rlim_max)) -} - -fn set_soft_nofile(pid: i32, soft: u64, hard: u64) -> io::Result<()> { - let new = libc::rlimit64 { rlim_cur: soft, rlim_max: hard }; - let r = unsafe { libc::prlimit64(pid, libc::RLIMIT_NOFILE, &new, std::ptr::null_mut()) }; - if r != 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) + Ok(old.rlim_cur) } /// Step one of a policy-checked exec: recognise the relay's own execve, or @@ -257,9 +288,6 @@ pub(crate) async fn prepare( let hold = ctx.exec_relay.holds.lock().unwrap().remove(&tgid); if let Some(hold) = hold { let ours = ident_of(Path::new(&format!("/proc/{pid}/exe"))) == Some(hold.memfd_ident); - if let Ok((_, hard)) = nofile_limits(tgid) { - let _ = set_soft_nofile(tgid, hold.old_soft.min(hard), hard); - } if ours { return Ok(Prepared::SecondExec); } @@ -468,25 +496,29 @@ fn sealed_memfd(image: &[u8]) -> io::Result { pub(crate) fn commit(pending: PendingExec, notif: &SeccompNotif, notif_fd: RawFd, ctx: &Arc) -> Result<(), i32> { let pid = notif.pid as i32; let PendingExec { request, args, tgid } = pending; - let (soft, hard) = nofile_limits(tgid).map_err(|e| errno_of(&e))?; + let soft = nofile_soft(tgid).map_err(|e| errno_of(&e))?; // Seven digits keep "/dev/fd/K" within 16 bytes; a free slot just below - // the soft limit is almost never in use. - let top = soft.min(hard).min(10_000_000); + // the soft limit is almost never in use, and ADDFD needs newfd < soft. + let top = soft.min(10_000_000); if top < 32 { return Err(libc::EAGAIN); } let k = (top - 16..top) .rev() .find(|k| std::fs::symlink_metadata(format!("/proc/{pid}/fd/{k}")).is_err()) - .ok_or(libc::EAGAIN)?; + .ok_or(libc::EAGAIN)? as i32; let k_link = format!("/proc/{pid}/fd/{k}"); - let image = build_image(&args, k as i32).map_err(|e| errno_of(&e))?; + let image = build_image(&args, k).map_err(|e| errno_of(&e))?; let memfd = sealed_memfd(&image).map_err(|e| errno_of(&e))?; let ident = std::fs::metadata(format!("/proc/self/fd/{}", memfd.as_raw_fd())) .map(|m| (m.dev(), m.ino())) .map_err(|e| errno_of(&e))?; - let restore = || { let _ = set_soft_nofile(tgid, soft, hard); }; + // Arm the hold before the install so `guard_dup` fences K for the whole + // window; drop it on any failure below. + ctx.exec_relay.holds.lock().unwrap().insert(tgid, Hold { memfd_ident: ident, k }); + let unwind = || { ctx.exec_relay.holds.lock().unwrap().remove(&tgid); }; + let addfd = SeccompNotifAddfd { id: notif.id, flags: SECCOMP_ADDFD_FLAG_SETFD, @@ -496,33 +528,58 @@ pub(crate) fn commit(pending: PendingExec, notif: &SeccompNotif, notif_fd: RawFd }; let installed = unsafe { libc::ioctl(notif_fd, SECCOMP_IOCTL_NOTIF_ADDFD as libc::Ioctl, &addfd as *const _) }; if installed < 0 { - restore(); + unwind(); return Err(libc::EAGAIN); } - if let Err(e) = set_soft_nofile(tgid, k, hard) { - restore(); - return Err(errno_of(&e)); - } - // Nothing in the sandbox can change fd K from here on, so this check - // settles what the kernel will open. + // ADDFD force-installed our memfd at K and dup2/dup3 onto K are now refused, + // so this settles what the kernel will open. if ident_of(Path::new(&k_link)) != Some(ident) { - restore(); + unwind(); return Err(libc::EAGAIN); } let new_path = format!("{}/{k}\0", fd_dir()); if rewrite_exec_path( notif_fd, notif.id, notif.pid, request.path_ptr, request.argv_ptr, request.envp_ptr, new_path.as_bytes(), ).is_err() { - restore(); + unwind(); return Err(libc::EFAULT); } - ctx.exec_relay.holds.lock().unwrap().insert(tgid, Hold { memfd_ident: ident, old_soft: soft }); Ok(()) } #[cfg(test)] mod tests { use super::*; + use crate::sys::structs::SeccompData; + + fn notif(nr: i64, args: [u64; 6], pid: u32) -> SeccompNotif { + SeccompNotif { + id: 1, + pid, + flags: 0, + data: SeccompData { nr: nr as i32, arch: 0, instruction_pointer: 0, args }, + } + } + + #[test] + fn guard_refuses_dup_onto_a_pinned_fd_only_while_a_hold_is_active() { + let relay = RelayState::default(); + let tgid = std::process::id() as i32; + const K: i32 = 1_048_560; + let dup3 = crate::arch::sys_dup2().unwrap_or(libc::SYS_dup3); + // dup2/dup3(oldfd, newfd, ..): newfd is args[1]. + let onto_k = notif(dup3, [0, K as u64, 0, 0, 0, 0], tgid as u32); + let onto_other = notif(dup3, [0, 5, 0, 0, 0, 0], tgid as u32); + + assert!(guard_dup(&onto_k, &relay).is_ok(), "no hold: allowed"); + + relay.holds.lock().unwrap().insert(tgid, Hold { memfd_ident: (0, 0), k: K }); + assert_eq!(guard_dup(&onto_k, &relay), Err(libc::EPERM), "hold: dup onto K refused"); + assert!(guard_dup(&onto_other, &relay).is_ok(), "hold: dup onto other fd allowed"); + + relay.holds.lock().unwrap().remove(&tgid); + assert!(guard_dup(&onto_k, &relay).is_ok(), "hold cleared: allowed again"); + } fn sample(mode: ArgsMode) -> ExecArgs { ExecArgs { diff --git a/crates/sandlock-core/src/resource.rs b/crates/sandlock-core/src/resource.rs index 621b9ba6..4642340a 100644 --- a/crates/sandlock-core/src/resource.rs +++ b/crates/sandlock-core/src/resource.rs @@ -25,7 +25,6 @@ use crate::sys::structs::{ /// CLONE_THREAD flag — threads don't count toward process limit. const CLONE_THREAD: u64 = 0x0001_0000; -const CLONE_FILES: u64 = 0x0000_0400; /// MAP_ANONYMOUS flag: anonymous and writable private file mappings count. const MAP_ANONYMOUS: u64 = 0x20; @@ -40,9 +39,8 @@ const MAP_ANONYMOUS: u64 = 0x20; /// TOCTOU note: the `clone3` read is from racy user memory — a sibling /// thread could mutate the struct between this read and the kernel's /// re-read after `Continue`. Callers use this only for resource -/// accounting (`proc_count`, the CLONE_FILES gate), never as a -/// security boundary, so a misread can throttle incorrectly but cannot -/// bypass any kernel-enforced deny. +/// accounting (`proc_count`), never as a security boundary, so a misread +/// can throttle incorrectly but cannot bypass any kernel-enforced deny. fn clone_flags(notif: &SeccompNotif, notif_fd: RawFd) -> Option { let args = ¬if.data.args; let nr = notif.data.nr as i64; @@ -84,21 +82,10 @@ pub(crate) async fn handle_fork( notif: &SeccompNotif, notif_fd: RawFd, ctx: &Arc, - policy: &NotifPolicy, ) -> NotifAction { let nr = notif.data.nr as i64; let args = ¬if.data.args; - // The exec relay pins its fd through the caller's RLIMIT_NOFILE, which a - // process sharing the fd table without sharing the limit could defeat. - if policy.argv_safety_required { - if let Some(flags) = clone_flags(notif, notif_fd) { - if flags & CLONE_FILES != 0 && flags & CLONE_THREAD == 0 { - return NotifAction::Errno(libc::EINVAL); - } - } - } - // Namespace flags are denied for clone (clone3's are caught by the // BPF arg filter; vfork takes no flags). if nr == libc::SYS_clone && (args[0] & CLONE_NS_FLAGS) != 0 { diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 02f9e195..29058637 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -279,15 +279,13 @@ pub(crate) fn build_dispatch_table( // Fork/clone family (always on) // ------------------------------------------------------------------ for nr in arch::fork_like_syscalls() { - let policy_for_fork = Arc::clone(policy); let ctx_for_fork = Arc::clone(ctx); table.register(nr, move |cx: &HandlerCtx| { let notif = cx.notif; let notif_fd = cx.notif_fd; - let policy = Arc::clone(&policy_for_fork); let ctx = Arc::clone(&ctx_for_fork); async move { - crate::resource::handle_fork(¬if, notif_fd, &ctx, &policy).await + crate::resource::handle_fork(¬if, notif_fd, &ctx).await } }); } diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index 866a01a7..c61d1dcf 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -2205,6 +2205,19 @@ async fn handle_notification( handle_relay_exec(notif, ctx, dispatch_table, fd).await; return; } + // Fence the relay's pin fd: refuse a dup2/dup3 onto a fd pinned by an + // in-flight exec, so the relay memfd cannot be swapped before the + // kernel opens it. Outside a hold this is a plain Continue. + if policy.argv_safety_required + && (nr == libc::SYS_dup3 || Some(nr) == crate::arch::sys_dup2()) + { + let action = match crate::exec_relay::guard_dup(¬if, &ctx.exec_relay) { + Ok(()) => NotifAction::Continue, + Err(errno) => NotifAction::Errno(errno), + }; + let _ = send_response(fd, notif.id, action); + return; + } } // Check dynamic path denials before dispatch. The gated syscall set is diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index fce7645b..40e22cb7 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -361,6 +361,16 @@ pub(crate) fn notif_syscalls_resolved(resolved: &ResolvedSandbox) -> Vec { let mut nrs = SyscallList::with(BASE_NOTIF_SYSCALLS); nrs.push_optional(arch::sys_vfork()); + // Under argv safety the exec relay pins its memfd at a fd K and rewrites + // the exec to /dev/fd/K. Only dup2/dup3 can force another file onto an + // already-occupied fd, so they are trapped and one whose newfd is a pinned + // K is refused while an exec is in flight (`guard_dup`). dup3 is the + // generic-ABI syscall (all arches); dup2 exists only on legacy (x86_64). + if features.argv_safety_required { + nrs.push(libc::SYS_dup3); + nrs.push_optional(arch::sys_dup2()); + } + if features.memory_limit { nrs.extend(MEMORY_NOTIF_SYSCALLS); // shmget is in notif only when SysV IPC is allowed. The BPF diff --git a/crates/sandlock-core/tests/integration/test_exec_relay.rs b/crates/sandlock-core/tests/integration/test_exec_relay.rs index e6d68073..3b698e92 100644 --- a/crates/sandlock-core/tests/integration/test_exec_relay.rs +++ b/crates/sandlock-core/tests/integration/test_exec_relay.rs @@ -185,21 +185,38 @@ async fn threads_spawning_subprocesses_all_succeed() { assert_eq!(stdout_of(&r), "SPAWNS 80", "stderr: {}", stderr_of(&r)); } -/// The relay borrows the soft NOFILE limit to pin its fd; the program that -/// finally runs must see the limit it would have had. +/// The relay no longer touches RLIMIT_NOFILE, so the program that runs sees the +/// limit it would have had. #[tokio::test] -async fn soft_nofile_limit_is_restored_for_the_program() { - let outside = std::process::Command::new("sh").args(["-c", "ulimit -Sn"]).output().unwrap(); +async fn nofile_limit_is_untouched_for_the_program() { + let outside = std::process::Command::new("sh").args(["-c", "ulimit -Sn; ulimit -Hn"]).output().unwrap(); let outside = String::from_utf8_lossy(&outside.stdout).trim().to_string(); let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); - let r = policy.clone().run(&["sh", "-c", "ulimit -Sn"]).await.unwrap(); + let r = policy.clone().run(&["sh", "-c", "ulimit -Sn; ulimit -Hn"]).await.unwrap(); assert_eq!(stdout_of(&r), outside); } -/// A process sharing its fd table with another process could repopulate the -/// pinned fd number, so that clone shape is refused under an argv policy. +/// The relay fences its pin fd by refusing dup2/dup3 onto it, so a program not +/// mid-exec must still be able to dup2 onto an arbitrary high fd normally. #[tokio::test] -async fn clone_files_without_thread_is_rejected() { +async fn program_can_dup2_onto_a_high_fd() { + let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); + let script = concat!( + "import os, resource\n", + "soft, _ = resource.getrlimit(resource.RLIMIT_NOFILE)\n", + "k = soft - 100\n", // near where the relay would pin, but no exec in flight + "os.dup2(0, k)\n", + "print('DUP2_OK' if os.fstat(k) else 'NO')\n", + ); + let r = policy.clone().run(&["python3", "-c", script]).await.unwrap(); + assert_eq!(stdout_of(&r), "DUP2_OK", "stderr: {}", stderr_of(&r)); +} + +/// The pin is fenced per-fd, not per-process, so a CLONE_FILES peer that shares +/// the fd table with its own rlimit is no longer a threat and that clone shape +/// is now allowed to run. +#[tokio::test] +async fn clone_files_without_thread_is_allowed() { let policy = base_policy().policy_fn(|_e, _c| Verdict::Allow).build().unwrap(); let script = concat!( "import ctypes, os, platform\n", @@ -208,10 +225,10 @@ async fn clone_files_without_thread_is_rejected() { "nr = 56 if platform.machine() == 'x86_64' else 220\n", "r = libc.syscall(nr, CLONE_FILES | SIGCHLD, 0, 0, 0, 0)\n", "if r == 0: os._exit(0)\n", - "print('EINVAL' if r < 0 and ctypes.get_errno() == 22 else 'RET %d' % r)\n", + "print('OK' if r > 0 else 'ERR %d' % ctypes.get_errno())\n", ); let r = policy.clone().run(&["python3", "-c", script]).await.unwrap(); - assert_eq!(stdout_of(&r), "EINVAL", "stderr: {}", stderr_of(&r)); + assert_eq!(stdout_of(&r), "OK", "stderr: {}", stderr_of(&r)); } /// Under chroot the relay hands the child's virtual path to the chroot exec