diff --git a/README.md b/README.md index a61de5d9..e458a902 100644 --- a/README.md +++ b/README.md @@ -531,7 +531,7 @@ The async notification supervisor (tokio) handles intercepted syscalls: | Syscall | Handler | |---|---| | `clone/fork/vfork` | Process count enforcement | -| `mmap/munmap/brk/mremap` | Memory limit tracking | +| `mmap/munmap/brk/mremap/mprotect` | Memory limit tracking | | `connect/sendto/sendmsg` | IP allowlist + on-behalf execution + HTTP ACL redirect | | `bind` | On-behalf bind + port remapping | | `openat` | /proc virtualization, COW interception | diff --git a/crates/sandlock-core/src/context/tests.rs b/crates/sandlock-core/src/context/tests.rs index 91e6cece..507fd06c 100644 --- a/crates/sandlock-core/src/context/tests.rs +++ b/crates/sandlock-core/src/context/tests.rs @@ -310,6 +310,32 @@ fn test_arg_filters_has_clone_ioctl_prctl_socket() { && f.k == PR_SET_DUMPABLE)); } +#[test] +fn test_arg_filters_mprotect_traps_only_with_memory_limit() { + use crate::sys::structs::{ + BPF_ABS, BPF_JEQ, BPF_JSET, BPF_JMP, BPF_K, BPF_LD, BPF_W, OFFSET_ARGS2_LO, + }; + // SYS_mprotect collides with AF_INET6 on x86_64, so the test looks for + // the nr check followed by the prot load rather than the bare constant. + let has_mprotect = |filters: &[crate::sys::structs::SockFilter]| { + filters.windows(2).any(|w| w[0].code == (BPF_JMP | BPF_JEQ | BPF_K) + && w[0].k == libc::SYS_mprotect as u32 + && w[1].code == (BPF_LD | BPF_W | BPF_ABS) + && w[1].k == OFFSET_ARGS2_LO) + }; + let unlimited = Sandbox::builder().build().unwrap(); + assert!(!has_mprotect(&arg_filters(&unlimited))); + + let limited = Sandbox::builder() + .max_memory(crate::sandbox::ByteSize::mib(256)) + .build() + .unwrap(); + let filters = arg_filters(&limited); + assert!(has_mprotect(&filters)); + assert!(filters.iter().any(|f| f.code == (BPF_JMP | BPF_JSET | BPF_K) + && f.k == libc::PROT_WRITE as u32)); +} + #[test] fn test_arg_filters_raw_sockets() { use crate::sys::structs::{BPF_ALU, BPF_AND, BPF_JEQ, BPF_JMP, BPF_K}; diff --git a/crates/sandlock-core/src/resource.rs b/crates/sandlock-core/src/resource.rs index 398d45a2..c11436d3 100644 --- a/crates/sandlock-core/src/resource.rs +++ b/crates/sandlock-core/src/resource.rs @@ -27,7 +27,7 @@ use crate::sys::structs::{ /// CLONE_THREAD flag — threads don't count toward process limit. const CLONE_THREAD: u64 = 0x0001_0000; -/// MAP_ANONYMOUS flag — only anonymous mappings count toward memory limit. +/// MAP_ANONYMOUS flag: anonymous and writable private file mappings count. const MAP_ANONYMOUS: u64 = 0x20; /// Effective clone flags for a fork-like notification. @@ -642,13 +642,39 @@ fn read_private_anon_bytes(pid: i32) -> Option { Some(pages.saturating_mul(page_size as u64)) } +/// Bytes of `[addr, addr + len)` that an mprotect granting write would +/// newly commit: the overlap with private mappings that are not yet +/// writable. Shared mappings are skipped; writing to one never creates +/// anonymous memory. +fn newly_writable_bytes(pid: i32, addr: u64, len: u64) -> Option { + let maps = std::fs::read_to_string(format!("/proc/{}/maps", pid)).ok()?; + let end = addr.checked_add(len)?; + let mut total = 0u64; + for line in maps.lines() { + let mut fields = line.split_whitespace(); + let (Some(range), Some(perms)) = (fields.next(), fields.next()) else { continue }; + let Some((lo, hi)) = range.split_once('-') else { continue }; + let (Ok(lo), Ok(hi)) = (u64::from_str_radix(lo, 16), u64::from_str_radix(hi, 16)) else { + continue; + }; + if perms.as_bytes().get(1) == Some(&b'w') || perms.as_bytes().get(3) != Some(&b'p') { + continue; + } + let (lo, hi) = (lo.max(addr), hi.min(end)); + if lo < hi { + total += hi - lo; + } + } + Some(total) +} + /// Raise a laundered ledger back to the measured footprint: only /// anonymous mappings are charged but every unmap is credited, so /// mapping and unmapping a file refunds memory that was never charged. /// -/// A floor, not an assignment: `mmap` charges `PROT_NONE` reservations -/// that `data_vm` excludes until an `mprotect` this handler never sees -/// makes them writable, so the ledger must be allowed to sit higher. +/// A floor, not an assignment: shared anonymous mappings and shmget +/// segments are charged but `data_vm` excludes them, so the ledger must +/// be allowed to sit higher. fn reconcile_floor(st: &mut ResourceState, per: Option<&mut PerProcessState>, pid: i32) { let Some(per) = per else { return }; let Some(measured) = read_private_anon_bytes(pid) else { return }; @@ -660,7 +686,7 @@ fn reconcile_floor(st: &mut ResourceState, per: Option<&mut PerProcessState>, pi } } -/// Handle memory-related notifications (mmap, munmap, brk, mremap, shmget). +/// Handle memory-related notifications (mmap, munmap, brk, mremap, mprotect, shmget). /// /// Tracks anonymous memory usage and enforces the configured memory limit. pub(crate) async fn handle_memory( @@ -714,10 +740,20 @@ pub(crate) async fn handle_memory( } if nr == libc::SYS_mmap { - // args[1] = len, args[3] = flags + // args[1] = len, args[2] = prot, args[3] = flags. A PROT_NONE + // reservation backs nothing until it is remapped writable (a later + // mmap this handler charges) or mprotect'd (judged above). A + // writable private file mapping is anonymous memory in waiting: + // /dev/zero mapped that way is a plain anonymous mapping by another + // name, and the kernel's own data_vm counts its full length. let len = args[1]; + let prot = args[2]; let flags = args[3]; - if (flags & MAP_ANONYMOUS) != 0 { + let anon = (flags & MAP_ANONYMOUS) != 0 && prot != libc::PROT_NONE as u64; + let private_writable = (flags & MAP_ANONYMOUS) == 0 + && (flags & libc::MAP_PRIVATE as u64) != 0 + && (prot & libc::PROT_WRITE as u64) != 0; + if anon || private_writable { if would_exceed(&st, len) { return kill; } @@ -764,6 +800,21 @@ pub(crate) async fn handle_memory( } else if new_len < old_len { credit(&mut st, per.as_deref_mut(), old_len - new_len); } + } else if nr == libc::SYS_mprotect { + // args[0] = addr, args[1] = len. Only calls granting PROT_WRITE + // arrive (BPF filter). Judged but never charged: the floor measures + // the result exactly at the next event, whereas charging the length + // would count already-writable pages twice with no way back down. + // The maps read that tells the two apart is deferred until the + // whole length would exceed the limit, so it costs nothing on the + // hot path. + let (addr, len) = (args[0], args[1]); + if would_exceed(&st, len) { + let newly = newly_writable_bytes(notif.pid as i32, addr, len).unwrap_or(len); + if would_exceed(&st, newly) { + return kill; + } + } } else if nr == libc::SYS_shmget { // shmget(key, size, shmflg) — args[1] = size let size = args[1]; @@ -879,8 +930,7 @@ mod memory_range_tests { assert_eq!(per.mem_charged, measured); assert_eq!(st.mem_used, per.mem_charged); - // A ledger above the measure is left alone: mmap charges PROT_NONE - // reservations that the kernel's measure excludes. + // A ledger above the measure is left alone. let inflated = per.mem_charged + (1 << 30); per.mem_charged = inflated; st.mem_used = inflated; @@ -987,6 +1037,122 @@ mod tests { }) } + fn mmap_notif(len: u64, prot: u64) -> SeccompNotif { + let flags = (libc::MAP_PRIVATE | libc::MAP_ANONYMOUS) as u64; + file_mmap_notif(len, prot, flags) + } + + fn file_mmap_notif(len: u64, prot: u64, flags: u64) -> SeccompNotif { + let mut n = fake_notif(libc::SYS_mmap, 0); + n.data.args = [0, len, prot, flags, 3, 0]; + n + } + + /// A writable private file mapping is anonymous memory in waiting: + /// every written page is copied, and `/dev/zero` mapped this way is + /// indistinguishable from an anonymous mapping. Shared and read-only + /// file mappings never create private pages and stay free. + #[tokio::test] + async fn writable_private_file_mapping_is_charged() { + let ctx = fake_supervisor_ctx(false); + let mut policy = fake_policy(false); + policy.max_memory_bytes = 1 << 20; + policy.has_memory_limit = true; + let rw = (libc::PROT_READ | libc::PROT_WRITE) as u64; + + let shared = file_mmap_notif(1 << 30, rw, libc::MAP_SHARED as u64); + assert!(matches!(handle_memory(&shared, &ctx, &policy).await, NotifAction::Continue)); + let readonly = file_mmap_notif(1 << 30, libc::PROT_READ as u64, libc::MAP_PRIVATE as u64); + assert!(matches!(handle_memory(&readonly, &ctx, &policy).await, NotifAction::Continue)); + assert_eq!(ctx.resource.lock().await.mem_used, 0); + + let small = file_mmap_notif(1 << 19, rw, libc::MAP_PRIVATE as u64); + assert!(matches!(handle_memory(&small, &ctx, &policy).await, NotifAction::Continue)); + assert_eq!(ctx.resource.lock().await.mem_used, 1 << 19); + + let big = file_mmap_notif(1 << 30, rw, libc::MAP_PRIVATE as u64); + assert!(matches!(handle_memory(&big, &ctx, &policy).await, NotifAction::KillTask { .. })); + } + + /// A PROT_NONE reservation backs nothing, so it must not count: the + /// Go runtime reserves over a gigabyte of address space at startup and + /// used to be killed by any limit smaller than that before main ran. + #[tokio::test] + async fn prot_none_reservation_is_not_charged() { + let ctx = fake_supervisor_ctx(false); + let mut policy = fake_policy(false); + policy.max_memory_bytes = 1 << 20; + policy.has_memory_limit = true; + + let reserve = mmap_notif(1 << 30, libc::PROT_NONE as u64); + let action = handle_memory(&reserve, &ctx, &policy).await; + assert!(matches!(action, NotifAction::Continue), "reservation killed"); + assert_eq!(ctx.resource.lock().await.mem_used, 0); + + let commit = mmap_notif(1 << 30, (libc::PROT_READ | libc::PROT_WRITE) as u64); + let action = handle_memory(&commit, &ctx, &policy).await; + assert!(matches!(action, NotifAction::KillTask { .. }), "commit not judged"); + } + + fn mprotect_notif(addr: u64, len: u64) -> SeccompNotif { + let mut n = fake_notif(libc::SYS_mprotect, 0); + n.pid = std::process::id(); + n.data.args = [addr, len, (libc::PROT_READ | libc::PROT_WRITE) as u64, 0, 0, 0]; + n + } + + fn map_anon(len: usize, prot: i32) -> u64 { + let p = unsafe { + libc::mmap( + std::ptr::null_mut(), + len, + prot, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert_ne!(p, libc::MAP_FAILED, "mmap failed"); + p as u64 + } + + /// Making a reservation writable is the moment it becomes real memory, + /// so it is judged against the limit; but it is never charged, since + /// the floor will measure it exactly at the next event. + #[tokio::test] + async fn mprotect_of_a_reservation_is_judged_but_not_charged() { + let ctx = fake_supervisor_ctx(false); + let mut policy = fake_policy(false); + policy.max_memory_bytes = 1 << 20; + policy.has_memory_limit = true; + let reserved = map_anon(1 << 30, libc::PROT_NONE); + + let over = handle_memory(&mprotect_notif(reserved, 1 << 30), &ctx, &policy).await; + assert!(matches!(over, NotifAction::KillTask { .. }), "1 GiB commit under 1 MiB"); + + let under = handle_memory(&mprotect_notif(reserved, 1 << 19), &ctx, &policy).await; + assert!(matches!(under, NotifAction::Continue), "512 KiB commit under 1 MiB"); + assert_eq!(ctx.resource.lock().await.mem_used, 0, "mprotect must not charge"); + unsafe { libc::munmap(reserved as *mut _, 1 << 30) }; + } + + /// Re-granting write on memory that is already writable adds nothing, + /// so a JIT flipping a large code cache back to RW near the limit must + /// not be killed for it. + #[tokio::test] + async fn mprotect_of_writable_memory_adds_nothing() { + let ctx = fake_supervisor_ctx(false); + let mut policy = fake_policy(false); + policy.max_memory_bytes = 64 << 20; + policy.has_memory_limit = true; + ctx.resource.lock().await.mem_used = 60 << 20; + let writable = map_anon(16 << 20, libc::PROT_READ | libc::PROT_WRITE); + + let action = handle_memory(&mprotect_notif(writable, 16 << 20), &ctx, &policy).await; + assert!(matches!(action, NotifAction::Continue), "already-writable range killed"); + 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); diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 87571d24..651caa2a 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -308,7 +308,7 @@ pub(crate) fn build_dispatch_table( if policy.has_memory_limit { for &nr in &[ libc::SYS_mmap, libc::SYS_munmap, libc::SYS_brk, - libc::SYS_mremap, libc::SYS_shmget, + libc::SYS_mremap, libc::SYS_mprotect, libc::SYS_shmget, ] { let policy_for_mem = Arc::clone(policy); let __sup = Arc::clone(ctx); diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index 465219ae..0e4e4950 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -1540,6 +1540,8 @@ fn syscall_name(nr: i64) -> &'static str { n if n == libc::SYS_mmap => "mmap", n if n == libc::SYS_munmap => "munmap", n if n == libc::SYS_brk => "brk", + n if n == libc::SYS_mremap => "mremap", + n if n == libc::SYS_mprotect => "mprotect", n if n == libc::SYS_getrandom => "getrandom", n if n == libc::SYS_unlinkat => "unlinkat", n if n == libc::SYS_mkdirat => "mkdirat", @@ -1588,6 +1590,7 @@ fn syscall_category(nr: i64) -> crate::policy_fn::SyscallCategory { || n == libc::SYS_execve || n == libc::SYS_execveat => SyscallCategory::Process, n if n == libc::SYS_mmap || n == libc::SYS_munmap || n == libc::SYS_brk || n == libc::SYS_mremap + || n == libc::SYS_mprotect => SyscallCategory::Memory, _ => SyscallCategory::File, // default } diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index 63cce0bf..520756d3 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -70,6 +70,8 @@ const MEMORY_NOTIF_SYSCALLS: &[i64] = &[ libc::SYS_munmap, libc::SYS_brk, libc::SYS_mremap, + // Only calls granting PROT_WRITE reach the supervisor; see arg_filters. + libc::SYS_mprotect, // exec destroys the address space and the kernel picks a fresh // randomized brk base, so brk accounting must observe it to drop the // old image's base; otherwise the new image's first brk is charged the @@ -660,5 +662,19 @@ pub(crate) fn arg_filters_resolved(resolved: &ResolvedSandbox) -> Vec RX constantly; the RX half must not pay a supervisor trip. + // mprotect(addr, len, prot): prot is arg2 + if features.memory_limit { + let nr_mprotect = libc::SYS_mprotect as u32; + insns.push(stmt(BPF_LD | BPF_W | BPF_ABS, OFFSET_NR)); + insns.push(jump(BPF_JMP | BPF_JEQ | BPF_K, nr_mprotect, 0, 3)); + insns.push(stmt(BPF_LD | BPF_W | BPF_ABS, OFFSET_ARGS2_LO)); + insns.push(jump(BPF_JMP | BPF_JSET | BPF_K, libc::PROT_WRITE as u32, 1, 0)); + insns.push(stmt(BPF_RET | BPF_K, SECCOMP_RET_ALLOW)); + } + insns } diff --git a/python/tests/test_sandbox.py b/python/tests/test_sandbox.py index 6bd904ca..a918e4de 100644 --- a/python/tests/test_sandbox.py +++ b/python/tests/test_sandbox.py @@ -226,6 +226,110 @@ def test_grandchild_over_the_limit_is_killed(self): ) +class TestMaxMemoryIgnoresReservations: + def test_prot_none_reservation_survives_a_small_limit(self): + """Reserving address space must not spend the memory budget. + + Anonymous mmaps were charged by length regardless of protection, + so a PROT_NONE reservation counted as if it were committed. The + Go runtime reserves over a gigabyte that way at startup, which + made every practical limit kill a Go program before main ran. + Committed memory must still be judged: the writable allocation + that follows is over the limit and must die. + """ + prog = ( + "import mmap\n" + "r = mmap.mmap(-1, 1 << 30, flags=mmap.MAP_PRIVATE | mmap.MAP_ANONYMOUS, prot=0)\n" + "print('RESERVED', flush=True)\n" + "b = bytearray(400 * 1024 * 1024)\n" + "b[::4096] = b'\\x01' * (len(b) // 4096)\n" + "print('COMMITTED', flush=True)\n" + ) + result = _policy(fs_writable=["/tmp"], max_memory="128M").run( + [sys.executable, "-c", prog], timeout=60 + ) + + assert b"RESERVED" in result.stdout, ( + f"reservation was charged: reason={result.reason} " + f"signal={result.signal} stdout={result.stdout!r}" + ) + assert b"COMMITTED" not in result.stdout, ( + "a writable allocation over the limit was not stopped" + ) + + +class TestMaxMemoryMprotectCommit: + def test_mprotect_of_a_reservation_is_judged(self): + """Committing a reservation with mprotect must not escape the limit. + + Reservations are free, so a workload could reserve PROT_NONE, + mprotect it writable, and touch it all without ever making the + memory syscall that would have corrected the ledger. A small + commit under the limit must still go through. + """ + prog = ( + "import ctypes\n" + "libc = ctypes.CDLL(None, use_errno=True)\n" + "libc.mmap.restype = ctypes.c_void_p\n" + "libc.mmap.argtypes = [ctypes.c_void_p, ctypes.c_size_t, ctypes.c_int," + " ctypes.c_int, ctypes.c_int, ctypes.c_long]\n" + "libc.mprotect.argtypes = [ctypes.c_void_p, ctypes.c_size_t, ctypes.c_int]\n" + "n = 512 << 20\n" + "p = libc.mmap(None, n, 0, 0x22, -1, 0)\n" + "print('RESERVED', flush=True)\n" + "small = 16 << 20\n" + "assert libc.mprotect(p, small, 3) == 0\n" + "ctypes.memset(p, 1, small)\n" + "print('SMALL-OK', flush=True)\n" + "assert libc.mprotect(p + small, n - small, 3) == 0\n" + "ctypes.memset(p + small, 1, n - small)\n" + "print('COMMITTED', flush=True)\n" + ) + result = _policy(fs_writable=["/tmp"], max_memory="128M").run( + [sys.executable, "-c", prog], timeout=60 + ) + + assert b"SMALL-OK" in result.stdout, ( + f"small commit was refused: reason={result.reason} " + f"signal={result.signal} stdout={result.stdout!r} stderr={result.stderr!r}" + ) + assert b"COMMITTED" not in result.stdout, ( + "mprotect committed 496 MiB under a 128 MiB limit" + ) + + +class TestMaxMemoryPrivateFileMapping: + def test_writable_private_file_mapping_is_charged(self): + """A writable MAP_PRIVATE file mapping must count like anonymous memory. + + Only MAP_ANONYMOUS was charged, so mapping /dev/zero private and + writable gave a workload arbitrary anonymous memory the ledger + never saw. A modest mapping under the limit must still work. + """ + prog = ( + "import mmap\n" + "f = open('/dev/zero', 'rb')\n" + "rw = mmap.PROT_READ | mmap.PROT_WRITE\n" + "m = mmap.mmap(f.fileno(), 8 << 20, flags=mmap.MAP_PRIVATE, prot=rw)\n" + "m[::4096] = b'\\x01' * (len(m) // 4096)\n" + "print('SMALL-OK', flush=True)\n" + "big = mmap.mmap(f.fileno(), 400 << 20, flags=mmap.MAP_PRIVATE, prot=rw)\n" + "big[::4096] = b'\\x01' * (len(big) // 4096)\n" + "print('COMMITTED', flush=True)\n" + ) + result = _policy(fs_writable=["/tmp"], max_memory="128M").run( + [sys.executable, "-c", prog], timeout=60 + ) + + assert b"SMALL-OK" in result.stdout, ( + f"small mapping was refused: reason={result.reason} " + f"signal={result.signal} stdout={result.stdout!r} stderr={result.stderr!r}" + ) + assert b"COMMITTED" not in result.stdout, ( + "wrote 400 MiB of private /dev/zero pages under a 128 MiB limit" + ) + + class TestNetAllowDenyAll: """An empty `net_allow` denies all outbound — including when fs grants are present, which turn on the named-`AF_UNIX` connect gate (`has_unix_fs_gate`)