From fdc983b6cdae29b8b535f046ec89ed6bc34e5f62 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 14:11:28 -0700 Subject: [PATCH 1/4] cow: mediate openat2 like open and openat COW registered only openat and legacy open, and the notif list did the same, so a raw openat2 under the workdir never reached the supervisor. The kernel ran it against the real directory with the child's Landlock write grant, so an O_CREAT or O_TRUNC open survived an abort and a read-only open could see stale lower content past a whiteout. Reusing decode_open_args also fixes the argument layout: for openat2 the flags and mode live in a struct open_how in child memory, not in the syscall args. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/dispatch.rs | 16 ++---- crates/sandlock-core/src/seccomp/dispatch.rs | 4 +- crates/sandlock-core/src/seccomp_plan.rs | 2 + .../tests/integration/test_cow.rs | 52 +++++++++++++++++++ 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index 9453906e..28ac4dc1 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -35,7 +35,7 @@ use crate::arch; use crate::cow::result::link_result; use crate::cow::seccomp::SeccompCowBranch; use crate::procfs::{build_dirent64, DT_DIR, DT_LNK, DT_REG}; -use crate::seccomp::notif::{read_child_mem, write_child_mem, write_child_mem_force, NotifAction}; +use crate::seccomp::notif::{decode_open_args, read_child_mem, write_child_mem, write_child_mem_force, NotifAction}; use crate::seccomp::state::{CowState, PerProcessState, ProcessIndex}; use crate::sys::structs::SeccompNotif; @@ -186,8 +186,7 @@ fn open_confined( crate::sys::fs::openat2_in_root(root, &rel, flags, mode) } -/// Handle openat under workdir: redirect to COW upper/lower. -/// openat(dirfd, pathname, flags, mode): args[0]=dirfd, args[1]=path, args[2]=flags +/// Handle open/openat/openat2 under workdir: redirect to COW upper/lower. pub(crate) async fn handle_cow_open( notif: &SeccompNotif, cow_state: &Arc>, @@ -196,14 +195,9 @@ pub(crate) async fn handle_cow_open( ) -> NotifAction { use crate::cow::seccomp::CowOpenPlan; - let nr = notif.data.nr as i64; - - // open(path, flags, mode): args[0]=path, args[1]=flags, args[2]=mode - // openat(dirfd, path, flags, mode): args[0]=dirfd, args[1]=path, args[2]=flags, args[3]=mode - let (path_ptr, dirfd, flags, mode) = if Some(nr) == arch::sys_open() { - (notif.data.args[0], libc::AT_FDCWD as i64, notif.data.args[1], notif.data.args[2]) - } else { - (notif.data.args[1], notif.data.args[0] as i64, notif.data.args[2], notif.data.args[3]) + let (dirfd, path_ptr, flags, mode) = match decode_open_args(notif, notif_fd) { + Some(a) => (a.dirfd, a.path_ptr, a.flags, a.mode), + None => return NotifAction::Continue, }; let rel_path = match read_path(notif, path_ptr, notif_fd) { diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 3673d84b..36d78bf2 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -1034,9 +1034,7 @@ fn register_cow_handlers(table: &mut DispatchTable, ctx: &Arc) { table.register(nr, cow_call!(crate::cow::dispatch::handle_cow_access)); } - let mut open_nrs = vec![libc::SYS_openat]; - open_nrs.extend(arch::sys_open()); - for nr in open_nrs { + for nr in open_family_syscalls() { table.register(nr, cow_call!(crate::cow::dispatch::handle_cow_open)); } diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index 03fc4560..fc8f0255 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -133,6 +133,8 @@ const NETLINK_NOTIF_SYSCALLS: &[i64] = &[ fn cow_path_syscalls() -> Vec { let mut v = vec![ libc::SYS_openat, + // Left to the kernel, openat2 writes straight into the real workdir. + arch::SYS_OPENAT2, libc::SYS_execve, libc::SYS_execveat, libc::SYS_unlinkat, diff --git a/crates/sandlock-core/tests/integration/test_cow.rs b/crates/sandlock-core/tests/integration/test_cow.rs index 3698cfed..e2d89bb3 100644 --- a/crates/sandlock-core/tests/integration/test_cow.rs +++ b/crates/sandlock-core/tests/integration/test_cow.rs @@ -313,6 +313,58 @@ async fn test_seccomp_cow_legacy_open_syscall() { let _ = fs::remove_file(&out_file); } +/// openat2 must land in the COW layer like open/openat. +/// +/// Regression test: COW registered only openat and legacy open, so a raw +/// openat2(O_CREAT|O_TRUNC) under the workdir went straight to the kernel and +/// mutated the real directory, surviving an abort. openat2 also keeps flags +/// and mode in a struct open_how rather than in the syscall args. +#[tokio::test] +async fn test_seccomp_cow_openat2_syscall() { + let workdir = temp_dir("seccomp-openat2"); + let out_file = std::env::temp_dir().join(format!( + "sandlock-test-openat2-{}", std::process::id() + )); + + let policy = 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(&workdir).fs_write("/tmp") + .workdir(&workdir) + .cwd(&workdir) + .on_exit(BranchAction::Abort) + .build() + .unwrap(); + + // SYS_openat2 = 437 on every supported arch; open_how = {flags, mode, resolve}. + let script = format!(concat!( + "import ctypes, os\n", + "libc = ctypes.CDLL('libc.so.6', use_errno=True)\n", + "class OpenHow(ctypes.Structure):\n", + " _fields_ = [('flags', ctypes.c_uint64), ('mode', ctypes.c_uint64), ('resolve', ctypes.c_uint64)]\n", + "O_WRONLY = 1; O_CREAT = 64; O_TRUNC = 512\n", + "how = OpenHow(O_WRONLY | O_CREAT | O_TRUNC, 0o644, 0)\n", + "fd = libc.syscall(437, -100, b'{wd}/newfile.txt', ctypes.byref(how), ctypes.sizeof(how))\n", + "err = ctypes.get_errno()\n", + "if fd >= 0:\n", + " os.write(fd, b'created via raw openat2')\n", + " os.close(fd)\n", + " content = open('{wd}/newfile.txt').read()\n", + " open('{out}', 'w').write(content)\n", + "else:\n", + " open('{out}', 'w').write(f'FAILED:errno={{err}}')\n", + ), wd = workdir.display(), out = out_file.display()); + + let result = policy.clone().run(&["python3", "-c", &script]).await.unwrap(); + assert!(result.success(), "exit={:?}, stderr={}", result.code(), result.stderr_str().unwrap_or("")); + let content = fs::read_to_string(&out_file).unwrap_or_default(); + assert_eq!(content, "created via raw openat2", "raw openat2 should work with COW"); + assert!(!workdir.join("newfile.txt").exists(), "newfile.txt should not exist after abort"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_file(&out_file); +} + /// Legacy stat/lstat/access must honor whiteouts. /// /// Regression test: handle_cow_stat parsed every syscall with the at-variant From eaac3c1e34512d0610ea1b5e0bf7946770ed7a4e Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 14:18:40 -0700 Subject: [PATCH 2/4] build: compile the restore stub without a stack protector A compiler that enables the stack protector by default (vanilla GCC; Ubuntu's spec exempts -ffreestanding) emits a canary load from %fs:0x28 in the stub's prologue. The stub runs with a zero thread pointer until it restores the checkpoint's, so that read faults at address 0x28 before the READY handshake and every restore dies with SIGSEGV. Pass -fno-stack-protector explicitly and add a unit test that scans the built stub for the canary load. The freshness check in build_static compared the binary only against its C source, so a flag change here never recompiled a stub that already existed in target/. Compare against build.rs as well. Signed-off-by: Cong Wang --- crates/sandlock-core/build.rs | 21 +++++++++++-------- crates/sandlock-core/src/checkpoint/resume.rs | 19 +++++++++++++++++ 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/crates/sandlock-core/build.rs b/crates/sandlock-core/build.rs index a40456ae..59800555 100644 --- a/crates/sandlock-core/build.rs +++ b/crates/sandlock-core/build.rs @@ -83,6 +83,10 @@ fn main() { "-O2", "-ffreestanding", "-fno-tree-loop-distribute-patterns", + // A compiler with default SSP (vanilla GCC; Ubuntu's exempts + // -ffreestanding) reads the canary at %fs:0x28, and the stub's + // thread pointer is zero until it restores the checkpoint's. + "-fno-stack-protector", text_segment, ], ) { @@ -97,8 +101,9 @@ fn main() { } /// Compile `src` to `bin` with the first working compiler in `ccs`, skipping the -/// work when `bin` is newer than `src`. Returns `false` only when the source is -/// present, newer than `bin`, and no compiler in `ccs` succeeded; a missing +/// work when `bin` is newer than both `src` and this build script (the flags +/// live here, so a flag change must recompile). Returns `false` only when the +/// source is present, stale, and no compiler in `ccs` succeeded; a missing /// source (a packaged crate) or an up-to-date `bin` reports success. The caller /// decides whether that failure is a hard error or a warning. fn build_static(src: &Path, bin: &Path, ccs: &[&str], args: &[&str]) -> bool { @@ -106,13 +111,11 @@ fn build_static(src: &Path, bin: &Path, ccs: &[&str], args: &[&str]) -> bool { if !src.exists() { return true; } - if bin.exists() { - if let (Ok(s), Ok(b)) = (src.metadata(), bin.metadata()) { - if let (Ok(st), Ok(bt)) = (s.modified(), b.modified()) { - if bt >= st { - return true; - } - } + let mtime = |p: &Path| p.metadata().and_then(|m| m.modified()).ok(); + let build_rs = Path::new(env!("CARGO_MANIFEST_DIR")).join("build.rs"); + if let (Some(bt), Some(st), Some(rt)) = (mtime(bin), mtime(src), mtime(&build_rs)) { + if bt >= st && bt >= rt { + return true; } } for cc in ccs { diff --git a/crates/sandlock-core/src/checkpoint/resume.rs b/crates/sandlock-core/src/checkpoint/resume.rs index 90528e25..ade96102 100644 --- a/crates/sandlock-core/src/checkpoint/resume.rs +++ b/crates/sandlock-core/src/checkpoint/resume.rs @@ -427,6 +427,25 @@ mod tests { assert!(loads > 0, "stub has no PT_LOAD segments"); } + /// The stub runs with a zero thread pointer until it restores the + /// checkpoint's, so a stack-protector prologue (`mov %fs:0x28,%rax`) faults + /// at address 0x28 before the handshake. Ubuntu's gcc exempts freestanding + /// builds from its default SSP; a vanilla gcc does not, so build.rs has to + /// disable it explicitly. + #[test] + #[cfg(target_arch = "x86_64")] + fn stub_carries_no_stack_protector() { + let Ok(elf) = std::fs::read(stub_path()) else { + eprintln!("skip: restore-stub not built"); + return; + }; + const CANARY_LOAD: &[u8] = &[0x64, 0x48, 0x8b, 0x04, 0x25, 0x28, 0x00, 0x00, 0x00]; + assert!( + !elf.windows(CANARY_LOAD.len()).any(|w| w == CANARY_LOAD), + "restore-stub reads the %fs:0x28 canary; build.rs must pass -fno-stack-protector" + ); + } + /// End-to-end proof that the serializer and the stub agree: build a real /// control blob for a hand-assembled one-page "program", exec the stub with /// the inherited fds, drive the handshake, and read the sentinel byte the From fa1de272663a280b64f6baffbec39077225254d8 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 14:24:32 -0700 Subject: [PATCH 3/4] cow: mediate renameat like rename and renameat2 COW registered renameat2 and legacy rename but not renameat, and the notif list did the same, so a raw renameat under the workdir went to the kernel and renamed the real file, surviving an abort. On aarch64 this covered every ordinary rename: the ABI has no rename(2), so libc's rename() compiles to renameat. The chroot lists already carried it for that reason. renameat shares renameat2's first four arguments, so the parser accepts both numbers in the same branch. Signed-off-by: Cong Wang --- crates/sandlock-core/src/cow/dispatch.rs | 2 +- crates/sandlock-core/src/seccomp/dispatch.rs | 4 +- crates/sandlock-core/src/seccomp_plan.rs | 2 + .../tests/integration/test_cow.rs | 55 +++++++++++++++++++ 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index 28ac4dc1..38a0ca5b 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -396,7 +396,7 @@ fn parse_cow_write( dev: notif.data.args[3], }); } - if nr == libc::SYS_renameat2 { + if nr == libc::SYS_renameat2 || Some(nr) == arch::sys_renameat() { let old_path = read_resolved(notif, 1, Some(0), notif_fd, virtual_cwd)?; let new_path = read_resolved(notif, 3, Some(2), notif_fd, virtual_cwd)?; return Some(CowWriteOp::Rename { old_path, new_path }); diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 36d78bf2..6855f553 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -1019,8 +1019,8 @@ fn register_cow_handlers(table: &mut DispatchTable, ctx: &Arc) { ]; write_nrs.extend([ arch::sys_unlink(), arch::sys_rmdir(), arch::sys_mkdir(), arch::sys_mknod(), - arch::sys_rename(), arch::sys_symlink(), arch::sys_link(), arch::sys_chmod(), - arch::sys_chown(), arch::sys_lchown(), + arch::sys_rename(), arch::sys_renameat(), arch::sys_symlink(), arch::sys_link(), + arch::sys_chmod(), arch::sys_chown(), arch::sys_lchown(), ].into_iter().flatten()); for nr in write_nrs { table.register(nr, cow_call!(crate::cow::dispatch::handle_cow_write)); diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index fc8f0255..e5b35112 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -164,6 +164,8 @@ fn cow_path_syscalls() -> Vec { arch::sys_mkdir(), arch::sys_mknod(), arch::sys_rename(), + // libc's rename() lands here on aarch64, which has no rename(2). + arch::sys_renameat(), arch::sys_symlink(), arch::sys_link(), arch::sys_chmod(), diff --git a/crates/sandlock-core/tests/integration/test_cow.rs b/crates/sandlock-core/tests/integration/test_cow.rs index e2d89bb3..ae288255 100644 --- a/crates/sandlock-core/tests/integration/test_cow.rs +++ b/crates/sandlock-core/tests/integration/test_cow.rs @@ -365,6 +365,61 @@ async fn test_seccomp_cow_openat2_syscall() { let _ = fs::remove_file(&out_file); } +/// renameat must land in the COW layer like rename and renameat2. +/// +/// Regression test: COW registered renameat2 and legacy rename but not +/// renameat, so a raw renameat under the workdir renamed the real file and +/// survived an abort. On aarch64 libc's rename() compiles to renameat, so +/// there every ordinary rename escaped the branch. riscv64 has no renameat, +/// so the test runs the same check through renameat2 with no flags. +#[tokio::test] +async fn test_seccomp_cow_renameat_syscall() { + let workdir = temp_dir("seccomp-renameat"); + fs::write(workdir.join("orig.txt"), "keep").unwrap(); + let out_file = std::env::temp_dir().join(format!( + "sandlock-test-renameat-{}", std::process::id() + )); + + let mut policy = 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(&workdir).fs_write("/tmp") + .workdir(&workdir) + .cwd(&workdir) + .on_exit(BranchAction::Abort) + .build() + .unwrap(); + + // renameat is 264 on x86_64 and 38 on aarch64; riscv64 only has renameat2 (276). + let script = format!(concat!( + "import ctypes, os, platform\n", + "libc = ctypes.CDLL('libc.so.6', use_errno=True)\n", + "old = b'{wd}/orig.txt'; new = b'{wd}/moved.txt'\n", + "m = platform.machine()\n", + "if m == 'x86_64':\n", + " r = libc.syscall(264, -100, old, -100, new)\n", + "elif m == 'aarch64':\n", + " r = libc.syscall(38, -100, old, -100, new)\n", + "else:\n", + " r = libc.syscall(276, -100, old, -100, new, 0)\n", + "err = ctypes.get_errno()\n", + "if r == 0 and not os.path.exists(old) and open(new).read() == 'keep':\n", + " open('{out}', 'w').write('renamed')\n", + "else:\n", + " open('{out}', 'w').write(f'FAILED:ret={{r}},errno={{err}}')\n", + ), wd = workdir.display(), out = out_file.display()); + + let result = policy.run(&["python3", "-c", &script]).await.unwrap(); + assert!(result.success(), "exit={:?}, stderr={}", result.code(), result.stderr_str().unwrap_or("")); + let content = fs::read_to_string(&out_file).unwrap_or_default(); + assert_eq!(content, "renamed", "raw renameat should work with COW"); + assert!(workdir.join("orig.txt").exists(), "orig.txt should be back after abort"); + assert!(!workdir.join("moved.txt").exists(), "moved.txt should not exist after abort"); + + let _ = fs::remove_dir_all(&workdir); + let _ = fs::remove_file(&out_file); +} + /// Legacy stat/lstat/access must honor whiteouts. /// /// Regression test: handle_cow_stat parsed every syscall with the at-variant From 34d54a48e1bff9b301f2e2c3390b48d3dd4e1225 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Wed, 2 Sep 2026 14:46:24 -0700 Subject: [PATCH 4/4] checkpoint: gate the x86_64 xstate constants by arch The fxsave/xstate frame constants only feed the x86_64 signal-frame builder, which is already cfg-gated, so the riscv64 build warned about four unused constants. Signed-off-by: Cong Wang --- crates/sandlock-core/src/checkpoint/restore_blob.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/sandlock-core/src/checkpoint/restore_blob.rs b/crates/sandlock-core/src/checkpoint/restore_blob.rs index 20dfd594..60390cb1 100644 --- a/crates/sandlock-core/src/checkpoint/restore_blob.rs +++ b/crates/sandlock-core/src/checkpoint/restore_blob.rs @@ -57,12 +57,16 @@ pub(crate) const STUB_SPAN: u64 = 0x40_0000; /// `sw_reserved` area of the 512-byte fxsave block tells the kernel the buffer /// holds a full xstate; without it the kernel falls back to `fxrstor` of the /// legacy area only. +#[cfg(target_arch = "x86_64")] const FP_XSTATE_MAGIC1: u32 = 0x4650_5853; +#[cfg(target_arch = "x86_64")] const FP_XSTATE_MAGIC2: u32 = 0x4650_5845; /// Offset of `struct _fpx_sw_bytes` within the 512-byte fxsave block. +#[cfg(target_arch = "x86_64")] const SW_RESERVED_OFF: usize = 464; /// fxsave block + xstate header: the smallest buffer the kernel accepts as a /// full xstate image. +#[cfg(target_arch = "x86_64")] const MIN_XSTATE_SIZE: usize = 512 + 64; /// A restore reduced to the three things the supervisor needs: the control blob