sandlock-core: close fs deny TOCTOU with race-free on-behalf openat

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

Author: congwang-mk

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

@@ -374,6 +374,264 @@ fn is_denied_with_symlink_resolve(
     false
 }
 
+/// `RESOLVE_NO_MAGICLINKS` — forbid `/proc` magic-link redirection during
+/// on-behalf resolution while still following ordinary symlinks the way the
+/// child's own open would.
+const RESOLVE_NO_MAGICLINKS: u64 = 0x02;
+
+/// Kernel `struct open_how` for `openat2`.
+#[repr(C)]
+struct OpenHow {
+    flags: u64,
+    mode: u64,
+    resolve: u64,
+}
+
+fn last_errno(fallback: i32) -> i32 {
+    io::Error::last_os_error().raw_os_error().unwrap_or(fallback)
+}
+
+/// `openat2` relative to `dirfd`. Returns an owned fd or the errno.
+fn openat2_at(dirfd: RawFd, path: &std::ffi::CStr, flags: u64, mode: u64, resolve: u64)
+    -> Result<OwnedFd, i32>
+{
+    use std::os::unix::io::FromRawFd;
+    let how = OpenHow { flags, mode, resolve };
+    let fd = unsafe {
+        libc::syscall(
+            arch::SYS_OPENAT2,
+            dirfd,
+            path.as_ptr(),
+            &how as *const OpenHow,
+            std::mem::size_of::<OpenHow>(),
+        )
+    } as i32;
+    if fd < 0 {
+        Err(last_errno(libc::ENOENT))
+    } else {
+        Ok(unsafe { OwnedFd::from_raw_fd(fd) })
+    }
+}
+
+/// Capture an `O_PATH` fd to the directory the child's open resolves against,
+/// taken from the child's own view so a concurrent `chdir`/dirfd swap cannot
+/// move the resolution base after we read it.
+fn open_base_dir(pid: u32, dirfd: i64) -> Result<OwnedFd, i32> {
+    use std::os::unix::io::FromRawFd;
+    if dirfd as i32 == libc::AT_FDCWD {
+        let cwd = std::ffi::CString::new(format!("/proc/{}/cwd", pid)).map_err(|_| libc::EINVAL)?;
+        let fd = unsafe {
+            libc::open(cwd.as_ptr(), libc::O_PATH | libc::O_DIRECTORY | libc::O_CLOEXEC)
+        };
+        if fd < 0 {
+            return Err(last_errno(libc::EACCES));
+        }
+        Ok(unsafe { OwnedFd::from_raw_fd(fd) })
+    } else {
+        dup_fd_from_pid(pid, dirfd as i32).map_err(|_| libc::EBADF)
+    }
+}
+
+/// Real path of an open fd via its `/proc/self/fd` magic link.
+fn realpath_of_fd(fd: RawFd) -> Option<std::path::PathBuf> {
+    std::fs::read_link(format!("/proc/self/fd/{}", fd)).ok()
+}
+
+fn path_under_any(path: &std::path::Path, list: &[std::path::PathBuf]) -> bool {
+    list.iter().any(|p| path.starts_with(p))
+}
+
+/// Decide whether `realpath` may be opened with `flags` under the deny set
+/// and the (conservative) grant lists. Returns `Some(errno)` to refuse,
+/// `None` to allow. Never over-allows relative to the configured grants: a
+/// path outside every grant is refused, so this can only be stricter than
+/// Landlock, never looser (an over-deny is a functional gap, an over-allow
+/// would be an escape).
+fn deny_open_verdict(
+    realpath: &std::path::Path,
+    flags: u64,
+    policy: &NotifPolicy,
+    pfs: &super::state::PolicyFnState,
+) -> Option<i32> {
+    if pfs.is_path_denied(&realpath.to_string_lossy()) {
+        return Some(libc::EACCES);
+    }
+    let acc = flags as i32 & libc::O_ACCMODE;
+    let is_write = acc == libc::O_WRONLY
+        || acc == libc::O_RDWR
+        || (flags & libc::O_TRUNC as u64) != 0
+        || (flags & libc::O_CREAT as u64) != 0;
+    let allowed = if is_write {
+        path_under_any(realpath, &policy.chroot_writable)
+    } else {
+        path_under_any(realpath, &policy.chroot_readable)
+            || path_under_any(realpath, &policy.chroot_writable)
+    };
+    if allowed { None } else { Some(libc::EACCES) }
+}
+
+/// openat/open argument layout, normalized across the two spellings.
+struct OpenArgs {
+    dirfd: i64,
+    path_ptr: u64,
+    flags: u64,
+    mode: u64,
+}
+
+fn decode_open_args(notif: &SeccompNotif) -> OpenArgs {
+    let a = &notif.data.args;
+    if notif.data.nr as i64 == libc::SYS_openat {
+        OpenArgs { dirfd: a[0] as i64, path_ptr: a[1], flags: a[2], mode: a[3] }
+    } else {
+        // legacy open(path, flags, mode) — AT_FDCWD implied.
+        OpenArgs { dirfd: libc::AT_FDCWD as i64, path_ptr: a[0], flags: a[1], mode: a[2] }
+    }
+}
+
+/// Wrap a freshly opened raw fd into an `InjectFdSend`, honoring the child's
+/// `O_CLOEXEC` request. Ownership of `raw_fd` moves into the action.
+fn inject_open_result(raw_fd: i32, flags: u64) -> NotifAction {
+    use std::os::unix::io::FromRawFd;
+    if raw_fd < 0 {
+        return NotifAction::Errno(last_errno(libc::EACCES));
+    }
+    let owned = unsafe { OwnedFd::from_raw_fd(raw_fd) };
+    let newfd_flags = if flags & libc::O_CLOEXEC as u64 != 0 {
+        libc::O_CLOEXEC as u32
+    } else {
+        0
+    };
+    NotifAction::InjectFdSend { srcfd: owned, newfd_flags }
+}
+
+/// Existing-file branch: vet the pinned inode behind `probe`, then reopen it
+/// race-free via its `/proc/self/fd` magic link with the child's real access
+/// mode (binds to the inode, not the original path).
+fn reopen_existing_on_behalf(
+    probe: OwnedFd,
+    flags: u64,
+    policy: &NotifPolicy,
+    pfs: &super::state::PolicyFnState,
+) -> NotifAction {
+    // File exists. Refuse O_CREAT|O_EXCL the way the kernel would.
+    if (flags & libc::O_CREAT as u64) != 0 && (flags & libc::O_EXCL as u64) != 0 {
+        return NotifAction::Errno(libc::EEXIST);
+    }
+    let realpath = match realpath_of_fd(probe.as_raw_fd()) {
+        Some(p) => p,
+        None => return NotifAction::Errno(libc::EACCES),
+    };
+    if let Some(errno) = deny_open_verdict(&realpath, flags, policy, pfs) {
+        return NotifAction::Errno(errno);
+    }
+    // Resolution-only flags are stripped from the reopen.
+    let reopen_flags =
+        flags as i32 & !(libc::O_CREAT | libc::O_EXCL | libc::O_PATH | libc::O_NOFOLLOW);
+    let proc_path = match std::ffi::CString::new(format!("/proc/self/fd/{}", probe.as_raw_fd())) {
+        Ok(c) => c,
+        Err(_) => return NotifAction::Errno(libc::EIO),
+    };
+    let fd = unsafe { libc::open(proc_path.as_ptr(), reopen_flags) };
+    inject_open_result(fd, flags)
+}
+
+/// O_CREAT branch: resolve the parent directory race-free, vet the would-be
+/// target, then create the leaf inside that pinned parent (the dir inode is
+/// fixed, only the leaf name is appended).
+fn create_new_on_behalf(
+    base: &OwnedFd,
+    path: &str,
+    flags: u64,
+    mode: u64,
+    policy: &NotifPolicy,
+    pfs: &super::state::PolicyFnState,
+) -> NotifAction {
+    let p = std::path::Path::new(path);
+    let file_name = match p.file_name() {
+        Some(f) => f,
+        None => return NotifAction::Errno(libc::ENOENT),
+    };
+    let parent = p.parent().unwrap_or(std::path::Path::new("."));
+    let parent_str = match parent.to_str() {
+        Some("") | None => ".",
+        Some(s) => s,
+    };
+    let c_parent = match std::ffi::CString::new(parent_str) {
+        Ok(c) => c,
+        Err(_) => return NotifAction::Errno(libc::EINVAL),
+    };
+    let parent_fd = match openat2_at(
+        base.as_raw_fd(),
+        &c_parent,
+        (libc::O_PATH | libc::O_DIRECTORY | libc::O_CLOEXEC) as u64,
+        0,
+        RESOLVE_NO_MAGICLINKS,
+    ) {
+        Ok(f) => f,
+        Err(e) => return NotifAction::Errno(e),
+    };
+    let parent_real = match realpath_of_fd(parent_fd.as_raw_fd()) {
+        Some(p) => p,
+        None => return NotifAction::Errno(libc::EACCES),
+    };
+    if let Some(errno) = deny_open_verdict(&parent_real.join(file_name), flags, policy, pfs) {
+        return NotifAction::Errno(errno);
+    }
+    let c_name = match std::ffi::CString::new(file_name.as_encoded_bytes()) {
+        Ok(c) => c,
+        Err(_) => return NotifAction::Errno(libc::EINVAL),
+    };
+    let create_flags = flags as i32 & !(libc::O_PATH | libc::O_NOFOLLOW);
+    let fd = unsafe { libc::openat(parent_fd.as_raw_fd(), c_name.as_ptr(), create_flags, mode) };
+    inject_open_result(fd, flags)
+}
+
+/// Perform `openat`/`open` on behalf of the child, race-free, when a deny is
+/// active. Resolves once (pinning the inode), enforces deny + grant on the
+/// pinned target, then hands the child an fd to that exact inode via
+/// `InjectFdSend`. Returns `Continue` only when no allow/deny decision was
+/// made on content we resolved (unreadable path / no allowlist configured),
+/// matching the precheck's existing soft fall-through.
+fn on_behalf_open_for_deny(
+    notif: &SeccompNotif,
+    policy: &NotifPolicy,
+    pfs: &super::state::PolicyFnState,
+    notif_fd: RawFd,
+) -> NotifAction {
+    // No allowlist configured (Landlock is not allowlisting the filesystem):
+    // there is no grant to check against, so taking over the open could only
+    // wrongly deny. Leave it to the existing precheck/kernel path.
+    if policy.chroot_readable.is_empty() && policy.chroot_writable.is_empty() {
+        return NotifAction::Continue;
+    }
+
+    let OpenArgs { dirfd, path_ptr, flags, mode } = decode_open_args(notif);
+
+    let path = match read_child_cstr(notif_fd, notif.id, notif.pid, path_ptr, 4096) {
+        Some(p) => p,
+        None => return NotifAction::Continue, // kernel's re-read fails the same way
+    };
+    let c_path = match std::ffi::CString::new(path.clone()) {
+        Ok(c) => c,
+        Err(_) => return NotifAction::Errno(libc::EINVAL),
+    };
+    let base = match open_base_dir(notif.pid, dirfd) {
+        Ok(b) => b,
+        Err(e) => return NotifAction::Errno(e),
+    };
+
+    // Side-effect-free probe; mirror the child's no-follow intent for the
+    // final component.
+    let probe_flags = (libc::O_PATH | libc::O_CLOEXEC) as u64 | (flags & libc::O_NOFOLLOW as u64);
+    match openat2_at(base.as_raw_fd(), &c_path, probe_flags, 0, RESOLVE_NO_MAGICLINKS) {
+        Ok(probe) => reopen_existing_on_behalf(probe, flags, policy, pfs),
+        Err(errno) if errno == libc::ENOENT && (flags & libc::O_CREAT as u64) != 0 => {
+            create_new_on_behalf(&base, &path, flags, mode, policy, pfs)
+        }
+        Err(errno) => NotifAction::Errno(errno),
+    }
+}
+
 /// Read the thread-group leader (Tgid) of a thread from `/proc/<tid>/status`.
 fn tgid_of(tid: u32) -> Option<u32> {
     let status = std::fs::read_to_string(format!("/proc/{}/status", tid)).ok()?;
@@ -1343,8 +1601,27 @@ async fn handle_notification(
             if is_path_denied_for_notif(&pfs, &notif, fd) {
                 NotifAction::Errno(libc::EACCES)
             } else {
+                let has_denied = pfs.has_denied_paths();
                 drop(pfs);
-                dispatch_table.dispatch(notif, fd).await
+                // Let normal dispatch run first so /proc virtualization and
+                // other handlers still win for their paths.
+                let action = dispatch_table.dispatch(notif, fd).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
+                // symlink to reach a denied carve-out inside a granted tree
+                // (issue #111). Run the open on-behalf against the pinned
+                // inode and inject the fd so the kernel never re-resolves.
+                // Other path syscalls keep the best-effort precheck above
+                // (documented follow-up — they return no fd to inject).
+                let is_openat_family =
+                    nr == libc::SYS_openat || Some(nr) == arch::sys_open();
+                if matches!(action, NotifAction::Continue) && is_openat_family && has_denied {
+                    let pfs = ctx.policy_fn.lock().await;
+                    on_behalf_open_for_deny(&notif, policy, &pfs, fd)
+                } else {
+                    action
+                }
             }
         } else {
             dispatch_table.dispatch(notif, fd).await

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

@@ -433,6 +433,16 @@ impl PolicyFnState {
             false
         }
     }
+
+    /// Whether any deny rule is currently in effect. Cheap gate for the
+    /// race-free on-behalf open path: with no denies there is no carve-out
+    /// to protect and opens are left to the kernel and Landlock.
+    pub fn has_denied_paths(&self) -> bool {
+        self.denied_paths
+            .read()
+            .map(|d| !d.is_empty())
+            .unwrap_or(false)
+    }
 }
 
 // ============================================================

crates/sandlock-core/tests/integration/test_landlock.rs

@@ -1497,3 +1497,64 @@ async fn test_isolate_signals_allows_self() {
 
     let _ = std::fs::remove_file(&out);
 }
+
+#[tokio::test]
+async fn test_deny_carveout_on_behalf_open_preserves_io() {
+    // Issue #111: with a deny active, openat/open run on-behalf in the
+    // supervisor (race-free) instead of Continue-ing to the kernel. This
+    // checks that the new path preserves normal I/O — allowed reads, allowed
+    // creates — while the denied carve-out inside the granted tree stays
+    // blocked.
+    let dir = temp_file("deny-onbehalf-dir");
+    let _ = std::fs::create_dir_all(&dir);
+    let ok = dir.join("ok.txt");
+    let secret = dir.join("secret.txt");
+    std::fs::write(&ok, "ok-data").unwrap();
+    std::fs::write(&secret, "secret-data").unwrap();
+    let created = dir.join("created.txt");
+    let _ = std::fs::remove_file(&created);
+
+    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(dir.to_str().unwrap()) // grant the whole tree writable
+        .fs_deny(secret.to_str().unwrap()) // carve-out inside the grant
+        .build()
+        .unwrap();
+
+    // Allowed read still works (on-behalf probe + reopen path).
+    let r = policy
+        .clone()
+        .with_name("t")
+        .run(&["cat", ok.to_str().unwrap()])
+        .await
+        .unwrap();
+    assert!(r.success(), "allowed read must work through on-behalf open");
+
+    // Allowed create still works (on-behalf O_CREAT parent-resolve path).
+    let cmd = format!("echo hi > {}", created.display());
+    let w = policy
+        .clone()
+        .with_name("t")
+        .run_interactive(&["sh", "-c", &cmd])
+        .await
+        .unwrap();
+    assert!(w.success(), "allowed create must work through on-behalf open");
+    assert_eq!(std::fs::read_to_string(&created).unwrap().trim(), "hi");
+
+    // The denied carve-out stays blocked.
+    let d = policy
+        .clone()
+        .with_name("t")
+        .run(&["cat", secret.to_str().unwrap()])
+        .await
+        .unwrap();
+    assert!(!d.success(), "denied carve-out must stay blocked");
+
+    let _ = std::fs::remove_dir_all(&dir);
+}