sandlock-core: force child path-arg rewrites through /proc/pid/mem to survive read-only buffers

Signed-off-by: Cong Wang <cwang@multikernel.io>

Author: congwang-mk

crates/sandlock-core/src/chroot/dispatch.rs

@@ -49,7 +49,7 @@ use tokio::sync::Mutex;
 
 use crate::chroot::resolve::{confine, resolve_existing_in_root, resolve_in_root};
 use crate::sys::fs::openat2_in_root;
-use crate::seccomp::notif::{read_child_mem, write_child_mem, NotifAction};
+use crate::seccomp::notif::{read_child_mem, write_child_mem, write_child_mem_force, NotifAction};
 use crate::seccomp::state::{ChrootState, CowState};
 use crate::sys::structs::{SeccompNotif, SeccompNotifAddfd, SECCOMP_IOCTL_NOTIF_ADDFD};
 
@@ -868,8 +868,12 @@ pub(crate) async fn handle_chroot_exec(
         return NotifAction::Errno(libc::EIO);
     }
 
+    // Force-write past read-only page protections: the child commonly passes a
+    // .rodata path literal to execve, which process_vm_writev can't overwrite.
+    // No length guard needed — execve replaces the address space on success, so
+    // a write past the original buffer is harmless.
     let fd_path = format!("/proc/self/fd/{}\0", child_fd);
-    if write_child_mem(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
+    if write_child_mem_force(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
         return NotifAction::Errno(libc::EFAULT);
     }
 
@@ -1637,6 +1641,9 @@ pub(crate) async fn handle_chroot_chdir(
         Some(p) => p,
         None => return NotifAction::Continue,
     };
+    // Bytes the child mapped for the path argument (string + NUL): the upper
+    // bound for an in-place rewrite that must not overflow into adjacent memory.
+    let orig_path_buf_len = path.len() + 1;
 
     // Build the full virtual path from AT_FDCWD + path.
     let was_absolute = Path::new(&path).is_absolute();
@@ -1725,7 +1732,17 @@ pub(crate) async fn handle_chroot_chdir(
     }
 
     let fd_path = format!("/proc/self/fd/{}\0", child_fd);
-    if write_child_mem(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
+    // chdir leaves the process running, so the rewrite must not overflow the
+    // child's path buffer into adjacent memory. Only force-write when the
+    // redirect fits; otherwise the original (short) buffer can't be redirected.
+    // src_fd is already closed (above); the injected child fd is O_CLOEXEC and
+    // is reclaimed on the child's next exec/exit.
+    if orig_path_buf_len < fd_path.len() {
+        return NotifAction::Errno(libc::ENAMETOOLONG);
+    }
+    // Force-write past read-only page protections (a .rodata chdir literal that
+    // process_vm_writev can't overwrite).
+    if write_child_mem_force(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
         return NotifAction::Errno(libc::EFAULT);
     }
 

crates/sandlock-core/src/cow/dispatch.rs

@@ -34,7 +34,7 @@ use tokio::sync::Mutex as AsyncMutex;
 use crate::arch;
 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, NotifAction};
+use crate::seccomp::notif::{read_child_mem, write_child_mem, write_child_mem_force, NotifAction};
 use crate::seccomp::state::{CowState, PerProcessState, ProcessIndex};
 use crate::sys::structs::SeccompNotif;
 
@@ -1015,8 +1015,11 @@ pub(crate) async fn handle_cow_exec(
     // injected fd. execve replaces the whole address space on success, so
     // (unlike chdir) writing past the original buffer is harmless — the
     // only memory the kernel still reads is argv/envp, which sit elsewhere.
+    // Force-write past read-only protections (a .rodata exec path literal). No
+    // length guard: execve replaces the address space on success (see above), so
+    // a write past the original buffer is harmless.
     let fd_path = format!("/proc/self/fd/{}\0", child_fd);
-    if write_child_mem(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
+    if write_child_mem_force(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
         return NotifAction::Errno(libc::EFAULT);
     }
 
@@ -1300,7 +1303,9 @@ pub(crate) async fn handle_cow_chdir(
         // fd has O_CLOEXEC so it will be cleaned up on exit/exec.
         return NotifAction::Errno(libc::ENOENT);
     }
-    if write_child_mem(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
+    // Force-write past read-only protections (a .rodata chdir path literal).
+    // The fit guard above keeps the redirect from overflowing the buffer.
+    if write_child_mem_force(notif_fd, notif.id, notif.pid, path_ptr, fd_path.as_bytes()).is_err() {
         return NotifAction::Errno(libc::EFAULT);
     }
 

crates/sandlock-core/src/error.rs

@@ -100,6 +100,9 @@ pub enum NotifError {
     #[error("child memory read failed: {0}")]
     ChildMemoryRead(#[source] std::io::Error),
 
+    #[error("child memory write failed: {0}")]
+    ChildMemoryWrite(#[source] std::io::Error),
+
     #[error("notification ioctl failed: {0}")]
     Ioctl(#[source] std::io::Error),
 }

crates/sandlock-core/src/seccomp/notif.rs

@@ -1120,6 +1120,62 @@ pub fn write_child_mem(
     Ok(())
 }
 
+/// Write bytes to a child, forcing past read-only page protections.
+///
+/// [`write_child_mem`] uses `process_vm_writev`, which honors the target VMA's
+/// protection bits and so returns `EFAULT` when the destination page is
+/// read-only — e.g. a `.rodata` path literal a program hands to
+/// `chdir`/`execve`. This variant writes through `/proc/<pid>/mem`, whose writes
+/// use `FOLL_FORCE` and therefore copy-on-write past a read-only mapping (the
+/// same mechanism a debugger uses to plant a breakpoint in read-only `.text`).
+/// It handles writable and read-only destinations through one path, so the
+/// argument-rewrite sites need no `EFAULT` fallback.
+///
+/// Use ONLY for rewriting an *input* path argument the child owns that the
+/// kernel re-reads under `SECCOMP_USER_NOTIF_FLAG_CONTINUE` (the `chdir`/`execve`
+/// path rewrites). Do NOT use it for syscall *output* buffers (`stat`,
+/// `getdents`, `getcwd`, `readlink`, the `getsockname`/`recvfrom` source
+/// address): if a child supplies a read-only output buffer the real syscall
+/// would `EFAULT`, and faithful emulation must too — keep those on
+/// [`write_child_mem`]. (`bind`/`connect` need nothing here: they are emulated
+/// on-behalf on a duped fd and never rewrite the child's sockaddr.)
+///
+/// Opening `/proc/<pid>/mem` for write needs `PTRACE_MODE_ATTACH_FSCREDS` over
+/// the child (no actual attach or stop): satisfied by the supervisor as the
+/// child's same-uid parent under the common Yama scopes. Returns `Err` (so the
+/// caller can still surface `EFAULT`) if the `mem` file can't be opened or
+/// written — it never silently no-ops. TOCTOU-safe: brackets the write with
+/// `id_valid` like [`write_child_mem`].
+pub fn write_child_mem_force(
+    notif_fd: RawFd,
+    id: u64,
+    pid: u32,
+    addr: u64,
+    data: &[u8],
+) -> Result<(), NotifError> {
+    id_valid(notif_fd, id).map_err(NotifError::Ioctl)?;
+    write_child_mem_proc(pid, addr, data)?;
+    id_valid(notif_fd, id).map_err(NotifError::Ioctl)?;
+    Ok(())
+}
+
+/// Write bytes to a process's memory via `/proc/<pid>/mem` (open + `pwrite`).
+///
+/// Inner helper for [`write_child_mem_force`]; the file offset is the target
+/// virtual address. Writes here use `FOLL_FORCE`, so they copy-on-write past a
+/// read-only destination page where [`write_child_mem_vm`] (`process_vm_writev`)
+/// returns `EFAULT`.
+fn write_child_mem_proc(pid: u32, addr: u64, data: &[u8]) -> Result<(), NotifError> {
+    use std::os::unix::fs::FileExt;
+    let mem = std::fs::OpenOptions::new()
+        .write(true)
+        .open(format!("/proc/{}/mem", pid))
+        .map_err(NotifError::ChildMemoryWrite)?;
+    mem.write_all_at(data, addr)
+        .map_err(NotifError::ChildMemoryWrite)?;
+    Ok(())
+}
+
 // ============================================================
 // Response dispatch
 // ============================================================
@@ -2267,6 +2323,57 @@ mod tests {
         assert_eq!(data, 0x1234567890ABCDEF);
     }
 
+    /// The force-write path (`/proc/<pid>/mem`, FOLL_FORCE) must overwrite a
+    /// read-only page where `process_vm_writev` (`write_child_mem_vm`) refuses.
+    /// A read-only private page stands in for the `.rodata` path literal a child
+    /// hands to chdir/execve — the exact case the chroot/cow rewrite sites hit.
+    #[test]
+    fn write_child_mem_proc_forces_past_readonly_page() {
+        const PAGE: usize = 4096;
+        let orig = b"/some/rodata/path\0";
+        let newb = b"/proc/self/fd/7\0\0"; // fits within the page, near orig's len
+
+        let addr = unsafe {
+            libc::mmap(
+                std::ptr::null_mut(),
+                PAGE,
+                libc::PROT_READ | libc::PROT_WRITE,
+                libc::MAP_PRIVATE | libc::MAP_ANONYMOUS,
+                -1,
+                0,
+            )
+        };
+        assert_ne!(addr, libc::MAP_FAILED, "mmap failed");
+        unsafe { std::ptr::copy_nonoverlapping(orig.as_ptr(), addr as *mut u8, orig.len()) };
+
+        // Flip the page read-only: a normal store would now SIGSEGV, and
+        // process_vm_writev honors the protection and fails.
+        assert_eq!(
+            unsafe { libc::mprotect(addr, PAGE, libc::PROT_READ) },
+            0,
+            "mprotect PROT_READ failed"
+        );
+
+        let pid = std::process::id();
+        let uaddr = addr as u64;
+
+        assert!(
+            write_child_mem_vm(pid, uaddr, newb).is_err(),
+            "process_vm_writev must fail on a read-only page"
+        );
+
+        // FOLL_FORCE via /proc/<pid>/mem copies-on-write past the RO page.
+        write_child_mem_proc(pid, uaddr, newb)
+            .expect("force-write through /proc/pid/mem must succeed on a read-only page");
+
+        // The write landed at the target address (page is still RO-mapped, so a
+        // plain read is fine and reflects the COW'd contents).
+        let got = unsafe { std::slice::from_raw_parts(addr as *const u8, newb.len()) };
+        assert_eq!(got, newb, "forced write must be visible at the target address");
+
+        unsafe { libc::munmap(addr, PAGE) };
+    }
+
     #[test]
     fn denylist_blocks_matching_cidr_allows_rest() {
         use crate::network::IpCidr;