diff --git a/crates/sandlock-core/src/checkpoint/capture.rs b/crates/sandlock-core/src/checkpoint/capture.rs index cd92476c..67e4e418 100644 --- a/crates/sandlock-core/src/checkpoint/capture.rs +++ b/crates/sandlock-core/src/checkpoint/capture.rs @@ -387,7 +387,11 @@ fn parse_fdinfo(pid: i32, fd: i32) -> io::Result<(i32, u64)> { /// Capture a checkpoint from a running, stopped sandbox. /// The sandbox must already be frozen (SIGSTOP'd and fork-held). -pub(crate) fn capture(pid: i32, policy: &Sandbox) -> Result { +pub(crate) fn capture( + pid: i32, + policy: &Sandbox, + mem_gate: Option<&crate::seccomp::gate::MemGate>, +) -> Result { // Credential-injection rules hold supervisor-only secrets that are // deliberately not serialized (`#[serde(skip)]`). A checkpoint image would // therefore restore with no injection rules and send every request @@ -411,6 +415,11 @@ pub(crate) fn capture(pid: i32, policy: &Sandbox) -> Result, + sun_path: &Path, +) -> Result { + let vpath = build_virtual_path( + notif, + libc::AT_FDCWD as i64, + sun_path.to_str().ok_or(libc::ECONNREFUSED)?, + ctx, + ) + .ok_or(libc::ECONNREFUSED)?; + let confined = confine(&vpath); + let (root, sub) = match ctx.mount_target(&confined) { + Some((mount, sub)) => (mount, sub), + None => (ctx.root, confined.to_string_lossy().into_owned()), + }; + let fd = openat2_in_root(root, &sub, libc::O_PATH | libc::O_CLOEXEC, 0) + .map_err(|_| libc::ECONNREFUSED)?; + let pinned = unsafe { OwnedFd::from_raw_fd(fd) }; + let host = std::fs::read_link(format!("/proc/self/fd/{}", fd)).map_err(|_| libc::EACCES)?; + match ctx.host_to_virtual(&host) { + Some(virtual_path) if ctx.can_write(&virtual_path) => Ok(pinned), + _ => Err(libc::EACCES), + } +} + /// Convert a Path to CString, returning Errno on failure. fn path_cstr(path: &Path, err: i32) -> Result { CString::new(path.to_str().unwrap_or("")).map_err(|_| NotifAction::Errno(err)) @@ -560,6 +593,35 @@ fn read_and_resolve_existing( } /// Perform a libc syscall on a host path; return ReturnValue(0) or Errno. +fn rename_on_host(old_host: &Path, new_host: &Path) -> NotifAction { + let c_old = match path_cstr(old_host, libc::EINVAL) { + Ok(c) => c, + Err(a) => return a, + }; + let c_new = match path_cstr(new_host, libc::EINVAL) { + Ok(c) => c, + Err(a) => return a, + }; + if unsafe { libc::rename(c_old.as_ptr(), c_new.as_ptr()) } < 0 { + NotifAction::Errno(last_errno(libc::EIO)) + } else { + NotifAction::ReturnValue(0) + } +} + +fn chown_on_host(host: &Path, uid: u32, gid: u32, nofollow: bool) -> NotifAction { + exec_on_host( + |p| unsafe { + if nofollow { + libc::lchown(p, uid, gid) + } else { + libc::chown(p, uid, gid) + } + }, + host, + ) +} + fn exec_on_host(f: impl FnOnce(*const libc::c_char) -> libc::c_int, host: &Path) -> NotifAction { let c = match path_cstr(host, libc::EINVAL) { Ok(c) => c, @@ -628,64 +690,10 @@ pub(crate) async fn handle_chroot_open( // COW path: the branch picks the layer, the kernel resolves the rest. // A symlink in the image is followed inside that layer's root, never on // the host, because the supervisor holds no chroot of its own. + #[cfg(feature = "cow")] + if let Some(action) = chroot_cow_open(notif, notif_fd, cow_state, &host_path, flags, mode).await { - #[cfg(feature = "cow")] - let mut cs = cow_state.lock().await; - #[cfg(feature = "cow")] - if let Some(cow) = cs.branch.as_mut() { - let host_str = host_path.to_string_lossy(); - if cow.matches(&host_str) { - match cow.handle_open(&host_str, flags) { - Ok(Some(real_path)) => { - let upper_root = cow.upper_dir().to_path_buf(); - let workdir_root = cow.workdir().to_path_buf(); - drop(cs); - let fd = match crate::cow::dispatch::open_confined( - &upper_root, - &workdir_root, - &real_path, - flags_i, - mode, - ) { - Ok(fd) => fd, - Err(errno) => return NotifAction::Errno(errno), - }; - let newfd_flags = if flags & libc::O_CLOEXEC as u64 != 0 { - libc::O_CLOEXEC as u32 - } else { - 0 - }; - let owned = unsafe { OwnedFd::from_raw_fd(fd) }; - return NotifAction::InjectFdSend { - srcfd: owned, - newfd_flags, - }; - } - Ok(None) => { - // Fall through to openat2_in_root below. This keeps - // directory opens and other non-COW / non-whiteout - // cases confined to the chroot instead of executing the - // original host syscall. (Whiteouts are handled by the - // BranchError::Deleted arm below, not here.) - } - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC); - } - Err(crate::error::BranchError::Exists) => { - return NotifAction::Errno(libc::EEXIST); - } - Err(crate::error::BranchError::Deleted) => { - // Whiteout read-open: the lower file still physically - // exists, but the branch deleted it. Return ENOENT - // rather than leaking pre-delete bytes — parity with the - // async cow open path (cow/dispatch.rs) and - // stat/statx/access/readlink. - return NotifAction::Errno(libc::ENOENT); - } - Err(_) => return NotifAction::Errno(libc::EIO), - } - } - } + return action; } // Resolve the path to an fd to hand the child: either a freshly opened tree @@ -702,6 +710,89 @@ pub(crate) async fn handle_chroot_open( } } +/// The answer for an open the COW branch owns, or `None` to open it in the +/// namespace. +#[cfg(feature = "cow")] +async fn chroot_cow_open( + notif: &SeccompNotif, + notif_fd: RawFd, + cow_state: &Arc>, + host_path: &Path, + flags: u64, + mode: u32, +) -> Option { + use crate::cow::dispatch::{Caller, CowOpen, Layers}; + + let host_str = host_path.to_string_lossy().into_owned(); + let mut cs = cow_state.lock().await; + let cow = cs.branch.as_mut()?; + if !cow.matches(&host_str) { + return None; + } + if flags & libc::O_DIRECTORY as u64 != 0 { + // The namespace open sees every layer but a whiteout: the lower + // directory survives one, where the merged answer is ENOENT. + let rel = cow.safe_rel(&host_str)?; + return cow + .is_deleted(&rel) + .then_some(NotifAction::Errno(libc::ENOENT)); + } + let open = CowOpen { + caller: Caller::of(notif, notif_fd), + layers: Layers::of(cow), + path: host_str, + flags, + open_flags: flags as i32, + mode, + may_continue: false, + }; + open.run(cow_state, cow) +} + +/// A branch handler's answer in chroot mode, where `host` is the syscall +/// done on the host path for what the branch leaves to the kernel. +#[cfg(feature = "cow")] +fn branch_answer( + r: Result, + host: impl FnOnce() -> NotifAction, +) -> NotifAction { + match r { + Ok(true) => NotifAction::ReturnValue(0), + Ok(false) | Err(crate::error::BranchError::NotOwned) => host(), + Err(e) => NotifAction::Errno(crate::cow::seccomp::branch_errno(e)), + } +} + +/// Answer a chroot write the branch owns: copy up what it needs off the +/// loop, then run `apply`, with `host` for what the branch leaves to the +/// kernel. +#[cfg(feature = "cow")] +fn chroot_cow_write( + cow_state: &Arc>, + caller: crate::cow::dispatch::Caller, + cow: &mut crate::cow::seccomp::SeccompCowBranch, + path: String, + host: impl Fn() -> NotifAction + Clone + Send + 'static, + apply: impl FnOnce( + &mut crate::cow::seccomp::SeccompCowBranch, + &str, + ) -> Result + + Send + + 'static, +) -> NotifAction { + use crate::cow::seccomp::CopyUp; + let need = cow.safe_rel(&path).map_or(CopyUp::Nothing, CopyUp::Entry); + let refuse_host = host.clone(); + crate::cow::dispatch::after_copy_up( + cow_state, + caller, + cow, + need, + move |e| branch_answer(Err(e), &refuse_host), + move |cow| branch_answer(apply(cow, &path), host), + ) +} + /// Open `virtual_path` within the child's virtual namespace, returning an fd to /// inject into the child. /// @@ -1447,30 +1538,35 @@ pub(crate) async fn handle_chroot_write( let mut cs = cow_state.lock().await; #[cfg(feature = "cow")] if let Some(cow) = cs.branch.as_mut() { - let old_str = old_host.to_string_lossy(); + let old_str = old_host.to_string_lossy().into_owned(); if cow.matches(&old_str) { - match cow.handle_rename(&old_str, &new_host.to_string_lossy()) { - Ok(true) => return NotifAction::ReturnValue(0), - Err(errno) => return NotifAction::Errno(errno), - Ok(false) => {} + let new_str = new_host.to_string_lossy().into_owned(); + // Refused before the copy, so a refused rename stages nothing. + if let Err(errno) = cow.check_rename(&old_str, &new_str) { + return NotifAction::Errno(errno); } + let need = cow + .safe_rel(&old_str) + .map_or(crate::cow::seccomp::CopyUp::Nothing, |r| { + cow.rename_copy_up(r) + }); + return crate::cow::dispatch::after_copy_up( + cow_state, + crate::cow::dispatch::Caller::of(notif, notif_fd), + cow, + need, + |e| NotifAction::Errno(crate::cow::seccomp::branch_errno(e)), + move |cow| match cow.handle_rename(&old_str, &new_str) { + Ok(true) => NotifAction::ReturnValue(0), + Err(errno) => NotifAction::Errno(errno), + Ok(false) => rename_on_host(&old_host, &new_host), + }, + ); } } } - let c_old = match path_cstr(&old_host, libc::EINVAL) { - Ok(c) => c, - Err(a) => return a, - }; - let c_new = match path_cstr(&new_host, libc::EINVAL) { - Ok(c) => c, - Err(a) => return a, - }; - return if unsafe { libc::rename(c_old.as_ptr(), c_new.as_ptr()) } < 0 { - NotifAction::Errno(last_errno(libc::EIO)) - } else { - NotifAction::ReturnValue(0) - }; + return rename_on_host(&old_host, &new_host); } if nr == libc::SYS_symlinkat { @@ -1604,7 +1700,20 @@ pub(crate) async fn handle_chroot_write( return NotifAction::Errno(libc::EXDEV); } if cow.matches(&new_s) { - return crate::cow::result::link_result(cow.handle_link(&old_s, &new_s)); + let (old_s, new_s) = (old_s.into_owned(), new_s.into_owned()); + let need = cow + .safe_rel(&old_s) + .map_or(crate::cow::seccomp::CopyUp::Nothing, |r| { + cow.link_copy_up(r) + }); + return crate::cow::dispatch::after_copy_up( + cow_state, + crate::cow::dispatch::Caller::of(notif, notif_fd), + cow, + need, + |e| crate::cow::result::link_result(Err(e)), + move |cow| crate::cow::result::link_result(cow.handle_link(&old_s, &new_s)), + ); } } } @@ -1669,15 +1778,17 @@ pub(crate) async fn handle_chroot_write( let mut cs = cow_state.lock().await; #[cfg(feature = "cow")] if let Some(cow) = cs.branch.as_mut() { - let s = host_path.to_string_lossy(); + let s = host_path.to_string_lossy().into_owned(); if cow.matches(&s) { - match cow.handle_chmod(&s, mode) { - Ok(true) => return NotifAction::ReturnValue(0), - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC) - } - _ => {} - } + let host = host_path.clone(); + return chroot_cow_write( + cow_state, + crate::cow::dispatch::Caller::of(notif, notif_fd), + cow, + s, + move || exec_on_host(|p| unsafe { libc::chmod(p, mode) }, &host), + move |cow, s| cow.handle_chmod(s, mode), + ); } } } @@ -1706,28 +1817,21 @@ pub(crate) async fn handle_chroot_write( let mut cs = cow_state.lock().await; #[cfg(feature = "cow")] if let Some(cow) = cs.branch.as_mut() { - let s = host_path.to_string_lossy(); + let s = host_path.to_string_lossy().into_owned(); if cow.matches(&s) { - match cow.handle_chown(&s, uid, gid) { - Ok(true) => return NotifAction::ReturnValue(0), - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC) - } - _ => {} - } + let host = host_path.clone(); + return chroot_cow_write( + cow_state, + crate::cow::dispatch::Caller::of(notif, notif_fd), + cow, + s, + move || chown_on_host(&host, uid, gid, nofollow), + move |cow, s| cow.handle_chown(s, uid, gid), + ); } } } - return exec_on_host( - |p| unsafe { - if nofollow { - libc::lchown(p, uid, gid) - } else { - libc::chown(p, uid, gid) - } - }, - &host_path, - ); + return chown_on_host(&host_path, uid, gid, nofollow); } if nr == libc::SYS_truncate { @@ -1749,15 +1853,17 @@ pub(crate) async fn handle_chroot_write( let mut cs = cow_state.lock().await; #[cfg(feature = "cow")] if let Some(cow) = cs.branch.as_mut() { - let s = host_path.to_string_lossy(); + let s = host_path.to_string_lossy().into_owned(); if cow.matches(&s) { - match cow.handle_truncate(&s, length) { - Ok(true) => return NotifAction::ReturnValue(0), - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC) - } - _ => {} - } + let host = host_path.clone(); + return chroot_cow_write( + cow_state, + crate::cow::dispatch::Caller::of(notif, notif_fd), + cow, + s, + move || exec_on_host(|p| unsafe { libc::truncate(p, length) }, &host), + move |cow, s| cow.handle_truncate(s, length), + ); } } } diff --git a/crates/sandlock-core/src/cow/dispatch.rs b/crates/sandlock-core/src/cow/dispatch.rs index b65a789e..d8f71fec 100644 --- a/crates/sandlock-core/src/cow/dispatch.rs +++ b/crates/sandlock-core/src/cow/dispatch.rs @@ -33,10 +33,11 @@ use tokio::sync::Mutex as AsyncMutex; use crate::arch; use crate::cow::result::link_result; -use crate::cow::seccomp::{branch_errno, SeccompCowBranch}; +use crate::cow::seccomp::{branch_errno, CopyUp, CowCopyPlan, CowOpenPlan, SeccompCowBranch}; +use crate::error::BranchError; 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, + decode_open_args, id_valid, read_child_mem, write_child_mem, write_child_mem_force, NotifAction, }; use crate::seccomp::state::{CowState, PerProcessState, ProcessIndex}; use crate::sys::structs::SeccompNotif; @@ -197,6 +198,254 @@ pub(crate) fn open_confined( crate::sys::fs::openat2_in_root(root, &rel, flags, mode) } +/// The branch's directories, taken under the lock for work done without it. +#[derive(Clone)] +pub(crate) struct Layers { + upper: PathBuf, + workdir: PathBuf, + staging: PathBuf, +} + +impl Layers { + pub(crate) fn of(cow: &SeccompCowBranch) -> Self { + Self { + upper: cow.upper_dir().to_path_buf(), + workdir: cow.workdir().to_path_buf(), + staging: cow.staging_dir().to_path_buf(), + } + } +} + +/// The trapped syscall a copy-up is made for. +#[derive(Clone, Copy)] +pub(crate) struct Caller { + fd: RawFd, + id: u64, +} + +impl Caller { + pub(crate) fn of(notif: &SeccompNotif, fd: RawFd) -> Self { + Self { fd, id: notif.id } + } + + fn waiting(self) -> bool { + id_valid(self.fd, self.id).is_ok() + } +} + +/// Run `apply` under the branch lock once `need` is in the upper. Only the +/// copy is slow, so only it leaves the loop, and `apply` runs under the lock +/// that found every copy in place. +pub(crate) fn after_copy_up( + cow_state: &Arc>, + caller: Caller, + cow: &mut SeccompCowBranch, + need: CopyUp, + refuse: impl Fn(BranchError) -> NotifAction + Send + 'static, + apply: impl FnOnce(&mut SeccompCowBranch) -> NotifAction + Send + 'static, +) -> NotifAction { + let plans = match &need { + CopyUp::Nothing => return apply(cow), + CopyUp::Entry(rel) => match cow.prepare_copies(std::slice::from_ref(rel)) { + Ok(plans) if plans.is_empty() => return apply(cow), + Ok(plans) => plans, + Err(e) => return refuse(e), + }, + // Walking the lower tree is I/O too. + CopyUp::Tree(_) => Vec::new(), + }; + copy_up_off_loop( + cow_state, + caller, + Layers::of(cow), + need, + plans, + refuse, + apply, + ) +} + +/// `after_copy_up` for a caller holding `plans` it already made for `need`. +pub(crate) fn copy_up_off_loop( + cow_state: &Arc>, + caller: Caller, + layers: Layers, + need: CopyUp, + mut plans: Vec<(String, CowCopyPlan)>, + refuse: impl Fn(BranchError) -> NotifAction + Send + 'static, + apply: impl FnOnce(&mut SeccompCowBranch) -> NotifAction + Send + 'static, +) -> NotifAction { + let cow_state = Arc::clone(cow_state); + NotifAction::native(move || { + let rels = match need { + CopyUp::Nothing => Vec::new(), + CopyUp::Entry(rel) => vec![rel], + CopyUp::Tree(root) => match SeccompCowBranch::lower_tree(&layers.workdir, &root) { + Ok(tree) => tree, + Err(errno) => return NotifAction::Errno(errno), + }, + }; + loop { + if let Err(errno) = run_copies(&cow_state, &layers, plans) { + return NotifAction::Errno(errno); + } + let mut st = cow_state.blocking_lock(); + let Some(cow) = st.branch.as_mut() else { + return NotifAction::Errno(libc::EIO); + }; + plans = match cow.prepare_copies(&rels) { + Ok(plans) => plans, + Err(e) => return refuse(e), + }; + // A caller interrupted meanwhile (a signal, or an exec freeze + // without a killable wait) restarts its syscall, which must not + // find the write already done. + if plans.is_empty() { + return match caller.waiting() { + true => apply(cow), + false => NotifAction::Errno(libc::EINTR), + }; + } + } + }) +} + +/// Blocks. Copies first, waits after: a caller that waits holds no ticket, +/// so two of them never wait on each other. A failed copy fails the syscall, +/// since continuing would reach the lower file. +fn run_copies( + cow_state: &Mutex, + layers: &Layers, + plans: Vec<(String, CowCopyPlan)>, +) -> Result<(), i32> { + let (copies, waits): (Vec<_>, Vec<_>) = plans + .into_iter() + .partition(|(_, plan)| matches!(plan, CowCopyPlan::NeedsCopy { .. })); + let mut copies = copies.into_iter(); + while let Some((rel, plan)) = copies.next() { + let CowCopyPlan::NeedsCopy { + file_size, ticket, .. + } = plan + else { + continue; + }; + let staged = SeccompCowBranch::stage_copy(&layers.workdir, &layers.staging, &rel); + let mut st = cow_state.blocking_lock(); + let cow = st.branch.as_mut().ok_or(libc::EIO)?; + if let Err(e) = cow.finish_copy(&rel, file_size, ticket, staged) { + cow.abandon_copies(copies); + return Err(e.raw_os_error().unwrap_or(libc::EIO)); + } + } + for (_, plan) in waits { + if let CowCopyPlan::InFlight(done) = plan { + done.wait(); + } + } + Ok(()) +} + +/// Where an open plan leads. +pub(crate) enum OpenStep { + /// Outside the branch's say: the caller opens it the way it would anyway. + Skip, + Open(PathBuf), + Copy(String, CowCopyPlan), + Answer(NotifAction), +} + +impl OpenStep { + pub(crate) fn of(plan: Result) -> Self { + match plan { + Ok(CowOpenPlan::Skip) | Err(BranchError::NotOwned) => OpenStep::Skip, + // Deleted in this branch (whiteout): the lower file still exists, so + // opening it would read its pre-delete content. Return ENOENT, + // matching the stat/access handlers. + Ok(CowOpenPlan::Deleted) => OpenStep::Answer(NotifAction::Errno(libc::ENOENT)), + Ok(CowOpenPlan::Resolved(p)) | Ok(CowOpenPlan::UpperReady { upper: p }) => { + OpenStep::Open(p) + } + Ok(CowOpenPlan::Copy { rel_path, plan }) => OpenStep::Copy(rel_path, plan), + Err(e) => OpenStep::Answer(NotifAction::Errno(branch_errno(e))), + } + } +} + +/// An open the branch may redirect. +pub(crate) struct CowOpen { + pub(crate) caller: Caller, + pub(crate) layers: Layers, + pub(crate) path: String, + pub(crate) flags: u64, + pub(crate) open_flags: i32, + pub(crate) mode: u32, + /// Chroot mode has no kernel open to fall back to after a copy. + pub(crate) may_continue: bool, +} + +impl CowOpen { + /// The answer for this open, or `None` when the branch leaves it alone. + pub(crate) fn run( + self, + cow_state: &Arc>, + cow: &mut SeccompCowBranch, + ) -> Option { + Some( + match OpenStep::of(cow.prepare_open(&self.path, self.flags)) { + OpenStep::Skip => return None, + OpenStep::Answer(action) => action, + OpenStep::Open(p) => self.inject(&p), + OpenStep::Copy(rel, plan) => copy_up_off_loop( + cow_state, + self.caller, + self.layers.clone(), + CopyUp::Entry(rel.clone()), + vec![(rel, plan)], + |e| NotifAction::Errno(branch_errno(e)), + move |cow| self.reopen(cow), + ), + }, + ) + } + + fn reopen(self, cow: &mut SeccompCowBranch) -> NotifAction { + match OpenStep::of(cow.prepare_open(&self.path, self.flags)) { + OpenStep::Open(p) => self.inject(&p), + OpenStep::Answer(action) => action, + OpenStep::Skip if self.may_continue => NotifAction::Continue, + OpenStep::Skip => NotifAction::Errno(libc::EIO), + OpenStep::Copy(rel, plan) => { + cow.abandon_copies([(rel, plan)]); + NotifAction::Errno(libc::EIO) + } + } + } + + fn inject(&self, real_path: &Path) -> NotifAction { + let fd = match open_confined( + &self.layers.upper, + &self.layers.workdir, + real_path, + self.open_flags, + self.mode, + ) { + Ok(fd) => fd, + // 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. + let newfd_flags = if self.flags & libc::O_CLOEXEC as u64 != 0 { + libc::O_CLOEXEC as u32 + } else { + 0 + }; + NotifAction::InjectFdSend { + srcfd: unsafe { OwnedFd::from_raw_fd(fd) }, + newfd_flags, + } + } +} + /// Handle open/openat/openat2 under workdir: redirect to COW upper/lower. pub(crate) async fn handle_cow_open( notif: &SeccompNotif, @@ -204,8 +453,6 @@ pub(crate) async fn handle_cow_open( processes: &Arc, notif_fd: RawFd, ) -> NotifAction { - use crate::cow::seccomp::CowOpenPlan; - 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, @@ -222,121 +469,42 @@ pub(crate) async fn handle_cow_open( }; let mut path = resolve_at_path_with_virtual(notif, dirfd, &rel_path, virtual_cwd.as_deref()); - // Phase 1: determine plan under lock (no heavy I/O) - let (plan, upper_root, workdir_root) = { - let mut st = cow_state.lock().await; - let cow = match st.branch.as_mut() { - Some(c) => c, - None => return NotifAction::Continue, - }; - let upper_root = cow.upper_dir().to_path_buf(); - let workdir_root = cow.workdir().to_path_buf(); - - path = map_cow_upper_path(cow, &path); - if !cow.matches(&path) { - return NotifAction::Continue; - } - - // Read-only opens don't need interception unless the file was - // modified or deleted in the COW layer. - const WRITE_FLAGS: u64 = - (libc::O_WRONLY | libc::O_RDWR | libc::O_CREAT | libc::O_TRUNC | libc::O_APPEND) as u64; - let is_write = flags & WRITE_FLAGS != 0; - if !is_write && !cow.needs_read_intercept(&path) { - return NotifAction::Continue; - } - - let plan = match cow.prepare_open(&path, flags) { - Ok(plan) => plan, - Err(crate::error::BranchError::NotOwned) => return NotifAction::Continue, - Err(e) => return NotifAction::Errno(branch_errno(e)), - }; - (plan, upper_root, workdir_root) - }; - // Lock is released here - - // Phase 2: execute I/O plan without holding the lock - let real_path = match plan { - CowOpenPlan::Skip => return NotifAction::Continue, - // Deleted in this branch (whiteout): the lower file still exists, so - // Continue would read its pre-delete content. Return ENOENT, matching - // the stat/access handlers. - CowOpenPlan::Deleted => return NotifAction::Errno(libc::ENOENT), - CowOpenPlan::Resolved(p) | CowOpenPlan::UpperReady { upper: p } => p, - CowOpenPlan::NeedsCopy { - upper, - lower: _lower, - file_size, - rel_path, - } => { - // Do the potentially-expensive copy on a blocking thread - let root = workdir_root.clone(); - let uroot = upper_root.clone(); - let rel = rel_path.clone(); - let copy_result = tokio::task::spawn_blocking(move || { - crate::cow::seccomp::SeccompCowBranch::execute_copy(&root, &uroot, &rel) - }) - .await; - - match copy_result { - Ok(Ok(digest)) => { - if let Some(d) = digest { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.record_digest(&rel_path, d); - } - } - upper - } - Ok(Err(_)) | Err(_) => { - // 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); - } - let errno = match copy_result { - Ok(Err(e)) => e.raw_os_error().unwrap_or(libc::EIO), - _ => libc::EIO, - }; - return NotifAction::Errno(errno); - } - } - } + let mut st = cow_state.lock().await; + let cow = match st.branch.as_mut() { + Some(c) => c, + None => return NotifAction::Continue, }; - // Phase 3: open the resolved path and inject fd - // Strip O_EXCL — the COW layer already verified exclusivity. Keeping it - // would fail because we may have just copied the file into upper. - let open_flags = (flags & !(libc::O_EXCL as u64)) as i32; - // Honor the child's requested creation mode (masked to permission bits). - // Hardcoding 0o666 dropped the execute bits, so a binary copied into the - // workdir (e.g. `cp /bin/echo m`) landed in upper non-executable and - // `./m` failed with EACCES. The kernel ignores mode unless O_CREAT is set. - let create_mode = (mode & 0o7777) as libc::c_uint; - let fd = match open_confined( - &upper_root, - &workdir_root, - &real_path, - open_flags, - create_mode, - ) { - Ok(fd) => fd, - // The kernel cannot see the upper layer, so its answer would be wrong. - Err(errno) => return NotifAction::Errno(errno), - }; + path = map_cow_upper_path(cow, &path); + if !cow.matches(&path) { + return NotifAction::Continue; + } - // Preserve O_CLOEXEC from the original openat flags. - let newfd_flags = if flags & libc::O_CLOEXEC as u64 != 0 { - libc::O_CLOEXEC as u32 - } else { - 0 - }; - let owned = unsafe { OwnedFd::from_raw_fd(fd) }; - NotifAction::InjectFdSend { - srcfd: owned, - newfd_flags, + // Read-only opens don't need interception unless the file was + // modified or deleted in the COW layer. + const WRITE_FLAGS: u64 = + (libc::O_WRONLY | libc::O_RDWR | libc::O_CREAT | libc::O_TRUNC | libc::O_APPEND) as u64; + let is_write = flags & WRITE_FLAGS != 0; + if !is_write && !cow.needs_read_intercept(&path) { + return NotifAction::Continue; } + + let open = CowOpen { + caller: Caller::of(notif, notif_fd), + layers: Layers::of(cow), + path, + flags, + // Strip O_EXCL — the COW layer already verified exclusivity. Keeping it + // would fail because we may have just copied the file into upper. + open_flags: (flags & !(libc::O_EXCL as u64)) as i32, + // Honor the child's requested creation mode (masked to permission bits). + // Hardcoding 0o666 dropped the execute bits, so a binary copied into the + // workdir (e.g. `cp /bin/echo m`) landed in upper non-executable and + // `./m` failed with EACCES. The kernel ignores mode unless O_CREAT is set. + mode: (mode & 0o7777) as u32, + may_continue: true, + }; + open.run(cow_state, cow).unwrap_or(NotifAction::Continue) } // ============================================================ @@ -568,16 +736,13 @@ fn parse_cow_write( } /// Map a BranchError result to a NotifAction. -fn cow_result(r: Result) -> NotifAction { +fn cow_result(r: Result) -> NotifAction { match r { Ok(true) => NotifAction::ReturnValue(0), - Err(crate::error::BranchError::QuotaExceeded) => NotifAction::Errno(libc::ENOSPC), - Err(crate::error::BranchError::Denied) => NotifAction::Errno(libc::EPERM), - // Whiteouted source: Continue would let the kernel act on the lower - // entry, which still exists with its pre-delete content. - Err(crate::error::BranchError::Deleted) => NotifAction::Errno(libc::ENOENT), - Err(crate::error::BranchError::NotOwned) => NotifAction::Continue, - _ => NotifAction::Continue, + Ok(false) | Err(BranchError::NotOwned) => NotifAction::Continue, + // Continue would let the kernel act on the lower entry, which the + // branch promised to leave alone. + Err(e) => NotifAction::Errno(branch_errno(e)), } } @@ -590,76 +755,38 @@ fn unlink_result(r: Result) -> NotifAction { } } -/// Determine which relative path (if any) needs a COW copy for this operation. -/// Returns `(match_path, copy_rel)` where match_path is checked against -/// `cow.matches()` and copy_rel is the relative path to pre-copy. -fn cow_copy_rel<'a>( - op: &'a CowWriteOp, - cow: &crate::cow::seccomp::SeccompCowBranch, -) -> Option<(&'a str, String)> { - let (match_path, copy_path) = match op { - // These ops call ensure_cow_copy internally — pre-copy the target - CowWriteOp::Chmod { ref path, .. } - | CowWriteOp::Chown { ref path, .. } - | CowWriteOp::Truncate { ref path, .. } => (path.as_str(), path.as_str()), - CowWriteOp::Rename { ref old_path, .. } => (old_path.as_str(), old_path.as_str()), - CowWriteOp::Link { - ref old_path, - ref new_path, - .. - } => (new_path.as_str(), old_path.as_str()), - // These ops don't need a pre-copy - _ => return None, - }; - if !cow.matches(match_path) { - return None; +impl CowWriteOp { + /// What the branch must copy up before this write can run. + fn copy_up(&self, cow: &SeccompCowBranch) -> CopyUp { + let entry = |path: &str| cow.matches(path).then(|| cow.safe_rel(path)).flatten(); + let need = match self { + CowWriteOp::Chmod { path, .. } + | CowWriteOp::Chown { path, .. } + | CowWriteOp::Truncate { path, .. } => entry(path).map(CopyUp::Entry), + CowWriteOp::Rename { old_path, .. } => entry(old_path).map(|r| cow.rename_copy_up(r)), + CowWriteOp::Link { old_path, new_path } if cow.matches(new_path) => { + entry(old_path).map(|r| cow.link_copy_up(r)) + } + _ => None, + }; + need.unwrap_or(CopyUp::Nothing) } - cow.safe_rel(copy_path).map(|rel| (match_path, rel)) } -/// Execute a deferred `CowCopyPlan::NeedsCopy` on a blocking thread. -/// Returns the upper path on success, or rolls back quota on failure. -async fn execute_deferred_copy( - cow_state: &Arc>, - workdir_root: std::path::PathBuf, - upper_root: std::path::PathBuf, - rel: String, - upper: std::path::PathBuf, - file_size: u64, -) -> Option { - let rel_path = rel.clone(); - let copy_result = tokio::task::spawn_blocking(move || { - crate::cow::seccomp::SeccompCowBranch::execute_copy(&workdir_root, &upper_root, &rel) - }) - .await; - match copy_result { - Ok(Ok(digest)) => { - if let Some(d) = digest { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.record_digest(&rel_path, d); - } - } - Some(upper) - } - _ => { - let mut st = cow_state.lock().await; - if let Some(cow) = st.branch.as_mut() { - cow.rollback_copy(file_size); - } - None - } +/// How a write answers a copy-up the branch refused. +fn refuse_write_copy(names_entry: bool, e: BranchError) -> NotifAction { + match e { + BranchError::QuotaExceeded => NotifAction::Errno(libc::ENOSPC), + // A kernel object: its metadata is the kernel's to change, but it + // cannot be staged under a new name. + BranchError::NotOwned if names_entry => NotifAction::Errno(libc::EXDEV), + BranchError::NotOwned => NotifAction::Continue, + e => NotifAction::Errno(branch_errno(e)), } } /// Handle all write-type syscalls: both *at variants (unlinkat, mkdirat, etc.) /// and legacy variants (unlink, rmdir, mkdir, etc.). -/// -/// For operations that modify existing files (chmod, chown, rename, link, -/// truncate), the handler uses a two-phase pattern: prepare the copy plan -/// under the lock, execute the potentially expensive file copy outside the -/// lock, then re-acquire the lock and run the actual operation (which finds -/// the file already in upper). pub(crate) async fn handle_cow_write( notif: &SeccompNotif, cow_state: &Arc>, @@ -672,79 +799,35 @@ pub(crate) async fn handle_cow_write( None => return NotifAction::Continue, }; - // Phase 1: check if we need to pre-copy a file (under lock, no heavy I/O). - // Capture both layer roots here so Phase 2 needs no second lock. - let (copy_plan, copy_workdir, copy_upper, copy_rel) = { - let mut st = cow_state.lock().await; - let cow = match st.branch.as_mut() { - Some(c) => c, - None => return NotifAction::Continue, - }; - - op.remap_upper_paths(cow); - match cow_copy_rel(&op, cow) { - Some((_match_path, ref rel)) => { - let workdir = cow.workdir().to_path_buf(); - let upper = cow.upper_dir().to_path_buf(); - match cow.prepare_copy(rel) { - Ok(plan) => (Some(plan), workdir, upper, rel.clone()), - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC) - } - // A kernel object: its metadata is the kernel's to change, - // but it cannot be staged under a new name. - Err(crate::error::BranchError::NotOwned) => { - return match op { - CowWriteOp::Rename { .. } | CowWriteOp::Link { .. } => { - NotifAction::Errno(libc::EXDEV) - } - _ => NotifAction::Continue, - } - } - Err(_) => return NotifAction::Continue, - } - } - None => ( - None, - std::path::PathBuf::new(), - std::path::PathBuf::new(), - String::new(), - ), - } - }; - // Lock is released here - - // Phase 2: execute the file copy outside the lock (if needed) - if let Some(crate::cow::seccomp::CowCopyPlan::NeedsCopy { - upper, - lower: _lower, - file_size, - }) = copy_plan - { - if execute_deferred_copy( - cow_state, - copy_workdir, - copy_upper, - copy_rel, - upper, - file_size, - ) - .await - .is_none() - { - return NotifAction::Continue; - } - } - - // Phase 3: execute the operation under lock (ensure_cow_copy is now a no-op - // for the pre-copied file since it's already in upper) let mut st = cow_state.lock().await; let cow = match st.branch.as_mut() { Some(c) => c, None => return NotifAction::Continue, }; + op.remap_upper_paths(cow); + if let CowWriteOp::Rename { old_path, new_path } = &op { + // Refused before the copy, so a refused rename stages nothing. + if cow.matches(old_path) { + if let Err(errno) = cow.check_rename(old_path, new_path) { + return NotifAction::Errno(errno); + } + } + } + let need = op.copy_up(cow); + let names_entry = matches!(op, CowWriteOp::Rename { .. } | CowWriteOp::Link { .. }); + after_copy_up( + cow_state, + Caller::of(notif, notif_fd), + cow, + need, + move |e| refuse_write_copy(names_entry, e), + move |cow| apply_write(cow, &op), + ) +} - match op { +/// Run a write operation once any file it changes is in the upper. +fn apply_write(cow: &mut SeccompCowBranch, op: &CowWriteOp) -> NotifAction { + match *op { CowWriteOp::Unlink { ref path, is_dir } => { if !cow.matches(path) { return NotifAction::Continue; @@ -918,36 +1001,8 @@ pub(crate) async fn handle_cow_utimensat( return NotifAction::Continue; } - let virtual_cwd = current_virtual_cwd(processes, notif.pid).await; - let path = match read_path(notif, path_ptr, notif_fd) { - Some(p) => resolve_at_path_with_virtual(notif, dirfd, &p, virtual_cwd.as_deref()), - None => return NotifAction::Continue, - }; - - let (upper_path, upper_root, workdir_root) = { - let mut st = cow_state.lock().await; - let cow = match st.branch.as_mut() { - Some(c) => c, - None => return NotifAction::Continue, - }; - let upper_root = cow.upper_dir().to_path_buf(); - let workdir_root = cow.workdir().to_path_buf(); - let path = map_cow_upper_path(cow, &path); - if !cow.matches(&path) { - return NotifAction::Continue; - } - let p = match cow.handle_utimensat(&path) { - Ok(Some(p)) => p, - Ok(None) => return NotifAction::Continue, - Err(crate::error::BranchError::QuotaExceeded) => { - return NotifAction::Errno(libc::ENOSPC) - } - Err(_) => return NotifAction::Continue, - }; - (p, upper_root, workdir_root) - }; - - // Read times from child memory (2 x struct timespec = 32 bytes on x86_64) + // The kernel faults on the times before it looks at the path, and a plan + // given up afterwards would leave its copy reserved in the quota. let times = if times_ptr != 0 { match read_child_mem(notif_fd, notif.id, notif.pid, times_ptr, 32) { Ok(data) => { @@ -962,8 +1017,49 @@ pub(crate) async fn handle_cow_utimensat( } else { None }; + let follow = (flags & libc::AT_SYMLINK_NOFOLLOW) == 0; - let (root, rel) = match pick_root_rel(&upper_root, &workdir_root, &upper_path) { + let virtual_cwd = current_virtual_cwd(processes, notif.pid).await; + let path = match read_path(notif, path_ptr, notif_fd) { + Some(p) => resolve_at_path_with_virtual(notif, dirfd, &p, virtual_cwd.as_deref()), + None => return NotifAction::Continue, + }; + + let mut st = cow_state.lock().await; + let cow = match st.branch.as_mut() { + Some(c) => c, + None => return NotifAction::Continue, + }; + let path = map_cow_upper_path(cow, &path); + if !cow.matches(&path) { + return NotifAction::Continue; + } + let Some(rel) = cow.safe_rel(&path) else { + return NotifAction::Continue; + }; + let layers = Layers::of(cow); + let upper = layers.upper.join(&rel); + after_copy_up( + cow_state, + Caller::of(notif, notif_fd), + cow, + CopyUp::Entry(rel), + refuse_times_copy, + move |_| set_times(&layers, &upper, times, follow), + ) +} + +fn refuse_times_copy(e: BranchError) -> NotifAction { + refuse_write_copy(false, e) +} + +fn set_times( + layers: &Layers, + upper_path: &Path, + times: Option<[libc::timespec; 2]>, + follow: bool, +) -> NotifAction { + let (root, rel) = match pick_root_rel(&layers.upper, &layers.workdir, upper_path) { Ok(v) => v, Err(e) => return NotifAction::Errno(e), }; @@ -971,7 +1067,6 @@ pub(crate) async fn handle_cow_utimensat( .as_ref() .map(|t| t.as_ptr()) .unwrap_or(std::ptr::null()); - let follow = (flags & libc::AT_SYMLINK_NOFOLLOW) == 0; if let Err(e) = crate::sys::fs::utimensat_in_root(root, &rel, times_raw, follow) { return NotifAction::Errno(e); } diff --git a/crates/sandlock-core/src/cow/seccomp.rs b/crates/sandlock-core/src/cow/seccomp.rs index 81101d4d..9e720289 100644 --- a/crates/sandlock-core/src/cow/seccomp.rs +++ b/crates/sandlock-core/src/cow/seccomp.rs @@ -5,12 +5,13 @@ //! Works on any Linux 5.9+ kernel with seccomp user notification. use crate::result::{Entry, EntryKind}; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fs; use std::os::unix::ffi::OsStringExt; -use std::os::unix::fs::{FileTypeExt, MetadataExt}; +use std::os::unix::fs::{FileTypeExt, MetadataExt, OpenOptionsExt}; use std::os::unix::io::FromRawFd; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Condvar, Mutex}; use std::time::Duration; use crate::error::BranchError; @@ -32,7 +33,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`. -pub(super) fn branch_errno(e: BranchError) -> i32 { +pub(crate) fn branch_errno(e: BranchError) -> i32 { match e { BranchError::QuotaExceeded => libc::ENOSPC, BranchError::Denied => libc::EPERM, @@ -57,7 +58,83 @@ pub enum CowCopyPlan { upper: PathBuf, lower: PathBuf, file_size: u64, + ticket: CopyTicket, }, + /// Another caller is copying this file; plan again once it is done. + InFlight(CopyDone), +} + +#[derive(Debug, Default)] +struct CopySignal { + done: Mutex, + cv: Condvar, +} + +/// Held by whoever runs a copy-up `prepare_copy` handed out. Dropping it +/// wakes the waiters, so a copier that fails cannot strand them. +#[derive(Debug)] +pub struct CopyTicket(Arc); + +impl Drop for CopyTicket { + fn drop(&mut self) { + *self.0.done.lock().unwrap_or_else(|e| e.into_inner()) = true; + self.0.cv.notify_all(); + } +} + +/// Released when the copy-up it watches has been installed or given up. +#[derive(Debug, Clone)] +pub struct CopyDone(Arc); + +impl CopyDone { + /// Blocks, so only work off the notification loop may wait. + pub fn wait(&self) { + let mut done = self.0.done.lock().unwrap_or_else(|e| e.into_inner()); + while !*done { + done = self.0.cv.wait(done).unwrap_or_else(|e| e.into_inner()); + } + } + + fn is_done(&self) -> bool { + *self.0.done.lock().unwrap_or_else(|e| e.into_inner()) + } +} + +/// What `plan_copy` found at a path. +enum LowerCopy { + Ready(PathBuf), + Needed { upper: PathBuf, file_size: u64 }, +} + +/// A copy-up streamed to a private file beside the upper, not yet visible in +/// the merged view. Dropping it discards the file. +pub struct StagedCopy { + tmp: PathBuf, + digest: Option<[u8; 32]>, +} + +impl Drop for StagedCopy { + fn drop(&mut self) { + let _ = fs::remove_file(&self.tmp); + } +} + +/// What became of a staged copy-up. +#[derive(Debug, PartialEq, Eq)] +pub enum CopyInstall { + /// The upper holds the file, from this copy or one that got there first. + Ready, + /// The path was deleted meanwhile; the caller plans again. + Replan, +} + +/// What a write must find in the upper before it runs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CopyUp { + Nothing, + Entry(String), + /// A directory with everything below it in the lower. + Tree(String), } /// Plan returned by `prepare_open` — describes what I/O to do after releasing the lock. @@ -71,13 +148,9 @@ pub enum CowOpenPlan { Deleted, /// File already resolved (upper or lower) — open this path directly. Resolved(PathBuf), - /// Need to copy lower to upper, then open upper. - NeedsCopy { - upper: PathBuf, - lower: PathBuf, - file_size: u64, - rel_path: String, - }, + /// The file must be copied into the upper first, by this caller or by + /// the one already copying it. + Copy { rel_path: String, plan: CowCopyPlan }, /// Upper path ready (already exists in upper, or new file placeholder). UpperReady { upper: PathBuf }, } @@ -729,6 +802,8 @@ pub struct SeccompCowBranch { keep_if_abandoned: bool, max_disk_bytes: u64, disk_used: u64, + /// Copy-ups staged but not yet installed, with the bytes each reserved. + copying: HashMap, } /// The entry at `rel` under `root`, confined and without following @@ -982,9 +1057,16 @@ impl SeccompCowBranch { keep_if_abandoned: false, max_disk_bytes, disk_used: 0, + copying: HashMap::new(), }) } + /// Where copy-ups are staged: on the upper's filesystem, but outside the + /// merged view and anything a commit publishes. + pub fn staging_dir(&self) -> &Path { + &self.storage_dir + } + /// The upper directory where writes are stored. pub fn upper_dir(&self) -> &Path { &self.upper @@ -1135,27 +1217,56 @@ impl SeccompCowBranch { Ok(()) } - /// Recalculate `disk_used` by walking the upper directory. + /// Recalculate `disk_used` by walking the upper directory. Staged copies + /// are not in it yet, so their reservations are added back. fn recalc_disk_used(&mut self) { - self.disk_used = dir_size(&self.upper); + self.copying.retain(|_, (_, done)| !done.is_done()); + let staged: u64 = self.copying.values().map(|(bytes, _)| bytes).sum(); + self.disk_used = dir_size(&self.upper) + staged; } - /// Prepare a COW copy: update metadata (deleted set, quota reservation) - /// and handle small items (symlinks, dirs) immediately, but defer large - /// file copies to the caller. This is the shared core used by both - /// `ensure_cow_copy` (synchronous) and the async two-phase dispatch. + /// `plan_copy` for a caller that copies outside the branch lock. A file + /// someone is already copying is not copied twice: the caller waits and + /// plans again. The ticket goes back to `finish_copy`. pub fn prepare_copy(&mut self, rel_path: &str) -> Result { + if let Some((_, done)) = self.copying.get(rel_path) { + if !done.is_done() { + return Ok(CowCopyPlan::InFlight(done.clone())); + } + self.copying.remove(rel_path); + } + Ok(match self.plan_copy(rel_path)? { + LowerCopy::Ready(upper) => CowCopyPlan::Ready(upper), + LowerCopy::Needed { upper, file_size } => { + let signal = Arc::new(CopySignal::default()); + self.copying.insert( + rel_path.to_string(), + (file_size, CopyDone(Arc::clone(&signal))), + ); + CowCopyPlan::NeedsCopy { + upper, + lower: self.workdir.join(rel_path), + file_size, + ticket: CopyTicket(signal), + } + } + }) + } + + /// Plan a COW copy: update metadata (deleted set, quota reservation) + /// and handle small items (symlinks, dirs) immediately, but leave the + /// potentially large file copy to the caller. + fn plan_copy(&mut self, rel_path: &str) -> Result { self.has_changes = true; self.touch_parents(rel_path); self.touch(rel_path); let upper_file = self.upper.join(rel_path); - let lower_file = self.workdir.join(rel_path); // Already materialized in upper? Confined lstat succeeds for any // existing entry (including a dangling symlink). if crate::sys::fs::statat_in_root(&self.upper, rel_path, false).is_ok() { - return Ok(CowCopyPlan::Ready(upper_file)); + return Ok(LowerCopy::Ready(upper_file)); } if let Some(p) = parent_rel(rel_path) { @@ -1168,7 +1279,7 @@ impl SeccompCowBranch { // in the merged view (and what makes a re-created directory opaque). if self.deleted.covers(rel_path) { self.check_quota(0)?; - return Ok(CowCopyPlan::Ready(upper_file)); + return Ok(LowerCopy::Ready(upper_file)); } // Classify the lower entry confined to the workdir root, so a symlinked @@ -1177,14 +1288,14 @@ impl SeccompCowBranch { let st = match crate::sys::fs::statat_in_root(&self.workdir, rel_path, false) { Ok(st) => st, // Absent or confined-out: treat as a new file created in upper. - // EACCES gets the same disposition as execute_copy's source-open + // EACCES gets the same disposition as stage_copy's source-open // gives it: the supervisor cannot see inside the lower directory // (e.g. /root under a workdir of "/"), so the write proceeds on // an empty upper file rather than falling through to a real // permission error the virtualized child was promised not to hit. Err(libc::ENOENT) | Err(libc::EACCES) => { self.check_quota(0)?; - return Ok(CowCopyPlan::Ready(upper_file)); + return Ok(LowerCopy::Ready(upper_file)); } Err(e) => { return Err(BranchError::Io { @@ -1212,7 +1323,7 @@ impl SeccompCowBranch { source: std::io::Error::from_raw_os_error(e), })?; self.disk_used += 256; - return Ok(CowCopyPlan::Ready(upper_file)); + return Ok(LowerCopy::Ready(upper_file)); } // Directory — create immediately (no data copy) @@ -1226,7 +1337,7 @@ impl SeccompCowBranch { )?; 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)); + return Ok(LowerCopy::Ready(upper_file)); } if Self::is_kernel_object_kind(kind) { @@ -1235,34 +1346,33 @@ impl SeccompCowBranch { // Regular file — defer the potentially expensive copy. Size comes from // the confined lstat, so the quota reservation matches the file - // execute_copy will actually read. + // stage_copy will actually read. let file_size = st.st_size as u64; self.check_quota(file_size)?; self.disk_used += file_size; - Ok(CowCopyPlan::NeedsCopy { + Ok(LowerCopy::Needed { upper: upper_file, - lower: lower_file, file_size, }) } - /// Execute a file copy synchronously. Used by `ensure_cow_copy` and the - /// async dispatch (via `spawn_blocking`). - pub fn execute_copy( + /// Stream the lower file at `rel` into a private file in `staging_dir`. + /// Nothing appears in the upper until `install_staged`, so a copy running + /// beside other handlers never shows them a half-written file. + pub fn stage_copy( workdir_root: &Path, - upper_root: &Path, + staging_dir: &Path, rel: &str, - ) -> Result, std::io::Error> { - let create_dest = || -> Result { - let fd = crate::sys::fs::openat2_in_root( - upper_root, - rel, - libc::O_WRONLY | libc::O_CREAT | libc::O_TRUNC | libc::O_NOFOLLOW | libc::O_CLOEXEC, - 0o600, - ) - .map_err(std::io::Error::from_raw_os_error)?; - Ok(unsafe { fs::File::from_raw_fd(fd) }) - }; + ) -> Result { + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let tmp = staging_dir.join(format!(".copy-{}-{}", std::process::id(), n)); + let dest = fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&tmp)?; + let mut staged = StagedCopy { tmp, digest: None }; // Read the lower source confined to the workdir root: a symlink or // `..` in `rel` cannot escape the tree (issue #112). @@ -1276,10 +1386,7 @@ impl SeccompCowBranch { // Unreadable (EACCES) or confined-out / absent (ENOENT): give the // child an empty COW file so writes proceed, never leaking the // escape target. - Err(libc::EACCES) | Err(libc::ENOENT) => { - create_dest()?; - return Ok(None); - } + Err(libc::EACCES) | Err(libc::ENOENT) => return Ok(staged), // On a kernel without openat2 (ENOSYS) the copy fails and the caller // rolls back / returns Continue; the child then hits Landlock, which // is the backstop. Do not "fix" this with an unconfined fs::copy. @@ -1293,105 +1400,183 @@ impl SeccompCowBranch { // #158). O_NONBLOCK above makes the FIFO open itself return instead // of waiting for a writer; on a regular file it is a no-op for reads. if !src.metadata()?.file_type().is_file() { - create_dest()?; - return Ok(None); + return Ok(staged); } - let mut dst = Sha256Writer::new(create_dest()?); + let mut dst = Sha256Writer::new(dest); std::io::copy(&mut src, &mut dst)?; if let Ok(meta) = src.metadata() { let _ = dst.inner.set_permissions(meta.permissions()); } - Ok(Some(dst.finish())) - } - - /// Ensure a COW copy exists in upper (synchronous). Returns the upper path. - /// For callers that don't need async two-phase behavior. - pub fn ensure_cow_copy(&mut self, rel_path: &str) -> Result { - match self.prepare_copy(rel_path)? { - CowCopyPlan::Ready(upper) => Ok(upper), - CowCopyPlan::NeedsCopy { - upper, - lower: _lower, - file_size, - } => match Self::execute_copy(&self.workdir, &self.upper, rel_path) { - Ok(digest) => { - if let Some(d) = digest { - self.record_digest(rel_path, d); + staged.digest = Some(dst.finish()); + Ok(staged) + } + + /// Move a staged copy into the upper. Never replaces: a copy that got + /// there first may already carry a writer's data. + fn install_staged( + &mut self, + rel_path: &str, + file_size: u64, + staged: Result, + ) -> Result { + let staged = match staged { + Ok(staged) => staged, + Err(e) => { + self.rollback_copy(file_size); + return Err(e); + } + }; + // Installing a path deleted while the copy ran would bring it back. + let result = if self.deleted.covers(rel_path) { + Ok(CopyInstall::Replan) + } else { + match crate::sys::fs::install_in_root(&staged.tmp, &self.upper, rel_path) { + Ok(()) => { + if let Some(digest) = staged.digest { + self.record_digest(rel_path, digest); } - Ok(upper) + return Ok(CopyInstall::Ready); } + Err(libc::EEXIST) => Ok(CopyInstall::Ready), + Err(e) => Err(std::io::Error::from_raw_os_error(e)), + } + }; + self.rollback_copy(file_size); + result + } + + /// Settle a copy `prepare_copy` handed out. The ticket drops last, so the + /// waiters it wakes find the outcome in place. + pub fn finish_copy( + &mut self, + rel_path: &str, + file_size: u64, + _ticket: CopyTicket, + staged: Result, + ) -> Result { + self.copying.remove(rel_path); + self.install_staged(rel_path, file_size, staged) + } + + /// `prepare_copy` for every entry in `rels`, all or nothing: on a refusal + /// the entries planned so far are given up and removed again, so a write + /// the branch refuses leaves no trace. Only the copies still to run come + /// back. + pub fn prepare_copies( + &mut self, + rels: &[String], + ) -> Result, BranchError> { + let mut created = Vec::new(); + let mut plans = Vec::new(); + for rel in rels { + // A whiteouted child of a staged tree stays deleted. + if self.deleted.covers(rel) && !self.upper_has(rel) && rel != &rels[0] { + continue; + } + let had = self.upper_has(rel); + match self.prepare_copy(rel) { + Ok(CowCopyPlan::Ready(_)) => { + if !had && self.upper_has(rel) { + created.push(rel.clone()); + } + } + Ok(plan) => plans.push((rel.clone(), plan)), Err(e) => { - self.rollback_copy(file_size); - Err(BranchError::Operation(format!("copy: {}", e))) + self.abandon_copies(plans); + self.remove_created(&created); + return Err(e); } - }, + } } + Ok(plans) } - /// Copy a lower tree into the upper, entry by entry, so a directory - /// rename can be staged in the branch without losing the contents - /// (issue #160). Each entry goes through the same confined, - /// quota-accounted single-entry copy-up, so symlinks are copied verbatim - /// and never followed (issue #112). - /// - /// Staging is all-or-nothing: a mid-tree failure (EACCES, EIO, quota) - /// must propagate rather than truncate the copy, because the caller - /// whiteouts the source afterwards and untraversed children would be - /// silently lost at commit. On failure every upper entry this staging - /// created is removed again and the quota re-derived. - fn copy_up_tree(&mut self, rel: &str) -> Result<(), i32> { - let mut created: Vec = Vec::new(); - let result = self.stage_tree(rel, &mut created); - if result.is_err() { - for c in created.iter().rev() { - let is_dir = crate::sys::fs::statat_in_root(&self.upper, c, false) - .map(|st| st.st_mode & libc::S_IFMT == libc::S_IFDIR) - .unwrap_or(false); - if is_dir { - let _ = crate::sys::fs::remove_dir_all_in_root(&self.upper, c); - } else { - let _ = crate::sys::fs::unlinkat_in_root(&self.upper, c, false); - } + /// Give back copies planned but never run. + pub fn abandon_copies(&mut self, plans: impl IntoIterator) { + for (rel, plan) in plans { + if let CowCopyPlan::NeedsCopy { + file_size, ticket, .. + } = plan + { + let cancelled = Err(std::io::Error::from_raw_os_error(libc::ECANCELED)); + let _ = self.finish_copy(&rel, file_size, ticket, cancelled); } - self.recalc_disk_used(); } - result } - /// The traversal half of `copy_up_tree`: an explicit worklist (child - /// directory depth must not become supervisor stack depth) that records - /// every upper entry it creates into `created` for rollback. - fn stage_tree(&mut self, root: &str, created: &mut Vec) -> Result<(), i32> { - let mut queue = std::collections::VecDeque::new(); - queue.push_back(root.to_string()); - while let Some(rel) = queue.pop_front() { - let was_in_upper = self.upper_has(&rel); - self.ensure_cow_copy(&rel).map_err(branch_errno)?; - if !was_in_upper && self.upper_has(&rel) { - created.push(rel.clone()); + fn remove_created(&mut self, created: &[String]) { + if created.is_empty() { + return; + } + for c in created.iter().rev() { + let is_dir = crate::sys::fs::statat_in_root(&self.upper, c, false) + .map(|st| st.st_mode & libc::S_IFMT == libc::S_IFDIR) + .unwrap_or(false); + if is_dir { + let _ = crate::sys::fs::remove_dir_all_in_root(&self.upper, c); + } else { + let _ = crate::sys::fs::unlinkat_in_root(&self.upper, c, false); + } + } + self.recalc_disk_used(); + } + + /// The upper copy of `rel`, which the dispatcher made before the write. + fn copied(&mut self, rel: &str) -> Result { + match self.plan_copy(rel)? { + LowerCopy::Ready(upper) => Ok(upper), + LowerCopy::Needed { file_size, .. } => { + self.rollback_copy(file_size); + Err(BranchError::Operation(format!("{rel} was not copied up"))) } - let st = match crate::sys::fs::statat_in_root(&self.workdir, &rel, false) { + } + } + + /// `root` and everything below it in the lower, parents first. It reads + /// only the lower, which the branch never writes, so it needs no lock. + /// An explicit worklist: child directory depth must not become supervisor + /// stack depth. + pub fn lower_tree(workdir: &Path, root: &str) -> Result, i32> { + let mut tree = Vec::new(); + let mut queue = std::collections::VecDeque::from([root.to_string()]); + while let Some(rel) = queue.pop_front() { + let st = crate::sys::fs::statat_in_root(workdir, &rel, false); + tree.push(rel.clone()); + let st = match st { Ok(st) => st, - // Upper-only entry: nothing below it in the lower to stage. + // Upper-only entry: nothing below it in the lower. Err(libc::ENOENT) => continue, Err(e) => return Err(e), }; if st.st_mode & libc::S_IFMT != libc::S_IFDIR { continue; } - let rd = fs::read_dir(self.workdir.join(&rel)) - .map_err(|e| e.raw_os_error().unwrap_or(libc::EIO))?; - for entry in rd { - let entry = entry.map_err(|e| e.raw_os_error().unwrap_or(libc::EIO))?; - let name = entry.file_name().to_string_lossy().into_owned(); - let child = format!("{}/{}", rel, name); - if self.deleted.covers(&child) && !self.upper_has(&child) { - continue; - } - queue.push_back(child); + let io = |e: std::io::Error| e.raw_os_error().unwrap_or(libc::EIO); + for entry in fs::read_dir(workdir.join(&rel)).map_err(io)? { + let name = entry.map_err(io)?.file_name(); + queue.push_back(format!("{}/{}", rel, name.to_string_lossy())); } } - Ok(()) + Ok(tree) + } + + /// What a rename of `rel` must copy up: a directory takes its lower tree + /// along, so the rename can be staged without losing the contents + /// (issue #160). + pub fn rename_copy_up(&self, rel: String) -> CopyUp { + match self.merged_entry_is_dir(&rel) { + Some(true) => CopyUp::Tree(rel), + _ => CopyUp::Entry(rel), + } + } + + /// What a hard link from `rel` must copy up. linkat on a directory is the + /// kernel's EPERM to give; staging it would leave a meaningless empty dir. + pub fn link_copy_up(&self, rel: String) -> CopyUp { + match self.merged_entry_is_dir(&rel) { + Some(true) => CopyUp::Nothing, + _ => CopyUp::Entry(rel), + } } /// Resolve a read path: upper if modified, else lower. @@ -1406,82 +1591,6 @@ impl SeccompCowBranch { // ---- Syscall handlers (called by cow::dispatch) ---- - /// Handle openat: resolve to upper or lower path. - /// - /// Returns `Err(QuotaExceeded)` when the write would exceed `max_disk`. - /// - /// When a quota is active and the open is a write, resync `disk_used` - /// from the real upper directory first. This catches growth from - /// `write()` syscalls on previously injected fds (which bypass the - /// seccomp supervisor) and prevents new opens once the quota is - /// exhausted. - pub fn handle_open(&mut self, path: &str, flags: u64) -> Result, BranchError> { - let rel = match self.safe_rel(path) { - Some(r) => r, - None => return Ok(None), - }; - if flags & O_DIRECTORY != 0 { - // A whiteouted directory must not fall through to the kernel: - // the lower directory still exists, so opendir would hand the - // child a live fd (and fstat its inode) where the merged answer - // is ENOENT. - if self.is_deleted(&rel) { - return Err(BranchError::Deleted); - } - return Ok(None); - } - - let is_write = flags & WRITE_FLAGS != 0; - - // Resync quota accounting before any write open so that bytes - // written through previously injected fds are counted. - if is_write && self.max_disk_bytes > 0 { - self.recalc_disk_used(); - self.check_quota(0)?; - } - - if self.is_deleted(&rel) { - if flags & O_CREAT != 0 { - return self.ensure_cow_copy(&rel).map(Some); - } - // Whiteout: the lower file still physically exists with its - // pre-delete bytes. Surface the deletion so the caller returns - // ENOENT rather than falling through to the lower file — matching - // the async prepare_open (CowOpenPlan::Deleted) and the stat/access - // handlers. - return Err(BranchError::Deleted); - } - - if self.is_kernel_object(&rel) { - return Ok(None); - } - - // O_EXCL: fail if file already exists (in upper or lower) - if flags & O_CREAT != 0 && flags & O_EXCL != 0 { - // Confined existence check: a symlinked parent component must not - // let this probe follow out of the tree, which would turn O_EXCL - // into a host-file existence oracle (issue #112). - if crate::sys::fs::statat_in_root(&self.upper, &rel, false).is_ok() - || crate::sys::fs::statat_in_root(&self.workdir, &rel, false).is_ok() - { - return Err(BranchError::Exists); - } - // File truly doesn't exist — create in upper - return self.ensure_cow_copy(&rel).map(Some); - } - - if is_write { - self.ensure_cow_copy(&rel).map(Some) - } else { - let resolved = self.resolve_read(&rel); - if resolved.exists() || resolved.is_symlink() { - Ok(Some(resolved)) - } else { - Ok(None) - } - } - } - /// Prepare an open without doing the file copy. /// /// Returns a plan that describes what I/O needs to happen after the lock @@ -1562,19 +1671,13 @@ impl SeccompCowBranch { /// Prepare a COW copy for openat — wraps `prepare_copy` into `CowOpenPlan`. fn prepare_cow_copy(&mut self, rel_path: &str) -> Result { - match self.prepare_copy(rel_path)? { - CowCopyPlan::Ready(upper) => Ok(CowOpenPlan::UpperReady { upper }), - CowCopyPlan::NeedsCopy { - upper, - lower, - file_size, - } => Ok(CowOpenPlan::NeedsCopy { - upper, - lower, - file_size, + Ok(match self.prepare_copy(rel_path)? { + CowCopyPlan::Ready(upper) => CowOpenPlan::UpperReady { upper }, + plan => CowOpenPlan::Copy { rel_path: rel_path.to_string(), - }), - } + plan, + }, + }) } /// Roll back quota reservation if the copy failed. @@ -1730,6 +1833,37 @@ impl SeccompCowBranch { .map(|st| st.st_mode & libc::S_IFMT == libc::S_IFDIR) } + /// The merged view's refusal of a rename, if any. + pub fn check_rename(&self, old_path: &str, new_path: &str) -> Result<(), i32> { + match (self.safe_rel(old_path), self.safe_rel(new_path)) { + (Some(old_rel), Some(new_rel)) => self.check_rename_rel(&old_rel, &new_rel), + _ => Ok(()), + } + } + + fn check_rename_rel(&self, old_rel: &str, new_rel: &str) -> Result<(), i32> { + let src_is_dir = match self.merged_entry_is_dir(old_rel) { + Some(d) => d, + None => return Err(libc::ENOENT), + }; + // Destination semantics come from the merged view: renaming onto an + // existing directory must replace it or refuse, never publish the + // union of the renamed tree and the pre-existing one (issue #160 + // review). The kernel cannot give these answers because it sees only + // one layer at a time. + if let Some(dest_is_dir) = self.merged_entry_is_dir(new_rel) { + match (src_is_dir, dest_is_dir) { + (false, true) => return Err(libc::EISDIR), + (true, false) => return Err(libc::ENOTDIR), + (true, true) if !self.list_merged_dir(new_rel).is_empty() => { + return Err(libc::ENOTEMPTY) + } + _ => {} + } + } + Ok(()) + } + /// Handle rename. /// /// Returns `Ok(true)` on success, `Ok(false)` when the branch does not @@ -1748,26 +1882,12 @@ impl SeccompCowBranch { }; self.touch_parents(&new_rel); self.touch(&new_rel); - let src_is_dir = match self.merged_entry_is_dir(&old_rel) { - Some(d) => d, - None => return Err(libc::ENOENT), - }; - // Destination semantics come from the merged view: renaming onto an - // existing directory must replace it or refuse, never publish the - // union of the renamed tree and the pre-existing one (issue #160 - // review). The kernel cannot give these answers because it sees only - // one layer at a time. - if let Some(dest_is_dir) = self.merged_entry_is_dir(&new_rel) { - match (src_is_dir, dest_is_dir) { - (false, true) => return Err(libc::EISDIR), - (true, false) => return Err(libc::ENOTDIR), - (true, true) if !self.list_merged_dir(&new_rel).is_empty() => { - return Err(libc::ENOTEMPTY) - } - _ => {} - } + self.check_rename_rel(&old_rel, &new_rel)?; + // The dispatcher copied the tree; a missing source would let the + // rename lose it. + if !self.upper_has(&old_rel) { + return Err(libc::EIO); } - self.copy_up_tree(&old_rel)?; if let Some(p) = parent_rel(&new_rel) { let _ = crate::sys::fs::mkdirp_in_root(&self.upper, p, 0o755); } @@ -1852,7 +1972,7 @@ impl SeccompCowBranch { if src_is_dir { return Ok(false); } - let _ = self.ensure_cow_copy(&old_rel)?; + self.copied(&old_rel)?; if let Some(p) = parent_rel(&new_rel) { let _ = crate::sys::fs::mkdirp_in_root(&self.upper, p, 0o755); } @@ -1867,7 +1987,7 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; - let _ = self.ensure_cow_copy(&rel)?; + self.copied(&rel)?; Ok(crate::sys::fs::chmod_in_root(&self.upper, &rel, mode).is_ok()) } @@ -1879,7 +1999,7 @@ impl SeccompCowBranch { Some(r) => r, None => return Ok(false), }; - let _ = self.ensure_cow_copy(&rel)?; + self.copied(&rel)?; // Best-effort: try the real chown but succeed either way — the // supervisor typically lacks CAP_CHOWN so this will fail, but // in COW/dry-run mode the ownership doesn't matter. @@ -1896,7 +2016,7 @@ impl SeccompCowBranch { None => return Ok(false), }; let new_len = length as u64; - let _ = self.ensure_cow_copy(&rel)?; + self.copied(&rel)?; let old_len = crate::sys::fs::statat_in_root(&self.upper, &rel, true) .map(|st| st.st_size as u64) .unwrap_or(0); @@ -1924,17 +2044,6 @@ impl SeccompCowBranch { Ok(ok) } - /// Handle utimensat — resolve to upper, return the upper path for the - /// caller to call utimensat on. - pub fn handle_utimensat(&mut self, path: &str) -> Result, BranchError> { - let rel = match self.safe_rel(path) { - Some(r) => r, - None => return Ok(None), - }; - let upper = self.ensure_cow_copy(&rel)?; - Ok(Some(upper)) - } - /// Handle readlink. pub fn handle_readlink(&self, path: &str) -> Option { let rel = self.safe_rel(path)?; @@ -2668,7 +2777,7 @@ impl SeccompCowBranch { /// leaves the page cache intact and the old unsynced write was already /// enough. These fsyncs change behaviour only on power loss or a kernel /// panic — and there they make the RECORD durable while the upper it names - /// is written by `ensure_cow_copy` and by the child with no sync at all, + /// is written by the copy-up and by the child with no sync at all, /// and the default storage base is `$XDG_RUNTIME_DIR` or `$TMPDIR`, i.e. /// tmpfs, where the whole tree is gone anyway. They are meaningful with an /// explicit disk-backed `fs_storage`, and inert without one. @@ -2796,9 +2905,103 @@ impl Drop for SeccompCowBranch { } } +/// Shorthands that run on the caller's thread what the dispatcher runs off +/// the loop. +#[cfg(test)] +impl SeccompCowBranch { + pub fn copy_up_now(&mut self, need: CopyUp) -> Result<(), BranchError> { + let rels = match need { + CopyUp::Nothing => return Ok(()), + CopyUp::Entry(rel) => vec![rel], + CopyUp::Tree(root) => { + Self::lower_tree(&self.workdir, &root).map_err(|e| BranchError::Io { + operation: "walk lower", + source: std::io::Error::from_raw_os_error(e), + })? + } + }; + for (rel, plan) in self.prepare_copies(&rels)? { + if let CowCopyPlan::NeedsCopy { + file_size, ticket, .. + } = plan + { + let staged = Self::stage_copy(&self.workdir, &self.storage_dir, &rel); + self.finish_copy(&rel, file_size, ticket, staged) + .map_err(|source| BranchError::Io { + operation: "copy", + source, + })?; + } + } + Ok(()) + } + + pub fn ensure_cow_copy(&mut self, rel: &str) -> Result { + self.copy_up_now(CopyUp::Entry(rel.to_string()))?; + Ok(self.upper.join(rel)) + } + + pub fn handle_open(&mut self, path: &str, flags: u64) -> Result, BranchError> { + match self.prepare_open(path, flags)? { + CowOpenPlan::Skip => Ok(None), + CowOpenPlan::Deleted => Err(BranchError::Deleted), + CowOpenPlan::Resolved(p) | CowOpenPlan::UpperReady { upper: p } => Ok(Some(p)), + CowOpenPlan::Copy { rel_path, plan } => { + self.abandon_copies([(rel_path.clone(), plan)]); + self.ensure_cow_copy(&rel_path).map(Some) + } + } + } +} + #[cfg(test)] mod tests { use super::*; + + /// Write handlers run the way the dispatcher runs them: copy-ups first. + fn chmod(b: &mut SeccompCowBranch, path: &str, mode: u32) -> Result { + copy_entry(b, path)?; + b.handle_chmod(path, mode) + } + + fn chown( + b: &mut SeccompCowBranch, + path: &str, + uid: u32, + gid: u32, + ) -> Result { + copy_entry(b, path)?; + b.handle_chown(path, uid, gid) + } + + fn truncate(b: &mut SeccompCowBranch, path: &str, length: i64) -> Result { + copy_entry(b, path)?; + b.handle_truncate(path, length) + } + + fn link(b: &mut SeccompCowBranch, old: &str, new: &str) -> Result { + if let Some(rel) = b.safe_rel(old) { + let need = b.link_copy_up(rel); + b.copy_up_now(need)?; + } + b.handle_link(old, new) + } + + fn rename(b: &mut SeccompCowBranch, old: &str, new: &str) -> Result { + b.check_rename(old, new)?; + if let Some(rel) = b.safe_rel(old) { + let need = b.rename_copy_up(rel); + b.copy_up_now(need).map_err(branch_errno)?; + } + b.handle_rename(old, new) + } + + fn copy_entry(b: &mut SeccompCowBranch, path: &str) -> Result<(), BranchError> { + match b.safe_rel(path) { + Some(rel) => b.copy_up_now(CopyUp::Entry(rel)), + None => Ok(()), + } + } use std::fs; #[test] @@ -4268,7 +4471,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 4).unwrap(); let old = abs(&branch, "existing.txt"); let new = abs(&branch, "renamed.txt"); - let err = branch.handle_rename(&old, &new).unwrap_err(); + let err = rename(&mut branch, &old, &new).unwrap_err(); assert_eq!(err, libc::ENOSPC); } @@ -4278,7 +4481,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 4).unwrap(); let old = abs(&branch, "existing.txt"); let new = abs(&branch, "hardlink.txt"); - let err = branch.handle_link(&old, &new).unwrap_err(); + let err = link(&mut branch, &old, &new).unwrap_err(); assert!(matches!(err, BranchError::QuotaExceeded)); } @@ -4288,7 +4491,7 @@ mod tests { // chmod triggers ensure_cow_copy. let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 4).unwrap(); let path = abs(&branch, "existing.txt"); - let err = branch.handle_chmod(&path, 0o644).unwrap_err(); + let err = chmod(&mut branch, &path, 0o644).unwrap_err(); assert!(matches!(err, BranchError::QuotaExceeded)); } @@ -4297,7 +4500,7 @@ mod tests { let (workdir, storage) = setup_workdir(); let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 4).unwrap(); let path = abs(&branch, "existing.txt"); - let err = branch.handle_chown(&path, 1000, 1000).unwrap_err(); + let err = chown(&mut branch, &path, 1000, 1000).unwrap_err(); assert!(matches!(err, BranchError::QuotaExceeded)); } @@ -4310,7 +4513,7 @@ mod tests { let path = abs(&branch, "existing.txt"); // cow copy uses 5 bytes (5 of 10 used). // Truncating to 20 bytes needs 15 more — exceeds remaining 5. - let err = branch.handle_truncate(&path, 20).unwrap_err(); + let err = truncate(&mut branch, &path, 20).unwrap_err(); assert!(matches!(err, BranchError::QuotaExceeded)); } @@ -4321,7 +4524,7 @@ mod tests { SeccompCowBranch::create(workdir.path(), Some(storage.path()), 10).unwrap(); let path = abs(&branch, "existing.txt"); // Truncate to 2 bytes — cow copy (5) + shrink is fine. - assert!(matches!(branch.handle_truncate(&path, 2), Ok(true))); + assert!(matches!(truncate(&mut branch, &path, 2), Ok(true))); // disk_used should now be 2, not 5. assert_eq!(branch.disk_used, 2); } @@ -4544,7 +4747,13 @@ mod tests { let path = abs(&branch, "existing.txt"); // O_WRONLY let plan = branch.prepare_open(&path, 0o1).unwrap(); - assert!(matches!(plan, CowOpenPlan::NeedsCopy { .. })); + assert!(matches!( + plan, + CowOpenPlan::Copy { + plan: CowCopyPlan::NeedsCopy { .. }, + .. + } + )); } #[test] @@ -4584,7 +4793,137 @@ mod tests { SeccompCowBranch::create(workdir.path(), Some(storage.path()), 100).unwrap(); let path = abs(&branch, "existing.txt"); let plan = branch.prepare_open(&path, 0o1).unwrap(); - assert!(matches!(plan, CowOpenPlan::NeedsCopy { file_size: 5, .. })); + assert!(matches!( + plan, + CowOpenPlan::Copy { + plan: CowCopyPlan::NeedsCopy { file_size: 5, .. }, + .. + } + )); + assert_eq!(branch.disk_used, 5); + } + + /// Run a copy-up of `rel` the way the off-loop dispatch does. + fn staged_copy( + branch: &mut SeccompCowBranch, + rel: &str, + ) -> (u64, CopyTicket, Result) { + let CowCopyPlan::NeedsCopy { + file_size, ticket, .. + } = branch.prepare_copy(rel).unwrap() + else { + panic!("expected a copy to be needed"); + }; + let staged = SeccompCowBranch::stage_copy(branch.workdir(), branch.staging_dir(), rel); + (file_size, ticket, staged) + } + + fn staged_files(branch: &SeccompCowBranch) -> usize { + fs::read_dir(branch.staging_dir()) + .unwrap() + .flatten() + .filter(|e| e.file_name().to_string_lossy().starts_with(".copy-")) + .count() + } + + #[test] + fn a_staged_copy_is_invisible_until_installed() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let (size, ticket, staged) = staged_copy(&mut branch, "existing.txt"); + assert!(!branch.upper_dir().join("existing.txt").exists()); + assert_eq!( + branch + .finish_copy("existing.txt", size, ticket, staged) + .unwrap(), + CopyInstall::Ready + ); + assert_eq!( + fs::read(branch.upper_dir().join("existing.txt")).unwrap(), + b"hello" + ); + assert_eq!(staged_files(&branch), 0); + } + + #[test] + fn a_second_copier_waits_until_the_first_finishes_or_gives_up() { + let (workdir, storage) = setup_workdir(); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let (size, ticket, staged) = staged_copy(&mut branch, "existing.txt"); + let CowCopyPlan::InFlight(done) = branch.prepare_copy("existing.txt").unwrap() else { + panic!("the second copier must wait"); + }; + assert!(!done.is_done()); + branch + .finish_copy("existing.txt", size, ticket, staged) + .unwrap(); + done.wait(); + assert!(matches!( + branch.prepare_copy("existing.txt").unwrap(), + CowCopyPlan::Ready(_) + )); + + let (_, ticket, _) = staged_copy(&mut branch, "subdir/nested.txt"); + drop(ticket); + assert!(matches!( + branch.prepare_copy("subdir/nested.txt").unwrap(), + CowCopyPlan::NeedsCopy { .. } + )); + } + + #[test] + fn a_copy_finished_after_the_path_was_deleted_is_not_installed() { + let (workdir, storage) = setup_workdir(); + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 100).unwrap(); + let (size, ticket, staged) = staged_copy(&mut branch, "existing.txt"); + let path = abs(&branch, "existing.txt"); + assert_eq!(branch.handle_unlink(&path, false), Ok(true)); + assert_eq!( + branch + .finish_copy("existing.txt", size, ticket, staged) + .unwrap(), + CopyInstall::Replan + ); + assert!(branch.is_deleted("existing.txt")); + assert_eq!(branch.disk_used, 0, "the reservation must be rolled back"); + assert_eq!(staged_files(&branch), 0); + } + + #[test] + fn a_copy_that_lost_the_race_keeps_the_winners_file() { + let (workdir, storage) = setup_workdir(); + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 100).unwrap(); + let (size, ticket, staged) = staged_copy(&mut branch, "existing.txt"); + let upper = branch.upper_dir().join("existing.txt"); + fs::write(&upper, "written").unwrap(); + assert_eq!( + branch + .finish_copy("existing.txt", size, ticket, staged) + .unwrap(), + CopyInstall::Ready + ); + assert_eq!(fs::read(&upper).unwrap(), b"written"); + assert_eq!( + branch.disk_used, 0, + "the losing copy gives its reservation back" + ); + assert_eq!(staged_files(&branch), 0); + } + + #[test] + fn a_quota_resync_keeps_the_reservation_of_a_staged_copy() { + let (workdir, storage) = setup_workdir(); + let mut branch = + SeccompCowBranch::create(workdir.path(), Some(storage.path()), 100).unwrap(); + let (size, ticket, staged) = staged_copy(&mut branch, "existing.txt"); + branch.recalc_disk_used(); + assert_eq!(branch.disk_used, size); + branch + .finish_copy("existing.txt", size, ticket, staged) + .unwrap(); + branch.recalc_disk_used(); assert_eq!(branch.disk_used, 5); } @@ -5054,12 +5393,12 @@ mod tests { fs::write(wd.join("d/sub/deep.txt"), "DEEP").unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); - assert!(branch - .handle_rename( - &format!("{}/d", wd.display()), - &format!("{}/d2", wd.display()) - ) - .unwrap()); + assert!(rename( + &mut branch, + &format!("{}/d", wd.display()), + &format!("{}/d2", wd.display()) + ) + .unwrap()); // Merged view before commit: d hidden, d2 holds the tree. assert!(branch.is_deleted("d")); @@ -5094,12 +5433,12 @@ mod tests { branch.handle_unlink(&format!("{}/d/gone.txt", wd.display()), false), Ok(true) ); - assert!(branch - .handle_rename( - &format!("{}/d", wd.display()), - &format!("{}/d2", wd.display()) - ) - .unwrap()); + assert!(rename( + &mut branch, + &format!("{}/d", wd.display()), + &format!("{}/d2", wd.display()) + ) + .unwrap()); branch.commit().unwrap(); assert!(wd.join("d2/keep.txt").exists()); assert!(!wd.join("d2/gone.txt").exists()); @@ -5112,7 +5451,8 @@ mod tests { let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); branch.mark_deleted("existing.txt"); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/existing.txt", wd.display()), &format!("{}/moved.txt", wd.display()) ), @@ -5133,7 +5473,8 @@ mod tests { fs::write(wd.join("b/b1"), "B").unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/a", wd.display()), &format!("{}/b", wd.display()) ), @@ -5157,7 +5498,8 @@ mod tests { fs::create_dir(wd.join("b")).unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/a", wd.display()), &format!("{}/b", wd.display()) ), @@ -5179,7 +5521,8 @@ mod tests { fs::write(wd.join("target.txt"), "OLD").unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/existing.txt", wd.display()), &format!("{}/target.txt", wd.display()) ), @@ -5205,7 +5548,8 @@ mod tests { let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/existing.txt", wd.display()), &format!("{}/dest", wd.display()) ), @@ -5233,14 +5577,16 @@ mod tests { let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); // File onto directory: EISDIR. Directory onto file: ENOTDIR. assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/existing.txt", wd.display()), &format!("{}/d", wd.display()) ), Err(libc::EISDIR) ); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/d", wd.display()), &format!("{}/existing.txt", wd.display()) ), @@ -5266,7 +5612,8 @@ mod tests { Ok(true) ); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/existing.txt", wd.display()), &format!("{}/b", wd.display()) ), @@ -5285,7 +5632,8 @@ mod tests { let wd = workdir.path().canonicalize().unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/nope", wd.display()), &format!("{}/dest", wd.display()) ), @@ -5309,7 +5657,8 @@ mod tests { fs::set_permissions(wd.join("d/inner"), fs::Permissions::from_mode(0o000)).unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); - let result = branch.handle_rename( + let result = rename( + &mut branch, &format!("{}/d", wd.display()), &format!("{}/moved", wd.display()), ); @@ -5343,7 +5692,8 @@ mod tests { // Quota fits the staged directory (4096) but not the file after it. let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 4100).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/d", wd.display()), &format!("{}/m", wd.display()) ), @@ -5375,7 +5725,8 @@ mod tests { let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch.handle_rename( + rename( + &mut branch, &format!("{}/d", wd.display()), &format!("{}/m", wd.display()) ), @@ -5395,12 +5746,12 @@ mod tests { let (workdir, storage) = setup_workdir(); let wd = workdir.path().canonicalize().unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); - assert!(branch - .handle_rename( - &format!("{}/existing.txt", wd.display()), - &format!("{}/renamed.txt", wd.display()) - ) - .unwrap()); + assert!(rename( + &mut branch, + &format!("{}/existing.txt", wd.display()), + &format!("{}/renamed.txt", wd.display()) + ) + .unwrap()); branch.commit().unwrap(); assert!(!wd.join("existing.txt").exists()); assert_eq!(fs::read_to_string(wd.join("renamed.txt")).unwrap(), "hello"); @@ -5415,12 +5766,12 @@ mod tests { fs::create_dir(wd.join("d")).unwrap(); let mut branch = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap(); assert_eq!( - branch - .handle_link( - &format!("{}/d", wd.display()), - &format!("{}/d2", wd.display()) - ) - .unwrap(), + link( + &mut branch, + &format!("{}/d", wd.display()), + &format!("{}/d2", wd.display()) + ) + .unwrap(), false ); assert!(!branch.upper_dir().join("d").exists()); @@ -5512,7 +5863,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); let path = format!("{}/pipe", branch.workdir_str()); assert!(matches!( - branch.handle_chmod(&path, 0o600), + chmod(&mut branch, &path, 0o600), Err(BranchError::NotOwned) )); assert!( @@ -5536,7 +5887,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); let wd = branch.workdir_str().to_string(); assert_eq!( - branch.handle_rename(&format!("{wd}/pipe"), &format!("{wd}/pipe2")), + rename(&mut branch, &format!("{wd}/pipe"), &format!("{wd}/pipe2")), Err(libc::EXDEV), ); assert!( @@ -5621,11 +5972,11 @@ mod tests { let storage = tempfile::tempdir().unwrap(); let mut branch = SeccompCowBranch::create(Path::new("/"), Some(storage.path()), 0).unwrap(); assert!(matches!( - branch.handle_chmod("/dev/null", 0o666), + chmod(&mut branch, "/dev/null", 0o666), Err(BranchError::NotOwned) )); assert!(matches!( - branch.handle_utimensat("/dev/null"), + branch.prepare_copy("dev/null"), Err(BranchError::NotOwned) )); assert_eq!( @@ -5638,19 +5989,21 @@ mod tests { } #[test] - fn execute_copy_guards_against_type_race() { + fn stage_copy_guards_against_type_race() { // Defense in depth: if the entry changes type between prepare_copy's - // lstat and execute_copy's open, the O_NONBLOCK+fstat guard must + // lstat and stage_copy's open, the O_NONBLOCK+fstat guard must // catch it and produce an empty destination instead of streaming. let (workdir, storage) = setup_workdir(); let fifo = workdir.path().join("race"); let c = std::ffi::CString::new(fifo.to_str().unwrap()).unwrap(); assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o644) }, 0); - let branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - let workdir_root = branch.workdir().to_path_buf(); - let upper_root = branch.upper_dir().to_path_buf(); - SeccompCowBranch::execute_copy(&workdir_root, &upper_root, "race").unwrap(); - assert_eq!(fs::read(upper_root.join("race")).unwrap(), b""); + let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); + let staged = SeccompCowBranch::stage_copy(branch.workdir(), branch.staging_dir(), "race"); + assert_eq!( + branch.install_staged("race", 0, staged).unwrap(), + CopyInstall::Ready + ); + assert_eq!(fs::read(branch.upper_dir().join("race")).unwrap(), b""); } /// Recursively snapshot the merged view as (rel path -> Option), @@ -5731,7 +6084,7 @@ mod tests { .unwrap(); fs::write(&up, "MODIFIED").unwrap(); assert_eq!(branch.handle_unlink(&w("d/a.txt"), false), Ok(true)); - assert!(branch.handle_rename(&w("d"), &w("moved")).unwrap()); + assert!(rename(&mut branch, &w("d"), &w("moved")).unwrap()); assert_eq!( branch.handle_unlink(&w("subdir/nested.txt"), false), Ok(true) @@ -6256,9 +6609,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); let wd = workdir.path().canonicalize().unwrap(); - assert!(branch - .handle_chmod(&format!("{}/f.txt", wd.display()), 0o600) - .unwrap()); + assert!(chmod(&mut branch, &format!("{}/f.txt", wd.display()), 0o600).unwrap()); assert_eq!( branch .changes() @@ -6288,7 +6639,7 @@ mod tests { /// /// The merge used to hardcode the destination mode, so a script or binary /// the run produced arrived in the workdir un-executable — a committed - /// result that cannot be run. `execute_copy` already carries the mode on + /// result that cannot be run. `stage_copy` already carries the mode on /// the way down into the upper; this is the same property on the way back. #[test] fn the_mode_of_a_file_created_in_the_upper_survives_the_merge() { @@ -6610,9 +6961,7 @@ mod tests { let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); let wd = branch.workdir_str().to_string(); - assert!(branch - .handle_rename(&format!("{wd}/d"), &format!("{wd}/e")) - .unwrap()); + assert!(rename(&mut branch, &format!("{wd}/d"), &format!("{wd}/e")).unwrap()); let all = by_path(&branch); assert_eq!(all["d"].kind(), ChangeKind::Deleted); @@ -6650,9 +6999,8 @@ mod tests { fs::write(workdir.path().join("f.txt"), "abc").unwrap(); let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap(); - assert!(branch - .handle_chmod(&format!("{}/f.txt", branch.workdir_str()), 0o755) - .unwrap()); + let path = format!("{}/f.txt", branch.workdir_str()); + assert!(chmod(&mut branch, &path, 0o755).unwrap()); let c = &by_path(&branch)["f.txt"]; assert_eq!(c.kind(), ChangeKind::Modified); @@ -6682,14 +7030,12 @@ mod tests { } #[test] - fn execute_copy_returns_the_digest_of_what_it_streamed() { + fn stage_copy_returns_the_digest_of_what_it_streamed() { let workdir = tempfile::tempdir().unwrap(); - let upper = tempfile::tempdir().unwrap(); + let staging = tempfile::tempdir().unwrap(); fs::write(workdir.path().join("f"), "abc").unwrap(); - let digest = SeccompCowBranch::execute_copy(workdir.path(), upper.path(), "f") - .unwrap() - .unwrap(); - assert_eq!(hex(&digest), SHA256_ABC); + let staged = SeccompCowBranch::stage_copy(workdir.path(), staging.path(), "f").unwrap(); + assert_eq!(hex(&staged.digest.unwrap()), SHA256_ABC); } fn sorted_changes(changes: Vec) -> Vec { diff --git a/crates/sandlock-core/src/network/connect.rs b/crates/sandlock-core/src/network/connect.rs index 2dfad96d..e0533455 100644 --- a/crates/sandlock-core/src/network/connect.rs +++ b/crates/sandlock-core/src/network/connect.rs @@ -17,7 +17,7 @@ use super::materialize::{ }; use super::query_socket_protocol; use super::unix::connect_named_unix_on_behalf; -use super::verdict::{classify_destination, path_under_any, Destination}; +use super::verdict::{classify_destination, Destination}; // ============================================================ // connect_on_behalf — perform connect() on behalf of the child (TOCTOU-safe) @@ -102,14 +102,22 @@ pub(super) async fn connect_on_behalf( let metadata_addr = addr_bytes.clone(); NotifAction::prepare(&metadata_addr, dup_fd.as_raw_fd(), move || { - // Record the original destination before connect so the proxy - // cannot receive the connection before its mapping is available. - if plan.record_orig_dest { - if let Some(ref map) = orig_dest_map { - record_orig_dest(map, dup_fd.as_raw_fd(), sockaddr_is_ipv6(&addr_bytes), ip); + // A blocking connect waits on the peer, which must not stall the loop. + NotifAction::native(move || { + // Record the original destination before connect so the proxy + // cannot receive the connection before its mapping is available. + if plan.record_orig_dest { + if let Some(ref map) = orig_dest_map { + record_orig_dest( + map, + dup_fd.as_raw_fd(), + sockaddr_is_ipv6(&addr_bytes), + ip, + ); + } } - } - connect_dup(dup_fd.as_raw_fd(), &plan.addr) + connect_dup(dup_fd.as_raw_fd(), &plan.addr) + }) }) .held_if(held) } else { @@ -126,28 +134,11 @@ pub(super) async fn connect_on_behalf( // never return Continue on the deny path, so it is TOCTOU-safe. // Abstract sockets (no path) are handled by the Landlock abstract scope. match named_unix_socket_path(&addr_bytes) { + // Resolve the symlink-followed target and connect on-behalf to + // the pinned inode, so a symlink inside a granted dir cannot + // redirect to an ungranted socket. Some(path) if ctx.policy.supervises_unix_connect() => { - if ctx.policy.chroot_root.is_some() { - // Chroot mode: the child's paths are virtual, so a lexical - // check against the (virtual) write grants is consistent, - // and host socket paths are absent from the chroot view - // anyway. Deny unless under a write grant. - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - } - } else { - // Non-chroot: resolve the symlink-followed real target and - // connect on-behalf to the pinned inode, so a symlink inside - // a granted dir cannot redirect to an ungranted socket. - connect_named_unix_on_behalf( - notif.pid, - sockfd, - &path, - &ctx.policy.chroot_writable, - ) - } + connect_named_unix_on_behalf(notif, ctx, sockfd, &path) } // Abstract/unnamed socket, non-AF_UNIX family, or gate disabled. _ => NotifAction::Continue, diff --git a/crates/sandlock-core/src/network/mod.rs b/crates/sandlock-core/src/network/mod.rs index 56604440..be821724 100644 --- a/crates/sandlock-core/src/network/mod.rs +++ b/crates/sandlock-core/src/network/mod.rs @@ -154,19 +154,18 @@ fn socket_is_unix(fd: RawFd) -> bool { /// 1. Non-IP families (AF_UNIX etc.): the IP policy doesn't apply; /// Landlock IPC scoping is the enforcement boundary. One sub-case is /// narrower: `sendto`'s non-IP fall-through does NOT Continue once a -/// destination policy is active. That fall-through has no chroot check — -/// it is whatever misses the named-path gate, i.e. a pathless AF_UNIX or a -/// non-AF_UNIX destination — so it fails closed under `--chroot` too. +/// destination policy is active. That fall-through is whatever misses the +/// named-path gate, i.e. a pathless AF_UNIX or a non-AF_UNIX destination, +/// so it fails closed under `--chroot` too. /// There the send goes on-behalf on a /// pinned fd — a named unix target resolved in the child's root view, or a /// non-unix destination on a unix socket (the NETLINK_ROUTE /// virtualization) — or fails closed with EAFNOSUPPORT for an AF_UNIX /// address with no pathname to pin, notably an abstract one: the /// supervisor has no Landlock domain, so an on-behalf abstract send would -/// escape the child's scope. The other non-IP arms still Continue under a -/// destination policy — `connect`'s abstract/unnamed/non-AF_UNIX arm and -/// both chroot named-unix arms (`connect`, `sendto`/`sendmsg`), which is -/// the residue tracked by issues #27 / #143. +/// escape the child's scope. `connect`'s abstract/unnamed/non-AF_UNIX +/// arm still Continues under a destination policy, which is the residue +/// tracked by issues #27 / #143. /// 2. Connected sockets with addr_ptr == 0 — the address was already /// validated at connect time, so the kernel re-read of (nothing) is /// moot. diff --git a/crates/sandlock-core/src/network/send.rs b/crates/sandlock-core/src/network/send.rs index 312c738e..59d5b133 100644 --- a/crates/sandlock-core/src/network/send.rs +++ b/crates/sandlock-core/src/network/send.rs @@ -20,9 +20,7 @@ use super::unix::{ mmsg_entry_named_unix_path, sendmmsg_named_unix_on_behalf, sendto_named_unix_on_behalf, sendto_pinned_unix_on_behalf, unix_sendmsg_gate, }; -use super::verdict::{ - check_ip_destination, classify_send_path, path_under_any, DestShape, Destination, SendPath, -}; +use super::verdict::{check_ip_destination, classify_send_path, DestShape, Destination, SendPath}; use super::{query_socket_protocol, socket_is_unix, Protocol}; // ============================================================ @@ -110,26 +108,9 @@ pub(super) async fn sendto_on_behalf( // sendto to a named socket is a WRITE on its inode, so deny unless the // resolved real target is under an fs-write grant. match named_unix_socket_path(&addr_bytes) { - Some(path) if ctx.policy.supervises_unix_send() => { - if ctx.policy.chroot_root.is_some() { - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - } - } else { - sendto_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - buf_ptr, - buf_len, - flags, - &path, - &ctx.policy.chroot_writable, - ) - } - } + Some(path) if ctx.policy.supervises_unix_send() => sendto_named_unix_on_behalf( + notif, ctx, notif_fd, sockfd, buf_ptr, buf_len, flags, &path, + ), _ => { // Non-IP destination with no fs-path gate: an abstract unix // address, a named unix address with the fs-gate off, a non-unix @@ -475,29 +456,9 @@ pub(super) async fn sendmmsg_on_behalf( } } if named_unix { - if ctx.policy.chroot_root.is_some() { - // Chroot: lexical check; deny the whole call if any named-unix - // entry is outside the (virtual) write grants. - for i in 0..vlen { - let entry_ptr = mmsg_entry_ptr(msgvec_ptr, i); - if let Some(path) = mmsg_entry_named_unix_path(notif, notif_fd, entry_ptr) { - if !path_under_any(&path, &ctx.policy.chroot_writable) { - return NotifAction::Errno(libc::EACCES); - } - } - } - // All granted: fall through to the existing path. - } else { - return sendmmsg_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - msgvec_ptr, - vlen, - flags, - &ctx.policy.chroot_writable, - ); - } + return sendmmsg_named_unix_on_behalf( + notif, ctx, notif_fd, sockfd, msgvec_ptr, vlen, flags, + ); } } diff --git a/crates/sandlock-core/src/network/unix.rs b/crates/sandlock-core/src/network/unix.rs index ed8ea359..ab320428 100644 --- a/crates/sandlock-core/src/network/unix.rs +++ b/crates/sandlock-core/src/network/unix.rs @@ -18,7 +18,7 @@ use super::materialize::{ MaterializedMsg, }; use super::send_engine::{prepare_batch, prepare_send, wants_blocking}; -use super::verdict::{path_under_any, real_path_under_any}; +use super::verdict::real_path_under_any; /// Address `sun_path` in child's context. Absolute paths use `/proc//root`, /// relative paths use `/proc//cwd`. @@ -34,7 +34,7 @@ fn child_lookup_path(child_pid: u32, sun_path: &std::path::Path) -> String { } /// Pin the inode a named unix socket `sun_path` resolves to **in the child's -/// root view** (see [`child_root_path`]), following the child's symlinks rather +/// root view** (see [`child_lookup_path`]), following the child's symlinks rather /// than the supervisor's. Returns an `O_PATH` fd to that exact inode; callers /// address it through `/proc/self/fd` so the resolved inode is the one acted on, /// immune to a path swap after the check (TOCTOU- and symlink-safe). @@ -55,20 +55,25 @@ fn pin_child_unix_target( } /// Resolve a named unix socket `sun_path` to its real, symlink-followed inode -/// in the child's root view (see [`pin_child_unix_target`]) and verify that -/// inode is under an fs-write grant. On success returns the pinned `O_PATH` fd; -/// on failure returns the deny/refuse `NotifAction`. +/// in the child's view and verify that inode is under an fs-write grant. On +/// success returns the pinned `O_PATH` fd; on failure returns the deny/refuse +/// `NotifAction`. fn resolve_named_unix_target( - child_pid: u32, + notif: &SeccompNotif, + ctx: &SupervisorCtx, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> Result { - let pinned = pin_child_unix_target(child_pid, sun_path)?; + if ctx.policy.chroot_root.is_some() { + let chroot = crate::chroot::dispatch::ChrootCtx::new(&ctx.policy, &ctx.processes); + return crate::chroot::dispatch::pin_writable_unix_target(notif, &chroot, sun_path) + .map_err(NotifAction::Errno); + } + let pinned = pin_child_unix_target(notif.pid, sun_path)?; // Canonical path of the pinned inode in our mount namespace. let real = std::fs::read_link(format!("/proc/self/fd/{}", pinned.as_raw_fd())) .map_err(|_| NotifAction::Errno(libc::EACCES))?; - if real_path_under_any(&real, writable) { + if real_path_under_any(&real, &ctx.policy.chroot_writable) { Ok(pinned) } else { Err(NotifAction::Errno(libc::EACCES)) @@ -104,16 +109,17 @@ fn sockaddr_un_bytes(sun: &libc::sockaddr_un, len: libc::socklen_t) -> Vec { .to_vec() } -/// On-behalf `connect()` for a NAMED `AF_UNIX` socket in non-chroot mode: -/// resolve+verify the target, then connect the child's socket to the pinned -/// inode through `/proc/self/fd`. +/// On-behalf `connect()` for a NAMED `AF_UNIX` socket: resolve+verify the +/// target, then connect the child's socket to the pinned inode through +/// `/proc/self/fd`. pub(super) fn connect_named_unix_on_behalf( - child_pid: u32, + notif: &SeccompNotif, + ctx: &SupervisorCtx, sockfd: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - let pinned = match resolve_named_unix_target(child_pid, sun_path, writable) { + let child_pid = notif.pid; + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(action) => return action, }; @@ -127,36 +133,39 @@ pub(super) fn connect_named_unix_on_behalf( }; let addr = sockaddr_un_bytes(&sun, len); NotifAction::prepare(&addr.clone(), dup_fd.as_raw_fd(), move || { - let _pinned = pinned; - let ret = unsafe { - libc::connect( - dup_fd.as_raw_fd(), - addr.as_ptr() as *const libc::sockaddr, - addr.len() as libc::socklen_t, - ) - }; - if ret == 0 { - NotifAction::ReturnValue(0) - } else { - NotifAction::Errno(unsafe { *libc::__errno_location() }) - } + // A listener with a full backlog blocks the connect. + NotifAction::native(move || { + let _pinned = pinned; + let ret = unsafe { + libc::connect( + dup_fd.as_raw_fd(), + addr.as_ptr() as *const libc::sockaddr, + addr.len() as libc::socklen_t, + ) + }; + if ret == 0 { + NotifAction::ReturnValue(0) + } else { + NotifAction::Errno(unsafe { *libc::__errno_location() }) + } + }) }) } -/// On-behalf `sendto()` for a NAMED `AF_UNIX` datagram in non-chroot mode: -/// resolve+verify the target, copy the child's data, then send to the pinned -/// inode through `/proc/self/fd`. +/// On-behalf `sendto()` for a NAMED `AF_UNIX` datagram: resolve+verify the +/// target, copy the child's data, then send to the pinned inode through +/// `/proc/self/fd`. pub(super) fn sendto_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, buf_ptr: u64, buf_len: usize, flags: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - let pinned = match resolve_named_unix_target(notif.pid, sun_path, writable) { + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(action) => return action, }; @@ -274,38 +283,24 @@ pub(super) fn unix_sendmsg_gate( .ok()?; // None unless this is a NAMED AF_UNIX target; IP/abstract fall through. let path = named_unix_socket_path(&addr_bytes)?; - - if ctx.policy.chroot_root.is_some() { - return Some(if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) - }); - } Some(sendmsg_named_unix_on_behalf( - notif, - notif_fd, - sockfd, - msghdr_ptr, - flags, - &path, - &ctx.policy.chroot_writable, + notif, ctx, notif_fd, sockfd, msghdr_ptr, flags, &path, )) } -/// On-behalf `sendmsg()` for a NAMED `AF_UNIX` datagram in non-chroot mode: -/// resolve+verify the target, copy the message's iovecs and control data, then -/// send to the pinned inode through `/proc/self/fd`. +/// On-behalf `sendmsg()` for a NAMED `AF_UNIX` datagram: resolve+verify the +/// target, copy the message's iovecs and control data, then send to the pinned +/// inode through `/proc/self/fd`. fn sendmsg_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msghdr_ptr: u64, flags: i32, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> NotifAction { - match send_named_unix_msghdr(notif, notif_fd, sockfd, msghdr_ptr, sun_path, writable) { + match send_named_unix_msghdr(notif, ctx, notif_fd, sockfd, msghdr_ptr, sun_path) { Ok((dup_fd, m)) => { let blocking = wants_blocking(dup_fd.as_raw_fd(), flags); prepare_send(dup_fd, m, flags, blocking) @@ -322,13 +317,13 @@ fn sendmsg_named_unix_on_behalf( /// the per-entry `sendmmsg` path. fn send_named_unix_msghdr( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msghdr_ptr: u64, sun_path: &std::path::Path, - writable: &[crate::grant::Grant], ) -> Result<(OwnedFd, MaterializedMsg), i32> { - let pinned = match resolve_named_unix_target(notif.pid, sun_path, writable) { + let pinned = match resolve_named_unix_target(notif, ctx, sun_path) { Ok(fd) => fd, Err(NotifAction::Errno(e)) => return Err(e), Err(_) => return Err(libc::EACCES), @@ -374,20 +369,19 @@ pub(super) fn mmsg_entry_named_unix_path( named_unix_socket_path(&addr_bytes) } -/// On-behalf `sendmmsg` for a batch containing NAMED `AF_UNIX` entries -/// (non-chroot). Each named-unix entry is resolved, verified, and sent to its +/// On-behalf `sendmmsg` for a batch containing NAMED `AF_UNIX` entries. Each named-unix entry is resolved, verified, and sent to its /// pinned inode; the loop stops at the first entry it cannot gate on-behalf /// (connected/abstract) or that is denied, returning the count sent so far /// (standard short-`sendmmsg` semantics). Never returns `Continue`, so a unix /// entry cannot ride out via the binary whole-call passthrough. pub(super) fn sendmmsg_named_unix_on_behalf( notif: &SeccompNotif, + ctx: &SupervisorCtx, notif_fd: RawFd, sockfd: i32, msgvec_ptr: u64, vlen: usize, flags: i32, - writable: &[crate::grant::Grant], ) -> NotifAction { let mut entries = Vec::new(); let mut batch_bytes = 0usize; @@ -401,7 +395,7 @@ pub(super) fn sendmmsg_named_unix_on_behalf( None => break, }; let (dup_fd, m) = - match send_named_unix_msghdr(notif, notif_fd, sockfd, entry_ptr, &path, writable) { + match send_named_unix_msghdr(notif, ctx, notif_fd, sockfd, entry_ptr, &path) { Ok(pair) => pair, Err(errno) => { first_errno = Some(errno); diff --git a/crates/sandlock-core/src/network/verdict.rs b/crates/sandlock-core/src/network/verdict.rs index 9175b0c4..102339ac 100644 --- a/crates/sandlock-core/src/network/verdict.rs +++ b/crates/sandlock-core/src/network/verdict.rs @@ -140,16 +140,6 @@ pub(crate) fn real_path_under_any(real: &std::path::Path, grants: &[crate::grant }) } -/// True if `path`, lexically normalized (`.`/`..` resolved without touching the -/// filesystem), is at or under any of the granted `prefixes`. Mirrors the -/// prefix matching the chroot fs enforcement uses. -pub(crate) fn path_under_any(path: &std::path::Path, grants: &[crate::grant::Grant]) -> bool { - let norm = crate::chroot::resolve::confine(&path.to_string_lossy()); - grants - .iter() - .any(|grant| grant.covers(std::path::Path::new(&norm))) -} - /// The shape of a non-IP destination sockaddr, keyed on its ADDRESS FAMILY /// first. Produced by `materialize::classify_dest_shape` from the copied /// address bytes. diff --git a/crates/sandlock-core/src/resource.rs b/crates/sandlock-core/src/resource.rs index 3bc3a1cc..bd506474 100644 --- a/crates/sandlock-core/src/resource.rs +++ b/crates/sandlock-core/src/resource.rs @@ -1034,6 +1034,7 @@ mod tests { policy: Arc::new(fake_policy(argv_safety_required)), child_pidfd: None, notif_fd: -1, + mem_gate: Default::default(), }) } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 9477ffbb..5742e9ec 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -605,6 +605,8 @@ struct Runtime { #[cfg(feature = "cow")] seccomp_cow: Option, supervisor_resource: Option>>, + #[cfg_attr(not(feature = "checkpoint"), allow(dead_code))] + supervisor_mem_gate: Option, supervisor_cow: Option>>, supervisor_network: Option>>, ctrl_fd: Option, @@ -1933,7 +1935,11 @@ impl Sandbox { return Err(SandlockError::Runtime(SandboxRuntimeError::NotRunning)); } self.freeze().await?; - let cp = crate::checkpoint::capture(target_pid, self); + let gate = self + .runtime + .as_ref() + .and_then(|rt| rt.supervisor_mem_gate.as_ref()); + let cp = crate::checkpoint::capture(target_pid, self, gate); self.thaw().await?; cp } @@ -2166,6 +2172,7 @@ impl Sandbox { #[cfg(feature = "cow")] seccomp_cow: None, supervisor_resource: None, + supervisor_mem_gate: None, supervisor_cow: None, supervisor_network: None, ctrl_fd: None, @@ -2274,6 +2281,7 @@ impl Sandbox { #[cfg(feature = "cow")] seccomp_cow: None, supervisor_resource: None, + supervisor_mem_gate: None, supervisor_cow: None, supervisor_network: None, ctrl_fd: None, @@ -2949,6 +2957,8 @@ impl Sandbox { let chroot_state = Arc::new(tokio::sync::Mutex::new(chroot_state)); let processes = Arc::new(crate::seccomp::state::ProcessIndex::new()); + let mem_gate = crate::seccomp::gate::MemGate::default(); + self.rt_mut().supervisor_mem_gate = Some(mem_gate.clone()); let ctx = Arc::new(SupervisorCtx { resource: Arc::clone(&res_state), cow: Arc::clone(&cow_state), @@ -2963,6 +2973,7 @@ impl Sandbox { policy: Arc::new(notif_policy), child_pidfd: child_pidfd_raw, notif_fd: notif_raw_fd, + mem_gate, }); // Snapshot the sandbox config for the control-socket `config` diff --git a/crates/sandlock-core/src/seccomp/ctx.rs b/crates/sandlock-core/src/seccomp/ctx.rs index b0feb728..9f770d22 100644 --- a/crates/sandlock-core/src/seccomp/ctx.rs +++ b/crates/sandlock-core/src/seccomp/ctx.rs @@ -41,4 +41,7 @@ pub struct SupervisorCtx { pub child_pidfd: Option, /// Seccomp notification fd (for on-behalf operations). pub notif_fd: RawFd, + /// Orders deferred work's writes to child memory against the freezes + /// that read it: exec and checkpoint. + pub(crate) mem_gate: super::gate::MemGate, } diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 57bc4d19..d26b2f30 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -246,25 +246,54 @@ impl DispatchTable { .push(handler); } - /// Dispatch a notification through the handler chain for its syscall number. - pub(crate) async fn dispatch( + /// The handler chain for a notification, not yet started. + pub(crate) fn chain( &self, notif: SeccompNotif, notif_fd: RawFd, open: Option>, - ) -> NotifAction { - let nr = notif.data.nr as i64; - if let Some(chain) = self.chains.get(&nr) { - let handler_ctx = HandlerCtx { + ) -> Chain { + let handlers = self + .chains + .get(&(notif.data.nr as i64)) + .map(|chain| chain.handlers.clone()) + .unwrap_or_default(); + Chain { + handlers: handlers.into_iter(), + cx: HandlerCtx { notif, notif_fd, open, - }; - for handler in &chain.handlers { - let action = handler.handle(&handler_ctx).await; - if !matches!(action, NotifAction::Continue) { - return action; - } + }, + } + } + + /// Dispatch a notification through the handler chain for its syscall number. + #[cfg(test)] + pub(crate) async fn dispatch( + &self, + notif: SeccompNotif, + notif_fd: RawFd, + open: Option>, + ) -> NotifAction { + self.chain(notif, notif_fd, open).next().await + } +} + +/// A handler chain that can stop at a deferral and pick up after it. +pub(crate) struct Chain { + handlers: std::vec::IntoIter>, + cx: HandlerCtx, +} + +impl Chain { + /// Run the remaining handlers until one decides or defers; `Continue` + /// means none intervened. + pub(crate) async fn next(&mut self) -> NotifAction { + for handler in self.handlers.by_ref() { + let action = handler.handle(&self.cx).await; + if !matches!(action, NotifAction::Continue) { + return action; } } NotifAction::Continue @@ -291,20 +320,6 @@ pub(crate) fn build_dispatch_table( ) -> DispatchTable { let mut table = DispatchTable::new(); - // ------------------------------------------------------------------ - // Fork/clone family (always on) - // ------------------------------------------------------------------ - for nr in arch::fork_like_syscalls() { - let policy_for_fork = Arc::clone(policy); - let ctx_for_fork = Arc::clone(ctx); - table.register(nr, move |cx: &HandlerCtx| { - let notif = cx.notif; - let policy = Arc::clone(&policy_for_fork); - let ctx = Arc::clone(&ctx_for_fork); - async move { crate::resource::handle_fork(¬if, &ctx, &policy).await } - }); - } - // ------------------------------------------------------------------ // Process groups (always on) // ------------------------------------------------------------------ @@ -1315,6 +1330,7 @@ mod handler_tests { }), child_pidfd: None, notif_fd: -1, + mem_gate: Default::default(), }) } @@ -1495,41 +1511,48 @@ mod handler_tests { ); } - /// A handler returning `Defer` is non-`Continue`, so it must short-circuit - /// the chain exactly like `Errno`/`ReturnValue`: later handlers on the same - /// syscall do not run. Deferral is therefore a terminal decision. + /// A deferral stops the chain; a `Continue` result picks it up at the next + /// handler, and any other result ends it. #[tokio::test] - async fn dispatch_short_circuits_on_defer() { + async fn chain_resumes_after_a_deferral_that_continues() { let mut table = DispatchTable::new(); let later_ran = Arc::new(AtomicUsize::new(0)); - table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { - NotifAction::defer(async { NotifAction::ReturnValue(1) }) + NotifAction::defer(async { NotifAction::Continue }) }); - let later = Arc::clone(&later_ran); table.register(libc::SYS_openat, move |_cx: &HandlerCtx| { - let later = Arc::clone(&later); - async move { - later.fetch_add(1, Ordering::SeqCst); - NotifAction::Continue - } + later.fetch_add(1, Ordering::SeqCst); + async { NotifAction::ReturnValue(42) } }); - let _ctx = fake_supervisor_ctx(); - let action = table - .dispatch(fake_notif(libc::SYS_openat as i32), -1, None) - .await; + let mut chain = table.chain(fake_notif(libc::SYS_openat as i32), -1, None); + let NotifAction::Defer(deferred) = chain.next().await else { + panic!("the first handler defers") + }; + assert_eq!(later_ran.load(Ordering::SeqCst), 0); + assert!(matches!(deferred.run().await, NotifAction::Continue)); + assert!(matches!(chain.next().await, NotifAction::ReturnValue(42))); + assert_eq!(later_ran.load(Ordering::SeqCst), 1); + } - assert!( - matches!(action, NotifAction::Defer(_)), - "dispatch must return the Defer produced by the first handler" - ); - assert_eq!( - later_ran.load(Ordering::SeqCst), - 0, - "Defer must short-circuit the chain like any non-Continue action" - ); + #[tokio::test] + async fn chain_ends_at_a_deferral_that_decides() { + let mut table = DispatchTable::new(); + table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { + NotifAction::defer(async { NotifAction::Errno(libc::EACCES) }) + }); + table.register(libc::SYS_openat, |_cx: &HandlerCtx| async { + panic!("a decided deferral must end the chain") + }); + let mut chain = table.chain(fake_notif(libc::SYS_openat as i32), -1, None); + let NotifAction::Defer(deferred) = chain.next().await else { + panic!("the first handler defers") + }; + assert!(matches!( + deferred.run().await, + NotifAction::Errno(libc::EACCES) + )); } /// `validate_handler_syscalls_against_policy` must reject handlers whose diff --git a/crates/sandlock-core/src/seccomp/gate.rs b/crates/sandlock-core/src/seccomp/gate.rs new file mode 100644 index 00000000..89f5ee67 --- /dev/null +++ b/crates/sandlock-core/src/seccomp/gate.rs @@ -0,0 +1,101 @@ +//! Orders writes to child memory against the freezes that read it. +//! +//! Every handler, inline or parked, runs with its sandbox's gate installed, +//! and each write to child memory holds the shared side for that one write. +//! The exec freeze and a checkpoint capture hold the exclusive side, so no +//! write lands on memory they are reading. The exec freeze writes nothing +//! while it holds the gate, so the loop never waits on itself. + +use std::cell::RefCell; +use std::future::Future; +use std::sync::{Arc, RwLock, RwLockWriteGuard}; + +#[derive(Clone, Default)] +pub(crate) struct MemGate(Arc>); + +tokio::task_local! { + static TASK_GATE: MemGate; +} + +thread_local! { + static THREAD_GATE: RefCell> = const { RefCell::new(None) }; +} + +impl MemGate { + /// Writes only hold the shared side for a single copy, so this waits + /// microseconds, never for parked work to finish. + pub(crate) fn exclusive(&self) -> RwLockWriteGuard<'_, ()> { + self.0.write().unwrap_or_else(|e| e.into_inner()) + } + + pub(crate) async fn scope(self, future: F) -> F::Output { + TASK_GATE.scope(self, future).await + } + + pub(crate) fn enter(self, work: impl FnOnce() -> T) -> T { + let previous = THREAD_GATE.with(|gate| gate.replace(Some(self))); + let result = work(); + THREAD_GATE.with(|gate| *gate.borrow_mut() = previous); + result + } +} + +/// Run one write to child memory under the installed gate, if any. +pub(crate) fn write(work: impl FnOnce() -> T) -> T { + let gate = TASK_GATE + .try_with(MemGate::clone) + .ok() + .or_else(|| THREAD_GATE.with(|gate| gate.borrow().clone())); + match gate { + Some(gate) => { + let _shared = gate.0.read().unwrap_or_else(|e| e.into_inner()); + work() + } + None => work(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn exclusive_waits_for_a_write_in_progress() { + let gate = MemGate::default(); + let (writing_tx, writing_rx) = std::sync::mpsc::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + let writer = std::thread::spawn({ + let gate = gate.clone(); + move || { + gate.enter(|| { + write(|| { + writing_tx.send(()).unwrap(); + release_rx.recv().unwrap(); + }) + }) + } + }); + writing_rx.recv().unwrap(); + assert!(gate.0.try_write().is_err()); + release_tx.send(()).unwrap(); + writer.join().unwrap(); + drop(gate.exclusive()); + } + + #[tokio::test] + async fn parked_futures_write_under_the_gate() { + let gate = MemGate::default(); + let held = gate.clone(); + gate.scope(async move { + write(|| assert!(held.0.try_write().is_err())); + }) + .await; + } + + #[test] + fn writes_outside_parked_work_are_ungated() { + let gate = MemGate::default(); + let _exclusive = gate.exclusive(); + write(|| ()); + } +} diff --git a/crates/sandlock-core/src/seccomp/mod.rs b/crates/sandlock-core/src/seccomp/mod.rs index 231fc867..b7b59e1b 100644 --- a/crates/sandlock-core/src/seccomp/mod.rs +++ b/crates/sandlock-core/src/seccomp/mod.rs @@ -1,6 +1,8 @@ pub mod bpf; pub(crate) mod ctx; pub mod dispatch; +pub(crate) mod gate; +pub(crate) mod native; pub mod notif; pub(crate) mod state; pub mod syscall; diff --git a/crates/sandlock-core/src/seccomp/native.rs b/crates/sandlock-core/src/seccomp/native.rs new file mode 100644 index 00000000..5b308819 --- /dev/null +++ b/crates/sandlock-core/src/seccomp/native.rs @@ -0,0 +1,71 @@ +//! Synchronous work run on behalf of a parked notification. +//! +//! A native closure cannot be cancelled, so it has no deadline: its caller +//! waits for it as for the syscall itself. A trapped task has at most one +//! notification outstanding, which bounds how many a sandbox can run. + +use std::sync::OnceLock; + +use tokio::runtime::Runtime; + +use super::gate::MemGate; +use super::notif::NotifAction; + +const MAX_THREADS: usize = 512; + +// The embedder's runtime waits for its blocking tasks on shutdown, and a +// closure stuck on a peer would hold that shutdown hostage. +fn runtime() -> &'static Runtime { + static RUNTIME: OnceLock = OnceLock::new(); + RUNTIME.get_or_init(|| { + tokio::runtime::Builder::new_current_thread() + .max_blocking_threads(MAX_THREADS) + .thread_name("sandlock-native") + .build() + .expect("building a runtime without drivers cannot fail") + }) +} + +/// Run `work` on a native thread under `gate`, or ungated outside a supervisor. +pub(crate) async fn run( + gate: Option, + work: Box NotifAction + Send>, +) -> NotifAction { + let job = runtime().spawn_blocking(move || match gate { + Some(gate) => gate.enter(work), + None => work(), + }); + job.await.unwrap_or(NotifAction::Errno(libc::EIO)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn abandoned_work_runs_to_completion() { + let (started_tx, started_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + let (done_tx, done_rx) = tokio::sync::oneshot::channel(); + let waiter = tokio::spawn(run( + None, + Box::new(move || { + let _ = started_tx.send(()); + let _ = release_rx.recv(); + let _ = done_tx.send(()); + NotifAction::Continue + }), + )); + started_rx.await.unwrap(); + waiter.abort(); + let _ = waiter.await; + release_tx.send(()).unwrap(); + done_rx.await.unwrap(); + } + + #[tokio::test] + async fn panicking_work_fails_closed() { + let action = run(None, Box::new(|| panic!("native panic"))).await; + assert!(matches!(action, NotifAction::Errno(libc::EIO))); + } +} diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index a0c604ee..55ce3262 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -43,68 +43,123 @@ impl OnInjectSuccess { } } -/// A deferred decision: an owned, `'static` future that produces the real -/// [`NotifAction`] off the supervisor's notification loop. +/// Work that decides a notification off the supervisor's notification loop. /// /// A handler returns [`NotifAction::Defer`] when computing the response is /// slow (a network round-trip, a blocking syscall) and must not stall the -/// single supervisor task that gates every other trapped syscall. The -/// supervisor moves the future onto a worker, lets the loop proceed, and -/// sends the response (via the still-valid `notif.id`) when the future -/// resolves. The future is `'static` because it outlives the borrowed -/// `HandlerCtx` — capture what you need (`notif` is `Copy`, `notif_fd` is a -/// `RawFd`) by value rather than borrowing `&self`. +/// loop that gates every other trapped syscall. The child stays blocked in +/// its syscall meanwhile. A deferral resumes where it was returned: when a handler +/// defers, a `Continue` result runs the next handler in the chain and any other +/// result ends it, so a deferred decision still passes every later check. /// -/// The deferred future need only be `Send` (not `Sync`): the supervisor -/// moves it onto a worker task and never shares it by reference. Requiring -/// `Sync` of user futures would be a leaky bound (it would reject a future -/// capturing, say, a `Cell`), so it is not required. +/// The future outlives the borrowed `HandlerCtx`; capture what it needs by +/// value. It need only be `Send`: the supervisor moves it onto a task and +/// never shares it. pub struct Deferred { - future: Pin + Send + 'static>>, + work: Work, patient: bool, } -// Safety: `NotifAction` must stay `Sync` so it can live in `Sync` contexts -// (handler `&self` state, etc.; the `Handler` trait is `Send + Sync`), which -// requires `Deferred: Sync`. A `Send`-only future is not `Sync`, but the -// boxed future is unreachable through a shared `&Deferred`: the field is -// private, `Debug` touches only a static string, and `run(self)` consumes -// the value (it is never callable through `&self`). With no path to poll or -// read the future via a shared reference, sharing `&Deferred` across threads -// cannot race, so asserting `Sync` is sound while keeping user futures -// `Send`-only. +enum Work { + Future(Pin + Send + 'static>>), + Native(Box NotifAction + Send + 'static>), +} + +// Safety: `NotifAction` must stay `Sync` because handlers keep it in `Sync` +// state. The work is private and only reachable by consuming the `Deferred`, so +// a shared `&Deferred` can never touch it. unsafe impl Sync for Deferred {} impl std::fmt::Debug for Deferred { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("Deferred()") + match &self.work { + Work::Future(_) => f.write_str("Deferred()"), + Work::Native(_) => f.write_str("Deferred()"), + } } } impl Deferred { pub fn new + Send + 'static>(f: F) -> Self { Self { - future: Box::pin(f), + work: Work::Future(Box::pin(f)), patient: false, } } + /// Synchronous work run on a native thread. It cannot be cancelled, so + /// it is patient: failing the syscall would not stop its side effects. + pub fn native(work: impl FnOnce() -> NotifAction + Send + 'static) -> Self { + Self { + work: Work::Native(Box::new(work)), + patient: true, + } + } + /// A deferral with no deadline, for a syscall that natively waits as long /// as it takes. The future has to end by itself once the child is gone. pub(crate) fn patient + Send + 'static>(f: F) -> Self { Self { - future: Box::pin(f), + work: Work::Future(Box::pin(f)), patient: true, } } - /// Drive the deferred future to its terminal action. Consumes `self` - /// because the future is run exactly once, on a worker task. + /// Run outside a supervisor, where native work is not ordered against exec. pub async fn run(self) -> NotifAction { - self.future.await + match self.work { + Work::Future(future) => future.await, + Work::Native(work) => super::native::run(None, work).await, + } + } + + async fn run_in(self, offload: &Offload) -> NotifAction { + let gate = offload.gate.clone(); + match self.work { + Work::Future(future) => gate.scope(future).await, + Work::Native(work) => { + // The closure may outlive the supervisor and still use the + // listener's fd number, which must not be reused meanwhile. + let listener = Arc::clone(&offload.listener); + let work = Box::new(move || { + let _listener = listener; + work() + }); + super::native::run(Some(gate), work).await + } + } + } + + /// Run within `deadline` unless patient; a deferral never resolves to + /// another unexecuted action. + async fn run_until(self, offload: &Offload, deadline: tokio::time::Instant) -> NotifAction { + let action = if self.patient { + self.run_in(offload).await + } else { + match tokio::time::timeout_at(deadline, self.run_in(offload)).await { + Ok(action) => action, + Err(_) => { + log::warn!("parked notification exceeded its deadline; failing with EIO"); + NotifAction::Errno(libc::EIO) + } + } + }; + match action { + NotifAction::Defer(_) | NotifAction::Prepared(_) => NotifAction::Errno(libc::EIO), + other => other, + } } } +/// What one sandbox's parked work shares: the gate ordering its child-memory +/// writes against exec and checkpoint, and the listener its native closures +/// may outlive. +#[derive(Clone)] +struct Offload { + gate: super::gate::MemGate, + listener: Arc, +} + /// An owned operation whose side effects require callback approval. pub struct PreparedOperation { pub(crate) host: Option, @@ -165,10 +220,7 @@ pub enum NotifAction { /// Signal one process, named by any pid in it. `SIGKILL` here ends /// the whole thread group, as it does for any task. KillTask { sig: i32, pid: i32 }, - /// Defer the response: run the carried future on a worker task and - /// send its terminal action later, keyed by `notif.id`. Non-`Continue`, - /// so it short-circuits the handler chain — a deferring handler makes a - /// terminal decision. See [`Deferred`]. + /// Decide off the notification loop. See [`Deferred`]. Defer(Deferred), /// Execute only after the policy callback permits the operation. Prepared(PreparedOperation), @@ -206,12 +258,14 @@ impl NotifAction { self } - /// Construct a [`NotifAction::Defer`] from a `'static` future. Ergonomic - /// shorthand for `NotifAction::Defer(Deferred::new(fut))`. pub fn defer + Send + 'static>(fut: F) -> Self { NotifAction::Defer(Deferred::new(fut)) } + pub fn native(work: impl FnOnce() -> NotifAction + Send + 'static) -> Self { + NotifAction::Defer(Deferred::native(work)) + } + /// Inject `content` into the child as the syscall's returned fd, backed by /// a sealed (read-only, fixed-size), `O_CLOEXEC` in-memory file. /// @@ -292,17 +346,6 @@ pub(crate) fn named_content_memfd(content: &[u8], seal: bool, name: &str) -> io: Ok(memfd) } -/// Collapse a deferred future's resolved action into a sendable terminal -/// action. A deferred future that itself resolves to `Defer` is a bug -/// (no nested deferral); collapse it to `EIO` so the trapped child gets a -/// definite response instead of wedging forever waiting for one. -fn finalize_deferred(action: NotifAction) -> NotifAction { - match action { - NotifAction::Defer(_) | NotifAction::Prepared(_) => NotifAction::Errno(libc::EIO), - other => other, - } -} - // ============================================================ // NetworkPolicy — network access policy enum // ============================================================ @@ -1366,15 +1409,17 @@ pub struct NotifPolicy { impl NotifPolicy { /// Whether a named unix `connect()` is decided by the supervisor. It then /// connects on-behalf, so the peer's `SO_PEERCRED` names the supervisor. + /// Under chroot the kernel would resolve the address against the host's + /// root and the child's frozen cwd, so Landlock cannot take it over. pub(crate) fn supervises_unix_connect(&self) -> bool { - self.has_unix_fs_gate && !self.landlock_unix_gate + self.has_unix_fs_gate && (!self.landlock_unix_gate || self.chroot_root.is_some()) } /// Whether a named unix send is decided by the supervisor. A destination /// policy keeps sends on-behalf even under Landlock: a `Continue` would let /// a racing `dup2` move the send onto an IP socket. pub(crate) fn supervises_unix_send(&self) -> bool { - self.has_unix_fs_gate && (!self.landlock_unix_gate || self.has_net_destination_policy) + self.supervises_unix_connect() || (self.has_unix_fs_gate && self.has_net_destination_policy) } /// Whether an open may land on something the supervisor answers for @@ -1759,8 +1804,12 @@ pub fn write_child_mem( addr: u64, data: &[u8], ) -> Result<(), NotifError> { - id_valid(notif_fd, id).map_err(NotifError::Ioctl)?; - write_child_mem_vm(pid, addr, data)?; + // Checked under the gate: a write that waited out an exec freeze must not + // follow the leader's pid into the image that exec installed. + super::gate::write(|| { + id_valid(notif_fd, id).map_err(NotifError::Ioctl)?; + write_child_mem_vm(pid, addr, data) + })?; id_valid(notif_fd, id).map_err(NotifError::Ioctl)?; Ok(()) } @@ -1798,8 +1847,10 @@ pub fn write_child_mem_force( addr: u64, data: &[u8], ) -> Result<(), NotifError> { - id_valid(notif_fd, id).map_err(NotifError::Ioctl)?; - write_child_mem_proc(pid, addr, data)?; + super::gate::write(|| { + 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(()) } @@ -2852,43 +2903,27 @@ async fn emit_policy_event( // Per-notification handler (runs in a spawned task) // ============================================================ -/// Process a single seccomp notification: vDSO re-patch, path denial check, -/// dispatch, policy event emission, and response. -/// Maximum number of deferred handler futures running concurrently. Caps -/// the worker fan-out (and any resources those workers hold, e.g. memfds or -/// sockets) so a burst of deferrals cannot exhaust the supervisor process. -const DEFER_MAX_INFLIGHT: usize = 64; - -/// Maximum time a deferred handler future may run before the supervisor gives -/// up and fails the trapped syscall closed. Bounds the worst case so a hung -/// future (e.g. a stalled network fetch in a token-injection handler) cannot -/// park the child forever or permanently leak its `DEFER_MAX_INFLIGHT` slot. -const DEFER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); - -/// Drive a deferred future to its terminal action, bounded by `limit`. -/// -/// On timeout, fail closed with `EIO` so the trapped child gets a definite -/// response instead of parking forever; `finalize_deferred` still guards a -/// future that resolves to a nested `Defer`. -async fn run_deferred_within(deferred: Deferred, limit: std::time::Duration) -> NotifAction { - match tokio::time::timeout(limit, deferred.run()).await { - Ok(action) => finalize_deferred(action), - Err(_) => { - log::warn!( - "deferred handler exceeded {:?}; failing syscall with EIO", - limit - ); - NotifAction::Errno(libc::EIO) - } - } -} +/// How long a notification may stay parked before it fails closed with EIO, +/// so a hung future cannot hold its caller forever. +const PARK_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); /// A notification whose response is decided off the loop. The loop sends /// it: a response sent while an exec freeze is held would release a task /// the freeze skipped because it was parked in seccomp. enum Parked { + /// A handler deferred; the chain picks up after it. + Chain(Box, NotifAction), Approval(Box, crate::policy_fn::Verdict), - Deferred(u64, NotifAction), + Response(u64, NotifAction), +} + +struct ParkedChain { + notif: SeccompNotif, + chain: super::dispatch::Chain, + open: Option>, + has_denied: bool, + /// Shared by every deferral of this notification's chain. + deadline: tokio::time::Instant, } /// Parked notifications and the tids waiting on them, which the exec freeze @@ -2896,13 +2931,15 @@ enum Parked { struct ParkedSet { tasks: tokio::task::JoinSet, callers: HashMap, + offload: Offload, } impl ParkedSet { - fn new() -> Self { + fn new(listener: Arc, gate: super::gate::MemGate) -> Self { Self { tasks: tokio::task::JoinSet::new(), callers: HashMap::new(), + offload: Offload { gate, listener }, } } @@ -2913,17 +2950,29 @@ impl ParkedSet { } async fn join_next(&mut self) -> Option { - loop { - match self.tasks.join_next_with_id().await? { - Ok((id, parked)) => { - self.callers.remove(&id); - return Some(parked); - } - Err(e) => { - self.callers.remove(&e.id()); - } - } - } + let joined = self.tasks.join_next_with_id().await?; + Some(self.settled(joined)) + } + + fn try_join_next(&mut self) -> Option { + let joined = self.tasks.try_join_next_with_id()?; + Some(self.settled(joined)) + } + + fn settled( + &mut self, + joined: Result<(tokio::task::Id, Parked), tokio::task::JoinError>, + ) -> Parked { + let (id, outcome) = match joined { + Ok((id, parked)) => (id, Ok(parked)), + Err(e) => (e.id(), Err(())), + }; + let caller = self.callers.remove(&id); + // A panicked task still owes its caller a response. + outcome.unwrap_or_else(|()| { + let notif_id = caller.map_or(0, |(_, notif_id)| notif_id); + Parked::Response(notif_id, NotifAction::Errno(libc::EIO)) + }) } /// The callers still parked: one that died since leaves its tid free for @@ -2945,30 +2994,12 @@ struct Prepared { exec_image: Option, } -fn park_deferred( - parked: &mut ParkedSet, - notif: &SeccompNotif, - deferred: Deferred, - permit: tokio::sync::OwnedSemaphorePermit, -) { - let id = notif.id; - parked.spawn(notif, async move { - let _permit = permit; - let action = match deferred.patient { - true => finalize_deferred(deferred.run().await), - false => run_deferred_within(deferred, DEFER_TIMEOUT).await, - }; - Parked::Deferred(id, action) - }); -} - async fn handle_notification( notif: SeccompNotif, ctx: &Arc, dispatch_table: &super::dispatch::DispatchTable, fd: RawFd, parked: &mut ParkedSet, - defer_sem: &Arc, ) { let policy = &ctx.policy; @@ -2997,38 +3028,92 @@ async fn handle_notification( .then(|| OpenRequest::decode(¬if, fd, policy, &ctx.processes)) .flatten() .map(Arc::new); - let mut action = { - let denied = has_denied - && crate::seccomp_plan::fs_denied_path_syscalls().contains(&nr) - && is_path_denied_for_notif(&*ctx.policy_fn.lock().await, ¬if, fd); - if denied { - NotifAction::Errno(libc::EACCES) - } else { - // Dispatch runs first so /proc virtualization and the other - // handlers still win for their paths. - let action = dispatch_table.dispatch(notif, fd, open.clone()).await; - // A bare Continue for an open is the racy window: the kernel - // re-resolves the path after it, and a racing thread can swap a - // link to reach a denied carve-out inside a granted tree (issue - // #111) or a procfs entry the handlers hide by name (issue - // #236). The open is done against the pinned inode instead. - let resolves = policy.chroot_root.is_none() && (has_denied || policy.resolves_opens()); - match (&action, &open) { - (NotifAction::Continue, Some(open)) if resolves => { - let pfs = ctx.policy_fn.lock().await; - on_behalf_open(¬if, open, ctx, &pfs, fd, has_denied).await - } - _ => action, - } - } + let denied = has_denied + && crate::seccomp_plan::fs_denied_path_syscalls().contains(&nr) + && is_path_denied_for_notif(&*ctx.policy_fn.lock().await, ¬if, fd); + // Dispatch runs first so /proc virtualization and the other handlers + // still win for their paths. + let mut chain = dispatch_table.chain(notif, fd, open.clone()); + let action = match denied { + true => NotifAction::Errno(libc::EACCES), + false => chain.next().await, + }; + let state = ParkedChain { + notif, + chain, + open, + has_denied, + deadline: tokio::time::Instant::now() + PARK_TIMEOUT, }; + settle_chain(state, action, ctx, fd, parked).await; +} + +async fn settle_chain( + state: ParkedChain, + action: NotifAction, + ctx: &Arc, + fd: RawFd, + parked: &mut ParkedSet, +) { + match action { + NotifAction::Defer(deferred) => { + let notif = state.notif; + let offload = parked.offload.clone(); + parked.spawn(¬if, async move { + let action = deferred.run_until(&offload, state.deadline).await; + Parked::Chain(Box::new(state), action) + }); + } + action => { + let ParkedChain { + notif, + open, + has_denied, + .. + } = state; + decide(notif, action, open, has_denied, ctx, fd, parked).await + } + } +} + +/// Everything after the handler chain settles: pinned opens, fork +/// accounting, the policy event and its verdict, and the response. +async fn decide( + notif: SeccompNotif, + mut action: NotifAction, + open: Option>, + has_denied: bool, + ctx: &Arc, + fd: RawFd, + parked: &mut ParkedSet, +) { + let policy = &ctx.policy; + let nr = notif.data.nr as i64; + + // A bare Continue for an open is the racy window: the kernel re-resolves + // the path after it, and a racing thread can swap a link to reach a + // denied carve-out inside a granted tree (issue #111) or a procfs entry + // the handlers hide by name (issue #236). The open is done against the + // pinned inode instead. + let resolves = policy.chroot_root.is_none() && (has_denied || policy.resolves_opens()); + if let (NotifAction::Continue, Some(open)) = (&action, &open) { + if resolves { + let pfs = ctx.policy_fn.lock().await; + action = on_behalf_open(¬if, open, ctx, &pfs, fd, has_denied).await; + } + } if let Some(open) = &open { action = crate::procfs::net_dispatch::guard_injection(action, ¬if, open, ctx).await; } - let fork_counted = matches!(action, NotifAction::Continue) - && crate::resource::fork_counted_on_continue(¬if); + // Counted only once no handler can still refuse or defer the fork. + let mut fork_counted = false; + if matches!(action, NotifAction::Continue) && arch::fork_like_syscalls().contains(&nr) { + action = crate::resource::handle_fork(¬if, ctx, policy).await; + fork_counted = matches!(action, NotifAction::Continue) + && crate::resource::fork_counted_on_continue(¬if); + } // Read without freezing so the sandbox keeps running while policy_fn // decides; `freeze_exec` re-reads it under the freeze before Continue. @@ -3052,7 +3137,7 @@ async fn handle_notification( Parked::Approval(Box::new(prepared), verdict) }); } - None => finish_notification(prepared, None, ctx, fd, parked, defer_sem).await, + None => finish_notification(prepared, None, ctx, fd, parked).await, } } @@ -3104,7 +3189,6 @@ async fn finish_notification( ctx: &Arc, fd: RawFd, parked: &mut ParkedSet, - defer_sem: &Arc, ) { let policy = &ctx.policy; let Prepared { @@ -3162,13 +3246,9 @@ async fn finish_notification( } } - // Deferred response: run the handler's future off the loop so slow work - // (a network round-trip, a blocking syscall) does not stall it. The - // trapped child stays parked; the loop sends the result when it lands. - // - // Deferral is refused on syscalls whose Continue path requires the - // execve argv-safety freeze or fork creation-tracking: answering later - // would skip that TOCTOU-closing work. + // Supervisor work decided after the verdict, such as an approved connect + // or a FIFO open, answers with its result. Answering a fork or exec later + // would skip its TOCTOU-closing work; neither is ever parked here. if let NotifAction::Defer(deferred) = action { if crate::freeze::requires_freeze_on_continue(nr) || crate::resource::requires_process_creation_tracking(¬if, policy) @@ -3176,42 +3256,51 @@ async fn finish_notification( let _ = send_response(fd, notif.id, NotifAction::Errno(libc::EPERM)); return; } - match Arc::clone(defer_sem).try_acquire_owned() { - Ok(permit) => park_deferred(parked, ¬if, deferred, permit), - // Too many deferrals in flight: fail fast with EAGAIN rather than - // blocking the loop or letting unbounded workers accrete. - Err(_) => { - let _ = send_response(fd, notif.id, NotifAction::Errno(libc::EAGAIN)); - } - } + let id = notif.id; + let deadline = tokio::time::Instant::now() + PARK_TIMEOUT; + let offload = parked.offload.clone(); + parked.spawn(¬if, async move { + Parked::Response(id, deferred.run_until(&offload, deadline).await) + }); return; } // From the freeze to its release nothing may await: the tracer is this // thread, and the loop must not release any parked task meanwhile. - let mut exec_freeze = None; - if let (NotifAction::Continue, Some(judged)) = (&action, &exec_image) { - let (frozen_action, freeze) = freeze_exec(¬if, judged, ctx, fd, parked); - action = frozen_action; - exec_freeze = Some(freeze); - } + // Parked work keeps running, so its child-memory writes wait on the gate + // until the kernel has copied argv. + let gate = parked.offload.gate.clone(); + let send_result = { + let mut exec_freeze = None; + let mut _writes_held = None; + if let (NotifAction::Continue, Some(judged)) = (&action, &exec_image) { + _writes_held = Some(gate.exclusive()); + let (frozen_action, freeze) = freeze_exec(¬if, judged, ctx, fd, parked); + action = frozen_action; + exec_freeze = Some(freeze); + } - // Ignore error — child may have exited between recv and response. - let exec_continued = exec_freeze.is_some() && matches!(action, NotifAction::Continue); - let send_result = send_response(fd, notif.id, action); + // Ignore error — child may have exited between recv and response. + let exec_continued = exec_freeze.is_some() && matches!(action, NotifAction::Continue); + let send_result = send_response(fd, notif.id, action); - if let Some(freeze) = exec_freeze { - if exec_continued && send_result.is_ok() { - crate::freeze::release_after_exec(&freeze); - } else { - crate::freeze::detach_all(&freeze); + if let Some(freeze) = exec_freeze { + if exec_continued && send_result.is_ok() { + crate::freeze::release_after_exec(&freeze); + } else { + crate::freeze::detach_all(&freeze); + } + // Now that the response is out, the kernel wait holding any pending + // task (the vfork parent waiting on this very execve) can clear; + // reap the queued interrupts and detach. + crate::freeze::reap_pending(&freeze.pending_tids); } - // Now that the response is out, the kernel wait holding any pending - // task (the vfork parent waiting on this very execve) can clear; - // reap the queued interrupts and detach. - crate::freeze::reap_pending(&freeze.pending_tids); - } + send_result + }; + if fork_counted && creation_trace.is_none() && send_result.is_err() { + crate::resource::rollback_fork_count(&ctx.resource).await; + } if let Some(trace) = creation_trace { if send_result.is_ok() { match crate::resource::finish_process_creation_tracking(trace).await { @@ -3240,13 +3329,22 @@ async fn settle_parked( ctx: &Arc, fd: RawFd, set: &mut ParkedSet, - defer_sem: &Arc, ) { match parked { + Parked::Chain(mut state, action) => { + if id_valid(fd, state.notif.id).is_err() { + return; + } + let action = match action { + NotifAction::Continue => state.chain.next().await, + decided => decided, + }; + settle_chain(*state, action, ctx, fd, set).await + } Parked::Approval(prepared, verdict) => { - finish_notification(*prepared, Some(verdict), ctx, fd, set, defer_sem).await + finish_notification(*prepared, Some(verdict), ctx, fd, set).await } - Parked::Deferred(id, action) => { + Parked::Response(id, action) => { let _ = send_response(fd, id, action); } } @@ -3271,14 +3369,16 @@ pub async fn supervisor( ) { // Register the notif fd with the Tokio IO driver so we can wait for // readiness via epoll instead of a dedicated blocking thread. - let async_fd = - match tokio::io::unix::AsyncFd::with_interest(notif_fd, tokio::io::Interest::READABLE) { - Ok(fd) => fd, - Err(err) => { - let _ = startup.send(Err(err)); - return; - } - }; + let async_fd = match tokio::io::unix::AsyncFd::with_interest( + Arc::new(notif_fd), + tokio::io::Interest::READABLE, + ) { + Ok(fd) => fd, + Err(err) => { + let _ = startup.send(Err(err)); + return; + } + }; let fd = async_fd.get_ref().as_raw_fd(); // Build the dispatch table once at startup. @@ -3304,11 +3404,7 @@ pub async fn supervisor( // still per-child pidfd readiness in `spawn_pid_watcher`. let gc = tokio::spawn(process_index_gc(Arc::clone(&ctx.processes))); - // Bounds the number of in-flight deferred handler futures (see - // `DEFER_MAX_INFLIGHT`). Shared across all notifications this supervisor - // processes. - let defer_sem = Arc::new(tokio::sync::Semaphore::new(DEFER_MAX_INFLIGHT)); - let mut parked = ParkedSet::new(); + let mut parked = ParkedSet::new(Arc::clone(async_fd.get_ref()), ctx.mem_gate.clone()); // Edge-triggered drain: each `readable().await` returns once per // epoll edge, then we drain the kernel queue via `probe_notif_fd` @@ -3319,7 +3415,7 @@ pub async fn supervisor( // // Notifications are processed sequentially (not spawned) to avoid // mutex contention between concurrent handlers. Policy verdicts and - // deferred handlers resolve in `parked` and are answered here too. + // parked handlers resolve in `parked` and are answered here too. 'outer: loop { let mut ready = tokio::select! { ready = async_fd.readable() => match ready { @@ -3327,7 +3423,7 @@ pub async fn supervisor( Err(_) => break 'outer, }, Some(done) = parked.join_next() => { - settle_parked(done, &ctx, fd, &mut parked, &defer_sem).await; + settle_parked(done, &ctx, fd, &mut parked).await; continue; } }; @@ -3335,6 +3431,10 @@ pub async fn supervisor( drop(ready); loop { + // A child that keeps the queue full must not postpone settled parks. + if let Some(done) = parked.try_join_next() { + settle_parked(done, &ctx, fd, &mut parked).await; + } match probe_notif_fd(fd) { NotifFdState::Pending => { let notif = match recv_notif(fd) { @@ -3351,8 +3451,15 @@ pub async fn supervisor( Err(e) if e.raw_os_error() == Some(libc::ENOENT) => continue, Err(_) => break 'outer, }; - handle_notification(notif, &ctx, &dispatch_table, fd, &mut parked, &defer_sem) - .await; + let gate = parked.offload.gate.clone(); + gate.scope(handle_notification( + notif, + &ctx, + &dispatch_table, + fd, + &mut parked, + )) + .await; } NotifFdState::Empty => break, NotifFdState::Terminal => break 'outer, @@ -3444,6 +3551,13 @@ pub(crate) async fn cleanup_pid(ctx: &super::ctx::SupervisorCtx, key: super::sta mod tests { use super::*; + fn test_offload() -> Offload { + Offload { + gate: super::super::gate::MemGate::default(), + listener: Arc::new(std::fs::File::open("/dev/null").unwrap().into()), + } + } + #[test] fn openat_how_matches_what_openat2_accepts() { let o_path = (libc::O_PATH | libc::O_DIRECTORY) as u64; @@ -3785,68 +3899,65 @@ mod tests { #[tokio::test] async fn deferred_future_need_not_be_sync() { - // A deferred future may capture Send-but-not-Sync state across an - // await. `Cell` is Send but never Sync; holding it across `.await` - // makes the future !Sync. Only `Send` is required (the supervisor - // moves the future to a worker, never shares it by reference). use std::cell::Cell; let action = NotifAction::defer(async move { let counter = Cell::new(0); counter.set(counter.get() + 41); - tokio::task::yield_now().await; // hold the !Sync Cell across await + tokio::task::yield_now().await; NotifAction::ReturnValue(counter.get() + 1) }); - let NotifAction::Defer(d) = action else { - panic!("expected Defer") - }; - assert!(matches!(d.run().await, NotifAction::ReturnValue(42))); - } - - #[tokio::test] - async fn deferred_runs_to_its_terminal_action() { - // A Defer carries a future; running it yields the deferred decision. - let action = NotifAction::defer(async { NotifAction::ReturnValue(7) }); let NotifAction::Defer(deferred) = action else { - panic!("defer() must construct a NotifAction::Defer"); + panic!("expected Defer") }; - assert!(matches!(deferred.run().await, NotifAction::ReturnValue(7))); + assert!(matches!(deferred.run().await, NotifAction::ReturnValue(42))); } #[tokio::test(start_paused = true)] - async fn deferred_times_out_to_eio() { - // A deferred future that exceeds its limit must fail closed (EIO) so - // the trapped child gets a definite response instead of parking - // forever (and leaking its DEFER_MAX_INFLIGHT slot). + async fn deferred_past_deadline_fails_with_eio() { let slow = Deferred::new(async { tokio::time::sleep(std::time::Duration::from_secs(60)).await; NotifAction::ReturnValue(7) }); - let action = run_deferred_within(slow, std::time::Duration::from_secs(1)).await; - assert!(matches!(action, NotifAction::Errno(e) if e == libc::EIO)); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(1); + assert!(matches!( + slow.run_until(&test_offload(), deadline).await, + NotifAction::Errno(libc::EIO) + )); } #[tokio::test(start_paused = true)] - async fn deferred_within_limit_passes_through() { - // A future that resolves within the limit returns its terminal action. - let fast = Deferred::new(async { NotifAction::ReturnValue(7) }); - let action = run_deferred_within(fast, std::time::Duration::from_secs(1)).await; - assert!(matches!(action, NotifAction::ReturnValue(7))); + async fn patient_deferral_ignores_deadline() { + let slow = Deferred::patient(async { + tokio::time::sleep(std::time::Duration::from_secs(60)).await; + NotifAction::ReturnValue(7) + }); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(1); + assert!(matches!( + slow.run_until(&test_offload(), deadline).await, + NotifAction::ReturnValue(7) + )); } - #[test] - fn finalize_deferred_collapses_nested_defer_to_eio() { - // A deferred future that itself resolves to Defer is a bug: collapse - // to EIO so the trapped child is never wedged waiting for a response. - let nested = NotifAction::defer(async { NotifAction::Continue }); - assert!(matches!(finalize_deferred(nested), NotifAction::Errno(e) if e == libc::EIO)); - // Non-nested terminal actions pass through unchanged. + #[tokio::test(start_paused = true)] + async fn native_deferral_ignores_deadline() { + let slow = Deferred::native(|| { + std::thread::sleep(std::time::Duration::from_millis(50)); + NotifAction::ReturnValue(7) + }); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_millis(1); assert!(matches!( - finalize_deferred(NotifAction::Continue), - NotifAction::Continue + slow.run_until(&test_offload(), deadline).await, + NotifAction::ReturnValue(7) )); + } + + #[tokio::test] + async fn deferred_resolving_to_unexecuted_work_fails_with_eio() { + let nested = Deferred::new(async { NotifAction::defer(async { NotifAction::Continue }) }); + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(1); assert!(matches!( - finalize_deferred(NotifAction::ReturnValue(3)), - NotifAction::ReturnValue(3) + nested.run_until(&test_offload(), deadline).await, + NotifAction::Errno(libc::EIO) )); } diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index f1c37d9e..20465940 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -393,11 +393,12 @@ pub(crate) fn notif_syscalls_resolved(resolved: &ResolvedSandbox) -> Vec { if features.network_supervision { nrs.extend(NETWORK_POLICY_SYSCALLS); - } else if features.unix_fs_gate && !features.landlock_unix_gate { + } else if features.unix_fs_gate && (!features.landlock_unix_gate || features.chroot) { // Named-unix gate: trap connect() (stream) and sendto()/sendmsg()/ // sendmmsg() (datagram) so reaching a unix socket outside the fs-write // grants is denied, even when no IP network rules are present. Landlock - // before ABI v9 cannot gate this. Handlers bail cheaply on addr-less + // before ABI v9 cannot gate this, and under chroot no Landlock can, + // since the kernel would resolve the address on the host. Handlers bail cheaply on addr-less // (connected) sends. nrs.push(libc::SYS_connect); nrs.push(libc::SYS_sendto); diff --git a/crates/sandlock-core/src/sys/fs.rs b/crates/sandlock-core/src/sys/fs.rs index f70ecfeb..5c645c90 100644 --- a/crates/sandlock-core/src/sys/fs.rs +++ b/crates/sandlock-core/src/sys/fs.rs @@ -489,6 +489,44 @@ pub(crate) fn renameat_in_root(root: &Path, old_rel: &str, new_rel: &str) -> Res } } +/// Move `src` to `rel` confined within `root`, failing with `EEXIST` rather +/// than replacing. NFS, 9p, ceph and ecryptfs reject `RENAME_NOREPLACE` but +/// can link, which is just as atomic; FAT cannot link, so neither alone does. +pub(crate) fn install_in_root(src: &Path, root: &Path, rel: &str) -> Result<(), i32> { + use std::os::unix::ffi::OsStrExt; + let csrc = CString::new(src.as_os_str().as_bytes()).map_err(|_| libc::EINVAL)?; + let (pfd, base) = parent_dir_in_root(root, rel)?; + let result = CString::new(base) + .map_err(|_| libc::EINVAL) + .and_then(|cbase| { + // Not every libc this builds against wraps renameat2. + let rc = unsafe { + libc::syscall( + libc::SYS_renameat2, + libc::AT_FDCWD, + csrc.as_ptr(), + pfd, + cbase.as_ptr(), + libc::RENAME_NOREPLACE, + ) + }; + if rc == 0 { + return Ok(()); + } + match last_errno(libc::EIO) { + libc::EINVAL | libc::ENOSYS => {} + e => return Err(e), + } + if unsafe { libc::linkat(libc::AT_FDCWD, csrc.as_ptr(), pfd, cbase.as_ptr(), 0) } < 0 { + return Err(last_errno(libc::EIO)); + } + unsafe { libc::unlink(csrc.as_ptr()) }; + Ok(()) + }); + unsafe { libc::close(pfd) }; + result +} + /// Create a hard link `new_rel -> old_rel` confined within `root`. The old path /// is not dereferenced (no `AT_SYMLINK_FOLLOW`). pub(crate) fn linkat_in_root(root: &Path, old_rel: &str, new_rel: &str) -> Result<(), i32> { @@ -652,6 +690,27 @@ mod tests { } } + #[test] + fn install_in_root_moves_the_file_and_never_replaces() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().join("root"); + std::fs::create_dir_all(root.join("d")).unwrap(); + let staged = tmp.path().join("staged"); + + std::fs::write(&staged, "first").unwrap(); + install_in_root(&staged, &root, "d/f").unwrap(); + assert_eq!(std::fs::read(root.join("d/f")).unwrap(), b"first"); + assert!(!staged.exists()); + + std::fs::write(&staged, "second").unwrap(); + assert_eq!(install_in_root(&staged, &root, "d/f"), Err(libc::EEXIST)); + assert_eq!(std::fs::read(root.join("d/f")).unwrap(), b"first"); + assert!( + staged.exists(), + "the loser is left for its owner to discard" + ); + } + #[test] fn openat2_in_root_clamps_parent_escape() { let tmp = TempDir::new().unwrap(); diff --git a/crates/sandlock-core/tests/integration/test_chroot.rs b/crates/sandlock-core/tests/integration/test_chroot.rs index 2486510e..3d7d08d8 100644 --- a/crates/sandlock-core/tests/integration/test_chroot.rs +++ b/crates/sandlock-core/tests/integration/test_chroot.rs @@ -2646,3 +2646,110 @@ async fn test_chroot_shebang_loop_is_eloop() { ); cleanup_rootfs(&rootfs); } + +/// Run `rootfs-helper unix-send` inside a chroot granting write on virtual +/// `/tmp`, returning the child's result. +async fn chroot_unix_send( + rootfs: &PathBuf, + kind: &str, + dir: &str, + path: &str, + payload: &str, +) -> sandlock_core::RunResult { + minimal_exec_policy(rootfs) + .fs_write("/tmp") + .build() + .unwrap() + .run(&["rootfs-helper", "unix-send", kind, dir, path, payload]) + .await + .unwrap() +} + +/// Named unix sockets are virtual paths in chroot mode: an absolute target +/// reaches the socket inside the rootfs, and a relative one resolves against +/// the child's virtual cwd. +#[tokio::test] +async fn test_chroot_unix_connect_resolves_in_root_and_cwd() { + use std::io::Read; + use std::os::unix::net::UnixListener; + + let rootfs = build_test_rootfs("unix-connect"); + let listener = UnixListener::bind(rootfs.join("tmp/svc.sock")).unwrap(); + listener.set_nonblocking(true).unwrap(); + + for (dir, path) in [ + ("/", "/tmp/svc.sock"), + ("/tmp", "svc.sock"), + ("/tmp", "./svc.sock"), + ] { + let r = chroot_unix_send(&rootfs, "stream", dir, path, "hello").await; + assert!( + r.success(), + "connect {path} from {dir}: {}", + r.stderr_str().unwrap_or("") + ); + let (mut conn, _) = listener + .accept() + .expect("connect must reach the rootfs socket"); + let mut buf = [0u8; 5]; + conn.read_exact(&mut buf).unwrap(); + assert_eq!(&buf, b"hello"); + } + + cleanup_rootfs(&rootfs); +} + +/// A relative datagram target resolves against the virtual cwd too. +#[tokio::test] +async fn test_chroot_unix_sendto_relative_after_chdir() { + use std::os::unix::net::UnixDatagram; + + let rootfs = build_test_rootfs("unix-sendto"); + let listener = UnixDatagram::bind(rootfs.join("tmp/svc.dgram")).unwrap(); + listener.set_nonblocking(true).unwrap(); + + let r = chroot_unix_send(&rootfs, "dgram", "/tmp", "svc.dgram", "datagram").await; + assert!(r.success(), "sendto: {}", r.stderr_str().unwrap_or("")); + let mut buf = [0u8; 16]; + let n = listener + .recv(&mut buf) + .expect("datagram must reach the rootfs socket"); + assert_eq!(&buf[..n], b"datagram"); + + cleanup_rootfs(&rootfs); +} + +/// The grant is checked on the virtual path the relative target resolves to: +/// `tmp/svc.sock` from cwd `/etc` is `/etc/tmp/svc.sock`, outside the `/tmp` +/// write grant, and must not be mistaken for `/tmp/svc.sock`. +#[tokio::test] +async fn test_chroot_unix_connect_relative_outside_grant_denied() { + use std::os::unix::net::UnixListener; + + let rootfs = build_test_rootfs("unix-deny"); + fs::create_dir_all(rootfs.join("etc/tmp")).unwrap(); + let outside = UnixListener::bind(rootfs.join("etc/tmp/svc.sock")).unwrap(); + outside.set_nonblocking(true).unwrap(); + let _granted = UnixListener::bind(rootfs.join("tmp/svc.sock")).unwrap(); + + let r = chroot_unix_send(&rootfs, "stream", "/etc", "tmp/svc.sock", "hello").await; + assert!(!r.success(), "connect outside the write grant must fail"); + assert!( + outside.accept().is_err(), + "the ungranted socket must not be reached" + ); + + // A symlink inside the grant is judged by the socket it reaches. + std::os::unix::fs::symlink("/etc/tmp/svc.sock", rootfs.join("tmp/link.sock")).unwrap(); + let r = chroot_unix_send(&rootfs, "stream", "/tmp", "link.sock", "hello").await; + assert!( + !r.success(), + "a symlink out of the write grant must be denied" + ); + assert!( + outside.accept().is_err(), + "the ungranted socket must not be reached" + ); + + cleanup_rootfs(&rootfs); +} diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index a172cadb..77cff780 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -1836,6 +1836,11 @@ void sandlock_action_set_kill(sandlock_action_out_t *out, int32_t sig, int32_t p * Allocate a handler container. `handler_fn` must be non-null; passing * `ud_drop = None` is legal when `ud` does not require cleanup. * + * Callbacks run on worker threads, concurrently and off the notification + * loop. A syscall waits for its callback however long it takes, and the + * run waits for every callback before returning, so one that never + * returns prevents the run from completing. + * * # Safety * `ud` is opaque to Rust — the caller guarantees that the pointer * remains valid until either (a) `sandlock_handler_free` is called or @@ -1852,24 +1857,6 @@ sandlock_handler_t *sandlock_handler_new(int32_t (*handler_fn)(void *ud, void (*ud_drop)(void *ud), uint32_t on_exception); -/** - * Mark a handler as deferred: the supervisor will run its callback off the - * notification loop so a slow callback does not block other trapped - * syscalls. Must be called before the handler is registered with a run. - * - * A deferred handler makes a *terminal* decision: deferral short-circuits - * the handler chain, so if a deferred handler would `Continue` and other - * user handlers are registered on the same syscall, those later handlers do - * not run. Builtins always run first regardless. Deferral is refused at - * dispatch time for syscalls whose Continue path needs the execve - * argv-safety freeze (`execve`/`execveat`) or fork creation-tracking. - * - * # Safety - * `h` must be a non-null pointer returned by `sandlock_handler_new` that has - * not yet been registered with a run or freed. - */ -void sandlock_handler_set_deferred(sandlock_handler_t *h, bool deferred); - /** * Free a handler container that has *not* been registered with a * sandbox. After successful registration the supervisor owns the diff --git a/crates/sandlock-ffi/src/handler/abi.rs b/crates/sandlock-ffi/src/handler/abi.rs index 53016875..c7ab4eac 100644 --- a/crates/sandlock-ffi/src/handler/abi.rs +++ b/crates/sandlock-ffi/src/handler/abi.rs @@ -458,18 +458,12 @@ pub struct sandlock_handler_t { pub(super) ud: *mut std::ffi::c_void, pub(super) ud_drop: Option, pub(super) on_exception: sandlock_exception_policy_t, - /// When true, the supervisor runs this handler's callback off the - /// notification loop (as a deferred future) instead of inline, so a - /// slow callback (network round-trip, blocking syscall) does not stall - /// every other trapped syscall. The container is an opaque box — C - /// never sees this field's layout — so adding it is not a C ABI break. - pub(super) deferred: bool, } // Safety: // -// `Send`: required so the supervisor can move the handler container into -// a `tokio::task::spawn_blocking` closure. The struct contains only +// `Send`: required so the supervisor can run the callback on a native +// worker thread. The struct contains only // pointers (function pointer + `void*` user-data) and a `#[repr(u32)]` // enum, all of which are `Send`-safe to move across threads. // @@ -504,6 +498,11 @@ impl Drop for sandlock_handler_t { /// Allocate a handler container. `handler_fn` must be non-null; passing /// `ud_drop = None` is legal when `ud` does not require cleanup. /// +/// Callbacks run on worker threads, concurrently and off the notification +/// loop. A syscall waits for its callback however long it takes, and the +/// run waits for every callback before returning, so one that never +/// returns prevents the run from completing. +/// /// # Safety /// `ud` is opaque to Rust — the caller guarantees that the pointer /// remains valid until either (a) `sandlock_handler_free` is called or @@ -550,33 +549,10 @@ pub unsafe extern "C" fn sandlock_handler_new( ud, ud_drop, on_exception, - deferred: false, }); Box::into_raw(h) } -/// Mark a handler as deferred: the supervisor will run its callback off the -/// notification loop so a slow callback does not block other trapped -/// syscalls. Must be called before the handler is registered with a run. -/// -/// A deferred handler makes a *terminal* decision: deferral short-circuits -/// the handler chain, so if a deferred handler would `Continue` and other -/// user handlers are registered on the same syscall, those later handlers do -/// not run. Builtins always run first regardless. Deferral is refused at -/// dispatch time for syscalls whose Continue path needs the execve -/// argv-safety freeze (`execve`/`execveat`) or fork creation-tracking. -/// -/// # Safety -/// `h` must be a non-null pointer returned by `sandlock_handler_new` that has -/// not yet been registered with a run or freed. -#[no_mangle] -pub unsafe extern "C" fn sandlock_handler_set_deferred(h: *mut sandlock_handler_t, deferred: bool) { - if h.is_null() { - return; - } - (*h).deferred = deferred; -} - /// Free a handler container that has *not* been registered with a /// sandbox. After successful registration the supervisor owns the /// handler; calling this on a registered handler is undefined behaviour diff --git a/crates/sandlock-ffi/src/handler/adapter.rs b/crates/sandlock-ffi/src/handler/adapter.rs index 5131e5cd..7cabd96f 100644 --- a/crates/sandlock-ffi/src/handler/adapter.rs +++ b/crates/sandlock-ffi/src/handler/adapter.rs @@ -7,6 +7,7 @@ use std::future::Future; use std::os::unix::io::FromRawFd; use std::pin::Pin; +use std::sync::{Arc, RwLock}; use sandlock_core::seccomp::dispatch::{Handler, HandlerCtx}; use sandlock_core::seccomp::notif::NotifAction; @@ -44,7 +45,23 @@ pub(crate) const UNSAFE_PGID: i32 = i32::MIN; /// `Handler`. Constructed when the supervisor accepts handlers passed /// through `sandlock_run_with_handlers`. pub struct FfiHandler { - inner: Box, + callback: Arc, + on_exception: sandlock_exception_policy_t, +} + +/// A callback can outlive the notification that started it, but not the +/// FFI run: the caller's userdata may be borrowed only until the run returns. +struct Callback(RwLock>>); + +/// Held for an FFI run. Dropping it waits for running callbacks, then +/// releases the container so queued callbacks find nothing to call. +pub(super) struct RunGuard(Arc); + +impl Drop for RunGuard { + fn drop(&mut self) { + let handler = self.0 .0.write().unwrap_or_else(|e| e.into_inner()).take(); + drop(handler); + } } impl FfiHandler { @@ -57,13 +74,19 @@ impl FfiHandler { /// supervisor owns the container. pub unsafe fn from_raw(raw: *mut sandlock_handler_t) -> Self { assert!(!raw.is_null(), "FfiHandler::from_raw on null pointer"); + let handler = Box::from_raw(raw); Self { - inner: Box::from_raw(raw), + on_exception: handler.on_exception, + callback: Arc::new(Callback(RwLock::new(Some(handler)))), } } + pub(super) fn run_guard(&self) -> RunGuard { + RunGuard(Arc::clone(&self.callback)) + } + fn exception_action(&self, child_pgid: i32) -> NotifAction { - match self.inner.on_exception { + match self.on_exception { sandlock_exception_policy_t::Kill => { if child_pgid == UNSAFE_PGID { // No safe pgid is available (nested PID namespace, @@ -88,23 +111,11 @@ impl FfiHandler { } } -/// `Send`-only wrapper around the C user-data pointer so it can travel -/// into `spawn_blocking`. Only the move (not sharing across threads) is -/// required; the deeper Send/Sync rationale for the underlying handler -/// container lives on `sandlock_handler_t`. -struct UdPtr(*mut std::ffi::c_void); -// Safety: ud is opaque to Rust; the spawn_blocking pipeline only moves -// (not shares) the wrapper. See `sandlock_handler_t` for the deeper -// Send/Sync rationale that justifies the underlying handler container. -unsafe impl Send for UdPtr {} - impl Handler for FfiHandler { fn handle<'a>( &'a self, cx: &'a HandlerCtx, ) -> Pin + Send + 'a>> { - // Capture the pieces we need by value so spawn_blocking can run - // the C callback on a worker thread without &self lifetime games. let notif_snap = crate::notif_repr::sandlock_notif_data_t::from(&cx.notif); let notif_fd = cx.notif_fd; let notif_id = cx.notif.id; @@ -156,82 +167,47 @@ impl Handler for FfiHandler { } } }; - let handler_fn = self.inner.handler_fn; - let ud = UdPtr(self.inner.ud); + let callback = Arc::clone(&self.callback); let on_exception_fallback = self.exception_action(child_pgid); - let deferred = self.inner.deferred; - // The whole callback (including its slow work) runs on a blocking - // worker. When `deferred` is set we hand this future to the - // supervisor as `NotifAction::Defer` so it runs off the notification - // loop; otherwise the supervisor awaits it inline as before. The - // future is fully owned (`'static`) either way — nothing borrows - // `self`/`cx`. - let run = async move { - let join = tokio::task::spawn_blocking(move || { - // Rust 2021 disjoint closure captures (RFC 2229) would - // otherwise capture `ud.0` (a bare `*mut c_void`, not - // `Send`) rather than the whole `UdPtr`. Binding `ud` to - // a fresh local at the top of the closure forces a - // whole-struct capture so the `Send` impl on `UdPtr` - // applies to the outer closure. - let ud = ud; - let UdPtr(ud_raw) = ud; - let mut mem = sandlock_mem_handle_t::new(notif_fd, notif_id, pid); - let mut out = sandlock_action_out_t::zeroed(); - let rc = match handler_fn { - Some(f) => std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - f(ud_raw, ¬if_snap, &mut mem, &mut out) - })), - None => Ok(-1), - }; - (rc, out) - }) - .await; - - let (rc_or_panic, out) = match join { - Ok(pair) => pair, - Err(_join_err) => return on_exception_fallback, + // A callback may wait on anything, so it never runs on the loop. + let action = NotifAction::native(move || { + let handler = callback.0.read().unwrap_or_else(|e| e.into_inner()); + let Some(handler) = handler.as_ref() else { + return NotifAction::Errno(libc::EIO); }; - - match rc_or_panic { + let mut mem = sandlock_mem_handle_t::new(notif_fd, notif_id, pid); + let mut out = sandlock_action_out_t::zeroed(); + let rc = match handler.handler_fn { + Some(f) => std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + f(handler.ud, ¬if_snap, &mut mem, &mut out) + })), + None => Ok(-1), + }; + match rc { Ok(0) => match translate_action(&out, child_pgid) { Some(action) => action, None => { // Action kind ended up Unset, unknown, or the - // reserved InjectFdSendTracked discriminant. - // Drain any inject-fd payload before falling - // back to the exception policy — otherwise the - // supervisor leaks the srcfd that was armed by - // the (failed) callback. + // reserved InjectFdSendTracked discriminant. Drain + // any inject-fd payload the callback armed before + // falling back, or its srcfd leaks. // SAFETY: `drain_pending_inject_fd` inspects - // `out.kind` itself before touching the union, - // and `out.kind` matches the union variant per - // the action setters' contract. + // `out.kind` itself before touching the union. unsafe { drain_pending_inject_fd(&out) }; on_exception_fallback } }, _ => { - // Either the callback returned a non-zero rc OR - // `catch_unwind` caught a panic. The callback may - // have armed an InjectFdSend{,Tracked} payload - // before failing; drain it so its srcfd doesn't - // leak in the supervisor. + // A non-zero rc or a caught panic may still have armed + // an inject-fd payload; drain it so its srcfd doesn't leak. // SAFETY: see the `Ok(0) -> None` branch above. unsafe { drain_pending_inject_fd(&out) }; on_exception_fallback } } - }; - - if deferred { - // Resolves immediately to Defer; the supervisor moves `run` onto - // a worker task and sends the response when it completes. - Box::pin(async move { NotifAction::defer(run) }) - } else { - Box::pin(run) - } + }); + Box::pin(async move { action }) } } @@ -358,3 +334,113 @@ fn translate_action(out: &sandlock_action_out_t, child_pgid: i32) -> Option>>, + release: Mutex>, + } + + extern "C-unwind" fn blocking_callback( + ud: *mut std::ffi::c_void, + _: *const crate::notif_repr::sandlock_notif_data_t, + _: *mut sandlock_mem_handle_t, + out: *mut sandlock_action_out_t, + ) -> i32 { + let gate = unsafe { &*ud.cast::() }; + if let Some(started) = gate.started.lock().unwrap().take() { + let _ = started.send(()); + } + let _ = gate.release.lock().unwrap().recv(); + unsafe { sandlock_action_set_return_value(out, 42) }; + 0 + } + + fn handler(gate: &Gate) -> FfiHandler { + let raw = unsafe { + sandlock_handler_new( + Some(blocking_callback), + (gate as *const Gate).cast_mut().cast(), + None, + sandlock_exception_policy_t::DenyEio as u32, + ) + }; + unsafe { FfiHandler::from_raw(raw) } + } + + fn context() -> HandlerCtx { + let notif = SeccompNotif { + id: 1, + pid: 0, + flags: 0, + data: SeccompData { + nr: libc::SYS_getpid as i32, + arch: 0, + instruction_pointer: 0, + args: [0; 6], + }, + }; + HandlerCtx::new(notif, -1) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn run_guard_waits_for_an_abandoned_callback() { + let (started_tx, started_rx) = mpsc::channel(); + let (release_tx, release_rx) = mpsc::channel(); + let gate = Gate { + started: Mutex::new(Some(started_tx)), + release: Mutex::new(release_rx), + }; + let h = handler(&gate); + let guard = h.run_guard(); + let NotifAction::Defer(deferred) = h.handle(&context()).await else { + panic!("expected a deferral"); + }; + let waiter = tokio::spawn(deferred.run()); + started_rx.recv().unwrap(); + waiter.abort(); + let _ = waiter.await; + drop(h); + + let (done_tx, done_rx) = mpsc::channel(); + let drain = std::thread::spawn(move || { + drop(guard); + done_tx.send(()).unwrap(); + }); + let returned_early = done_rx + .recv_timeout(std::time::Duration::from_millis(50)) + .is_ok(); + release_tx.send(()).unwrap(); + drain.join().unwrap(); + assert!( + !returned_early, + "the run returned while userdata was in use" + ); + } + + #[tokio::test] + async fn callbacks_queued_past_the_run_do_not_touch_userdata() { + let (_release_tx, release_rx) = mpsc::channel(); + let gate = Gate { + started: Mutex::new(None), + release: Mutex::new(release_rx), + }; + let h = handler(&gate); + let NotifAction::Defer(deferred) = h.handle(&context()).await else { + panic!("expected a deferral"); + }; + drop(h.run_guard()); + assert!(matches!( + deferred.run().await, + NotifAction::Errno(libc::EIO) + )); + } +} diff --git a/crates/sandlock-ffi/src/handler/run.rs b/crates/sandlock-ffi/src/handler/run.rs index 0ed5a4e2..3a145655 100644 --- a/crates/sandlock-ffi/src/handler/run.rs +++ b/crates/sandlock-ffi/src/handler/run.rs @@ -116,6 +116,7 @@ fn block_on_run( // `crate::runtime` for why this is `current_thread`. This path is // reached from `extern "C-unwind"` entry points, so user callback // panics are intentionally allowed to propagate. + let _callbacks: Vec<_> = handlers.iter().map(|(_, h)| h.run_guard()).collect(); crate::runtime::with_runtime_unwind(|rt| { rt.block_on(async move { if interactive { diff --git a/crates/sandlock-ffi/tests/handler_smoke.rs b/crates/sandlock-ffi/tests/handler_smoke.rs index ee6498c4..7dfb94cf 100644 --- a/crates/sandlock-ffi/tests/handler_smoke.rs +++ b/crates/sandlock-ffi/tests/handler_smoke.rs @@ -280,6 +280,14 @@ use sandlock_core::seccomp::notif::NotifAction; use sandlock_core::{SeccompData, SeccompNotif}; use sandlock_ffi::handler::FfiHandler; +/// What the supervisor would answer once the handler's deferral resolves. +async fn resolve(h: &FfiHandler, cx: &HandlerCtx) -> NotifAction { + match h.handle(cx).await { + NotifAction::Defer(deferred) => deferred.run().await, + decided => decided, + } +} + fn fake_ctx() -> HandlerCtx { HandlerCtx::new( SeccompNotif { @@ -394,15 +402,12 @@ async fn ffi_handler_translates_return_value() { }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; assert!(matches!(action, NotifAction::ReturnValue(42))); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn deferred_ffi_handler_returns_defer_that_resolves_to_callback() { - // A handler flagged deferred must return NotifAction::Defer (so the - // supervisor runs it off-loop), and driving that future must yield the - // same action the C callback would have produced inline. +async fn ffi_handler_defers_its_callback_off_the_loop() { let raw = unsafe { sandlock_ffi::handler::sandlock_handler_new( Some(return_value_42), @@ -411,33 +416,14 @@ async fn deferred_ffi_handler_returns_defer_that_resolves_to_callback() { sandlock_exception_policy_t::Kill as u32, ) }; - unsafe { sandlock_ffi::handler::sandlock_handler_set_deferred(raw, true) }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; - let deferred = match action { - NotifAction::Defer(d) => d, - other => panic!("deferred handler must return Defer, got {other:?}"), + let NotifAction::Defer(deferred) = h.handle(&cx).await else { + panic!("a C callback must not run on the notification loop"); }; assert!(matches!(deferred.run().await, NotifAction::ReturnValue(42))); } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn non_deferred_ffi_handler_stays_inline() { - // Without the flag, handle() resolves the action inline (no Defer). - let raw = unsafe { - sandlock_ffi::handler::sandlock_handler_new( - Some(return_value_42), - std::ptr::null_mut(), - None, - sandlock_exception_policy_t::Kill as u32, - ) - }; - let h = unsafe { FfiHandler::from_raw(raw) }; - let cx = fake_ctx(); - assert!(matches!(h.handle(&cx).await, NotifAction::ReturnValue(42))); -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn ffi_handler_applies_exception_policy_on_failure() { let raw = unsafe { @@ -450,7 +436,7 @@ async fn ffi_handler_applies_exception_policy_on_failure() { }; let h = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; assert!(matches!(action, NotifAction::Errno(e) if e == libc::EPERM)); } @@ -709,7 +695,7 @@ async fn ffi_handler_translates_continue() { // Safety: `raw` was just produced by `sandlock_handler_new` and is // non-null; ownership transfers into the adapter. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Continue)); } @@ -725,7 +711,7 @@ async fn ffi_handler_translates_errno() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Errno(e) if e == 13)); } @@ -741,7 +727,7 @@ async fn ffi_handler_translates_hold() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Hold)); } @@ -757,7 +743,7 @@ async fn ffi_handler_translates_kill_with_explicit_pgid() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!(matches!( action, NotifAction::KillGroup { sig, pgid } if sig == libc::SIGTERM && pgid == 1234 @@ -839,7 +825,7 @@ async fn ffi_handler_translates_kill_zero_pgid_substitutes_child_pgid() { }, -1, ); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; // Reap the child regardless of assertion outcome. let _ = child.kill(); @@ -911,7 +897,7 @@ async fn k1_pgid_resolution_rejects_pid_zero() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(0); - let action = handler.handle(&cx).await; + let action = resolve(&handler, &cx).await; assert!( matches!(action, NotifAction::Continue), "expected exception-policy fallback (Continue) when no safe pgid available, got {action:?}", @@ -965,7 +951,7 @@ async fn k1_no_supervisor_signal_on_pid_zero_kill() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(0); // pid=0 -> UNSAFE_PGID - let action = handler.handle(&cx).await; + let action = resolve(&handler, &cx).await; // The action must be Continue (exception-policy fallback), NOT a // Kill that send_response would forward to killpg(0). @@ -1019,7 +1005,7 @@ async fn k2_pgid_resolution_rejects_supervisor_pgid_match() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(child_pid as u32); - let action = handler.handle(&cx).await; + let action = resolve(&handler, &cx).await; // Reap the child regardless of assertion outcome. let _ = child.kill(); @@ -1054,7 +1040,7 @@ async fn k3_pgid_resolution_falls_back_on_esrch() { }; let handler = unsafe { FfiHandler::from_raw(raw) }; let cx = fake_ctx_with_pid(dead_pid); - let action = handler.handle(&cx).await; + let action = resolve(&handler, &cx).await; assert!( matches!(action, NotifAction::Continue), "expected exception-policy fallback (Continue) on ESRCH, got {action:?}", @@ -1081,7 +1067,7 @@ async fn ffi_handler_kill_policy_on_callback_rc_nonzero() { // suicide, and the assertion below would not exercise the // Kill-path the test exists to cover. let (cx, mut child) = fake_ctx_with_isolated_child(); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; let _ = child.kill(); let _ = child.wait(); assert!(matches!(action, NotifAction::KillGroup { sig, .. } if sig == libc::SIGKILL)); @@ -1099,7 +1085,7 @@ async fn ffi_handler_continue_policy_on_callback_rc_nonzero() { }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!(matches!(action, NotifAction::Continue)); } @@ -1136,7 +1122,7 @@ async fn ffi_handler_recovers_from_callback_panic() { // Use an isolated child so the Kill exception policy is observable // (rationale identical to `ffi_handler_kill_policy_on_callback_rc_nonzero`). let (cx, mut child) = fake_ctx_with_isolated_child(); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; let _ = child.kill(); let _ = child.wait(); // The `catch_unwind` inside `spawn_blocking` swallows the panic and @@ -1167,7 +1153,7 @@ async fn ffi_handler_callback_returns_zero_but_never_sets_action_triggers_fallba }; // Safety: see `ffi_handler_translates_continue`. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; // `translate_action` returns `None` for `Unset`, which routes the // dispatcher onto the exception policy fallback. assert!(matches!(action, NotifAction::Errno(e) if e == libc::EPERM)); @@ -2028,7 +2014,7 @@ async fn a1_ffi_handler_drains_inject_fd_on_panic() { }; // SAFETY: `raw` was just produced and is non-null. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EPERM), "panic must route to the exception-policy fallback (Kill degraded to EPERM under UNSAFE_PGID), got {action:?}", @@ -2103,7 +2089,7 @@ async fn a2_ffi_handler_drains_inject_fd_tracked_discriminant() { }; // SAFETY: `raw` was just produced and is non-null. let h = unsafe { FfiHandler::from_raw(raw) }; - let action = h.handle(&fake_ctx()).await; + let action = resolve(&h, &fake_ctx()).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EPERM), "unsupported tracked discriminant must route to the exception-policy fallback (Kill degraded to EPERM under UNSAFE_PGID), got {action:?}", @@ -2318,7 +2304,7 @@ async fn ffi_handler_deny_eio_policy_on_callback_rc_nonzero() { }; let h = unsafe { sandlock_ffi::handler::FfiHandler::from_raw(raw) }; let cx = fake_ctx(); - let action = h.handle(&cx).await; + let action = resolve(&h, &cx).await; assert!( matches!(action, NotifAction::Errno(e) if e == libc::EIO), "expected Errno(EIO), got {:?}", diff --git a/crates/sandlock-fixtures/rootfs-helper.c b/crates/sandlock-fixtures/rootfs-helper.c index abe0659c..ad46e694 100644 --- a/crates/sandlock-fixtures/rootfs-helper.c +++ b/crates/sandlock-fixtures/rootfs-helper.c @@ -19,8 +19,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -877,10 +879,41 @@ static int cmd_fexecve(int argc, char **argv) { return 1; } +/* ── unix-send (deliver a payload to a named unix socket) ───── */ +/* + * unix-send stream|dgram : chdir() so a relative + * has a cwd to resolve against, then connect+send (stream) or sendto + * (dgram) . + */ +static int cmd_unix_send(int argc, char **argv) { + if (argc < 4) { fprintf(stderr, "unix-send: need stream|dgram \n"); return 1; } + int dgram = strcmp(argv[0], "dgram") == 0; + if (chdir(argv[1]) != 0) { fprintf(stderr, "unix-send: chdir %s: %s\n", argv[1], strerror(errno)); return 1; } + struct sockaddr_un sun = { .sun_family = AF_UNIX }; + strncpy(sun.sun_path, argv[2], sizeof(sun.sun_path) - 1); + int s = socket(AF_UNIX, dgram ? SOCK_DGRAM : SOCK_STREAM, 0); + if (s < 0) { fprintf(stderr, "unix-send: socket: %s\n", strerror(errno)); return 1; } + size_t len = strlen(argv[3]); + ssize_t n; + if (dgram) { + n = sendto(s, argv[3], len, 0, (struct sockaddr *)&sun, sizeof(sun)); + } else { + if (connect(s, (struct sockaddr *)&sun, sizeof(sun)) != 0) { + fprintf(stderr, "unix-send: connect %s: %s\n", argv[2], strerror(errno)); + return 1; + } + n = send(s, argv[3], len, 0); + } + if (n != (ssize_t)len) { fprintf(stderr, "unix-send: send %s: %s\n", argv[2], strerror(errno)); return 1; } + close(s); + return 0; +} + /* ── dispatch ───────────────────────────────────────────────── */ static int dispatch(const char *cmd, int argc, char **argv) { if (strcmp(cmd, "chdir") == 0) return cmd_chdir(argc, argv); + if (strcmp(cmd, "unix-send") == 0) return cmd_unix_send(argc, argv); if (strcmp(cmd, "fchdir") == 0) return cmd_fchdir(argc, argv); if (strcmp(cmd, "openat2") == 0) return cmd_openat2(argc, argv); if (strcmp(cmd, "chdir-self") == 0) return cmd_chdir_self(argc, argv); diff --git a/docs/extension-handlers.md b/docs/extension-handlers.md index cb207241..54a931d3 100644 --- a/docs/extension-handlers.md +++ b/docs/extension-handlers.md @@ -377,7 +377,7 @@ The contract is exercised at two layers: | `InjectFd { srcfd, targetfd }` | Inject `srcfd` into the guest at slot `targetfd`, then continue. | | `InjectFdSendTracked { srcfd, newfd_flags, on_success }` | Inject `srcfd`; `on_success` callback runs synchronously when the kernel returns the slot, so downstream tracking cannot race with the guest seeing the new fd. | | `Kill { sig, pgid }` | Send `sig` to the guest's process group. | -| `Defer(Deferred)` | Run the carried future off the supervisor loop; its terminal action is sent later, keyed by `notif.id`. Non-`Continue`, so it ends the chain. See [Deferred handlers](#deferred-handlers). | +| `Defer(Deferred)` | Run the carried future off the supervisor loop. A `Continue` result resumes the chain at the next handler; any other result is the decision. See [Deferred handlers](#deferred-handlers). | `InjectFd`/`InjectFdSend`/`InjectFdSendTracked` carry a file descriptor. To synthesise *content* (rather than inject an fd you already hold), construct the action with @@ -464,13 +464,13 @@ user handlers. ### Deferred handlers -Continue-site safety exists because the supervisor processes notifications sequentially: a handler -that blocks (a network round-trip, a blocking syscall, a slow lock) stalls every other trapped -syscall until it returns. `NotifAction::Defer` is the escape hatch. A handler that returns `Defer` -hands the supervisor an owned, `'static` future; the supervisor moves it onto a worker task, lets -the notification loop proceed immediately, and sends the response (keyed by `notif.id`) when the -future resolves. The trapped child stays parked in the syscall until then, so `notif.id` stays -valid and the child-memory helpers keep working inside the deferred future. +The supervisor processes notifications sequentially: a handler that blocks (a network round-trip, +a blocking syscall, a slow lock) stalls every other trapped syscall until it returns. +`NotifAction::Defer` is the escape hatch. A handler that returns `Defer` hands the supervisor an +owned, `'static` future; the supervisor runs it on a task, lets the notification loop proceed, and +picks the notification up again when the future resolves. The trapped child stays blocked in the +syscall until then, so `notif.id` stays valid and the child-memory helpers keep working inside the +deferred future. ```rust use std::future::Future; @@ -482,17 +482,14 @@ fn handle<'a>( &'a self, cx: &'a HandlerCtx, ) -> Pin + Send + 'a>> { - // Copy out what the deferred future needs: `notif` is `Copy`, `notif_fd` - // is a `RawFd`, and `Arc` state is cloned. Never borrow `&self`/`cx`: - // the deferred future is `'static` and outlives this call. + // The deferred future is `'static` and outlives this call: copy `notif`, + // clone `Arc` state, never borrow `&self`/`cx`. let (fd, id, pid) = (cx.notif_fd, cx.notif.id, cx.notif.pid); let key = read_child_cstr(fd, id, pid, cx.notif.data.args[1], 4096); let backend = self.backend.clone(); Box::pin(async move { let Some(key) = key else { return NotifAction::Continue }; NotifAction::defer(async move { - // Runs on a worker, off the supervisor loop. The child is still - // parked, so child-memory helpers and `id` are valid here. match backend.get(&key).await { Ok(_data) => NotifAction::ReturnValue(0), // e.g. inject a memfd of `_data` Err(_) => NotifAction::Errno(libc::EIO), @@ -502,30 +499,34 @@ fn handle<'a>( } ``` -The deferred future must be `Send + 'static`: `Send` so the supervisor can move it onto a worker, -and `'static` so it can outlive the borrowed `HandlerCtx`. It does **not** need to be `Sync` (the -supervisor moves it, never shares it by reference). Capture owned data only: copy `notif` (it is -`Copy`), clone an `Arc` for shared state, and never borrow `&self`/`cx`. +The deferred future must be `Send + 'static`; it does **not** need to be `Sync` (the supervisor +moves it, never shares it by reference). + +For synchronous work (a blocking syscall, a C library call), return +`NotifAction::native(move || ...)` instead of wrapping it in a future. The closure runs on a +native thread and cannot be cancelled, so it has no deadline: the syscall waits for it as it would +for the kernel. It may still outlive its syscall if the child dies, so capture owned resources +rather than raw fd numbers or pointers. Contract: -- **Terminal decision.** `Defer` is non-`Continue`, so it short-circuits the handler chain exactly - like `Errno`/`ReturnValue`: later handlers on the same syscall do not run. A deferring handler - decides the outcome. -- **No deferral on freeze/fork syscalls.** Deferral is refused (with `EPERM`) on - `execve`/`execveat` and fork-creating syscalls, because moving the response off-loop would skip - the argv-safety freeze (see [issue #27][i27]) and process creation-tracking that those paths - require before `Continue`. -- **Bounded fan-out.** At most `DEFER_MAX_INFLIGHT` deferred futures run concurrently; beyond that, - further deferrals fail fast with `EAGAIN` rather than queuing. The cap also bounds the resources - workers hold (memfds, sockets). -- **No nesting.** A deferred future that itself resolves to `Defer` is a bug; the supervisor - collapses it to `EIO` so the child is never left wedged. -- **Stale id.** If the child exits mid-defer, the eventual `send_response` is a no-op and the - child-memory helpers fail safe (they are `id_valid`-bracketed), matching the inline path's - "child may have exited" tolerance. - -Do not defer trivial fast handlers: the worker hop adds latency. Defer only when the work would +- **Resumes the chain.** A `Continue` result runs the next handler on the same syscall; any + other result ends the chain. Either way the outcome then passes every check a handler's inline + result would: pinned opens, fork accounting, the policy callback and the execve argv-safety + freeze (see [issue #27][i27]). Deferring on `execve` or `fork` is therefore safe. +- **Writes wait for exec.** Child-memory writes from a deferred future or native closure are ordered + against the execve argv-safety freeze: an exec holds them off until the kernel has copied its + argv. Write from the deferred future or closure itself; a task you spawn from it is not ordered. +- **Deadline.** All deferred futures of one notification share a 30 second deadline; past it the + syscall fails with `EIO`. Native closures have none. +- **No admission limit.** Each trapped task has at most one outstanding notification, so deferred + work is already bounded by the number of tasks in the sandbox. +- **No nesting.** A deferred future that resolves to another `Defer` is a bug; the supervisor turns it + into `EIO` so the child is never left wedged. +- **Stale id.** If the child exits mid-defer, the chain is dropped and the child-memory helpers + fail safe (they are `id_valid`-bracketed). + +Do not defer trivial fast handlers: the task hop adds latency. Defer only when the work would otherwise block the loop. ## Security boundary diff --git a/docs/python-handlers.md b/docs/python-handlers.md index fc46c336..2a4ff96a 100644 --- a/docs/python-handlers.md +++ b/docs/python-handlers.md @@ -22,9 +22,8 @@ sb.run_with_handlers( ## Core types - `Handler` — base class. Subclass and override `handle(ctx) -> NotifAction`. - Define `handle` as `async def` to run it off the supervisor loop so it can - `await` slow work without stalling other trapped syscalls; see - [Deferred handlers](#deferred-handlers). Set the class attribute + `handle` runs on a worker thread off the supervisor loop and may be + `async def`; see [Deferred handlers](#deferred-handlers). Set the class attribute `on_exception` (default `ExceptionPolicy.KILL`) to choose what the supervisor does when the handler errors. - `HandlerCtx` — frozen dataclass with the notification fields (`id`, `pid`, @@ -265,13 +264,10 @@ syscall list rather than against an empty dispatch table. - **GIL contention.** Each handler dispatch holds the GIL for the duration of `handle()`. The supervisor may dispatch handler - callbacks concurrently across different notifications, so design - `handle()` to be fast (sub-millisecond) and to protect any mutable - handler state with your own synchronization. High-frequency + callbacks concurrently across different notifications, so protect any + mutable handler state with your own synchronization. High-frequency interception (e.g. per-`SYS_openat` audit on a busy workload) will - serialize on the GIL and can stall the supervisor. For a `handle()` - that must do slow work (a network call, a blocking read), define it as - `async def` so it runs off the loop instead; see + serialize on the GIL and slow down the sandbox's trapped syscalls. See [Deferred handlers](#deferred-handlers). - **Interpreter finalization.** If `Py_FinalizeEx` runs while the @@ -294,12 +290,10 @@ syscall list rather than against an empty dispatch table. ## Deferred handlers -By default `handle()` runs synchronously inside the supervisor's -notification loop, so a slow callback stalls every other trapped syscall -until it returns. Define `handle` as `async def` to run it off that loop -instead: the coroutine is driven to completion on a worker thread, so it -can `await` slow work without blocking the supervisor. That is the only -change; you do not set any flag. +Every `handle()` runs on a worker thread, off the supervisor's notification +loop, so a slow callback delays only its own syscall. Define `handle` as +`async def` to `await` slow work; the coroutine is driven to completion on +that worker thread. You do not set any flag. ```python class FetchHandler(Handler): @@ -310,39 +304,30 @@ class FetchHandler(Handler): key = ctx.read_path() # read the path before the slow work if key is None: return NotifAction.continue_() - data = await self.backend.get(key) # slow network GET, awaited off-loop + data = await self.backend.get(key) # slow network GET # inject_bytes builds the sealed memfd and transfers fd ownership to # the supervisor (see "Inject synthetic content" above). return NotifAction.inject_bytes(data) ``` -Why it helps Python specifically: the coroutine runs on a worker thread, -and CPython releases the GIL while it `await`s I/O, so multiple async -handlers doing network work genuinely overlap while the supervisor loop -stays free. It does not parallelize CPU-bound Python work (the GIL still -serializes that); for that, push the hot path into a C extension that -releases the GIL. +CPython releases the GIL while a coroutine `await`s I/O, so multiple async +handlers doing network work overlap. It does not parallelize CPU-bound +Python work (the GIL still serializes that); for that, push the hot path +into a C extension that releases the GIL. `ctx` and its child-memory accessors (`read_cstr`, `read`, `write`) stay -valid for the whole coroutine, so you can read paths and write results -across `await` points. +valid for the whole call, so you can read paths and write results across +`await` points. Contract (enforced by the supervisor): -- **Terminal decision.** Deferral short-circuits the handler chain, so if - an async handler returns `continue_()` and other handlers are registered - on the same syscall, those later handlers do not run. Built-ins always - run first regardless. -- **Refused on `execve`/`execveat` and fork-creating syscalls.** The - response cannot be moved off-loop there without skipping argv-safety - work, so such a call is denied with `EPERM`. -- **Bounded.** Beyond an internal in-flight cap, further deferrals fail - fast with `EAGAIN` rather than queuing. - -Make `handle` async only when it must do slow work. For fast handlers -(audit counters, path checks) a synchronous `handle` runs inline at lower -latency. See -[`extension-handlers.md`](extension-handlers.md#deferred-handlers) for the +- **Same chain as any handler.** Returning `continue_()` hands the syscall + to the next handler registered on it, and the result still passes the + execve argv-safety checks and fork accounting. +- **No deadline.** A syscall waits for its handler to return, however + long that takes, and the run waits for every handler before completing. + +See [`extension-handlers.md`](extension-handlers.md#deferred-handlers) for the underlying Rust/C contract. ## Ownership rules diff --git a/python/src/sandlock/_handler_ffi.py b/python/src/sandlock/_handler_ffi.py index 42e83519..beda761f 100644 --- a/python/src/sandlock/_handler_ffi.py +++ b/python/src/sandlock/_handler_ffi.py @@ -129,11 +129,9 @@ def _trampoline_impl(ud, notif_ptr, mem_ptr, out_ptr) -> int: action = handler.handle(ctx) if inspect.iscoroutine(action): # `async def handle`: drive the coroutine to completion on - # this worker thread. Async handlers are always deferred (see - # sandbox._is_deferred_handler), so this runs off the - # supervisor loop and `await` does not stall other syscalls. - # The mem handle stays valid until the finally below, so - # child-memory access inside the coroutine works; awaits + # this worker thread, which is already off the supervisor + # loop. The mem handle stays valid until the finally below, + # so child-memory access inside the coroutine works; awaits # release the GIL, so concurrent async handlers overlap. action = asyncio.run(action) except BaseException: diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index ad3d6d5a..2dfcf3d4 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -655,9 +655,6 @@ class _SandlockHandlerRegistration(ctypes.Structure): _lib.sandlock_handler_free.restype = None _lib.sandlock_handler_free.argtypes = [_c_handler_p] -_lib.sandlock_handler_set_deferred.restype = None -_lib.sandlock_handler_set_deferred.argtypes = [_c_handler_p, ctypes.c_bool] - _lib.sandlock_run_with_handlers.restype = _c_result_p _lib.sandlock_run_with_handlers.argtypes = [ _c_policy_p, ctypes.c_char_p, diff --git a/python/src/sandlock/handler.py b/python/src/sandlock/handler.py index 371f7aaa..f2dd07ac 100644 --- a/python/src/sandlock/handler.py +++ b/python/src/sandlock/handler.py @@ -179,23 +179,13 @@ class Handler: guard it with your own synchronization — the wrapper does not serialize handler dispatch. - Promptness: ``handle()`` must return quickly. It runs synchronously - inside the supervisor's dispatch path while holding the GIL; a - handler that blocks (a long sleep, a blocking I/O call, an infinite - loop) stalls the supervisor and can wedge the entire run. - - Deferral: define ``handle`` as ``async def`` to run it off the - supervisor's notification loop, so a slow handler (a network round-trip, - a blocking call) does not stall every other trapped syscall. The - coroutine is driven to completion on a worker thread, and for I/O-bound - work multiple async handlers overlap (the GIL is released during - blocking I/O). Trade-offs: an async handler makes a *terminal* - decision: deferral short-circuits the handler chain, so if it returns - ``continue_()`` and other handlers are registered on the same syscall, - those later handlers do not run. Deferral is refused for - ``execve``/``execveat`` and fork-creating syscalls (the response cannot - be moved off-loop without skipping argv-safety work); such a call is - denied with EPERM. + Deferral: every ``handle()`` runs on a worker thread, off the + supervisor's notification loop, so a slow handler (a network + round-trip, a blocking call) delays only its own syscall. ``handle`` + may also be ``async def``; the coroutine is driven to completion on + that worker thread. Returning ``continue_()`` hands the syscall to the + next handler registered on it. The syscall waits for the handler + however long it takes, and the run waits for it before completing. """ on_exception: ExceptionPolicy = ExceptionPolicy.KILL diff --git a/python/src/sandlock/sandbox.py b/python/src/sandlock/sandbox.py index dd939566..944f2c62 100644 --- a/python/src/sandlock/sandbox.py +++ b/python/src/sandlock/sandbox.py @@ -8,7 +8,6 @@ from __future__ import annotations -import inspect import itertools import os import re @@ -954,10 +953,6 @@ def run_with_handlers( "sandlock_handler_new returned NULL for syscall " f"{syscall_nr}" ) - # An async `handle` runs off the supervisor loop. Flag the - # container before it is handed to the run. - if _is_deferred_handler(handler): - _lib.sandlock_handler_set_deferred(container, True) regs[i].syscall_nr = int(syscall_nr) regs[i].handler = container # Ownership now lives in regs[i]; clear the pending ref so @@ -1542,16 +1537,6 @@ def _encode(s: str) -> bytes: return result -def _is_deferred_handler(handler) -> bool: - """Whether a handler runs off the supervisor loop. - - True when its ``handle`` is an ``async def`` (a coroutine function): the - coroutine is driven to completion on a worker thread, which would block - the supervisor loop if run inline, so it must be deferred. - """ - return inspect.iscoroutinefunction(getattr(handler, "handle", None)) - - def _resolve_syscall(key) -> int: """Resolve a handler-registration key to a kernel syscall number. diff --git a/python/tests/test_handler_smoke.py b/python/tests/test_handler_smoke.py index d76f7f54..401ec83f 100644 --- a/python/tests/test_handler_smoke.py +++ b/python/tests/test_handler_smoke.py @@ -728,27 +728,10 @@ def test_run_with_handlers_rejects_non_handler(): # ---------------------------------------------------------------- -# Deferred (off-loop) handler dispatch. +# Async handler dispatch. # ---------------------------------------------------------------- -def test_is_deferred_handler_detects_async(): - """Registration treats a handler as deferred iff its `handle` is an - `async def`. A synchronous handler runs inline. This pins the helper.""" - from sandlock.sandbox import _is_deferred_handler - - class Sync(Handler): - def handle(self, ctx): - return NotifAction.continue_() - - class Async(Handler): - async def handle(self, ctx): - return NotifAction.continue_() - - assert _is_deferred_handler(Sync()) is False - assert _is_deferred_handler(Async()) is True - - def test_async_handle_delivers_errno_e2e(tmp_dir): """An `async def handle` that awaits and then returns errno(EACCES) must make the child's openat fail with EACCES (exit 7). Proves the trampoline @@ -793,13 +776,11 @@ async def handle(self, ctx): assert result.exit_code == 7, (result.exit_code, result.stderr) -def test_async_handle_short_circuits_chain_e2e(tmp_dir): - """An `async def handle` is auto-deferred, hence terminal: when it - continues, a later handler on the same syscall must NOT run. First - handler is async and always continues; second denies the probe. The - child opening successfully proves the async handler auto-deferred (a - non-deferred async handler driven inline would Continue, letting the - second handler deny -> child fails).""" +def test_async_handle_continue_resumes_chain_e2e(tmp_dir): + """An `async def handle` that continues hands the syscall to the next + handler on the same syscall, like a synchronous one. First handler is + async and always continues; second denies the probe, so the child's + open must fail with EACCES.""" if not os.path.exists(_SYSTEM_PYTHON): pytest.skip(f"{_SYSTEM_PYTHON} not available") @@ -829,17 +810,18 @@ def handle(self, ctx): sb = sandlock.Sandbox(fs_readable=[*_PYTHON_READABLE, str(tmp_dir)]) script = ( "import os, sys\n" - "fd = os.open(%r, os.O_RDONLY)\n" - "os.close(fd)\n" % probe_path + "try:\n" + " os.close(os.open(%r, os.O_RDONLY))\n" + "except OSError as e:\n" + " sys.stderr.write('errno=%%d' %% e.errno)\n" + " sys.exit(3)\n" % probe_path ) result = sb.run_with_handlers( cmd=[_SYSTEM_PYTHON, "-c", script], handlers=[("openat", _AsyncContinue()), ("openat", _DenyProbe())], ) - assert result.success, ( - "async handler must auto-defer (terminal), so the deny handler never " - f"runs; got {result}" - ) + assert result.exit_code == 3, (result.exit_code, result.stderr) + assert b"errno=%d" % _errno.EACCES in result.stderr, result.stderr # ----------------------------------------------------------------