cow: serve O_PATH and other openat opens of branch entries honestly

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

@@ -33,7 +33,7 @@ use tokio::sync::Mutex as AsyncMutex;
 
 use crate::arch;
 use crate::cow::result::link_result;
-use crate::cow::seccomp::SeccompCowBranch;
+use crate::cow::seccomp::{branch_errno, SeccompCowBranch};
 use crate::procfs::{build_dirent64, DT_DIR, DT_LNK, DT_REG};
 use crate::seccomp::notif::{
     decode_open_args, read_child_mem, write_child_mem, write_child_mem_force, NotifAction,
@@ -248,11 +248,8 @@ pub(crate) async fn handle_cow_open(
 
         let plan = match cow.prepare_open(&path, flags) {
             Ok(plan) => plan,
-            Err(crate::error::BranchError::QuotaExceeded) => {
-                return NotifAction::Errno(libc::ENOSPC)
-            }
-            Err(crate::error::BranchError::Exists) => return NotifAction::Errno(libc::EEXIST),
-            Err(_) => return NotifAction::Continue,
+            Err(crate::error::BranchError::NotOwned) => return NotifAction::Continue,
+            Err(e) => return NotifAction::Errno(branch_errno(e)),
         };
         (plan, upper_root, workdir_root)
     };
@@ -292,12 +289,17 @@ pub(crate) async fn handle_cow_open(
                     upper
                 }
                 Ok(Err(_)) | Err(_) => {
-                    // Copy failed — roll back quota and let kernel handle it
+                    // Continuing would let the kernel open the lower file
+                    // itself, so a write would land outside the branch.
                     let mut st = cow_state.lock().await;
                     if let Some(cow) = st.branch.as_mut() {
                         cow.rollback_copy(file_size);
                     }
-                    return NotifAction::Continue;
+                    let errno = match copy_result {
+                        Ok(Err(e)) => e.raw_os_error().unwrap_or(libc::EIO),
+                        _ => libc::EIO,
+                    };
+                    return NotifAction::Errno(errno);
                 }
             }
         }
@@ -320,7 +322,8 @@ pub(crate) async fn handle_cow_open(
         create_mode,
     ) {
         Ok(fd) => fd,
-        Err(_) => return NotifAction::Continue,
+        // The kernel cannot see the upper layer, so its answer would be wrong.
+        Err(errno) => return NotifAction::Errno(errno),
     };
 
     // Preserve O_CLOEXEC from the original openat flags.

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

@@ -32,7 +32,7 @@ fn parent_rel(rel: &str) -> Option<&str> {
 
 /// Errno for a branch failure, for handlers that answer the child directly
 /// (the `handle_unlink` convention) rather than returning a `BranchError`.
-fn branch_errno(e: BranchError) -> i32 {
+pub(super) fn branch_errno(e: BranchError) -> i32 {
     match e {
         BranchError::QuotaExceeded => libc::ENOSPC,
         BranchError::Denied => libc::EPERM,
@@ -41,6 +41,7 @@ fn branch_errno(e: BranchError) -> i32 {
         // A kernel object cannot be staged in the upper; EXDEV makes mv and
         // ln fall back to mknod plus unlink, which the branch virtualizes.
         BranchError::NotOwned => libc::EXDEV,
+        BranchError::Io { source, .. } => source.raw_os_error().unwrap_or(libc::EIO),
         BranchError::Operation(_) | BranchError::Conflict(_) => libc::EIO,
     }
 }
@@ -1185,27 +1186,44 @@ impl SeccompCowBranch {
                 self.check_quota(0)?;
                 return Ok(CowCopyPlan::Ready(upper_file));
             }
-            Err(e) => return Err(BranchError::Operation(format!("stat lower: {}", e))),
+            Err(e) => {
+                return Err(BranchError::Io {
+                    operation: "stat lower",
+                    source: std::io::Error::from_raw_os_error(e),
+                })
+            }
         };
         let kind = st.st_mode & libc::S_IFMT;
 
         // Symlink — copy verbatim (tiny, not worth deferring)
         if kind == libc::S_IFLNK {
             self.check_quota(256)?;
-            let target = crate::sys::fs::readlink_in_root(&self.workdir, rel_path)
-                .map_err(|e| BranchError::Operation(format!("readlink: {}", e)))?;
+            let target =
+                crate::sys::fs::readlink_in_root(&self.workdir, rel_path).map_err(|e| {
+                    BranchError::Io {
+                        operation: "readlink",
+                        source: std::io::Error::from_raw_os_error(e),
+                    }
+                })?;
             let target = std::path::PathBuf::from(std::ffi::OsString::from_vec(target));
             crate::sys::fs::symlinkat_in_root(&self.upper, rel_path, &target.to_string_lossy())
-                .map_err(|e| BranchError::Operation(format!("symlink: {}", e)))?;
+                .map_err(|e| BranchError::Io {
+                    operation: "symlink",
+                    source: std::io::Error::from_raw_os_error(e),
+                })?;
             self.disk_used += 256;
             return Ok(CowCopyPlan::Ready(upper_file));
         }
 
         // Directory — create immediately (no data copy)
         if kind == libc::S_IFDIR {
             self.check_quota(4096)?;
-            crate::sys::fs::mkdirp_in_root(&self.upper, rel_path, st.st_mode & 0o7777)
-                .map_err(|e| BranchError::Operation(format!("create dir: {}", e)))?;
+            crate::sys::fs::mkdirp_in_root(&self.upper, rel_path, st.st_mode & 0o7777).map_err(
+                |e| BranchError::Io {
+                    operation: "create dir",
+                    source: std::io::Error::from_raw_os_error(e),
+                },
+            )?;
             let _ = crate::sys::fs::chmod_in_root(&self.upper, rel_path, st.st_mode & 0o7777);
             self.disk_used += 4096;
             return Ok(CowCopyPlan::Ready(upper_file));
@@ -4503,6 +4521,22 @@ mod tests {
         assert!(matches!(err, BranchError::Deleted));
     }
 
+    #[test]
+    fn test_prepare_open_preserves_path_errors() {
+        let (workdir, storage) = setup_workdir();
+        fs::write(workdir.path().join("file"), "original").unwrap();
+        std::os::unix::fs::symlink("loop", workdir.path().join("loop")).unwrap();
+        let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap();
+
+        for (path, errno) in [("file/child", libc::ENOTDIR), ("loop/child", libc::ELOOP)] {
+            let path = workdir.path().join(path);
+            let err = branch
+                .prepare_open(path.to_str().unwrap(), libc::O_WRONLY as u64)
+                .unwrap_err();
+            assert_eq!(branch_errno(err), errno);
+        }
+    }
+
     #[test]
     fn test_prepare_open_write_existing_needs_copy() {
         let (workdir, storage) = setup_workdir();

crates/sandlock-core/src/error.rs

@@ -115,6 +115,13 @@ pub enum BranchError {
     #[error("branch operation failed: {0}")]
     Operation(String),
 
+    #[error("branch operation failed: {operation}: {source}")]
+    Io {
+        operation: &'static str,
+        #[source]
+        source: std::io::Error,
+    },
+
     #[error("branch conflict: {0}")]
     Conflict(String),
 

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

@@ -6,7 +6,7 @@ use std::collections::{HashMap, HashSet};
 use std::future::Future;
 use std::io;
 use std::net::IpAddr;
-use std::os::unix::io::{AsRawFd, IntoRawFd, OwnedFd, RawFd};
+use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, IntoRawFd, OwnedFd, RawFd};
 use std::pin::Pin;
 use std::sync::Arc;
 
@@ -736,11 +736,12 @@ pub(crate) fn decode_open_args(notif: &SeccompNotif, notif_fd: RawFd) -> Option<
     let a = &notif.data.args;
     let nr = notif.data.nr as i64;
     if nr == libc::SYS_openat {
+        let (flags, mode) = openat_how(a[2], a[3]);
         Some(OpenArgs {
             dirfd: a[0] as i64,
             path_ptr: a[1],
-            flags: a[2],
-            mode: a[3],
+            flags,
+            mode,
             resolve: 0,
         })
     } else if nr == arch::SYS_OPENAT2 {
@@ -771,16 +772,61 @@ pub(crate) fn decode_open_args(notif: &SeccompNotif, notif_fd: RawFd) -> Option<
         })
     } else {
         // legacy open(path, flags, mode) — AT_FDCWD implied.
+        let (flags, mode) = openat_how(a[1], a[2]);
         Some(OpenArgs {
             dirfd: libc::AT_FDCWD as i64,
             path_ptr: a[0],
-            flags: a[1],
-            mode: a[2],
+            flags,
+            mode,
             resolve: 0,
         })
     }
 }
 
+/// The `open_how` the kernel builds for an `open`/`openat` caller. Handlers
+/// serve opens with `openat2`, which rejects what `openat` silently drops:
+/// unknown flag bits, stray flags beside `O_PATH` (glibc adds `O_LARGEFILE`),
+/// and a mode without `O_CREAT`/`O_TMPFILE`.
+fn openat_how(flags: u64, mode: u64) -> (u64, u64) {
+    // libc exposes O_LARGEFILE as zero on some 64-bit targets, unlike the syscall ABI.
+    const O_LARGEFILE: u64 = if cfg!(target_arch = "aarch64") {
+        0o400000
+    } else {
+        0o100000
+    };
+    const VALID_OPEN_FLAGS: u64 = (libc::O_ACCMODE
+        | libc::O_CREAT
+        | libc::O_EXCL
+        | libc::O_NOCTTY
+        | libc::O_TRUNC
+        | libc::O_APPEND
+        | libc::O_NDELAY
+        | libc::O_NONBLOCK
+        | libc::O_SYNC
+        | libc::O_DSYNC
+        | libc::O_ASYNC
+        | libc::O_DIRECT
+        | libc::O_DIRECTORY
+        | libc::O_NOFOLLOW
+        | libc::O_NOATIME
+        | libc::O_CLOEXEC
+        | libc::O_PATH
+        | libc::O_TMPFILE) as u64
+        | O_LARGEFILE;
+    const O_PATH_FLAGS: u64 =
+        (libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_PATH | libc::O_CLOEXEC) as u64;
+    let flags = flags & VALID_OPEN_FLAGS;
+    let flags = if flags & libc::O_PATH as u64 != 0 {
+        flags & O_PATH_FLAGS
+    } else {
+        flags
+    };
+    let creates = flags & libc::O_CREAT as u64 != 0
+        || flags & libc::O_TMPFILE as u64 == libc::O_TMPFILE as u64;
+    let mode = if creates { mode & 0o7777 } else { 0 };
+    (flags, mode)
+}
+
 /// An open-family request, decoded once per notification and shared by every
 /// handler in the chain. Each handler used to read the string again, which
 /// added nothing: a handler is safe by how it answers, not by rereading.
@@ -1469,16 +1515,23 @@ fn respond_value(fd: RawFd, id: u64, val: i64) -> io::Result<()> {
 
 /// Fail-closed response used when fd injection fails.
 ///
-/// Denies the syscall with `EACCES` rather than letting it continue: a
+/// Denies the syscall rather than letting it continue: a
 /// `SECCOMP_USER_NOTIF_FLAG_CONTINUE` here would let the child's original
 /// syscall run unmediated against the host path, silently bypassing
 /// chroot/file confinement. (Regression guard: this must never be a CONTINUE
-/// response.)
-fn inject_failure_resp(id: u64) -> SeccompNotifResp {
+/// response.) Kernels whose ADDFD takes the source with `fget()` refuse an
+/// `O_PATH` file with `EBADF`; the child hears `EOPNOTSUPP`, not a denial.
+fn inject_failure_resp(id: u64, srcfd: BorrowedFd<'_>, err: io::Error) -> SeccompNotifResp {
+    let o_path = unsafe { libc::fcntl(srcfd.as_raw_fd(), libc::F_GETFL) } & libc::O_PATH != 0;
+    let errno = if o_path && err.raw_os_error() == Some(libc::EBADF) {
+        libc::EOPNOTSUPP
+    } else {
+        libc::EACCES
+    };
     SeccompNotifResp {
         id,
         val: 0,
-        error: -libc::EACCES,
+        error: -errno,
         flags: 0,
     }
 }
@@ -2040,7 +2093,7 @@ fn send_response(fd: RawFd, id: u64, action: NotifAction) -> io::Result<()> {
             // srcfd (OwnedFd) is dropped at end of this arm, closing the fd.
             match inject_fd_and_send(fd, id, srcfd.as_raw_fd(), newfd_flags) {
                 Ok(_new_fd) => Ok(()),
-                Err(_) => send_resp_raw(fd, &inject_failure_resp(id)),
+                Err(e) => send_resp_raw(fd, &inject_failure_resp(id, srcfd.as_fd(), e)),
             }
         }
         NotifAction::InjectFdSendTracked {
@@ -2052,7 +2105,7 @@ fn send_response(fd: RawFd, id: u64, action: NotifAction) -> io::Result<()> {
                 (on_success.0)(new_fd);
                 Ok(())
             }
-            Err(_) => send_resp_raw(fd, &inject_failure_resp(id)),
+            Err(e) => send_resp_raw(fd, &inject_failure_resp(id, srcfd.as_fd(), e)),
         },
         NotifAction::ReturnValue(val) => respond_value(fd, id, val),
         NotifAction::Hold => Ok(()), // Don't send a response.
@@ -3390,6 +3443,21 @@ pub(crate) async fn cleanup_pid(ctx: &super::ctx::SupervisorCtx, key: super::sta
 mod tests {
     use super::*;
 
+    #[test]
+    fn openat_how_matches_what_openat2_accepts() {
+        let o_path = (libc::O_PATH | libc::O_DIRECTORY) as u64;
+        assert_eq!(openat_how(o_path | libc::O_NONBLOCK as u64, 0), (o_path, 0));
+        assert_eq!(openat_how(o_path | libc::O_RDWR as u64, 0), (o_path, 0));
+        assert_eq!(openat_how(libc::O_RDONLY as u64, 0o755), (0, 0));
+        let creat = (libc::O_WRONLY | libc::O_CREAT) as u64;
+        assert_eq!(openat_how(creat, 0o100644), (creat, 0o644));
+        let tmpfile = (libc::O_RDWR | libc::O_TMPFILE) as u64;
+        assert_eq!(openat_how(tmpfile, 0o600), (tmpfile, 0o600));
+        assert_eq!(openat_how(libc::O_DIRECTORY as u64, 0o600).1, 0);
+        assert_eq!(openat_how(creat | 0x4000_0000, 0o644), (creat, 0o644));
+        assert_eq!(openat_how(creat | (1 << 63), 0o644), (creat, 0o644));
+    }
+
     #[test]
     fn fifo_writer_probe_sees_a_writer_and_consumes_nothing() {
         use std::os::unix::io::FromRawFd;
@@ -3601,7 +3669,9 @@ mod tests {
         // When fd injection fails, the supervisor must fail closed: deny the
         // syscall instead of letting it continue unmediated against the host
         // path (which would silently bypass chroot/file confinement).
-        let resp = inject_failure_resp(123);
+        let file = std::fs::File::open("/").unwrap();
+        let resp =
+            inject_failure_resp(123, file.as_fd(), io::Error::from_raw_os_error(libc::EBADF));
         assert_eq!(resp.id, 123);
         assert_eq!(
             resp.flags & SECCOMP_USER_NOTIF_FLAG_CONTINUE,
@@ -3612,6 +3682,23 @@ mod tests {
         assert_eq!(resp.error, -libc::EACCES);
     }
 
+    #[test]
+    fn inject_failure_of_an_o_path_source_is_eopnotsupp() {
+        let raw = unsafe { libc::open(c"/".as_ptr(), libc::O_PATH | libc::O_CLOEXEC) };
+        assert!(raw >= 0);
+        let fd = unsafe { OwnedFd::from_raw_fd(raw) };
+        let ebadf = || io::Error::from_raw_os_error(libc::EBADF);
+        assert_eq!(
+            inject_failure_resp(1, fd.as_fd(), ebadf()).error,
+            -libc::EOPNOTSUPP
+        );
+        let enomem = io::Error::from_raw_os_error(libc::ENOMEM);
+        assert_eq!(
+            inject_failure_resp(1, fd.as_fd(), enomem).error,
+            -libc::EACCES
+        );
+    }
+
     #[test]
     fn tgid_of_main_thread_is_own_pid() {
         // The main thread's tid equals the process pid, and its Tgid is the pid.