From 37d312e00147173ecdf219651b73c8a5edf7c5eb Mon Sep 17 00:00:00 2001 From: Jay Bosamiya Date: Fri, 30 Oct 2026 17:49:39 -0700 Subject: [PATCH] Separate fs resolution from opening; improve perm handling --- litebox_broker_core/src/fs/backend.rs | 307 ++++---- litebox_broker_core/src/fs/composer.rs | 584 ++++++++++------ litebox_broker_core/src/fs/devices.rs | 183 +++-- litebox_broker_core/src/fs/errors.rs | 12 + litebox_broker_core/src/fs/in_mem.rs | 531 +++++++++++--- litebox_broker_core/src/fs/nine_p/mod.rs | 359 ++++------ litebox_broker_core/src/fs/nine_p/tests.rs | 71 ++ litebox_broker_core/src/fs/overlay.rs | 774 +++++++++++++-------- litebox_broker_core/src/fs/resolver.rs | 724 +++++++------------ litebox_broker_core/src/fs/tar_ro.rs | 174 ++--- litebox_broker_core/src/fs/tests.rs | 203 +++++- litebox_broker_userland/src/fs/mod.rs | 226 +++--- litebox_broker_userland/tests/local_fs.rs | 267 ++++--- litebox_broker_vm_kernel/src/share.rs | 70 +- 14 files changed, 2650 insertions(+), 1835 deletions(-) diff --git a/litebox_broker_core/src/fs/backend.rs b/litebox_broker_core/src/fs/backend.rs index 44bfa6352..57bbc0100 100644 --- a/litebox_broker_core/src/fs/backend.rs +++ b/litebox_broker_core/src/fs/backend.rs @@ -14,7 +14,8 @@ use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, ReadDirError, ReadError, - RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, WriteError, + RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, + WriteError, }; use super::{DirEntry, FileStatus, FileType, Mode, OFlags, UserInfo}; use crate::readiness::ReadinessSource; @@ -63,50 +64,48 @@ pub enum SeekBehavior { /// A backend that can be used to support a (full or subset of) a LiteBox filesystem. pub trait Backend: Send + Sync + Any { - /// Obtain access to the root directory of the backend. - fn root(&self) -> WalkingDirHandle<'_>; + /// Resolve the root directory. + fn root(&self) -> Result, WalkError>; - /// Walk one or more `components` starting from the `from` handle. + /// Resolve `components` without opening the target. /// - /// `components` must be non-empty. Backends may panic if called with an empty slice. + /// Before looking up a child, backends reporting [`PermissionCheck::ByResolver`] must invoke + /// `authorize_dir_lookup` on its parent directory. Callback failure stops resolution immediately, + /// before inspecting the child. Self-enforcing backends omit the callback. /// - /// This function explicitly does not walk into files. If the next component exists but is not a - /// directory, the backend should stop at its parent and return - /// `WalkStopReason::StoppedAtNonDirectory`. - fn walk_directories<'a>( + /// The final component does not invoke `authorize_dir_lookup` upon itself, only its parent. + /// An empty sequence resolves to the starting directory without checking its permissions. + /// + /// Backends never follow symlinks. A non-directory (including a symlink) before the final + /// component fails with [`super::errors::PathError::ComponentNotADirectory`], with the error's + /// component index naming the next child (`non_directory_index + 1`). Only a missing final name + /// returns [`Resolution::MissingFinal`]; missing intermediate names are errors. An existing final + /// name that must be neither opened nor created may fail with + /// [`super::errors::PathError::NoSuchFileOrDirectory`] at its own index. + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError>; + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError>; /// Read the unexpanded target of the symlink `name` in `dir`. - fn read_link_at(&self, dir: &WalkingDirHandle<'_>, name: &str) -> Result; + fn read_link_at(&self, dir: &ResolvedDir<'_>, name: &str) -> Result; - /// Take an owned handle to a `dir` found via a walk, validating any open `flags`. - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result; + /// Open the already-resolved directory after resolver authorization, validating `flags`. + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result; - /// Obtain a walking handle to an existing owned dir. + /// Obtain a resolved directory from an existing owned directory handle, without opening it. /// /// This operation always succeeds and returns a `Some` _unless_ on a networked backend where /// owned handles can go stale. /// /// XXX(jayb): We will likely migrate away from `Option` here when we do a bit of an overhaul of /// the `errors` module in order to more consistently support stale errors everywhere. - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option>; + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option>; - /// Open an (existing) file at `dir`. - /// - /// To create a file, you need [`Self::create_file_at`]. - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError>; + /// Open the already-resolved file after resolver authorization, validating `flags`. + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result; /// Read directory entries at `dir`. fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError>; @@ -174,6 +173,9 @@ pub trait Backend: Send + Sync + Any { /// Status of an open file or directory handle. fn status(&self, h: HandleRef<'_>) -> Result; + /// Query metadata without consuming or opening the resolved target. + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result; + /// The terminal that an open file handle refers to. A file is a terminal /// exactly when it refers to one. #[expect(unused_variables, reason = "default body, non-underscored param names")] @@ -188,10 +190,10 @@ pub trait Backend: Send + Sync + Any { None } - /// Create a new file at `parent` with the given `name` and metadata. + /// Create a new file within `parent`. fn create_file_at( &self, - dir: DirHandle, + parent: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result; @@ -199,26 +201,26 @@ pub trait Backend: Send + Sync + Any { /// Create a symbolic link storing `target` verbatim at `dir` with the given `name` and metadata. fn symlink_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, target: &str, metadata: CreationMetadata, ) -> Result<(), SymlinkError>; - /// Create a new directory at `parent` with the given `name` and metadata. + /// Create a new directory within `parent`. fn mkdir_at( &self, - dir: DirHandle, + parent: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result; - /// Remove the file `name` at `parent`. - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError>; + /// Remove `name` after the resolver authorizes search and write access to `parent`. + fn unlink_at(&self, parent: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError>; /// Remove the directory `name` at `parent`. // XXX(jayb): I don't like that unlink and rmdir exist separately, we should probably merge them. - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError>; + fn rmdir_at(&self, parent: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError>; /// Atomically move an entry, replacing a compatible destination without following final /// symbolic links. Backends must reject directory cycles and preserve open file contents. @@ -231,13 +233,13 @@ pub trait Backend: Send + Sync + Any { new_name: &str, ) -> Result<(), RenameError>; - /// Update the permissions for the file/dir `h` refers to. - fn chmod(&self, h: HandleRef<'_>, mode: Mode) -> Result<(), ChmodError>; + /// Update a resolved target's permissions. + fn chmod(&self, h: ResolvedTarget<'_>, mode: Mode) -> Result<(), ChmodError>; - /// Update the owner/group for the file/dir `h` refers to. + /// Update a resolved target's owner/group. fn chown( &self, - h: HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError>; @@ -249,19 +251,38 @@ pub trait Backend: Send + Sync + Any { /// using erased handle wrappers, while concrete backend implementations can use these associated /// types at their own boundaries instead of spelling out manual erased-handle downcasts. pub trait BackendHandles { - /// Supporting walk through the backend - type WalkingDirHandle<'a>: 'a; + /// Scoped state identifying an unopened directory. + type ResolvedDir<'a>: 'a; + /// Scoped state identifying an unopened non-directory target. + type ResolvedFile<'a>: 'a; /// An owned handle to an open file type FileHandle: Clone + Send + Sync + 'static; /// An owned handle to an open directory type DirHandle: Clone + Send + Sync + 'static; } -/// Supporting walk through the backend -pub struct WalkingDirHandle<'a> { - backend_type: TypeId, - raw: Box, - _invariant: PhantomData &'a ()>, +/// A resolved, unopened directory, including permission metadata for resolver authorization. +pub struct ResolvedDir<'a> { + /// How permissions on this directory should be checked. + pub permissions: PermissionCheck, + raw: ErasedResolved<'a, DirKind>, +} + +/// A resolved, unopened non-directory target, including permission metadata for authorization. +pub struct ResolvedFile<'a> { + /// How permissions on this target should be checked. + pub permissions: PermissionCheck, + /// Lets the resolver detect symlinks, which it (not the backend) expands. + pub file_type: FileType, + raw: ErasedResolved<'a, FileKind>, +} + +/// A resolved target. +pub enum ResolvedTarget<'a> { + /// A directory target. + Dir(ResolvedDir<'a>), + /// A non-directory target. + File(ResolvedFile<'a>), } /// An owned handle to an open file @@ -305,63 +326,137 @@ pub enum HandleRef<'a> { Dir(&'a DirHandle), } -trait ErasedWalkingDirHandle { +trait ErasedResolvedHandle { + fn as_raw(&self) -> *const (); fn into_raw(self: Box) -> *mut (); } -impl ErasedWalkingDirHandle for H { +impl ErasedResolvedHandle for H { + fn as_raw(&self) -> *const () { + core::ptr::from_ref(self).cast() + } + fn into_raw(self: Box) -> *mut () { Box::into_raw(self).cast() } } -impl<'a> WalkingDirHandle<'a> { - /// Erase the concrete type of the handle. - pub fn from_typed(handle: B::WalkingDirHandle<'a>) -> Self { +/// Selects which [`BackendHandles`] resolved type an [`ErasedResolved`] stores. +trait ResolvedKind { + type Handle<'a, B: BackendHandles + 'static>: 'a; +} + +struct DirKind; +impl ResolvedKind for DirKind { + type Handle<'a, B: BackendHandles + 'static> = B::ResolvedDir<'a>; +} + +struct FileKind; +impl ResolvedKind for FileKind { + type Handle<'a, B: BackendHandles + 'static> = B::ResolvedFile<'a>; +} + +/// A type-erased `K::Handle<'a, B>`, recoverable only with the same `B`. +struct ErasedResolved<'a, K: ResolvedKind> { + backend_type: TypeId, + raw: Box, + _kind: PhantomData, + _invariant: PhantomData &'a ()>, +} + +impl<'a, K: ResolvedKind> ErasedResolved<'a, K> { + fn new(handle: K::Handle<'a, B>) -> Self { Self { backend_type: TypeId::of::(), raw: Box::new(handle), + _kind: PhantomData, _invariant: PhantomData, } } - /// Recover the concrete handle stored in this erased handle. + fn check(&self) { + assert_eq!( + self.backend_type, + TypeId::of::(), + "backend resolved handle type mismatch" + ); + } + + fn get(&self) -> &K::Handle<'a, B> { + self.check::(); + // SAFETY: `new::` stored a `K::Handle<'a, B>`: `K` is fixed by this type, `B` is + // checked above, and `'a` is invariant. The reference borrows `self`, which owns the box. + unsafe { &*(*self.raw).as_raw().cast::>() } + } + + fn into_inner(self) -> K::Handle<'a, B> { + self.check::(); + // SAFETY: as in `get`, the allocation holds a `K::Handle<'a, B>`. + unsafe { *Box::from_raw(self.raw.into_raw().cast::>()) } + } +} + +impl<'a> ResolvedDir<'a> { + /// Erase a concrete resolved directory and its permission metadata. + pub fn from_typed( + handle: B::ResolvedDir<'a>, + permissions: PermissionCheck, + ) -> Self { + Self { + permissions, + raw: ErasedResolved::new::(handle), + } + } + + /// Borrow the concrete resolved directory stored in this handle. /// - /// Intended to be called by backend implementations as `handle.into_typed::()` on - /// handles that the resolver passed back to the same backend. + /// # Panics + /// + /// Panics if `B` is not the backend type used to create this handle. + pub fn get_typed(&self) -> &B::ResolvedDir<'a> { + self.raw.get::() + } + + /// Recover scoped directory state passed back to the same backend. /// /// # Panics /// /// Panics if `B` is not the backend type used to create this handle. - pub fn into_typed(self) -> B::WalkingDirHandle<'a> { - assert_eq!( - self.backend_type, - TypeId::of::(), - "backend walking directory handle type mismatch" - ); - // SAFETY: `from_typed::` records `TypeId::of::()` and stores a - // `B::WalkingDirHandle<'a>`. `WalkingDirHandle<'a>` is invariant in `'a`, so the lifetime - // parameter cannot have been changed since construction. The assertion above confirms that - // this handle is being recovered for the same backend type, and together these guarantee - // that the allocation has the expected concrete type. - unsafe { *Box::from_raw(self.raw.into_raw().cast::>()) } + pub fn into_typed(self) -> B::ResolvedDir<'a> { + self.raw.into_inner::() + } +} + +impl<'a> ResolvedFile<'a> { + /// Erase a concrete resolved file and its permission metadata and type. + pub fn from_typed( + handle: B::ResolvedFile<'a>, + permissions: PermissionCheck, + file_type: FileType, + ) -> Self { + Self { + permissions, + file_type, + raw: ErasedResolved::new::(handle), + } } - /// Borrowing version of [`Self::into_typed`]. + /// Borrow the concrete resolved file stored in this handle. /// /// # Panics /// /// Panics if `B` is not the backend type used to create this handle. - pub fn as_typed(&self) -> &B::WalkingDirHandle<'a> { - assert_eq!( - self.backend_type, - TypeId::of::(), - "backend walking directory handle type mismatch" - ); - // SAFETY: as in `into_typed`, the boxed value is a `B::WalkingDirHandle<'a>`; casting the - // trait-object pointer to a thin pointer keeps its data address, and the borrow is tied - // to `self`, which owns the allocation. - unsafe { &*core::ptr::from_ref(&*self.raw).cast::>() } + pub fn get_typed(&self) -> &B::ResolvedFile<'a> { + self.raw.get::() + } + + /// Recover scoped file state passed back to the same backend. + /// + /// # Panics + /// + /// Panics if `B` is not the backend type used to create this handle. + pub fn into_typed(self) -> B::ResolvedFile<'a> { + self.raw.into_inner::() } } @@ -425,32 +520,16 @@ impl DirHandle { } } -/// A successful walk of directories through the backend -pub struct WalkOutcome { - /// A component per walked element. - /// - /// This vector can be empty when the first input component is a non-directory element. - /// - /// Components are in natural order (i.e., the last element is the last component visited thus - /// far). - pub components: Vec, - /// The last handle of the walk thus far. - /// - pub last: Walking, - /// Why this walk stopped at `last`. - pub stop_reason: WalkStopReason, -} - -/// Why a backend directory walk stopped. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +/// The result of [`Backend::resolve`]ing a component sequence. #[must_use] -pub enum WalkStopReason { - /// All requested components were walked, and `last` is the requested directory. - CompleteDirectory, - /// The next requested component exists but is not a directory; `last` is its parent directory. - StoppedAtNonDirectory { file_type: FileType }, - /// The backend stopped early; the resolver should continue walking from `last`. - Continue, +pub enum Resolution<'a> { + /// The target exists, including when an empty sequence names the starting directory. + Found(ResolvedTarget<'a>), + /// Only the final component is missing. A missing parent results in a [`ResolutionError`]. + MissingFinal { + /// The directory containing the missing name. + parent: ResolvedDir<'a>, + }, } /// The metadata a backend stamps onto a newly created file or directory. @@ -463,14 +542,6 @@ pub struct CreationMetadata { pub owner: UserInfo, } -/// A backend item plus permission metadata for resolver-side checks. -pub struct Permissioned { - /// The backend item. - pub item: H, - /// How permissions for this item should be checked. - pub permissions: PermissionCheck, -} - /// Whether a resolved component should be permission-checked by the resolver. #[derive(Clone, Debug)] #[must_use] @@ -481,14 +552,6 @@ pub enum PermissionCheck { ByResolver(PermissionInfo), } -/// Per-component status returned by a backend walk -#[derive(Clone, Debug)] -#[must_use] -pub struct WalkedComponent { - /// How permissions for this component should be checked. - pub permissions: PermissionCheck, -} - /// Permission information for a particular component of the walk. #[derive(Clone, Debug)] pub struct PermissionInfo { @@ -496,6 +559,10 @@ pub struct PermissionInfo { pub mode: Mode, /// Owner of the component. pub owner: UserInfo, + /// Whether the filesystem rejects writes, so file opens report EROFS before mode-based denial. + /// Self-enforcing backends order their own errors through [`PermissionCheck::ByBackend`]. + // TODO(jayb): consider a top-level "is read-only fs" backend method instead of a per-node flag. + pub on_read_only_fs: bool, } #[cfg(test)] diff --git a/litebox_broker_core/src/fs/composer.rs b/litebox_broker_core/src/fs/composer.rs index 07350f4e5..fbb5b8c23 100644 --- a/litebox_broker_core/src/fs/composer.rs +++ b/litebox_broker_core/src/fs/composer.rs @@ -15,12 +15,12 @@ use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use super::backend::{ Backend, BackendHandles, CreationMetadata, DirHandle, FileHandle, HandleRef, PermissionCheck, - Permissioned, SeekBehavior, WalkOutcome, WalkStopReason, WalkedComponent, WalkingDirHandle, + PermissionInfo, Resolution, ResolvedDir, ResolvedFile, ResolvedTarget, SeekBehavior, }; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, - WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }; use super::inode_allocator::{InodeAllocator, InodeAllocators}; use super::{DirEntry, FileStatus, FileType, Mode, NodeInfo, OFlags, UserInfo}; @@ -201,15 +201,16 @@ enum MountRelation { impl MountRelation { fn of(mounts: &[Mount], path: &[String]) -> Self { + let mut relation = MountRelation::Unrelated; for mount in mounts { if mount.path == path { return MountRelation::Exact; } if path.len() < mount.path.len() && mount.path.starts_with(path) { - return MountRelation::AncestorOfMount; + relation = MountRelation::AncestorOfMount; } } - MountRelation::Unrelated + relation } } @@ -275,25 +276,34 @@ impl Composer { entries } - fn exact_mount_root(&self, path: &[String]) -> Option<(usize, WalkingDirHandle<'_>)> { + fn exact_mount_root( + &self, + path: &[String], + ) -> Result)>, WalkError> { self.mounts .iter() .enumerate() .find(|(_, mount)| mount.path == path) - .map(|(index, mount)| (index, mount.backend.root())) + .map(|(index, mount)| mount.backend.root().map(|root| (index, root))) + .transpose() } - fn root_handle(&self) -> ComposerWalkingDirHandle<'_> { - if let Some((mount_index, handle)) = self.exact_mount_root(&[]) { - ComposerWalkingDirHandleInner::Mounted { - path: vec![], - mount_index, - handle, + fn resolved_dir<'a>(&self, dir: ComposerWalkingDirHandle<'a>) -> ResolvedDir<'a> { + let permissions = match &dir.inner { + ComposerWalkingDirHandleInner::Virtual { path } => { + let status = self.virtual_dir_status(path); + PermissionCheck::ByResolver(PermissionInfo { + mode: status.mode, + owner: status.owner, + on_read_only_fs: true, + }) } - .into() - } else { - ComposerWalkingDirHandleInner::Virtual { path: vec![] }.into() - } + ComposerWalkingDirHandleInner::Mounted { target, .. } => match target { + ResolvedTarget::Dir(handle) => handle.permissions.clone(), + ResolvedTarget::File(_) => unreachable!(), + }, + }; + ResolvedDir::from_typed::(dir, permissions) } fn virtual_dir_status(&self, path: &[String]) -> FileStatus { @@ -339,7 +349,7 @@ enum ComposerWalkingDirHandleInner<'a> { Mounted { path: Vec, mount_index: usize, - handle: WalkingDirHandle<'a>, + target: ResolvedTarget<'a>, }, } @@ -349,6 +359,12 @@ impl<'a> From> for ComposerWalkingDirHandle<'a } } +/// Resolved, unopened file in a composed filesystem namespace. +pub struct ComposerResolvedFile<'a> { + mount_index: usize, + target: ResolvedTarget<'a>, +} + /// File handle in a composed filesystem namespace. pub struct ComposerFileHandle { mount_index: usize, @@ -412,163 +428,197 @@ impl Clone for ComposerDirHandleInner { } impl BackendHandles for Composer { - type WalkingDirHandle<'a> = ComposerWalkingDirHandle<'a>; + type ResolvedDir<'a> = ComposerWalkingDirHandle<'a>; + type ResolvedFile<'a> = ComposerResolvedFile<'a>; type FileHandle = ComposerFileHandle; type DirHandle = ComposerDirHandle; } impl Backend for Composer { - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::(self.root_handle()) + fn root(&self) -> Result, WalkError> { + let inner = match self.exact_mount_root(&[])? { + Some((mount_index, handle)) => ComposerWalkingDirHandleInner::Mounted { + path: vec![], + mount_index, + target: ResolvedTarget::Dir(handle), + }, + None => ComposerWalkingDirHandleInner::Virtual { path: vec![] }, + }; + Ok(self.resolved_dir(inner.into())) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { - const BY_BACKEND: WalkedComponent = WalkedComponent { - permissions: PermissionCheck::ByBackend, - }; - let mut current = from.into_typed::(); - let mut walked_components = Vec::with_capacity(components.len()); + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + let mut current = from; let mut index = 0; while index < components.len() { + let fail = |error| ResolutionError { + component: Some(index), + error, + }; + let shift = |mut error: ResolutionError| { + error.component = error.component.map(|component| component + index); + error + }; let component = components[index]; - match current.inner { + let permissions = current.permissions.clone(); + match current.into_typed::().inner { ComposerWalkingDirHandleInner::Virtual { path } => { - let path = append_components(path, &[component]); - if let Some((mount_index, handle)) = self.exact_mount_root(&path) { - walked_components.push(BY_BACKEND); - current = ComposerWalkingDirHandleInner::Mounted { - path, + if let PermissionCheck::ByResolver(permissions) = permissions { + authorize_dir_lookup(&permissions).map_err(fail)?; + } + let child_path = append_components(path.clone(), &[component]); + let inner = if let Some((mount_index, handle)) = + self.exact_mount_root(&child_path).map_err(fail)? + { + ComposerWalkingDirHandleInner::Mounted { + path: child_path, mount_index, - handle, + target: ResolvedTarget::Dir(handle), } - .into(); - } else if self.mount_relation(&path) == MountRelation::AncestorOfMount { - walked_components.push(BY_BACKEND); - current = ComposerWalkingDirHandleInner::Virtual { path }.into(); + } else if self.mount_relation(&child_path) == MountRelation::AncestorOfMount { + ComposerWalkingDirHandleInner::Virtual { path: child_path } + } else if index + 1 == components.len() { + return Ok(Resolution::MissingFinal { + parent: self.resolved_dir( + ComposerWalkingDirHandleInner::Virtual { path }.into(), + ), + }); } else { - return Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)); - } + return Err(fail(PathError::NoSuchFileOrDirectory.into())); + }; + current = self.resolved_dir(inner.into()); index += 1; } ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target, } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; let child_path = append_components(path.clone(), &[component]); - if let Some((child_mount_index, child_handle)) = - self.exact_mount_root(&child_path) - { - walked_components.push(BY_BACKEND); - current = ComposerWalkingDirHandleInner::Mounted { - path: child_path, - mount_index: child_mount_index, - handle: child_handle, - } - .into(); - index += 1; - } else if self.mount_relation(&child_path) == MountRelation::AncestorOfMount { - match self.mounts[mount_index] - .backend - .walk_directories(handle, &[component]) - { - Ok(outcome) => { - let walked_len = outcome.components.len(); - assert!(walked_len <= 1); - assert!( - outcome.stop_reason != WalkStopReason::CompleteDirectory - || walked_len == 1 - ); - walked_components.extend(outcome.components); - let last = ComposerWalkingDirHandleInner::Mounted { - path: append_components( - path, - &components[index..index + walked_len], - ), - mount_index, - handle: outcome.last, - } - .into(); - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - index += walked_len; - current = last; - } - WalkStopReason::StoppedAtNonDirectory { .. } - | WalkStopReason::Continue => { - return Ok(WalkOutcome { - components: walked_components, - last: WalkingDirHandle::from_typed::(last), - stop_reason: outcome.stop_reason, - }); - } - } + let relation = self.mount_relation(&child_path); + let backend = &self.mounts[mount_index].backend; + if relation == MountRelation::Exact { + // XXX: an overlay can hide a nested server's search denial at a mount + // crossing; precise enforcement needs a backend/9P API change. + match &handle.permissions { + PermissionCheck::ByResolver(permissions) => { + authorize_dir_lookup(permissions).map_err(fail)?; + drop(handle); } - Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)) => { - walked_components.push(BY_BACKEND); - current = - ComposerWalkingDirHandleInner::Virtual { path: child_path } - .into(); - index += 1; + // A server-authorized parent must still be searched before crossing a mount. + PermissionCheck::ByBackend => { + let _ = backend + .resolve(handle, &[component], authorize_dir_lookup) + .map_err(shift)?; } - Err(error) => return Err(error), } + let (mount_index, handle) = + self.exact_mount_root(&child_path).map_err(fail)?.unwrap(); + current = self.resolved_dir( + ComposerWalkingDirHandleInner::Mounted { + path: child_path, + mount_index, + target: ResolvedTarget::Dir(handle), + } + .into(), + ); + index += 1; + continue; + } + let prefix_len = if relation == MountRelation::AncestorOfMount { + 1 } else { // TODO(jayb): Decide whether future backends need absolute-ish namespace // views instead of this mount-root-relative suffix view. The resolver handles // `..`, including across mount roots, before walking. - let prefix_len = self.mounted_walk_prefix_len(&path, &components[index..]); - assert!(prefix_len > 0); - let outcome = self.mounts[mount_index] - .backend - .walk_directories(handle, &components[index..index + prefix_len])?; - let walked_len = outcome.components.len(); - assert!( - outcome.stop_reason != WalkStopReason::CompleteDirectory - || walked_len == prefix_len - ); - walked_components.extend(outcome.components); - let last = ComposerWalkingDirHandleInner::Mounted { - path: append_components(path, &components[index..index + walked_len]), - mount_index, - handle: outcome.last, + self.mounted_walk_prefix_len(&path, &components[index..]) + }; + let end = index + prefix_len; + match backend + .resolve(handle, &components[index..end], authorize_dir_lookup) + .map_err(shift)? + { + Resolution::Found(ResolvedTarget::Dir(handle)) => { + current = self.resolved_dir( + ComposerWalkingDirHandleInner::Mounted { + path: append_components(path, &components[index..end]), + mount_index, + target: ResolvedTarget::Dir(handle), + } + .into(), + ); } - .into(); - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - index += walked_len; - current = last; - } - WalkStopReason::StoppedAtNonDirectory { .. } - | WalkStopReason::Continue => { - return Ok(WalkOutcome { - components: walked_components, - last: WalkingDirHandle::from_typed::(last), - stop_reason: outcome.stop_reason, + Resolution::Found(ResolvedTarget::File(file)) + if relation == MountRelation::AncestorOfMount + && file.file_type != FileType::SymbolicLink => + { + // TODO(jayb): files shadowed by a deeper mount return indexed ENOENT + // (never MissingFinal) so open/create can't touch them; symlinks still + // pass through. + return Err(fail(PathError::NoSuchFileOrDirectory.into())); + } + Resolution::Found(ResolvedTarget::File(handle)) => { + if end != components.len() { + return Err(ResolutionError { + component: Some(end), + error: PathError::ComponentNotADirectory.into(), }); } + let permissions = handle.permissions.clone(); + let file_type = handle.file_type; + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + ComposerResolvedFile { + mount_index, + target: ResolvedTarget::File(handle), + }, + permissions, + file_type, + ), + ))); + } + Resolution::MissingFinal { .. } + if relation == MountRelation::AncestorOfMount => + { + current = self.resolved_dir( + ComposerWalkingDirHandleInner::Virtual { path: child_path }.into(), + ); + } + Resolution::MissingFinal { parent } if end == components.len() => { + return Ok(Resolution::MissingFinal { + parent: self.resolved_dir( + ComposerWalkingDirHandleInner::Mounted { + path: append_components(path, &components[index..end - 1]), + mount_index, + target: ResolvedTarget::Dir(parent), + } + .into(), + ), + }); + } + Resolution::MissingFinal { .. } => { + return Err(ResolutionError { + component: Some(end - 1), + error: PathError::NoSuchFileOrDirectory.into(), + }); } } + index = end; } } } - - Ok(WalkOutcome { - components: walked_components, - last: WalkingDirHandle::from_typed::(current), - stop_reason: WalkStopReason::CompleteDirectory, - }) + Ok(Resolution::Found(ResolvedTarget::Dir(current))) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result { let dir = dir.into_typed::(); let inner = match dir.inner { ComposerWalkingDirHandleInner::Virtual { path } => { @@ -577,22 +627,25 @@ impl Backend for Composer { ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, - } => ComposerDirHandleInner::Mounted { - path, - mount_index, - handle: self.mounts[mount_index] - .backend - .owned_dir_at(handle, flags)?, - }, + target, + } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; + ComposerDirHandleInner::Mounted { + path, + mount_index, + handle: self.mounts[mount_index].backend.open_dir(handle, flags)?, + } + } }; Ok(DirHandle::from_typed::(ComposerDirHandle { inner })) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { let dir = dir.get_typed::(); match &dir.inner { - ComposerDirHandleInner::Virtual { path } => Some(WalkingDirHandle::from_typed::( + ComposerDirHandleInner::Virtual { path } => Some(self.resolved_dir( ComposerWalkingDirHandleInner::Virtual { path: path.clone() }.into(), )), ComposerDirHandleInner::Mounted { @@ -603,11 +656,11 @@ impl Backend for Composer { .backend .walking_dir_at(handle) .map(|handle| { - WalkingDirHandle::from_typed::( + self.resolved_dir( ComposerWalkingDirHandleInner::Mounted { path: path.clone(), mount_index: *mount_index, - handle, + target: ResolvedTarget::Dir(handle), } .into(), ) @@ -615,54 +668,39 @@ impl Backend for Composer { } } - fn read_link_at(&self, dir: &WalkingDirHandle<'_>, name: &str) -> Result { - match &dir.as_typed::().inner { + fn read_link_at(&self, dir: &ResolvedDir<'_>, name: &str) -> Result { + match &dir.get_typed::().inner { ComposerWalkingDirHandleInner::Virtual { .. } => { Err(PathError::NoSuchFileOrDirectory.into()) } ComposerWalkingDirHandleInner::Mounted { mount_index, - handle, + target, .. - } => self.mounts[*mount_index].backend.read_link_at(handle, name), - } - } - - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { - let dir = dir.into_typed::(); - match dir.inner { - ComposerWalkingDirHandleInner::Virtual { .. } => { - Err(OpenError::PathError(PathError::NoSuchFileOrDirectory)) - } - ComposerWalkingDirHandleInner::Mounted { - path, - mount_index, - handle, } => { - self.checked_child_path( - path, - name, - OpenError::PathError(PathError::NoSuchFileOrDirectory), - )?; - self.mounts[mount_index] - .backend - .open_file_at(handle, name, flags) - .map(|file| Permissioned { - item: FileHandle::from_typed::(ComposerFileHandle { - mount_index, - handle: file.item, - }), - permissions: file.permissions, - }) + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; + self.mounts[*mount_index].backend.read_link_at(handle, name) } } } + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { + let ComposerResolvedFile { + mount_index, + target, + } = file.into_typed::(); + let ResolvedTarget::File(handle) = target else { + unreachable!() + }; + let handle = self.mounts[mount_index].backend.open_file(handle, flags)?; + Ok(FileHandle::from_typed::(ComposerFileHandle { + mount_index, + handle, + })) + } + fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { let handle = handle.into_typed::(); match handle.inner { @@ -754,20 +792,44 @@ impl Backend for Composer { .readiness_source(&h.handle) } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + match target { + ResolvedTarget::File(h) => { + let h = h.get_typed::(); + self.mounts[h.mount_index] + .backend + .resolved_status(&h.target) + } + ResolvedTarget::Dir(h) => match &h.get_typed::().inner { + ComposerWalkingDirHandleInner::Virtual { path } => { + Ok(self.virtual_dir_status(path)) + } + ComposerWalkingDirHandleInner::Mounted { + mount_index, + target, + .. + } => self.mounts[*mount_index].backend.resolved_status(target), + }, + } + } + fn create_file_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { let dir = dir.into_typed::(); match dir.inner { - ComposerDirHandleInner::Virtual { .. } => Err(OpenError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ComposerWalkingDirHandleInner::Virtual { .. } => Err(OpenError::ReadOnlyFileSystem), + ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target, } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; self.checked_child_path(path, name, OpenError::ReadOnlyFileSystem)?; self.mounts[mount_index] .backend @@ -784,19 +846,22 @@ impl Backend for Composer { fn symlink_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, target: &str, metadata: CreationMetadata, ) -> Result<(), SymlinkError> { let dir = dir.into_typed::(); match dir.inner { - ComposerDirHandleInner::Virtual { .. } => Err(SymlinkError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ComposerWalkingDirHandleInner::Virtual { .. } => Err(SymlinkError::ReadOnlyFileSystem), + ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target: parent, } => { + let ResolvedTarget::Dir(handle) = parent else { + unreachable!() + }; self.checked_child_path(path, name, SymlinkError::ReadOnlyFileSystem)?; self.mounts[mount_index] .backend @@ -807,18 +872,21 @@ impl Backend for Composer { fn mkdir_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { let dir = dir.into_typed::(); match dir.inner { - ComposerDirHandleInner::Virtual { .. } => Err(MkdirError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ComposerWalkingDirHandleInner::Virtual { .. } => Err(MkdirError::ReadOnlyFileSystem), + ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target, } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; let path = self.checked_child_path(path, name, MkdirError::ReadOnlyFileSystem)?; self.mounts[mount_index] .backend @@ -837,30 +905,36 @@ impl Backend for Composer { } } - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError> { let dir = dir.into_typed::(); match dir.inner { - ComposerDirHandleInner::Virtual { .. } => Err(UnlinkError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ComposerWalkingDirHandleInner::Virtual { .. } => Err(UnlinkError::ReadOnlyFileSystem), + ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target, } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; self.checked_child_path(path, name, UnlinkError::ReadOnlyFileSystem)?; self.mounts[mount_index].backend.unlink_at(handle, name) } } } - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError> { let dir = dir.into_typed::(); match dir.inner { - ComposerDirHandleInner::Virtual { .. } => Err(RmdirError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ComposerWalkingDirHandleInner::Virtual { .. } => Err(RmdirError::ReadOnlyFileSystem), + ComposerWalkingDirHandleInner::Mounted { path, mount_index, - handle, + target, } => { + let ResolvedTarget::Dir(handle) = target else { + unreachable!() + }; self.checked_child_path(path, name, RmdirError::ReadOnlyFileSystem)?; self.mounts[mount_index].backend.rmdir_at(handle, name) } @@ -911,50 +985,104 @@ impl Backend for Composer { } } - fn chmod(&self, h: HandleRef<'_>, mode: Mode) -> Result<(), ChmodError> { + fn chmod(&self, h: ResolvedTarget<'_>, mode: Mode) -> Result<(), ChmodError> { match h { - HandleRef::File(h) => { - let h = h.get_typed::(); - self.mounts[h.mount_index] - .backend - .chmod(HandleRef::File(&h.handle), mode) + ResolvedTarget::File(h) => { + let h = h.into_typed::(); + self.mounts[h.mount_index].backend.chmod(h.target, mode) } - HandleRef::Dir(h) => match &h.get_typed::().inner { - ComposerDirHandleInner::Virtual { .. } => Err(ChmodError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ResolvedTarget::Dir(h) => match h.into_typed::().inner { + ComposerWalkingDirHandleInner::Virtual { .. } => { + Err(ChmodError::ReadOnlyFileSystem) + } + ComposerWalkingDirHandleInner::Mounted { mount_index, - handle, + target, .. - } => self.mounts[*mount_index] - .backend - .chmod(HandleRef::Dir(handle), mode), + } => self.mounts[mount_index].backend.chmod(target, mode), }, } } fn chown( &self, - h: HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError> { match h { - HandleRef::File(h) => { - let h = h.get_typed::(); + ResolvedTarget::File(h) => { + let h = h.into_typed::(); self.mounts[h.mount_index] .backend - .chown(HandleRef::File(&h.handle), user, group) + .chown(h.target, user, group) } - HandleRef::Dir(h) => match &h.get_typed::().inner { - ComposerDirHandleInner::Virtual { .. } => Err(ChownError::ReadOnlyFileSystem), - ComposerDirHandleInner::Mounted { + ResolvedTarget::Dir(h) => match h.into_typed::().inner { + ComposerWalkingDirHandleInner::Virtual { .. } => { + Err(ChownError::ReadOnlyFileSystem) + } + ComposerWalkingDirHandleInner::Mounted { mount_index, - handle, + target, .. - } => self.mounts[*mount_index] - .backend - .chown(HandleRef::Dir(handle), user, group), + } => self.mounts[mount_index].backend.chown(target, user, group), }, } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::fs::in_mem::{InMem, InitialNode}; + use crate::test_platform::TestPlatform; + + #[test] + fn child_mount_before_parent_does_not_truncate_shadowed_file() { + let file = || InitialNode::File { + mode: Mode::RWXU, + owner: UserInfo::ROOT, + data: b"preserve".as_slice().into(), + }; + let composer = Composer::builder() + .mount("/", |_| { + InMem::::new_initialized([ + ( + "/a", + InitialNode::Directory { + mode: Mode::RWXU, + owner: UserInfo::ROOT, + }, + ), + ("/a/file", file()), + ]) + }) + .mount("/a/b", InMem::::new) + .mount("/a", |_| { + InMem::::new_initialized([("/file", file())]) + }) + .build() + .unwrap(); + + let Resolution::Found(ResolvedTarget::File(target)) = composer + .resolve(composer.root().unwrap(), &["a", "file"], &|_| Ok(())) + .unwrap() + else { + panic!("expected mounted file"); + }; + composer + .open_file(target, OFlags::WRONLY | OFlags::TRUNC) + .unwrap(); + + for (index, components, expected) in [(0, &["a", "file"][..], 8), (2, &["file"][..], 0)] { + let backend = &composer.mounts[index].backend; + let Resolution::Found(target) = backend + .resolve(backend.root().unwrap(), components, &|_| Ok(())) + .unwrap() + else { + panic!("expected existing file"); + }; + assert_eq!(backend.resolved_status(&target).unwrap().size, expected); + } + } +} diff --git a/litebox_broker_core/src/fs/devices.rs b/litebox_broker_core/src/fs/devices.rs index f5dcb8513..c75624562 100644 --- a/litebox_broker_core/src/fs/devices.rs +++ b/litebox_broker_core/src/fs/devices.rs @@ -9,7 +9,6 @@ use alloc::string::{String, ToString}; use alloc::sync::Arc; -use alloc::vec; use alloc::vec::Vec; use litebox_broker_protocol::readiness::ReadinessFlags; @@ -17,13 +16,13 @@ use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use super::backend::{ Backend, BackendHandles, CreationMetadata, DirHandle, FileHandle, FileTerminal, HandleRef, - PermissionCheck, Permissioned, SeekBehavior, WalkOutcome, WalkStopReason, WalkedComponent, - WalkingDirHandle, + PermissionCheck, PermissionInfo, Resolution, ResolvedDir, ResolvedFile, ResolvedTarget, + SeekBehavior, }; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, - WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }; use super::inode_allocator::InodeAllocator; use super::{DirEntry, FileStatus, FileType, Mode, NodeInfo, OFlags, UserInfo}; @@ -271,6 +270,17 @@ impl ReadinessSource for StdioReadiness { } } +/// Resolved device, without opening it or creating a pseudo-terminal pair. +pub struct DeviceResolvedFile(DeviceResolvedTarget); + +enum DeviceResolvedTarget { + Device(Device), + Subsidiary { + index: u32, + pair: Arc, + }, +} + /// Owned file handle; identifies which device backs this fd. #[derive(Clone)] pub struct DeviceFileHandle(DeviceFile); @@ -312,7 +322,8 @@ pub enum DeviceDirHandle { } impl BackendHandles for Devices { - type WalkingDirHandle<'a> = DeviceDirHandle; + type ResolvedDir<'a> = DeviceDirHandle; + type ResolvedFile<'a> = DeviceResolvedFile; type FileHandle = DeviceFileHandle; type DirHandle = DeviceDirHandle; } @@ -326,92 +337,101 @@ impl Backend for Devices { } } - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::(DeviceDirHandle::Root) + fn root(&self) -> Result, WalkError> { + Ok(ResolvedDir::from_typed::( + DeviceDirHandle::Root, + PermissionCheck::ByBackend, + )) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { + _authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + if components.is_empty() { + return Ok(Resolution::Found(ResolvedTarget::Dir(from))); + } let mut dir = from.into_typed::(); - let mut walked = vec![]; - for &component in components { - let exists = match dir { + for (index, &component) in components.iter().enumerate() { + let target = match dir { DeviceDirHandle::Root if component == PTS_DIR_NAME => { - walked.push(WalkedComponent { - permissions: PermissionCheck::ByBackend, - }); dir = DeviceDirHandle::Pts; continue; } - DeviceDirHandle::Root => Device::from_name(component).is_some(), - DeviceDirHandle::Pts => self.pseudo_terminal(component).is_some(), + DeviceDirHandle::Root => { + Device::from_name(component).map(DeviceResolvedTarget::Device) + } + DeviceDirHandle::Pts => { + self.pseudo_terminal(component) + .map(|pair| DeviceResolvedTarget::Subsidiary { + index: pts_index(component).unwrap(), + pair, + }) + } }; - if !exists { - return Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)); + match target { + Some(target) if index + 1 == components.len() => { + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + DeviceResolvedFile(target), + PermissionCheck::ByBackend, + FileType::CharacterDevice, + ), + ))); + } + Some(_) => { + return Err(ResolutionError { + component: Some(index + 1), + error: PathError::ComponentNotADirectory.into(), + }); + } + None if index + 1 == components.len() => { + return Ok(Resolution::MissingFinal { + parent: ResolvedDir::from_typed::(dir, PermissionCheck::ByBackend), + }); + } + None => { + return Err(ResolutionError { + component: Some(index), + error: PathError::NoSuchFileOrDirectory.into(), + }); + } } - // Device files are final path targets, so directory walking must stop before them. - return Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(dir), - stop_reason: WalkStopReason::StoppedAtNonDirectory { - file_type: FileType::CharacterDevice, - }, - }); } - Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(dir), - stop_reason: WalkStopReason::CompleteDirectory, - }) + Ok(Resolution::Found(ResolvedTarget::Dir( + ResolvedDir::from_typed::(dir, PermissionCheck::ByBackend), + ))) } - fn read_link_at(&self, _dir: &WalkingDirHandle<'_>, _name: &str) -> Result { + fn read_link_at(&self, _dir: &ResolvedDir<'_>, _name: &str) -> Result { Err(PathError::InvalidPathname.into()) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - _flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, _flags: OFlags) -> Result { Ok(DirHandle::from_typed::(dir.into_typed::())) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { - Some(WalkingDirHandle::from_typed::( + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + Some(ResolvedDir::from_typed::( *dir.get_typed::(), + PermissionCheck::ByBackend, )) } - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { - let not_found = || OpenError::PathError(PathError::NoSuchFileOrDirectory); + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { + let DeviceResolvedFile(target) = file.into_typed::(); + if flags.contains(OFlags::DIRECTORY) { + return Err(OpenError::PathError(PathError::ComponentNotADirectory)); + } let path_only = flags.contains(OFlags::PATH); - let file = match dir.into_typed::() { - DeviceDirHandle::Root => { - let device = Device::from_name(name).ok_or_else(not_found)?; - if flags.contains(OFlags::DIRECTORY) { - return Err(OpenError::PathError(PathError::ComponentNotADirectory)); - } - if device == Device::Ptmx && !path_only { - DeviceFile::Manager(Arc::new(self.pseudo_terminals.open_manager()?)) - } else { - DeviceFile::Device(device) - } + let file = match target { + DeviceResolvedTarget::Device(Device::Ptmx) if !path_only => { + DeviceFile::Manager(Arc::new(self.pseudo_terminals.open_manager()?)) } - DeviceDirHandle::Pts => { - let index = pts_index(name).ok_or_else(not_found)?; - let pair = self.pseudo_terminals.get(index).ok_or_else(not_found)?; - if flags.contains(OFlags::DIRECTORY) { - return Err(OpenError::PathError(PathError::ComponentNotADirectory)); - } + DeviceResolvedTarget::Device(device) => DeviceFile::Device(device), + DeviceResolvedTarget::Subsidiary { index, pair } => { let file = if path_only { None } else { @@ -431,10 +451,7 @@ impl Backend for Devices { )); } - Ok(Permissioned { - item, - permissions: PermissionCheck::ByBackend, - }) + Ok(item) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { @@ -588,9 +605,21 @@ impl Backend for Devices { } } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + match target { + ResolvedTarget::File(h) => Ok(match &h.get_typed::().0 { + DeviceResolvedTarget::Device(device) => device.file_status(), + DeviceResolvedTarget::Subsidiary { index, .. } => self.subsidiary_status(*index), + }), + ResolvedTarget::Dir(h) => self.status(HandleRef::Dir(&DirHandle::from_typed::( + *h.get_typed::(), + ))), + } + } + fn create_file_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { @@ -599,7 +628,7 @@ impl Backend for Devices { fn symlink_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _target: &str, _metadata: CreationMetadata, @@ -609,14 +638,14 @@ impl Backend for Devices { fn mkdir_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { Err(MkdirError::ReadOnlyFileSystem) } - fn unlink_at(&self, _dir: DirHandle, _name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, _dir: ResolvedDir<'_>, _name: &str) -> Result<(), UnlinkError> { Err(UnlinkError::ReadOnlyFileSystem) } @@ -630,17 +659,17 @@ impl Backend for Devices { Err(RenameError::ReadOnlyFileSystem) } - fn rmdir_at(&self, _dir: DirHandle, _name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, _dir: ResolvedDir<'_>, _name: &str) -> Result<(), RmdirError> { Err(RmdirError::ReadOnlyFileSystem) } - fn chmod(&self, _h: HandleRef<'_>, _mode: Mode) -> Result<(), ChmodError> { + fn chmod(&self, _h: ResolvedTarget<'_>, _mode: Mode) -> Result<(), ChmodError> { Err(ChmodError::ReadOnlyFileSystem) } fn chown( &self, - _h: HandleRef<'_>, + _h: ResolvedTarget<'_>, _user: Option, _group: Option, ) -> Result<(), ChownError> { diff --git a/litebox_broker_core/src/fs/errors.rs b/litebox_broker_core/src/fs/errors.rs index 2b3937c83..a55ce896d 100644 --- a/litebox_broker_core/src/fs/errors.rs +++ b/litebox_broker_core/src/fs/errors.rs @@ -280,6 +280,18 @@ pub enum WalkError { PathError(#[from] PathError), } +/// A resolution failure optionally located relative to the component slice passed to a backend. +#[derive(Debug, Error)] +#[error("resolution failed (component: {component:?}): {error}")] +pub struct ResolutionError { + /// Zero-based index of the child whose lookup failed; wrappers adjust indices for subslices. + /// `None` covers failures such as transport errors with no known component location. + pub component: Option, + /// The underlying lookup or authorization failure. + #[source] + pub error: WalkError, +} + /// Possible errors in any file-system function due to path errors. #[derive(Error, Debug)] pub enum PathError { diff --git a/litebox_broker_core/src/fs/in_mem.rs b/litebox_broker_core/src/fs/in_mem.rs index 54d10c173..b1b889284 100644 --- a/litebox_broker_core/src/fs/in_mem.rs +++ b/litebox_broker_core/src/fs/in_mem.rs @@ -11,9 +11,13 @@ use hashbrown::HashMap; use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use litebox_platform::sync; +use super::backend::{ + PermissionCheck, PermissionInfo, Resolution, ResolvedDir, ResolvedFile, ResolvedTarget, +}; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }; use super::inode_allocator::InodeAllocator; use super::{DirEntry, FileStatus, FileType, Mode, NodeInfo, UserInfo}; @@ -35,6 +39,11 @@ pub struct InMem { } impl InMem { + fn resolved_dir<'a>(dir: InMemDirHandle) -> ResolvedDir<'a> { + let permissions = dir.dir.read().perms.permission_check(); + ResolvedDir::from_typed::(dir, permissions) + } + /// Construct a new `InMem` backend. #[must_use] pub fn new(inode_allocator: InodeAllocator) -> Self { @@ -322,76 +331,79 @@ impl Clone for InMemFileHandle super::backend::BackendHandles for InMem { - type WalkingDirHandle<'a> = InMemDirHandle; + type ResolvedDir<'a> = InMemDirHandle; + type ResolvedFile<'a> = InMemFileHandle; type FileHandle = InMemFileHandle; type DirHandle = InMemDirHandle; } impl super::backend::Backend for InMem { - fn root(&self) -> super::backend::WalkingDirHandle<'_> { - super::backend::WalkingDirHandle::from_typed::(InMemDirHandle { + fn root(&self) -> Result, WalkError> { + Ok(Self::resolved_dir(InMemDirHandle { dir: self.root.clone(), flags: super::OFlags::PATH, - }) + })) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: super::backend::WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result< - super::backend::WalkOutcome>, - super::errors::WalkError, - > { - let mut current = from.into_typed::(); - let mut walked_components = Vec::with_capacity(components.len()); - for component in components { - let child = current - .dir - .read() - .children - .get(*component) - .ok_or(PathError::NoSuchFileOrDirectory)? - .clone(); - let child = match child { - Node::Dir(child) => child, - Node::File(file) => { - return Ok(super::backend::WalkOutcome { - components: walked_components, - last: super::backend::WalkingDirHandle::from_typed::(current), - stop_reason: super::backend::WalkStopReason::StoppedAtNonDirectory { - file_type: file.read().file_type, - }, + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + let mut current = from; + for (index, component) in components.iter().enumerate() { + let fail = |error| ResolutionError { + component: Some(index), + error, + }; + let PermissionCheck::ByResolver(permissions) = ¤t.permissions else { + unreachable!() + }; + authorize_dir_lookup(permissions).map_err(fail)?; + let dir = current.into_typed::(); + let child = dir.dir.read().children.get(*component).cloned(); + let Some(child) = child else { + if index + 1 == components.len() { + return Ok(Resolution::MissingFinal { + parent: Self::resolved_dir(dir), }); } + return Err(fail(PathError::NoSuchFileOrDirectory.into())); }; - let perms = child.read().perms.clone(); - walked_components.push(super::backend::WalkedComponent { - permissions: super::backend::PermissionCheck::ByResolver( - super::backend::PermissionInfo { - mode: perms.mode, - owner: perms.userinfo, - }, - ), - }); - current = InMemDirHandle { - dir: child, - flags: super::OFlags::PATH, - }; + match child { + Node::Dir(child) => { + current = Self::resolved_dir(InMemDirHandle { + dir: child, + flags: super::OFlags::PATH, + }); + } + Node::File(file) => { + if index + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + 1), + error: PathError::ComponentNotADirectory.into(), + }); + } + let (permissions, file_type) = { + let file = file.read(); + (file.perms.permission_check(), file.file_type) + }; + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + InMemFileHandle { file }, + permissions, + file_type, + ), + ))); + } + } } - Ok(super::backend::WalkOutcome { - components: walked_components, - last: super::backend::WalkingDirHandle::from_typed::(current), - stop_reason: super::backend::WalkStopReason::CompleteDirectory, - }) + Ok(Resolution::Found(ResolvedTarget::Dir(current))) } - fn read_link_at( - &self, - dir: &super::backend::WalkingDirHandle<'_>, - name: &str, - ) -> Result { - let dir = dir.as_typed::().dir.read(); + fn read_link_at(&self, dir: &ResolvedDir<'_>, name: &str) -> Result { + let dir = dir.get_typed::().dir.read(); let file = match dir.children.get(name) { Some(Node::File(file)) => file.read(), Some(Node::Dir(_)) => return Err(PathError::InvalidPathname.into()), @@ -405,9 +417,9 @@ impl super::backend::Backend for InMe .into()) } - fn owned_dir_at( + fn open_dir( &self, - dir: super::backend::WalkingDirHandle<'_>, + dir: ResolvedDir<'_>, flags: super::OFlags, ) -> Result { assert_supported_oflags(flags); @@ -419,60 +431,30 @@ impl super::backend::Backend for InMe )) } - fn walking_dir_at<'a>( - &'a self, - dir: &super::backend::DirHandle, - ) -> Option> { - Some(super::backend::WalkingDirHandle::from_typed::( - InMemDirHandle { - dir: dir.get_typed::().dir.clone(), - flags: super::OFlags::PATH, - }, - )) + fn walking_dir_at<'a>(&'a self, dir: &super::backend::DirHandle) -> Option> { + Some(Self::resolved_dir(InMemDirHandle { + dir: dir.get_typed::().dir.clone(), + flags: super::OFlags::PATH, + })) } - fn open_file_at( + fn open_file( &self, - dir: super::backend::WalkingDirHandle<'_>, - name: &str, + file: ResolvedFile<'_>, flags: super::OFlags, - ) -> Result, OpenError> { + ) -> Result { assert_supported_oflags(flags); - let dir = dir.into_typed::(); - let child = dir - .dir - .read() - .children - .get(name) - .ok_or(PathError::NoSuchFileOrDirectory)? - .clone(); - let Node::File(file) = child else { - return Err(PathError::ComponentNotADirectory.into()); - }; if flags.contains(super::OFlags::DIRECTORY) { return Err(PathError::ComponentNotADirectory.into()); } - let perms = file.read().perms.clone(); - let handle = super::backend::FileHandle::from_typed::(InMemFileHandle { file }); + let handle = super::backend::FileHandle::from_typed::(file.into_typed::()); if flags.contains(super::OFlags::TRUNC) && !flags.contains(super::OFlags::PATH) { // Linux truncates whenever the open succeeds, regardless of the access mode (an // `O_RDONLY|O_TRUNC` open of a writable file does truncate it); `O_PATH` opens ignore // `O_TRUNC` entirely. - // - // TODO(jayb): Linux's `may_open` also adds `MAY_WRITE` for `O_TRUNC`, and checks - // permissions _before_ truncating; the resolver does neither, so a denied - // `O_RDONLY|O_TRUNC` open still empties the file here. self.truncate(&handle, 0)?; } - Ok(super::backend::Permissioned { - item: handle, - permissions: super::backend::PermissionCheck::ByResolver( - super::backend::PermissionInfo { - mode: perms.mode, - owner: perms.userinfo, - }, - ), - }) + Ok(handle) } fn list_dir_at( @@ -583,9 +565,20 @@ impl super::backend::Backend for InMe } } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + match target { + ResolvedTarget::File(h) => self.status(super::backend::HandleRef::File( + &super::backend::FileHandle::from_typed::(h.get_typed::().clone()), + )), + ResolvedTarget::Dir(h) => self.status(super::backend::HandleRef::Dir( + &super::backend::DirHandle::from_typed::(h.get_typed::().clone()), + )), + } + } + fn create_file_at( &self, - dir: super::backend::DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: super::backend::CreationMetadata, ) -> Result { @@ -616,7 +609,7 @@ impl super::backend::Backend for InMe fn symlink_at( &self, - dir: super::backend::DirHandle, + dir: ResolvedDir<'_>, name: &str, target: &str, metadata: super::backend::CreationMetadata, @@ -641,7 +634,7 @@ impl super::backend::Backend for InMe fn mkdir_at( &self, - dir: super::backend::DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: super::backend::CreationMetadata, ) -> Result { @@ -673,7 +666,7 @@ impl super::backend::Backend for InMe )) } - fn unlink_at(&self, dir: super::backend::DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError> { // TODO(jayb): Nothing checks write permission on the parent directory before removing; // the resolver should do so before calling this. let parent = dir.into_typed::(); @@ -688,7 +681,7 @@ impl super::backend::Backend for InMe } } - fn rmdir_at(&self, dir: super::backend::DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError> { // TODO(jayb): Nothing checks write permission on the parent directory before removing; // the resolver should do so before calling this. let parent = dir.into_typed::(); @@ -803,18 +796,20 @@ impl super::backend::Backend for InMe } } - fn chmod(&self, h: super::backend::HandleRef<'_>, mode: Mode) -> Result<(), ChmodError> { - let mut perms = match h { - super::backend::HandleRef::File(h) => { - let file = h.get_typed::().file.write(); + fn chmod(&self, h: ResolvedTarget<'_>, mode: Mode) -> Result<(), ChmodError> { + let node = match h { + ResolvedTarget::File(h) => Node::File(h.into_typed::().file), + ResolvedTarget::Dir(h) => Node::Dir(h.into_typed::().dir), + }; + let mut perms = match &node { + Node::File(file) => { + let file = file.write(); if file.file_type == FileType::SymbolicLink { return Err(ChmodError::IsSymlink); } sync::RwLockWriteGuard::map(file, |f| &mut f.perms) } - super::backend::HandleRef::Dir(h) => { - sync::RwLockWriteGuard::map(h.get_typed::().dir.write(), |d| &mut d.perms) - } + Node::Dir(dir) => sync::RwLockWriteGuard::map(dir.write(), |d| &mut d.perms), }; perms.mode = mode; Ok(()) @@ -822,21 +817,23 @@ impl super::backend::Backend for InMe fn chown( &self, - h: super::backend::HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError> { - let mut perms = match h { - super::backend::HandleRef::File(h) => { - let file = h.get_typed::().file.write(); + let node = match h { + ResolvedTarget::File(h) => Node::File(h.into_typed::().file), + ResolvedTarget::Dir(h) => Node::Dir(h.into_typed::().dir), + }; + let mut perms = match &node { + Node::File(file) => { + let file = file.write(); if file.file_type == FileType::SymbolicLink { return Err(ChownError::IsSymlink); } sync::RwLockWriteGuard::map(file, |f| &mut f.perms) } - super::backend::HandleRef::Dir(h) => { - sync::RwLockWriteGuard::map(h.get_typed::().dir.write(), |d| &mut d.perms) - } + Node::Dir(dir) => sync::RwLockWriteGuard::map(dir.write(), |d| &mut d.perms), }; if let Some(new_user) = user { perms.userinfo.user = new_user; @@ -945,3 +942,307 @@ struct Permissions { mode: Mode, userinfo: UserInfo, } + +impl Permissions { + fn permission_check(&self) -> PermissionCheck { + PermissionCheck::ByResolver(PermissionInfo { + mode: self.mode, + owner: self.userinfo, + on_read_only_fs: false, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::fs::OFlags; + use crate::fs::backend::Backend; + use crate::fs::composer::Composer; + use crate::fs::overlay::Overlay; + use crate::fs::resolver::Resolver; + use crate::test_platform::TestPlatform; + use core::cell::RefCell; + + const OWNER: UserInfo = UserInfo { + user: 1000, + group: 1000, + }; + const ALL: Mode = Mode::RWXU.union(Mode::RWXG).union(Mode::RWXO); + + #[test] + fn mounted_ancestor_files_are_unreachable_without_mutation() { + let backend = InMem::::new_initialized([ + ( + "/a", + InitialNode::File { + mode: ALL, + owner: OWNER, + data: b"preserve".as_slice().into(), + }, + ), + ( + "/target", + InitialNode::File { + mode: ALL, + owner: OWNER, + data: b"target".as_slice().into(), + }, + ), + ( + "/link", + InitialNode::Symlink { + mode: ALL, + owner: OWNER, + target: "/target".into(), + }, + ), + ]); + let backing_file = match backend.root.read().children.get("a").unwrap() { + Node::File(file) => file.clone(), + Node::Dir(_) => panic!("expected backing file"), + }; + let composer = Composer::builder() + .mount("/", |_| backend) + .mount("/a/b", InMem::::new) + .mount("/link/child", InMem::::new) + .build() + .unwrap(); + assert!(matches!( + composer.resolve(composer.root().unwrap(), &["a"], &|_| Ok(())), + Err(ResolutionError { + component: Some(0), + error: WalkError::PathError(PathError::NoSuchFileOrDirectory), + }) + )); + let fs = Resolver::::new(composer); + for flags in [ + OFlags::WRONLY | OFlags::TRUNC, + OFlags::CREAT | OFlags::WRONLY, + ] { + assert!(matches!( + fs.open(OWNER, "/a", flags, ALL), + Err(OpenError::PathError(PathError::NoSuchFileOrDirectory)) + )); + } + assert_eq!(backing_file.read().data.as_ref(), b"preserve"); + let fd = fs + .open(OWNER, "/link", OFlags::RDONLY, Mode::empty()) + .unwrap(); + assert_eq!(fd.path(), "/target"); + assert_eq!(fs.read_link(OWNER, "/link").unwrap(), "/target"); + } + + #[test] + fn denied_overlay_mutations_do_not_copy_up_or_materialize_ancestors() { + let upper = InMem::::new(InodeAllocator::standalone()); + let upper_root = upper.root.clone(); + let lower = InMem::::new_initialized([ + ( + "/a", + InitialNode::Directory { + mode: ALL, + owner: OWNER, + }, + ), + ( + "/a/b", + InitialNode::Directory { + mode: ALL, + owner: OWNER, + }, + ), + ( + "/a/b/file", + InitialNode::File { + mode: Mode::RUSR | Mode::WUSR | Mode::ROTH, + owner: OWNER, + data: b"keep lower".as_slice().into(), + }, + ), + ]); + let overlay = Overlay::::new(upper, lower, InodeAllocator::standalone()); + let backend = Composer::builder() + .mount("/mnt", |_| overlay) + .build() + .unwrap(); + let fs = Resolver::::new(backend); + let other_user = UserInfo { + user: 2000, + group: 2000, + }; + for flags in [ + OFlags::WRONLY, + OFlags::WRONLY | OFlags::TRUNC, + OFlags::RDWR | OFlags::TRUNC, + ] { + assert!(matches!( + fs.open(other_user, "/mnt/a/b/file", flags, Mode::empty()), + Err(OpenError::AccessNotAllowed) + )); + assert!(upper_root.read().children.is_empty()); + } + assert!(matches!( + fs.chmod(other_user, "/mnt/a/b/file", ALL), + Err(ChmodError::NotTheOwner) + )); + assert!(upper_root.read().children.is_empty()); + assert!(matches!( + fs.chown(other_user, "/mnt/a/b/file", Some(2000), None), + Err(ChownError::NotTheOwner) + )); + assert!(upper_root.read().children.is_empty()); + let fd = fs + .open( + other_user, + "/mnt/a/b/file", + OFlags::PATH | OFlags::WRONLY | OFlags::TRUNC, + Mode::empty(), + ) + .unwrap(); + assert_eq!(fs.handle_status(&fd).unwrap().size, 10); + drop(fd); + assert!(upper_root.read().children.is_empty()); + let mut fd = fs + .open(other_user, "/mnt/a/b/file", OFlags::RDONLY, Mode::empty()) + .unwrap(); + let mut buf = [0; 32]; + let count = fs.read(&mut fd, &mut buf, None).unwrap(); + assert_eq!(&buf[..count], b"keep lower"); + drop(fd); + assert!(upper_root.read().children.is_empty()); + let fd = fs + .open( + OWNER, + "/mnt/a/b/file", + OFlags::WRONLY | OFlags::TRUNC, + Mode::empty(), + ) + .unwrap(); + assert_eq!(fs.handle_status(&fd).unwrap().size, 0); + drop(fd); + assert!(upper_root.read().children.contains_key("a")); + } + + #[test] + fn root_and_final_directory_authorization_are_distinct_from_search() { + let backend = InMem::::new_initialized([ + ( + "/", + InitialNode::Directory { + mode: Mode::RUSR, + owner: OWNER, + }, + ), + ( + "/dir", + InitialNode::Directory { + mode: Mode::RUSR, + owner: OWNER, + }, + ), + ]); + let fs = Resolver::::new(backend); + let fd = fs.open(OWNER, "/", OFlags::RDONLY, Mode::empty()).unwrap(); + drop(fd); + assert!(matches!( + fs.open(OWNER, "/absent", OFlags::PATH, Mode::empty()), + Err(OpenError::PathError(PathError::NoSearchPerms)) + )); + fs.chmod(OWNER, "/", Mode::RUSR | Mode::XUSR).unwrap(); + let fd = fs + .open(OWNER, "/dir", OFlags::RDONLY, Mode::empty()) + .unwrap(); + drop(fd); + assert!(matches!( + fs.open(OWNER, "/dir/absent", OFlags::PATH, Mode::empty()), + Err(OpenError::PathError(PathError::NoSearchPerms)) + )); + let other_user = UserInfo { + user: 2000, + group: 2000, + }; + assert!(matches!( + fs.open(other_user, "/", OFlags::RDONLY, Mode::empty()), + Err(OpenError::AccessNotAllowed) + )); + let fd = fs + .open(other_user, "/", OFlags::PATH, Mode::empty()) + .unwrap(); + drop(fd); + fs.file_status(other_user, "/", true).unwrap(); + } + + #[test] + fn lookup_authorization_precedes_missing_and_target_metadata() { + let backend = InMem::::new_initialized([ + ( + "/dir", + InitialNode::Directory { + mode: Mode::RUSR, + owner: OWNER, + }, + ), + ( + "/dir/file", + InitialNode::File { + mode: Mode::WUSR, + owner: OWNER, + data: b"keep".as_slice().into(), + }, + ), + ]); + for components in [ + &["dir", "missing"][..], + &["dir", "missing", "child"], + &["dir", "file"], + &["dir", "file", "child"], + ] { + let calls = RefCell::new(Vec::new()); + let result = backend.resolve(backend.root().unwrap(), components, &|permissions| { + calls + .borrow_mut() + .push((permissions.mode, permissions.owner)); + if permissions.owner == OWNER { + Err(PathError::NoSearchPerms.into()) + } else { + Ok(()) + } + }); + assert!(matches!( + result, + Err(ResolutionError { + component: Some(1), + error: WalkError::PathError(PathError::NoSearchPerms), + }) + )); + assert_eq!(calls.borrow().len(), 2); + assert_eq!(calls.borrow()[1], (Mode::RUSR, OWNER)); + } + + assert!(matches!( + backend.resolve(backend.root().unwrap(), &["dir", "missing"], &|_| Ok(())), + Ok(Resolution::MissingFinal { .. }) + )); + assert!(matches!( + backend.resolve( + backend.root().unwrap(), + &["dir", "missing", "child"], + &|_| Ok(()) + ), + Err(ResolutionError { + component: Some(1), + error: WalkError::PathError(PathError::NoSuchFileOrDirectory), + }) + )); + assert!(matches!( + backend.resolve(backend.root().unwrap(), &["dir", "file", "child"], &|_| Ok( + () + )), + Err(ResolutionError { + component: Some(2), + error: WalkError::PathError(PathError::ComponentNotADirectory), + }) + )); + } +} diff --git a/litebox_broker_core/src/fs/nine_p/mod.rs b/litebox_broker_core/src/fs/nine_p/mod.rs index 0aaf69cc2..604e0abb8 100644 --- a/litebox_broker_core/src/fs/nine_p/mod.rs +++ b/litebox_broker_core/src/fs/nine_p/mod.rs @@ -19,13 +19,13 @@ use thiserror::Error; use self::fcall::Rlerror; use super::OFlags; use super::backend::{ - DirHandle, FileHandle, HandleRef, PermissionCheck, Permissioned, SeekBehavior, WalkOutcome, - WalkStopReason, WalkedComponent, WalkingDirHandle, + DirHandle, FileHandle, HandleRef, PermissionCheck, PermissionInfo, Resolution, ResolvedDir, + ResolvedFile, ResolvedTarget, SeekBehavior, }; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SeekError, SymlinkError, TruncateError, UnlinkError, - WalkError, WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SeekError, SymlinkError, TruncateError, + UnlinkError, WalkError, WriteError, }; use litebox_platform::buffer::{Fault, ReadBuffer, WriteBuffer}; use litebox_platform::sync; @@ -109,35 +109,10 @@ impl, - ) -> Result<(Arc>, bool), Error> { - match handle.inner { - NinePWalkingDirHandleInner::Dir { - fid, - is_backend_root, - } => Ok((fid, is_backend_root)), - NinePWalkingDirHandleInner::StoppedAtNonDir { - from, - from_is_backend_root, - prefix, - .. - } => { - if prefix.is_empty() { - return Ok((from, from_is_backend_root)); - } - let fid = self.client.walk(&from.fid, &prefix)?.into_complete_fid()?; - Ok((self.own(fid), false)) - } - } - } - /// Remove `name` from `dir`, via `Tunlinkat` where the server supports it. fn remove_at( &self, - dir: &NinePDirHandle, + dir: &NinePWalkingDirHandle, name: &str, is_file: bool, ) -> Result<(), Error> { @@ -188,46 +163,9 @@ pub struct NinePWalkingDirHandle< Platform: sync::RawSyncPrimitivesProvider, T: transport::Read + transport::Write, > { - inner: NinePWalkingDirHandleInner, -} - -enum NinePWalkingDirHandleInner< - Platform: sync::RawSyncPrimitivesProvider, - T: transport::Read + transport::Write, -> { - /// A fid on the directory itself. - Dir { - fid: Arc>, - /// Whether `fid` is the backend's own attach fid, handed out by - /// [`Backend::root`](super::backend::Backend::root). - /// - /// Such a fid is shared with the backend itself, so any operation that mutates it - /// server-side (`Tlopen`, `Tlcreate`) must be performed on a private clone instead. - is_backend_root: bool, - }, - /// The walk stopped because `name` is not a directory. - /// - /// The parent directory is kept as the walk's starting fid plus the components walked from it, - /// rather than a fid of its own, so it costs a walk only when actually needed (e.g. to follow a - /// relative symlink target). - /// - /// `child` is the walk's fid on `name` itself, or `None` when the path continued *through* the - /// non-directory, as a short walk establishes no fid. - StoppedAtNonDir { - from: Arc>, - from_is_backend_root: bool, - prefix: Vec, - name: String, - child: Option>>, - }, -} - -impl - From> for NinePWalkingDirHandle -{ - fn from(inner: NinePWalkingDirHandleInner) -> Self { - Self { inner } - } + fid: Arc>, + /// The shared attach fid must be cloned before a server-side open or create. + is_backend_root: bool, } /// Directory handle @@ -269,7 +207,8 @@ where Platform: sync::RawSyncPrimitivesProvider + 'static, T: transport::Read + transport::Write + Send + 'static, { - type WalkingDirHandle<'a> = NinePWalkingDirHandle; + type ResolvedDir<'a> = NinePWalkingDirHandle; + type ResolvedFile<'a> = NinePFileHandle; type FileHandle = NinePFileHandle; type DirHandle = NinePDirHandle; } @@ -279,115 +218,124 @@ where Platform: sync::RawSyncPrimitivesProvider + 'static, T: transport::Read + transport::Write + Send + 'static, { - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::( - NinePWalkingDirHandleInner::Dir { + fn root(&self) -> Result, WalkError> { + Ok(ResolvedDir::from_typed::( + NinePWalkingDirHandle { fid: Arc::clone(&self.root), is_backend_root: true, - } - .into(), - ) + }, + PermissionCheck::ByBackend, + )) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { - assert_ne!(components, [] as [&str; 0]); - let (from, from_is_backend_root) = self.walked_dir(from.into_typed::())?; - // 9P walks happily into files, so the qids have to be inspected to find where this walk - // must stop for the resolver's purposes. - let result = self.client.walk(&from.fid, components)?; - let first_non_dir = result - .wqids - .iter() - .position(|qid| !qid.typ.contains(fcall::QidType::DIR)); - - let Some(stopped_at) = first_non_dir else { - let Some(fid) = result.fid else { - // A short walk whose walked components are all directories means the next - // component simply does not exist. - return Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)); + _authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + let mut current = from.into_typed::(); + let mut index = 0; + for chunk in components.chunks(fcall::MAXWELEM) { + let result = match self.client.walk(¤t.fid.fid, chunk) { + Ok(result) => result, + Err(Error::Remote(ENOENT)) if index + 1 == components.len() => { + return Ok(Resolution::MissingFinal { + parent: ResolvedDir::from_typed::( + current, + PermissionCheck::ByBackend, + ), + }); + } + Err(error) => { + return Err(ResolutionError { + component: None, + error: error.into(), + }); + } }; - debug_assert_eq!(result.wqids.len(), components.len()); - return Ok(WalkOutcome { - components: backend_checked_components(components.len()), - last: WalkingDirHandle::from_typed::( - NinePWalkingDirHandleInner::Dir { - fid: self.own(fid), - is_backend_root: false, - } - .into(), - ), - stop_reason: WalkStopReason::CompleteDirectory, - }); - }; - - let child = result.fid.map(|fid| { - // A completed walk lands on the last component, so its fid names the non-directory the - // walk stopped at. Anything else would mean the server walked *through* a - // non-directory, which 9P2000.L does not permit. - assert_eq!( - stopped_at + 1, - components.len(), - "server completed a walk through a non-directory" - ); - // Holding on to the fid saves `open_file_at` a walk of its own. - self.own(fid) - }); - Ok(WalkOutcome { - components: backend_checked_components(stopped_at), - last: WalkingDirHandle::from_typed::( - NinePWalkingDirHandleInner::StoppedAtNonDir { - from, - from_is_backend_root, - prefix: components[..stopped_at] - .iter() - .map(|&part| String::from(part)) - .collect(), - name: String::from(components[stopped_at]), - child, + let fid = result.fid.map(|fid| self.own(fid)); + if let Some(non_dir) = result + .wqids + .iter() + .position(|qid| !qid.typ.contains(fcall::QidType::DIR)) + { + if index + non_dir + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + non_dir + 1), + error: PathError::ComponentNotADirectory.into(), + }); } - .into(), - ), - stop_reason: WalkStopReason::StoppedAtNonDirectory { - file_type: qid_type_to_file_type(result.wqids[stopped_at].typ), - }, - }) - } - - fn read_link_at(&self, dir: &WalkingDirHandle<'_>, name: &str) -> Result { - let walked; - let fid = match &dir.as_typed::().inner { - NinePWalkingDirHandleInner::StoppedAtNonDir { - name: stopped, - child: Some(child), - .. - } if stopped == name => child, - NinePWalkingDirHandleInner::StoppedAtNonDir { from, prefix, .. } => { - let path: Vec<&str> = prefix.iter().map(String::as_str).chain([name]).collect(); - walked = self.own(self.client.walk(&from.fid, &path)?.into_complete_fid()?); - &walked + let fid = fid.ok_or(ResolutionError { + component: None, + error: WalkError::Io, + })?; + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + NinePFileHandle { fid }, + PermissionCheck::ByBackend, + qid_type_to_file_type(result.wqids[non_dir].typ), + ), + ))); } - NinePWalkingDirHandleInner::Dir { fid, .. } => { - walked = self.own(self.client.walk(&fid.fid, &[name])?.into_complete_fid()?); - &walked + if let Some(fid) = fid { + current = NinePWalkingDirHandle { + fid, + is_backend_root: false, + }; + index += chunk.len(); + continue; } - }; + let walked = result.wqids.len(); + if index + walked + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + walked), + error: PathError::NoSuchFileOrDirectory.into(), + }); + } + // Short walks establish no fid. Recover the parent before reporting a creatable name. + if walked != 0 { + let fid = self + .client + .walk(¤t.fid.fid, &chunk[..walked]) + .and_then(client::WalkResult::into_complete_fid) + .map_err(|error| ResolutionError { + component: None, + error: error.into(), + })?; + current = NinePWalkingDirHandle { + fid: self.own(fid), + is_backend_root: false, + }; + } + return Ok(Resolution::MissingFinal { + parent: ResolvedDir::from_typed::(current, PermissionCheck::ByBackend), + }); + } + Ok(Resolution::Found(ResolvedTarget::Dir( + ResolvedDir::from_typed::(current, PermissionCheck::ByBackend), + ))) + } + + fn read_link_at(&self, dir: &ResolvedDir<'_>, name: &str) -> Result { + let dir = dir.get_typed::(); + let fid = self.own( + self.client + .walk(&dir.fid.fid, &[name])? + .into_complete_fid()?, + ); let target = self.client.readlink(&fid.fid)?; // Targets are passed through unchanged: an absolute target resolves from the guest root, // which is wherever this export is mounted. String::from_utf8(target).map_err(|_| PathError::InvalidPathname.into()) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result { assert_supported_oflags(flags); - let (fid, is_backend_root) = self.walked_dir(dir.into_typed::())?; + let NinePWalkingDirHandle { + fid, + is_backend_root, + } = dir.into_typed::(); if flags.contains(OFlags::PATH) { // An `O_PATH` handle is never opened server-side, so the walked fid can be handed over // as-is, even when it is the shared root fid. @@ -403,28 +351,23 @@ where Ok(DirHandle::from_typed::(NinePDirHandle { fid })) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { - // The walking handle can end up being opened (via `owned_dir_at`), which must not affect + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + // The walking handle can end up being opened (via `open_dir`), which must not affect // the directory handle it came from, so this hands out a private clone of the fid. let fid = self .client .clone_fid(&dir.get_typed::().fid.fid) .ok()?; - Some(WalkingDirHandle::from_typed::( - NinePWalkingDirHandleInner::Dir { + Some(ResolvedDir::from_typed::( + NinePWalkingDirHandle { fid: self.own(fid), is_backend_root: false, - } - .into(), + }, + PermissionCheck::ByBackend, )) } - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { assert_supported_oflags(flags); // TODO: we do not support non-blocking, so ignore that flag instead of returning an error. let flags = flags - OFlags::NONBLOCK; @@ -432,18 +375,7 @@ where return Err(OpenError::PathError(PathError::ComponentNotADirectory)); } - let fid = match dir.into_typed::().inner { - // The walk already ended up holding a fid on this very file. - NinePWalkingDirHandleInner::StoppedAtNonDir { - name: walked, - child: Some(child), - .. - } if walked == name => child, - inner => { - let (fid, _) = self.walked_dir(inner.into())?; - self.own(self.client.walk(&fid.fid, &[name])?.into_complete_fid()?) - } - }; + let NinePFileHandle { fid } = file.into_typed::(); if !flags.contains(OFlags::PATH) { // An `O_PATH` handle addresses the file without opening it server-side. @@ -455,10 +387,7 @@ where oflags_to_lopen(flags - OFlags::CREAT - OFlags::EXCL), )?; } - Ok(Permissioned { - item: FileHandle::from_typed::(NinePFileHandle { fid }), - permissions: PermissionCheck::ByBackend, - }) + Ok(FileHandle::from_typed::(NinePFileHandle { fid })) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { @@ -536,15 +465,28 @@ where Ok(rgetattr_to_file_status(&attr, self.device_id)?) } + fn resolved_status( + &self, + target: &ResolvedTarget<'_>, + ) -> Result { + let fid = match target { + ResolvedTarget::File(h) => &h.get_typed::().fid, + ResolvedTarget::Dir(h) => &h.get_typed::().fid, + }; + let attr = self.client.getattr(&fid.fid, fcall::GetattrMask::ALL)?; + Ok(rgetattr_to_file_status(&attr, self.device_id)?) + } + fn create_file_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: super::backend::CreationMetadata, ) -> Result { // `Tlcreate` turns the directory fid into the new file's fid server-side, so it must be // handed a private clone rather than the caller's directory handle. - let fid = self.client.clone_fid(&dir.get_typed::().fid.fid)?; + let dir = dir.into_typed::(); + let fid = self.client.clone_fid(&dir.fid.fid)?; // NOTE: 9P needs to commit to an access mode at creation time. The resolver still enforces // the caller's read/write intent via its own `read_allowed`/`write_allowed`. // @@ -564,7 +506,7 @@ where fn symlink_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _target: &str, _metadata: super::backend::CreationMetadata, @@ -577,7 +519,7 @@ where fn mkdir_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: super::backend::CreationMetadata, ) -> Result { @@ -604,11 +546,11 @@ where })) } - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError> { Ok(self.remove_at(&dir.into_typed::(), name, true)?) } - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError> { Ok(self.remove_at(&dir.into_typed::(), name, false)?) } @@ -637,10 +579,10 @@ where .rename(&source.fid, &new_dir.fid.fid, new_name)?) } - fn chmod(&self, h: HandleRef<'_>, mode: super::Mode) -> Result<(), ChmodError> { + fn chmod(&self, h: ResolvedTarget<'_>, mode: super::Mode) -> Result<(), ChmodError> { let fid = match h { - HandleRef::File(h) => &h.get_typed::().fid, - HandleRef::Dir(h) => &h.get_typed::().fid, + ResolvedTarget::File(h) => h.into_typed::().fid, + ResolvedTarget::Dir(h) => h.into_typed::().fid, }; let stat = fcall::SetAttr { mode: mode.bits().into(), @@ -653,13 +595,13 @@ where fn chown( &self, - h: HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError> { let fid = match h { - HandleRef::File(h) => &h.get_typed::().fid, - HandleRef::Dir(h) => &h.get_typed::().fid, + ResolvedTarget::File(h) => h.into_typed::().fid, + ResolvedTarget::Dir(h) => h.into_typed::().fid, }; // Only the fields actually supplied are marked valid, so the rest are left alone. let mut valid = fcall::SetattrMask::empty(); @@ -686,17 +628,6 @@ where } } -/// The 9P server is authoritative for permissions, so every component the backend reports is left -/// for it to check. -fn backend_checked_components(count: usize) -> Vec { - alloc::vec![ - WalkedComponent { - permissions: PermissionCheck::ByBackend - }; - count - ] -} - /// Flags this backend knows how to honor when opening files/directories. const SUPPORTED_OFLAGS: OFlags = OFlags::CREAT .union(OFlags::RDONLY) diff --git a/litebox_broker_core/src/fs/nine_p/tests.rs b/litebox_broker_core/src/fs/nine_p/tests.rs index 53c7e8996..d8777c4fc 100644 --- a/litebox_broker_core/src/fs/nine_p/tests.rs +++ b/litebox_broker_core/src/fs/nine_p/tests.rs @@ -145,6 +145,77 @@ fn connect_9p(server: &DiodServer) -> NinePFs { // Tests // --------------------------------------------------------------------------- +#[test] +fn overlay_authorizes_logical_targets_even_with_a_self_enforcing_lower() { + use crate::fs::UserInfo; + use crate::fs::errors::{ChmodError, ChownError, PathError}; + use crate::fs::in_mem::{InMem, InitialNode}; + use crate::fs::overlay::Overlay; + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let server = DiodServer::start(); + let file = server.export_path().join("private-file"); + std::fs::write(&file, b"server-owned data").unwrap(); + std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o600)).unwrap(); + let dir = server.export_path().join("private-dir"); + std::fs::create_dir(&dir).unwrap(); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + std::fs::write(dir.join("hidden"), b"hidden").unwrap(); + let metadata = std::fs::metadata(&file).unwrap(); + let user = UserInfo { + user: u16::try_from(metadata.uid()).unwrap().wrapping_add(1), + group: u16::try_from(metadata.gid()).unwrap().wrapping_add(1), + }; + let direct = connect_9p(&server); + drop( + direct + .open(user, "/private-file", OFlags::RDONLY, Mode::empty()) + .unwrap(), + ); + + let upper = InMem::::new_initialized([( + "/", + InitialNode::Directory { + mode: Mode::RWXU | Mode::RWXG | Mode::RWXO, + owner: UserInfo::ROOT, + }, + )]); + let backend = Overlay::::new( + upper, + attach(server.connect(), &server), + InodeAllocator::standalone(), + ); + let fs = Resolver::::new(backend); + for flags in [OFlags::RDONLY, OFlags::WRONLY | OFlags::TRUNC] { + assert!(matches!( + fs.open(user, "/private-file", flags, Mode::empty()), + Err(OpenError::AccessNotAllowed) + )); + } + assert!(matches!( + fs.open(user, "/private-dir", OFlags::RDONLY, Mode::empty()), + Err(OpenError::AccessNotAllowed) + )); + assert!(matches!( + fs.open(user, "/private-dir/hidden", OFlags::PATH, Mode::empty()), + Err(OpenError::PathError(PathError::NoSearchPerms)) + )); + assert!(matches!( + fs.chmod(user, "/private-file", Mode::RWXU), + Err(ChmodError::NotTheOwner) + )); + assert!(matches!( + fs.chown(user, "/private-file", Some(user.user), None), + Err(ChownError::NotTheOwner) + )); + let fd = fs + .open(user, "/private-file", OFlags::PATH, Mode::empty()) + .unwrap(); + assert_eq!(fs.handle_status(&fd).unwrap().size, 17); + drop(fd); + assert_eq!(std::fs::read(&file).unwrap(), b"server-owned data"); +} + #[test] fn test_nine_p_create_and_read_file() { let server = DiodServer::start(); diff --git a/litebox_broker_core/src/fs/overlay.rs b/litebox_broker_core/src/fs/overlay.rs index 2f295360c..20ff550f5 100644 --- a/litebox_broker_core/src/fs/overlay.rs +++ b/litebox_broker_core/src/fs/overlay.rs @@ -26,14 +26,14 @@ use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use litebox_platform::sync::{Mutex, MutexGuard, RawSyncPrimitivesProvider}; use super::backend::{ - Backend, BackendHandles, CreationMetadata, DirHandle, FileHandle, Handle, HandleRef, - PermissionCheck, PermissionInfo, Permissioned, SeekBehavior, WalkOutcome, WalkStopReason, - WalkedComponent, WalkingDirHandle, + Backend, BackendHandles, CreationMetadata, DirHandle, FileHandle, HandleRef, PermissionCheck, + PermissionInfo, Resolution, ResolvedDir as ScopedDir, ResolvedFile, ResolvedTarget, + SeekBehavior, }; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, - WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }; use super::inode_allocator::InodeAllocator; use super::{DirEntry, FileStatus, FileType, Mode, NodeInfo, OFlags, UserInfo}; @@ -52,8 +52,7 @@ pub struct Overlay { // Lower backends are ordered from highest to lowest precedence. lowers: Vec>, alloc: InodeAllocator, - /// Makes a resolve-then-mutate sequence atomic against other overlay mutations. It is - /// deliberately *not* taken by read paths. + /// Keeps resolution, authorization, and any subsequent copy-up or mutation atomic. namespace: Mutex, state: Mutex, } @@ -77,8 +76,19 @@ enum LayerNode { Lower(usize, NodeInfo), } -pub struct OverlayWalkingDir { +// Holds no nested-layer scopes: re-entering a layer during lazy lookup would deadlock. +pub struct OverlayWalkingDir<'a, Platform: RawSyncPrimitivesProvider> { path: Vec, + resolved: ResolvedDir, + locked: NamespaceGuard<'a, Platform>, +} + +pub struct OverlayResolvedFile<'a, Platform: RawSyncPrimitivesProvider> { + parent: Vec, + name: String, + layer: Option, + target: ResolvedTarget<'a>, + locked: NamespaceGuard<'a, Platform>, } #[derive(Clone)] @@ -93,11 +103,6 @@ pub struct OverlayFile { /// The layer this file was _opened_ against; a later copy-up can move it, which is what /// [`State::copied_up`] records. layer: OverlayFileLayer, - // Only upper files may be renamed. Their handles already address the - // object; copied-up lower handles migrate through `State::copied_up`, so neither needs this - // recorded location after moving. Lower-only files cannot move before copy-up. - parent: Vec, - name: String, } /// The layer backing an open overlay file. @@ -170,11 +175,17 @@ impl Overlay { } fn resolve_root(&self) -> Result { - let upper = self.upper.owned_dir_at(self.upper.root(), OFlags::PATH)?; + let upper = self + .upper + .open_dir(self.upper.root().map_err(walk_to_open_error)?, OFlags::PATH)?; let lowers = self .lowers .iter() - .map(|lower| lower.owned_dir_at(lower.root(), OFlags::PATH).map(Some)) + .map(|lower| { + lower + .open_dir(lower.root().map_err(walk_to_open_error)?, OFlags::PATH) + .map(Some) + }) .collect::, _>>()?; Ok(ResolvedDir { upper: Some(upper), @@ -182,49 +193,37 @@ impl Overlay { }) } - /// Resolve the directory `dir_name` within the already-resolved `parent`, given its `entry` - /// there, along with the [`WalkedComponent`] reported by the layer that owns it. + /// Resolve a child without inspecting the child's entries. fn resolve_child_dir( &self, parent: &ResolvedDir, dir_name: &str, entry: &ResolvedEntry, - ) -> Result<(ResolvedDir, WalkedComponent), OpenError> { + ) -> Result { fn walk_into_dir( backend: &dyn Backend, parent: &DirHandle, dir_name: &str, - ) -> Result<(DirHandle, WalkedComponent), OpenError> { + ) -> Result { let walking = backend.walking_dir_at(parent).ok_or(OpenError::Io)?; - let outcome = backend - .walk_directories(walking, &[dir_name]) - .map_err(|error| match error { - WalkError::PathError(error) => OpenError::PathError(error), - WalkError::Io => OpenError::Io, - })?; - let [component] = &outcome.components[..] else { - return Err(PathError::ComponentNotADirectory.into()); - }; - if outcome.stop_reason != WalkStopReason::CompleteDirectory { - return Err(PathError::ComponentNotADirectory.into()); + match backend + .resolve(walking, &[dir_name], &|_| Ok(())) + .map_err(|error| walk_to_open_error(error.error))? + { + Resolution::Found(ResolvedTarget::Dir(dir)) => backend.open_dir(dir, OFlags::PATH), + Resolution::Found(ResolvedTarget::File(_)) => { + Err(PathError::ComponentNotADirectory.into()) + } + Resolution::MissingFinal { .. } => Err(PathError::NoSuchFileOrDirectory.into()), } - let component = component.clone(); - let owned = backend.owned_dir_at(outcome.last, OFlags::PATH)?; - Ok((owned, component)) } if entry.entry.file_type != FileType::Directory { return Err(OpenError::PathError(PathError::ComponentNotADirectory)); } - let mut owner_component = None; let upper = match (&parent.upper, entry.upper) { - (Some(parent), true) => { - let (handle, component) = walk_into_dir(self.upper.as_ref(), parent, dir_name)?; - // The upper backend owns any name it has. - owner_component = Some(component); - Some(handle) - } + (Some(parent), true) => Some(walk_into_dir(self.upper.as_ref(), parent, dir_name)?), _ => None, }; @@ -232,19 +231,13 @@ impl Overlay { for ((layer, lower), parent) in self.lowers.iter().enumerate().zip(&parent.lowers) { let child = match parent { Some(parent) if entry.lower_directories[layer] => { - let (handle, component) = walk_into_dir(lower.as_ref(), parent, dir_name)?; - // Otherwise the highest-precedence lower with this name owns it. - owner_component.get_or_insert(component); - Some(handle) + Some(walk_into_dir(lower.as_ref(), parent, dir_name)?) } _ => None, }; lowers.push(child); } - let component = - owner_component.expect("a merged directory is owned by upper or by a lower directory"); - - Ok((ResolvedDir { upper, lowers }, component)) + Ok(ResolvedDir { upper, lowers }) } /// Resolve a logical `path` (relative to the overlay root) to its per-layer directories. @@ -254,11 +247,37 @@ impl Overlay { let entry = self .entry(¤t, name)? .ok_or(OpenError::PathError(PathError::MissingComponent))?; - current = self.resolve_child_dir(¤t, name, &entry)?.0; + current = self.resolve_child_dir(¤t, name, &entry)?; } Ok(current) } + fn scoped_dir<'a>( + &'a self, + path: Vec, + resolved: ResolvedDir, + locked: NamespaceGuard<'a, Platform>, + ) -> Result, OpenError> { + let (_, backend, handle) = self.owning_dir(&resolved).ok_or(OpenError::Io)?; + let status = backend + .resolved_status(&ResolvedTarget::Dir( + backend.walking_dir_at(handle).ok_or(OpenError::Io)?, + )) + .map_err(file_status_to_open_error)?; + Ok(ScopedDir::from_typed::( + OverlayWalkingDir { + path, + resolved, + locked, + }, + PermissionCheck::ByResolver(PermissionInfo { + mode: status.mode, + owner: status.owner, + on_read_only_fs: false, + }), + )) + } + /// Copy the lower file `lower` up into the upper backend as `name` within `upper_dir`. /// /// The namespace lock is held for the whole copy, so a partially written (or failed and @@ -277,7 +296,7 @@ impl Overlay { ) -> Result { let (layer, lower) = lower; let upper = self.upper.create_file_at( - upper_dir.clone(), + self.upper.walking_dir_at(upper_dir).ok_or(OpenError::Io)?, name, CreationMetadata { mode: status.mode, @@ -292,7 +311,11 @@ impl Overlay { if let Err(error) = copied { // Ancestor directories materialised for this copy-up deliberately stay behind. - let _rollback_result = self.upper.unlink_at(upper_dir.clone(), name); + let _rollback_result = self + .upper + .walking_dir_at(upper_dir) + .ok_or(UnlinkError::Io) + .and_then(|dir| self.upper.unlink_at(dir, name)); return Err(error); } // The copied-up file keeps the identity it had in the lower backend, so existing @@ -338,48 +361,65 @@ impl Overlay { } } - /// Materialise whatever `h` refers to in the upper backend, and return a handle to it. - fn ensure_upper( + /// Materialise the authorized target and retain the namespace lock through its mutation. + fn with_upper( &self, - locked: NamespaceGuard<'_, Platform>, - h: HandleRef<'_>, - ) -> Result { - let file = match h { - HandleRef::Dir(dir) => { - let path = &dir.get_typed::().path; - return Ok(Handle::Dir(self.ensure_upper_dir(&locked, path)?)); + h: ResolvedTarget<'_>, + f: impl FnOnce(ResolvedTarget<'_>) -> R, + ) -> Result { + match h { + ResolvedTarget::Dir(dir) => { + let OverlayWalkingDir { + path, + resolved, + locked, + } = dir.into_typed::(); + let upper = match resolved.upper { + Some(upper) => upper, + None => self.ensure_upper_dir(&locked, &path)?, + }; + let target = self.upper.walking_dir_at(&upper).ok_or(OpenError::Io)?; + Ok(f(ResolvedTarget::Dir(target))) + } + ResolvedTarget::File(file) => { + let OverlayResolvedFile { + parent, + name, + layer, + target, + locked, + } = file.into_typed::(); + let Some(layer) = layer else { + return Ok(f(target)); + }; + let status = self.lowers[layer] + .resolved_status(&target) + .map_err(file_status_to_open_error)?; + if status.file_type != FileType::RegularFile { + return Err(OpenError::ReadOnlyFileSystem); + } + let ResolvedTarget::File(file) = target else { + unreachable!() + }; + // Release any nested overlay scope before resolving ancestor directories there. + let lower = self.lowers[layer].open_file(file, OFlags::RDONLY)?; + let upper_dir = self.ensure_upper_dir(&locked, &parent)?; + self.copy_up_file(&locked, &upper_dir, &name, (layer, &lower), &status, false)?; + let parent = self.upper.walking_dir_at(&upper_dir).ok_or(OpenError::Io)?; + match self + .upper + .resolve(parent, &[&name], &|_| Ok(())) + .map_err(|error| walk_to_open_error(error.error))? + { + Resolution::Found(target @ ResolvedTarget::File(_)) => Ok(f(target)), + _ => Err(OpenError::Io), + } } - HandleRef::File(file) => file.get_typed::(), - }; - let (layer, lower) = match &file.layer { - OverlayFileLayer::Upper(handle) => return Ok(Handle::File(handle.clone())), - OverlayFileLayer::Lower { layer, handle, .. } => (*layer, handle), - }; - if let Some(upper) = self.migrated(file) { - return Ok(Handle::File(upper)); } - - let status = self.lowers[layer] - .status(HandleRef::File(lower)) - .map_err(file_status_to_open_error)?; - if status.file_type != FileType::RegularFile { - // Only regular files can be copied up. - return Err(OpenError::ReadOnlyFileSystem); - } - let upper_dir = self.ensure_upper_dir(&locked, &file.parent)?; - let upper = self.copy_up_file( - &locked, - &upper_dir, - &file.name, - (layer, lower), - &status, - false, - )?; - Ok(Handle::File(upper)) } fn marker_present(&self, dir: &DirHandle, marker: &str) -> Result { - Ok(self.upper.lookup_at(dir, marker)?.is_some()) + Ok(lookup_layer_entry(self.upper.as_ref(), dir, marker)?.is_some()) } /// Create `marker` in the upper directory `dir`, if not already there. @@ -398,7 +438,7 @@ impl Overlay { // Markers are overlay-internal bookkeeping, never visible to callers, so they are owned by // root rather than by whoever happened to trigger the write. self.upper.create_file_at( - dir.clone(), + self.upper.walking_dir_at(dir).ok_or(OpenError::Io)?, marker, CreationMetadata { mode: Mode::empty(), @@ -418,7 +458,10 @@ impl Overlay { .marker_present(dir, marker) .map_err(|_| UnlinkError::Io)? { - self.upper.unlink_at(dir.clone(), marker)?; + self.upper.unlink_at( + self.upper.walking_dir_at(dir).ok_or(UnlinkError::Io)?, + marker, + )?; } Ok(()) } @@ -430,13 +473,13 @@ impl Overlay { _locked: &NamespaceGuard<'_, Platform>, dir: &DirHandle, ) -> Result, UnlinkError> { - let entries = self - .upper - .list_dir_at(dir.clone()) - .map_err(|_| UnlinkError::Io)?; + let entries = list_layer_dir(self.upper.as_ref(), dir).map_err(|_| UnlinkError::Io)?; let mut removed = Vec::new(); for entry in entries.iter().filter(|entry| !valid(&entry.name)) { - self.upper.unlink_at(dir.clone(), &entry.name)?; + self.upper.unlink_at( + self.upper.walking_dir_at(dir).ok_or(UnlinkError::Io)?, + &entry.name, + )?; removed.push(entry.name.clone()); } Ok(removed) @@ -450,7 +493,9 @@ impl Overlay { locked: &NamespaceGuard<'_, Platform>, path: &[String], ) -> Result { - let mut upper = self.upper.owned_dir_at(self.upper.root(), OFlags::PATH)?; + let mut upper = self + .upper + .open_dir(self.upper.root().map_err(walk_to_open_error)?, OFlags::PATH)?; for index in 0..path.len() { // Re-resolve after each materialisation, since it changed the upper namespace. let resolved = self.resolve_dir(&path[..=index])?; @@ -478,7 +523,7 @@ impl Overlay { let child = self .upper .mkdir_at( - parent.clone(), + self.upper.walking_dir_at(parent).ok_or(OpenError::Io)?, name, CreationMetadata { mode: status.mode, @@ -501,10 +546,10 @@ impl Overlay { /// The layer that owns a resolved directory, and its handle within that layer: the upper /// directory when there is one, the highest-precedence lower directory otherwise. - fn owning_dir<'a>( + fn owning_dir<'a, 'd>( &'a self, - dir: &'a ResolvedDir, - ) -> Option<(Option, &'a dyn Backend, &'a DirHandle)> { + dir: &'d ResolvedDir, + ) -> Option<(Option, &'a dyn Backend, &'d DirHandle)> { match &dir.upper { Some(handle) => Some((None, self.upper.as_ref(), handle)), None => dir.lowers.iter().zip(&self.lowers).enumerate().find_map( @@ -593,16 +638,10 @@ impl Overlay { let mut upper = None; let mut lowers = Vec::new(); if let Some(handle) = &dir.upper { - // Read paths do not take the namespace lock, so read the mutable upper directory in a - // single listing: separate lookups of `name` and of the markers hiding it could - // straddle a concurrent writer and expose deleted lower entries. + // Read the upper entry and the markers hiding its lower counterparts together. let whiteout = whiteout(name); let mut hidden = false; - for entry in self - .upper - .list_dir_at(handle.clone()) - .map_err(|_| OpenError::Io)? - { + for entry in list_layer_dir(self.upper.as_ref(), handle).map_err(|_| OpenError::Io)? { if entry.name == name { upper = Some(entry); } else if entry.name == whiteout || entry.name == OPAQUE_MARKER { @@ -617,7 +656,8 @@ impl Overlay { // Lower backends are immutable, so they can be looked up by name. for (layer, (backend, handle)) in self.lowers.iter().zip(&dir.lowers).enumerate() { if let Some(handle) = handle - && let Some(entry) = backend.lookup_at(handle, name).map_err(|_| OpenError::Io)? + && let Some(entry) = + lookup_layer_entry(backend.as_ref(), handle, name).map_err(|_| OpenError::Io)? { lowers.push((layer, entry)); } @@ -628,7 +668,7 @@ impl Overlay { /// Every overlay-visible entry of `dir`, sorted by name. fn entries(&self, dir: &ResolvedDir) -> Result, OpenError> { fn list(backend: &dyn Backend, dir: &DirHandle) -> Result, OpenError> { - backend.list_dir_at(dir.clone()).map_err(|_| OpenError::Io) + list_layer_dir(backend, dir).map_err(|_| OpenError::Io) } let mut names: BTreeMap = BTreeMap::new(); @@ -750,6 +790,29 @@ fn whiteout(name: &str) -> String { out } +fn lookup_layer_entry( + backend: &dyn Backend, + dir: &DirHandle, + name: &str, +) -> Result, ReadDirError> { + // The default lookup scans a directory, so it needs the same private read-open as readdir. + let dir = backend.walking_dir_at(dir).ok_or(ReadDirError::Io)?; + let dir = backend + .open_dir(dir, OFlags::RDONLY) + .map_err(|_| ReadDirError::Io)?; + backend.lookup_at(&dir, name) +} + +fn list_layer_dir(backend: &dyn Backend, dir: &DirHandle) -> Result, ReadDirError> { + // Traversal retains O_PATH handles. A server may require a private, read-opened fid for + // readdir; never open the retained handle itself or bypass the server's checks. + let dir = backend.walking_dir_at(dir).ok_or(ReadDirError::Io)?; + let dir = backend + .open_dir(dir, OFlags::RDONLY) + .map_err(|_| ReadDirError::Io)?; + backend.list_dir_at(dir) +} + fn unlink_to_open_error(error: UnlinkError) -> OpenError { match error { UnlinkError::PathError(error) => OpenError::PathError(error), @@ -759,6 +822,20 @@ fn unlink_to_open_error(error: UnlinkError) -> OpenError { } } +fn walk_to_open_error(error: WalkError) -> OpenError { + match error { + WalkError::PathError(error) => OpenError::PathError(error), + WalkError::Io => OpenError::Io, + } +} + +fn open_to_walk_error(error: OpenError) -> WalkError { + match error { + OpenError::PathError(error) => WalkError::PathError(error), + _ => WalkError::Io, + } +} + fn file_status_to_open_error(error: FileStatusError) -> OpenError { match error { FileStatusError::PathError(error) => OpenError::PathError(error), @@ -767,202 +844,226 @@ fn file_status_to_open_error(error: FileStatusError) -> OpenError { } impl BackendHandles for Overlay { - type WalkingDirHandle<'a> = OverlayWalkingDir; + type ResolvedDir<'a> = OverlayWalkingDir<'a, Platform>; + type ResolvedFile<'a> = OverlayResolvedFile<'a, Platform>; type FileHandle = OverlayFile; type DirHandle = OverlayDir; } impl Backend for Overlay { - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::(OverlayWalkingDir { path: Vec::new() }) + fn root(&self) -> Result, WalkError> { + let locked = self.namespace.lock(); + let resolved = self.resolve_root().map_err(open_to_walk_error)?; + self.scoped_dir(Vec::new(), resolved, locked) + .map_err(open_to_walk_error) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ScopedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { - fn open_to_walk_error(error: OpenError) -> WalkError { - match error { - OpenError::PathError(error) => WalkError::PathError(error), - _ => WalkError::Io, - } - } - let mut path = from.into_typed::().path; - let mut walked = Vec::with_capacity(components.len()); - let mut current = self.resolve_dir(&path).map_err(open_to_walk_error)?; - for name in components { + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + let mut current = from; + for (index, name) in components.iter().enumerate() { + let fail = |error| ResolutionError { + component: Some(index), + error, + }; + let PermissionCheck::ByResolver(permissions) = ¤t.permissions else { + unreachable!() + }; + authorize_dir_lookup(permissions).map_err(fail)?; if !valid(name) { - return Err(PathError::InvalidPathname.into()); + return Err(fail(PathError::InvalidPathname.into())); } - let entry = self - .entry(¤t, name) - .map_err(open_to_walk_error)? - .ok_or(PathError::NoSuchFileOrDirectory)?; - if entry.entry.file_type != FileType::Directory { - return Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(OverlayWalkingDir { path }), - stop_reason: WalkStopReason::StoppedAtNonDirectory { - file_type: entry.entry.file_type, - }, + let OverlayWalkingDir { + mut path, + resolved, + locked, + } = current.into_typed::(); + let Some(entry) = self + .entry(&resolved, name) + .map_err(open_to_walk_error) + .map_err(fail)? + else { + if index + 1 == components.len() { + let parent = self + .scoped_dir(path, resolved, locked) + .map_err(open_to_walk_error) + .map_err(fail)?; + return Ok(Resolution::MissingFinal { parent }); + } + return Err(fail(PathError::NoSuchFileOrDirectory.into())); + }; + if entry.entry.file_type == FileType::Directory { + let child = self + .resolve_child_dir(&resolved, name, &entry) + .map_err(open_to_walk_error) + .map_err(fail)?; + path.push(String::from(*name)); + current = self + .scoped_dir(path, child, locked) + .map_err(open_to_walk_error) + .map_err(fail)?; + continue; + } + if index + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + 1), + error: PathError::ComponentNotADirectory.into(), }); } - let (child, component) = self - .resolve_child_dir(¤t, name, &entry) - .map_err(open_to_walk_error)?; - current = child; - path.push(String::from(*name)); - walked.push(component); - } - Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(OverlayWalkingDir { path }), - stop_reason: WalkStopReason::CompleteDirectory, - }) + let file_type = entry.entry.file_type; + let (layer, backend, parent) = if entry.upper { + (None, self.upper.as_ref(), resolved.upper.as_ref()) + } else { + let layer = entry.lower.ok_or_else(|| fail(WalkError::Io))?; + ( + Some(layer), + self.lowers[layer].as_ref(), + resolved.lowers[layer].as_ref(), + ) + }; + let parent = parent + .and_then(|dir| backend.walking_dir_at(dir)) + .ok_or_else(|| fail(WalkError::Io))?; + let Resolution::Found(target @ ResolvedTarget::File(_)) = backend + .resolve(parent, &[name], &|_| Ok(())) + .map_err(|mut error| { + error.component = error.component.map(|component| component + index); + error + })? + else { + return Err(fail(WalkError::Io)); + }; + let status = backend + .resolved_status(&target) + .map_err(file_status_to_open_error) + .map_err(open_to_walk_error) + .map_err(fail)?; + let permissions = PermissionCheck::ByResolver(PermissionInfo { + mode: status.mode, + owner: status.owner, + on_read_only_fs: false, + }); + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + OverlayResolvedFile { + parent: path, + name: String::from(*name), + layer, + target, + locked, + }, + permissions, + file_type, + ), + ))); + } + Ok(Resolution::Found(ResolvedTarget::Dir(current))) } - fn read_link_at(&self, dir: &WalkingDirHandle<'_>, name: &str) -> Result { - let open_to_walk_error = |error| match error { - OpenError::PathError(error) => WalkError::PathError(error), - _ => WalkError::Io, - }; - let current = self - .resolve_dir(&dir.as_typed::().path) - .map_err(open_to_walk_error)?; + fn read_link_at(&self, dir: &ScopedDir<'_>, name: &str) -> Result { + let OverlayWalkingDir { resolved, .. } = dir.get_typed::(); let entry = self - .entry(¤t, name) + .entry(resolved, name) .map_err(open_to_walk_error)? .ok_or(PathError::NoSuchFileOrDirectory)?; - let (backend, parent): (&dyn Backend, _) = if entry.upper { - (self.upper.as_ref(), current.upper.as_ref()) + let layer = if entry.upper { + None } else { - let layer = entry.lower.ok_or(WalkError::Io)?; - (self.lowers[layer].as_ref(), current.lowers[layer].as_ref()) + Some(entry.lower.ok_or(WalkError::Io)?) }; - let parent = parent.ok_or(WalkError::Io)?; - let walking = backend.walking_dir_at(parent).ok_or(WalkError::Io)?; - backend.read_link_at(&walking, name) + let backend = layer.map_or(self.upper.as_ref(), |layer| self.lowers[layer].as_ref()); + let parent = match layer { + None => resolved.upper.as_ref(), + Some(layer) => resolved.lowers[layer].as_ref(), + }; + let parent = parent + .and_then(|dir| backend.walking_dir_at(dir)) + .ok_or(WalkError::Io)?; + backend.read_link_at(&parent, name) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { - let path = dir.into_typed::().path; - let resolved = self.resolve_dir(&path)?; + fn open_dir(&self, dir: ScopedDir<'_>, flags: OFlags) -> Result { + let OverlayWalkingDir { + path, + resolved, + locked: _locked, + } = dir.into_typed::(); let (_, backend, handle) = self.owning_dir(&resolved).ok_or(OpenError::Io)?; - let walking = backend.walking_dir_at(handle).ok_or(OpenError::Io)?; - backend.owned_dir_at(walking, flags)?; + backend.open_dir(backend.walking_dir_at(handle).ok_or(OpenError::Io)?, flags)?; Ok(DirHandle::from_typed::(OverlayDir { path })) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { - Some(WalkingDirHandle::from_typed::(OverlayWalkingDir { - path: dir.get_typed::().path.clone(), - })) + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + let locked = self.namespace.lock(); + let path = dir.get_typed::().path.clone(); + let resolved = self.resolve_dir(&path).ok()?; + self.scoped_dir(path, resolved, locked).ok() } - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { - if !valid(name) { - return Err(PathError::InvalidPathname.into()); - } + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { if flags.contains(OFlags::DIRECTORY) { return Err(PathError::ComponentNotADirectory.into()); } - let path = dir.into_typed::().path; - let guard = self.namespace.lock(); - let resolved = self.resolve_dir(&path)?; - let entry = self - .entry(&resolved, name)? - .ok_or(OpenError::PathError(PathError::NoSuchFileOrDirectory))?; - // The resolver only reaches `create_file_at` once a walk reported the name as missing, so - // an existing entry means an exclusive create must fail here. - if flags.contains(OFlags::CREAT | OFlags::EXCL) { - return Err(OpenError::AlreadyExists); - } - - let (layer, permissions) = if entry.upper { - let upper = resolved.upper.as_ref().ok_or(OpenError::Io)?; - let walking = self.upper.walking_dir_at(upper).ok_or(OpenError::Io)?; - let file = self.upper.open_file_at(walking, name, flags)?; - (OverlayFileLayer::Upper(file.item), file.permissions) - } else { - let layer = entry.lower.ok_or(OpenError::Io)?; - let lower_dir = resolved.lowers[layer].as_ref().ok_or(OpenError::Io)?; - let walking = self.lowers[layer] - .walking_dir_at(lower_dir) - .ok_or(OpenError::Io)?; - // An open that may modify the file has to copy it up first; the lower backends are - // immutable, so such an open is read-only down there. - // - // XXX(jayb): the resolver authorizes an open only after this returns, so a - // writable open can copy up before a later permission denial. A preflight - // authorization hook in `Backend` would make copy-up properly two-phase. - let writing = - flags.intersects(OFlags::WRONLY | OFlags::RDWR | OFlags::APPEND | OFlags::TRUNC); + let OverlayResolvedFile { + parent, + name, + layer, + target, + locked, + } = file.into_typed::(); + let layer = if let Some(layer) = layer { + let status = self.lowers[layer] + .resolved_status(&target) + .map_err(file_status_to_open_error)?; + let ResolvedTarget::File(file) = target else { + unreachable!() + }; + let writing = !flags.contains(OFlags::PATH) + && flags.intersects(OFlags::WRONLY | OFlags::RDWR | OFlags::APPEND | OFlags::TRUNC); let lower_flags = if writing { OFlags::RDONLY } else { - flags.difference(OFlags::CREAT) + flags - OFlags::CREAT }; - let file = self.lowers[layer].open_file_at(walking, name, lower_flags)?; - // The file's own identity, which is also the key a later copy-up records itself under. - let status = self.lowers[layer] - .status(HandleRef::File(&file.item)) - .map_err(file_status_to_open_error)?; + let file = self.lowers[layer].open_file(file, lower_flags)?; if writing { - if entry.entry.file_type != FileType::RegularFile { - // Only regular files can be copied up, and the lowers do not accept writes. + if status.file_type != FileType::RegularFile { return Err(OpenError::ReadOnlyFileSystem); } - let upper_dir = self.ensure_upper_dir(&guard, &path)?; + let upper_dir = self.ensure_upper_dir(&locked, &parent)?; let upper = self.copy_up_file( - &guard, + &locked, &upper_dir, - name, - (layer, &file.item), + &name, + (layer, &file), &status, flags.contains(OFlags::TRUNC), )?; - // The lower open was substituted with a read-only one, so its `PermissionCheck` - // says nothing about the caller's write access; check the file's own mode instead. - let permissions = PermissionCheck::ByResolver(PermissionInfo { - mode: status.mode, - owner: status.owner, - }); - (OverlayFileLayer::Upper(upper), permissions) + OverlayFileLayer::Upper(upper) } else { let node = self.map_node(&mut self.state.lock().ids, Some(layer), status.node_info); - ( - OverlayFileLayer::Lower { - layer, - handle: file.item, - node, - }, - file.permissions, - ) + OverlayFileLayer::Lower { + layer, + handle: file, + node, + } } + } else { + let ResolvedTarget::File(file) = target else { + unreachable!() + }; + OverlayFileLayer::Upper(self.upper.open_file(file, flags)?) }; - - Ok(Permissioned { - item: FileHandle::from_typed::(OverlayFile { - layer, - parent: path, - name: String::from(name), - }), - permissions, - }) + Ok(FileHandle::from_typed::(OverlayFile { layer })) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { let path = handle.into_typed::().path; + let _locked = self.namespace.lock(); let resolved = self.resolve_dir(&path).map_err(|_| ReadDirError::Io)?; self.entries(&resolved).map_err(|_| ReadDirError::Io) } @@ -1040,6 +1141,7 @@ impl Backend for Overlay { } HandleRef::Dir(handle) => { let path = &handle.get_typed::().path; + let _locked = self.namespace.lock(); let resolved = self.resolve_dir(path).map_err(|_| FileStatusError::Io)?; let (layer, backend, handle) = self.owning_dir(&resolved).ok_or(FileStatusError::Io)?; @@ -1049,36 +1151,66 @@ impl Backend for Overlay { } } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + match target { + ResolvedTarget::Dir(dir) => { + let dir = dir.get_typed::(); + let (layer, backend, handle) = + self.owning_dir(&dir.resolved).ok_or(FileStatusError::Io)?; + let status = backend.resolved_status(&ResolvedTarget::Dir( + backend.walking_dir_at(handle).ok_or(FileStatusError::Io)?, + ))?; + Ok(self.map_status(status, layer)) + } + ResolvedTarget::File(file) => { + let file = file.get_typed::(); + let backend = file + .layer + .map_or(self.upper.as_ref(), |layer| self.lowers[layer].as_ref()); + Ok(self.map_status(backend.resolved_status(&file.target)?, file.layer)) + } + } + } + fn create_file_at( &self, - dir: DirHandle, + dir: ScopedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { if !valid(name) { return Err(PathError::InvalidPathname.into()); } - let path = dir.into_typed::().path; - let locked = self.namespace.lock(); - if self.entry(&self.resolve_dir(&path)?, name)?.is_some() { + let OverlayWalkingDir { + path, + resolved, + locked, + } = dir.into_typed::(); + if self.entry(&resolved, name)?.is_some() { return Err(OpenError::AlreadyExists); } let upper = self.ensure_upper_dir(&locked, &path)?; - let file = self.upper.create_file_at(upper.clone(), name, metadata)?; + let file = self.upper.create_file_at( + self.upper.walking_dir_at(&upper).ok_or(OpenError::Io)?, + name, + metadata, + )?; if let Err(error) = self.remove_marker(&locked, &upper, &whiteout(name)) { - let _rollback_result = self.upper.unlink_at(upper, name); + let _rollback_result = self + .upper + .walking_dir_at(&upper) + .ok_or(UnlinkError::Io) + .and_then(|dir| self.upper.unlink_at(dir, name)); return Err(unlink_to_open_error(error)); } Ok(FileHandle::from_typed::(OverlayFile { layer: OverlayFileLayer::Upper(file), - parent: path, - name: String::from(name), })) } fn symlink_at( &self, - dir: DirHandle, + dir: ScopedDir<'_>, name: &str, target: &str, metadata: CreationMetadata, @@ -1095,9 +1227,11 @@ impl Backend for Overlay { if !valid(name) { return Err(PathError::InvalidPathname.into()); } - let path = dir.into_typed::().path; - let locked = self.namespace.lock(); - let resolved = self.resolve_dir(&path).map_err(open_to_symlink_error)?; + let OverlayWalkingDir { + path, + resolved, + locked, + } = dir.into_typed::(); if self .entry(&resolved, name) .map_err(open_to_symlink_error)? @@ -1108,12 +1242,20 @@ impl Backend for Overlay { let upper = self .ensure_upper_dir(&locked, &path) .map_err(open_to_symlink_error)?; - self.upper - .symlink_at(upper.clone(), name, target, metadata)?; + self.upper.symlink_at( + self.upper.walking_dir_at(&upper).ok_or(SymlinkError::Io)?, + name, + target, + metadata, + )?; // Like a regular file, a replacement link clears its whiteout but // needs no opaque marker: only directories can merge with lower entries. if let Err(error) = self.remove_marker(&locked, &upper, &whiteout(name)) { - let _rollback_result = self.upper.unlink_at(upper, name); + let _rollback_result = self + .upper + .walking_dir_at(&upper) + .ok_or(UnlinkError::Io) + .and_then(|dir| self.upper.unlink_at(dir, name)); return Err(open_to_symlink_error(unlink_to_open_error(error))); } Ok(()) @@ -1121,7 +1263,7 @@ impl Backend for Overlay { fn mkdir_at( &self, - dir: DirHandle, + dir: ScopedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { @@ -1137,9 +1279,11 @@ impl Backend for Overlay { if !valid(name) { return Err(PathError::InvalidPathname.into()); } - let mut path = dir.into_typed::().path; - let locked = self.namespace.lock(); - let resolved = self.resolve_dir(&path).map_err(open_to_mkdir_error)?; + let OverlayWalkingDir { + mut path, + resolved, + locked, + } = dir.into_typed::(); if self .entry(&resolved, name) .map_err(open_to_mkdir_error)? @@ -1154,7 +1298,11 @@ impl Backend for Overlay { let recreated = self .marker_present(&upper, &whiteout) .map_err(|_| MkdirError::Io)?; - let child = self.upper.mkdir_at(upper.clone(), name, metadata)?; + let child = self.upper.mkdir_at( + self.upper.walking_dir_at(&upper).ok_or(MkdirError::Io)?, + name, + metadata, + )?; // A directory recreated over a whiteout must not re-merge with the lower directory it // replaces, so it starts out opaque. @@ -1170,7 +1318,11 @@ impl Backend for Overlay { }; if let Err(error) = cleared { let _rollback_marker = self.clear_markers(&locked, &child); - let _rollback_dir = self.upper.rmdir_at(upper, name); + let _rollback_dir = self + .upper + .walking_dir_at(&upper) + .ok_or(RmdirError::Io) + .and_then(|dir| self.upper.rmdir_at(dir, name)); return Err(error); } @@ -1178,7 +1330,7 @@ impl Backend for Overlay { Ok(DirHandle::from_typed::(OverlayDir { path })) } - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ScopedDir<'_>, name: &str) -> Result<(), UnlinkError> { fn open_to_unlink_error(error: OpenError) -> UnlinkError { match error { OpenError::PathError(error) => UnlinkError::PathError(error), @@ -1190,9 +1342,11 @@ impl Backend for Overlay { if !valid(name) { return Err(PathError::InvalidPathname.into()); } - let path = dir.into_typed::().path; - let locked = self.namespace.lock(); - let resolved = self.resolve_dir(&path).map_err(open_to_unlink_error)?; + let OverlayWalkingDir { + path, + resolved, + locked, + } = dir.into_typed::(); let entry = self .entry(&resolved, name) .map_err(open_to_unlink_error)? @@ -1214,12 +1368,15 @@ impl Backend for Overlay { None => resolved.upper.ok_or(UnlinkError::Io)?, }; if entry.upper { - self.upper.unlink_at(upper, name)?; + self.upper.unlink_at( + self.upper.walking_dir_at(&upper).ok_or(UnlinkError::Io)?, + name, + )?; } Ok(()) } - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ScopedDir<'_>, name: &str) -> Result<(), RmdirError> { fn open_to_rmdir_error(error: OpenError) -> RmdirError { match error { OpenError::PathError(error) => RmdirError::PathError(error), @@ -1231,9 +1388,11 @@ impl Backend for Overlay { if !valid(name) { return Err(PathError::InvalidPathname.into()); } - let path = dir.into_typed::().path; - let locked = self.namespace.lock(); - let resolved = self.resolve_dir(&path).map_err(open_to_rmdir_error)?; + let OverlayWalkingDir { + path, + resolved, + locked, + } = dir.into_typed::(); let entry = self .entry(&resolved, name) .map_err(open_to_rmdir_error)? @@ -1241,7 +1400,7 @@ impl Backend for Overlay { if entry.entry.file_type != FileType::Directory { return Err(RmdirError::NotADirectory); } - let (child, _) = self + let child = self .resolve_child_dir(&resolved, name, &entry) .map_err(open_to_rmdir_error)?; if !self @@ -1269,7 +1428,10 @@ impl Backend for Overlay { .clear_markers(&locked, child) .map_err(unlink_to_open_error) .map_err(open_to_rmdir_error)?; - if let Err(error) = self.upper.rmdir_at(upper, name) { + if let Err(error) = self.upper.rmdir_at( + self.upper.walking_dir_at(&upper).ok_or(RmdirError::Io)?, + name, + ) { for marker in cleared { let _rollback_marker = self.create_marker(&locked, child, &marker); } @@ -1336,7 +1498,7 @@ impl Backend for Overlay { (true, false) => return Err(RenameError::NotADirectory), (false, true) => return Err(RenameError::IsADirectory), (true, true) => { - let (child, _) = self + let child = self .resolve_child_dir(&new_dir, new_name, destination) .map_err(open_to_rename_error)?; if !self @@ -1389,38 +1551,36 @@ impl Backend for Overlay { .rename_at(old_upper, old_name, new_upper, new_name) } - fn chmod(&self, h: HandleRef<'_>, mode: Mode) -> Result<(), ChmodError> { - if let HandleRef::File(_) = h - && self.status(h).map_err(|_| ChmodError::Io)?.file_type == FileType::SymbolicLink + fn chmod(&self, h: ResolvedTarget<'_>, mode: Mode) -> Result<(), ChmodError> { + if let ResolvedTarget::File(file) = &h + && file.file_type == FileType::SymbolicLink { return Err(ChmodError::IsSymlink); } - let locked = self.namespace.lock(); - let handle = self.ensure_upper(locked, h).map_err(|error| match error { - OpenError::PathError(error) => ChmodError::PathError(error), - OpenError::ReadOnlyFileSystem => ChmodError::ReadOnlyFileSystem, - _ => ChmodError::Io, - })?; - self.upper.chmod(handle.as_ref(), mode) + self.with_upper(h, |target| self.upper.chmod(target, mode)) + .map_err(|error| match error { + OpenError::PathError(error) => ChmodError::PathError(error), + OpenError::ReadOnlyFileSystem => ChmodError::ReadOnlyFileSystem, + _ => ChmodError::Io, + })? } fn chown( &self, - h: HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError> { - if let HandleRef::File(_) = h - && self.status(h).map_err(|_| ChownError::Io)?.file_type == FileType::SymbolicLink + if let ResolvedTarget::File(file) = &h + && file.file_type == FileType::SymbolicLink { return Err(ChownError::IsSymlink); } - let locked = self.namespace.lock(); - let handle = self.ensure_upper(locked, h).map_err(|error| match error { - OpenError::PathError(error) => ChownError::PathError(error), - OpenError::ReadOnlyFileSystem => ChownError::ReadOnlyFileSystem, - _ => ChownError::Io, - })?; - self.upper.chown(handle.as_ref(), user, group) + self.with_upper(h, |target| self.upper.chown(target, user, group)) + .map_err(|error| match error { + OpenError::PathError(error) => ChownError::PathError(error), + OpenError::ReadOnlyFileSystem => ChownError::ReadOnlyFileSystem, + _ => ChownError::Io, + })? } } diff --git a/litebox_broker_core/src/fs/resolver.rs b/litebox_broker_core/src/fs/resolver.rs index ba505e28c..770ef3289 100644 --- a/litebox_broker_core/src/fs/resolver.rs +++ b/litebox_broker_core/src/fs/resolver.rs @@ -14,14 +14,14 @@ use litebox_platform::buffer::{ReadBuffer, WriteBuffer}; use super::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SeekError, SymlinkError, TruncateError, UnlinkError, - WalkError, WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SeekError, SymlinkError, TruncateError, + UnlinkError, WalkError, WriteError, }; use super::{ FileType, Mode, OFlags, backend::{ - CreationMetadata, DirHandle, FileTerminal, Handle, HandleRef, PermissionCheck, - PermissionInfo, Permissioned, SeekBehavior, WalkOutcome, WalkStopReason, WalkingDirHandle, + CreationMetadata, FileTerminal, Handle, HandleRef, PermissionCheck, PermissionInfo, + Resolution, ResolvedDir, ResolvedTarget, SeekBehavior, }, }; use super::{SeekWhence, UserInfo}; @@ -116,26 +116,6 @@ impl ResolvedPath { } } -/// A directory reached by a walk, plus the permission metadata to check against it. -struct WalkedDir<'a> { - handle: WalkingDirHandle<'a>, - /// `None` when the walk ended at the backend root, which reports no permission metadata. - permissions: Option, -} - -/// Which directories along a walk must grant search (execute) permission. -#[derive(Clone, Copy)] -enum SearchScope { - /// Every walked directory, including a final directory component, must be searchable. - AllComponents, - /// The directories leading to the object the path names must be searchable; target is not - /// checked. - ParentsOnly, - /// Like [`SearchScope::ParentsOnly`], but the final directory component is checked to be - /// readable. - AndReadableTarget, -} - impl Resolver { /// Resolve an absolute path to its physical location, following symbolic links. pub fn resolve_following_symlinks( @@ -170,9 +150,10 @@ impl Resolver = Vec::new(); let mut links = 0; + // XXX(jayb): each run re-resolves from the root, since `Backend::resolve` consumes its + // starting directory and does not report symlinks mid-walk. while let Some(name) = pending.front() { match name.as_str() { "." => { @@ -181,87 +162,72 @@ impl Resolver { pending.pop_front(); + // The remaining prefix was already resolved (and search-checked) above. resolved.pop(); - let components: Vec<_> = resolved.iter().map(String::as_str).collect(); - from = self - .walk_to_directory(context, self.backend.root(), &components, &components)? - .handle; continue; } _ => {} } - let components: Vec<_> = pending + let run_len = pending .iter() .take_while(|part| !matches!(part.as_str(), "." | "..")) - .map(String::as_str) - .collect(); - let run_len = components.len(); - let outcome = match self.backend.walk_directories(from, &components) { - Ok(outcome) => outcome, - Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)) => { - if allow_missing && !require_directory && run_len == pending.len() { - // A failed batch does not identify the missing component. Validate its - // parent before allowing creation of only the final name. - let parent: Vec<_> = resolved - .iter() - .map(String::as_str) - .chain(components[..run_len - 1].iter().copied()) - .collect(); - self.walk_to_directory(context, self.backend.root(), &parent, &parent)?; - resolved.extend(pending); - return Ok(ResolvedPath { - components: resolved, - }); + .count(); + let run_start = resolved.len(); + resolved.extend(pending.drain(..run_len)); + let mut end = resolved.len(); + let resolution = loop { + let components: Vec<_> = resolved[..end].iter().map(String::as_str).collect(); + match self.resolve_components(context, &components) { + // Retry from the failing child's parent so a mid-run symlink gets followed. + Err(ResolutionError { + component, + error: WalkError::PathError(PathError::ComponentNotADirectory), + }) if end > run_start + 1 => { + end = component + .filter(|&index| run_start < index && index < end) + .unwrap_or(end - 1); } - return Err(if run_len == pending.len() { - PathError::NoSuchFileOrDirectory - } else { - PathError::MissingComponent - } - .into()); + result => break result.map_err(|error| error.error)?, } - Err(error) => return Err(error), }; - let continuing = matches!(outcome.stop_reason, WalkStopReason::Continue); - Self::check_walk_permissions( - context, - &components, - &outcome, - if run_len == pending.len() && !continuing { - SearchScope::ParentsOnly - } else { - SearchScope::AllComponents - }, - )?; - let walked = outcome.components.len(); - assert!(walked <= run_len); - resolved.extend(pending.drain(..walked)); - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - assert_eq!(walked, run_len); - from = outcome.last; + for part in resolved.drain(end..).rev() { + pending.push_front(part); + } + let final_component = pending.is_empty(); + match resolution { + Resolution::Found(ResolvedTarget::Dir(dir)) => { + // `sub/../f` must still require search permission on `sub`. + if !pending.is_empty() + && let PermissionCheck::ByResolver(permissions) = &dir.permissions + { + Self::check_search_permission(context, permissions)?; + } } - WalkStopReason::StoppedAtNonDirectory { - file_type: FileType::SymbolicLink, - } => { - let name = pending.pop_front().expect("symlink is the next component"); - let final_component = pending.is_empty(); + Resolution::Found(ResolvedTarget::File(file)) + if file.file_type == FileType::SymbolicLink => + { if final_component && !follow_final && !require_directory { - resolved.push(name); - return Ok(ResolvedPath { - components: resolved, - }); + break; } + // TODO(jayb): add a resolved-file read_link to avoid re-resolving the parent. + drop(file); links += 1; if links > MAX_SYMLINKS { return Err(PathError::TooManySymlinks.into()); } - let target = self.backend.read_link_at(&outcome.last, &name)?; + let name = resolved + .pop() + .expect("symlink is the last resolved component"); + let parent_components: Vec<_> = resolved.iter().map(String::as_str).collect(); + let Resolution::Found(ResolvedTarget::Dir(parent)) = self + .resolve_components(context, &parent_components) + .map_err(|error| error.error)? + else { + return Err(PathError::MissingComponent.into()); + }; + let target = self.backend.read_link_at(&parent, &name)?; if target.starts_with('/') { resolved.clear(); - from = self.backend.root(); - } else { - from = outcome.last; } if final_component && target.ends_with('/') { require_directory = true; @@ -270,21 +236,21 @@ impl Resolver { - let name = pending - .pop_front() - .expect("non-directory is the next component"); - if !pending.is_empty() || require_directory { + Resolution::Found(ResolvedTarget::File(_)) => { + if !final_component || require_directory { return Err(PathError::ComponentNotADirectory.into()); } - resolved.push(name); - return Ok(ResolvedPath { - components: resolved, - }); } - WalkStopReason::Continue => { - assert!(walked > 0 && walked < run_len); - from = outcome.last; + Resolution::MissingFinal { .. } => { + if final_component && allow_missing && !require_directory { + break; + } + return Err(if final_component { + PathError::NoSuchFileOrDirectory + } else { + PathError::MissingComponent + } + .into()); } } } @@ -297,10 +263,8 @@ impl Resolver Result, String)>, WalkError> { - // Return the walking handle rather than an owned directory handle so backends can keep any - // locks acquired during path resolution held across the final operation. This lets e.g. - // "walk parent + mutate child" stay atomic. + ) -> Result, String)>, WalkError> { + // Keep scoped resolution state alive across the mutation so backend locks stay held. let trimmed = path.trim_end_matches('/'); if trimmed.is_empty() { return Ok(None); @@ -314,12 +278,19 @@ impl Resolver = parent_path.components.iter().map(String::as_str).collect(); - let parent = self.walk_to_directory( - context, - self.backend.root(), - &parent_components, - &parent_components, - )?; + let parent = match self + .resolve_components(context, &parent_components) + .map_err(|error| error.error)? + { + Resolution::Found(ResolvedTarget::Dir(parent)) => parent, + Resolution::Found(ResolvedTarget::File(_)) => { + return Err(PathError::ComponentNotADirectory.into()); + } + Resolution::MissingFinal { .. } => return Err(PathError::MissingComponent.into()), + }; + if let PermissionCheck::ByResolver(permissions) = &parent.permissions { + Self::check_search_permission(context, permissions)?; + } Ok(Some((parent, String::from(name)))) } @@ -330,247 +301,96 @@ impl Resolver self.backend.read_link_at(&outcome.last, name), - _ => Err(PathError::InvalidPathname.into()), + let all: Vec<_> = path.components.iter().map(String::as_str).collect(); + let is_link = matches!( + self.resolve_components(&context, &all) + .map_err(|error| error.error)?, + Resolution::Found(ResolvedTarget::File(file)) + if file.file_type == FileType::SymbolicLink + ); + if !is_link { + return Err(PathError::InvalidPathname.into()); } + let Resolution::Found(ResolvedTarget::Dir(parent)) = self + .resolve_components(&context, &components) + .map_err(|error| error.error)? + else { + return Err(PathError::MissingComponent.into()); + }; + self.backend.read_link_at(&parent, name) } /// Whether `context` may add or remove entries in `dir`. - /// - /// A `dir` without permission metadata is the backend root, which the backend does not report - /// permissions for; such directories are currently left unchecked. - // TODO(jayb): Check write permission on the root directory too. That needs the backend to - // report permissions for [`super::backend::Backend::root`]. // TODO(jayb): Prioritize `EROFS` before this permission check runs; currently not an issue due // to 0777 from read-only backends, but needs an update then. - fn can_change_entries_in_dir(context: &Context, dir: &WalkedDir<'_>) -> bool { + fn can_change_entries_in_dir(context: &Context, dir: &ResolvedDir<'_>) -> bool { match &dir.permissions { - None | Some(PermissionCheck::ByBackend) => true, - Some(PermissionCheck::ByResolver(permissions)) => context.can_write(permissions), + PermissionCheck::ByBackend => true, + PermissionCheck::ByResolver(permissions) => context.can_write(permissions), } } - fn owned_parent_dir(&self, dir: WalkingDirHandle<'_>) -> Result { - self.backend - .owned_dir_at(dir, OFlags::PATH) - .map_err(|error| match error { - OpenError::PathError(PathError::NoSuchFileOrDirectory) => { - PathError::MissingComponent.into() - } - OpenError::PathError(error) => error.into(), - _ => WalkError::Io, - }) - } - - /// Resolve `path` to an owned handle on the file or directory it names, plus how permissions - /// on it are to be checked. - /// - /// The handle is taken with [`OFlags::PATH`], as it addresses the object for operations that - /// do not read or write its contents, and thus needs no access permissions on it. - fn path_handle( + fn resolve_target( &self, context: &Context, path: &ResolvedPath, - ) -> Result, WalkError> { - let map_open_error = |error| match error { - OpenError::PathError(error) => WalkError::PathError(error), - _ => WalkError::Io, - }; + ) -> Result, WalkError> { let components: Vec<_> = path.components.iter().map(String::as_str).collect(); - if components.is_empty() { - let root = self - .backend - .owned_dir_at(self.backend.root(), OFlags::PATH) - .map_err(map_open_error)?; - // A backend root reports no permission metadata, so the backend is left to enforce - // whatever it wants on it. - return Ok(Permissioned { - item: Handle::Dir(root), - permissions: PermissionCheck::ByBackend, - }); - } - let (outcome, walked) = self.walk_path( - context, - self.backend.root(), - &components, - &components, - SearchScope::ParentsOnly, - )?; - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - let permissions = outcome - .components - .last() - .map_or(PermissionCheck::ByBackend, |component| { - component.permissions.clone() - }); - let dir = self - .backend - .owned_dir_at(outcome.last, OFlags::PATH) - .map_err(map_open_error)?; - Ok(Permissioned { - item: Handle::Dir(dir), - permissions, - }) - } - WalkStopReason::StoppedAtNonDirectory { .. } => { - let file = self - .backend - .open_file_at(outcome.last, components[walked], OFlags::PATH) - .map_err(map_open_error)?; - Ok(Permissioned { - item: Handle::File(file.item), - permissions: file.permissions, - }) - } - WalkStopReason::Continue => { - // `walk_path` validates stop reasons before returning. - unreachable!() - } + match self + .resolve_components(context, &components) + .map_err(|error| error.error)? + { + Resolution::Found(target) => Ok(target), + Resolution::MissingFinal { .. } => Err(PathError::NoSuchFileOrDirectory.into()), } } - fn walk_to_directory<'a>( - &'a self, + fn check_search_permission( context: &Context, - from: WalkingDirHandle<'a>, - components: &[&str], - absolute_components: &[&str], - ) -> Result, WalkError> { - if components.is_empty() { - // TODO(jayb): Decide whether empty walks from a non-root handle need permission checks. - return Ok(WalkedDir { - handle: from, - permissions: None, - }); - } - - let mut from = from; - let mut offset = 0; - loop { - let outcome = self - .backend - .walk_directories(from, &components[offset..]) - .map_err(|error| match error { - WalkError::PathError(PathError::NoSuchFileOrDirectory) => { - PathError::MissingComponent.into() - } - error => error, - })?; - let continuing = matches!(outcome.stop_reason, WalkStopReason::Continue); - Self::check_walk_permissions( - context, - &absolute_components[offset..], - &outcome, - SearchScope::AllComponents, - )?; - let walked = outcome.components.len(); - offset += walked; - - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - assert_eq!(walked, components.len() - (offset - walked)); - let permissions = outcome - .components - .last() - .map(|component| component.permissions.clone()); - return Ok(WalkedDir { - handle: outcome.last, - permissions, - }); - } - WalkStopReason::StoppedAtNonDirectory { .. } => { - return Err(WalkError::PathError(PathError::ComponentNotADirectory)); - } - WalkStopReason::Continue => { - assert!(continuing && walked > 0 && offset < components.len()); - from = outcome.last; - } - } + permissions: &PermissionInfo, + ) -> Result<(), WalkError> { + if !context.can_execute(permissions) { + litebox_util_log::debug!(perms:? = permissions.mode; "no search permission"); + return Err(PathError::NoSearchPerms.into()); } + Ok(()) } - fn walk_path<'a>( - &'a self, + fn resolve_components( + &self, context: &Context, - from: WalkingDirHandle<'a>, components: &[&str], - absolute_components: &[&str], - scope: SearchScope, - ) -> Result<(WalkOutcome>, usize), WalkError> { - assert_ne!(components, [] as [&str; 0]); - let mut from = from; - let mut offset = 0; - loop { - let outcome = self.backend.walk_directories(from, &components[offset..])?; - let continuing = matches!(outcome.stop_reason, WalkStopReason::Continue); - Self::check_walk_permissions( - context, - &absolute_components[offset..], - &outcome, - if continuing { - SearchScope::AllComponents - } else { - scope - }, - )?; - - let walked = outcome.components.len(); - offset += walked; - match outcome.stop_reason { - WalkStopReason::CompleteDirectory => { - assert_eq!(walked, components.len() - (offset - walked)); - return Ok((outcome, offset)); - } - WalkStopReason::StoppedAtNonDirectory { .. } if offset == components.len() - 1 => { - return Ok((outcome, offset)); - } - WalkStopReason::StoppedAtNonDirectory { .. } => { - return Err(WalkError::PathError(PathError::ComponentNotADirectory)); + ) -> Result, ResolutionError> { + let root = self.backend.root().map_err(|error| ResolutionError { + component: None, + error, + })?; + self.backend + .resolve(root, components, &|permissions| { + Self::check_search_permission(context, permissions) + }) + .map_err(|mut error| { + if let Some(index) = error.component + && let WalkError::PathError(PathError::NoSearchPerms) = &error.error + { + litebox_util_log::debug!( + dir:% = alloc::format!("/{}", components[..index].join("/")); + "no search permission" + ); } - WalkStopReason::Continue => { - assert!(continuing && walked > 0 && offset < components.len()); - from = outcome.last; + if matches!( + error.error, + WalkError::PathError(PathError::NoSuchFileOrDirectory) + ) && error + .component + .is_none_or(|index| index < components.len().saturating_sub(1)) + { + // TODO(jayb): keeps ENOENT for a final name hidden by Composer (see composer + // shadowed-file handling). + error.error = PathError::MissingComponent.into(); } - } - } - } - - fn check_walk_permissions( - context: &Context, - absolute_components: &[&str], - outcome: &WalkOutcome>, - scope: SearchScope, - ) -> Result<(), PathError> { - for (idx, walked) in outcome.components.iter().enumerate() { - let PermissionCheck::ByResolver(permissions) = &walked.permissions else { - continue; - }; - let is_target_dir = idx + 1 == outcome.components.len() - && matches!(outcome.stop_reason, WalkStopReason::CompleteDirectory); - let allowed = match (is_target_dir, scope) { - (true, SearchScope::ParentsOnly) => continue, - (true, SearchScope::AndReadableTarget) => context.can_read(permissions), - _ => context.can_execute(permissions), - }; - if !allowed { - // TODO(jayb): a [`SearchScope::AndReadableTarget`] target denying *read* permission - // reports `NoSearchPerms` too. Clean up during filesystem errors overhaul. - litebox_util_log::debug!( - dir:% = alloc::format!("/{}", absolute_components[..=idx].join("/")), - perms:? = permissions.mode; - "no search permission" - ); - return Err(PathError::NoSearchPerms); - } - } - Ok(()) + error + }) } } @@ -686,55 +506,37 @@ impl Resolver = path.components.iter().map(String::as_str).collect(); - let walk = self.walk_path( - &context, - self.backend.root(), - &components, - &components, - if path_only { - SearchScope::ParentsOnly - } else if read_allowed { - SearchScope::AndReadableTarget - } else { - // XXX: necessary due to #884/#888, ideally we would not have this path hit - SearchScope::AllComponents - }, - ); - match walk { - Ok((outcome, _)) if outcome.stop_reason == WalkStopReason::CompleteDirectory => { - if flags.contains(OFlags::CREAT) && flags.contains(OFlags::EXCL) { - return Err(OpenError::AlreadyExists); + let resolution = + self.resolve_components(&context, &components) + .map_err(|error| match error.error { + WalkError::Io => OpenError::Io, + WalkError::PathError(error) => error.into(), + })?; + match resolution { + Resolution::Found(_) if flags.contains(OFlags::CREAT | OFlags::EXCL) => { + Err(OpenError::AlreadyExists) + } + Resolution::Found(ResolvedTarget::Dir(dir)) => { + // XXX(jayb): non-read, non-PATH dir opens check search instead of read (#884/#888 + // workaround); Linux returns EISDIR for write opens. + if !path_only + && let PermissionCheck::ByResolver(permissions) = &dir.permissions + && if read_allowed { + !context.can_read(permissions) + } else { + !context.can_execute(permissions) + } + { + return Err(OpenError::AccessNotAllowed); } Ok(entry( - Handle::Dir(self.backend.owned_dir_at(outcome.last, flags)?), + Handle::Dir(self.backend.open_dir(dir, flags)?), SeekBehavior::NonSeekable, )) } - Ok((outcome, walked)) - if matches!( - outcome.stop_reason, - WalkStopReason::StoppedAtNonDirectory { .. } - ) => - { - let name = components[walked]; - if flags.contains(OFlags::CREAT) && flags.contains(OFlags::EXCL) { - return Err(OpenError::AlreadyExists); - } - if outcome.stop_reason - == (WalkStopReason::StoppedAtNonDirectory { - file_type: FileType::SymbolicLink, - }) + Resolution::Found(ResolvedTarget::File(file)) => { + if file.file_type == FileType::SymbolicLink && !flags.contains(OFlags::PATH | OFlags::NOFOLLOW) { return Err(if flags.contains(OFlags::DIRECTORY) { @@ -744,47 +546,33 @@ impl Resolver { - // `walk_path` validates stop reasons before returning. - unreachable!() + let wants_write = write_allowed || flags.contains(OFlags::TRUNC); + if !path_only && let PermissionCheck::ByResolver(permissions) = &file.permissions { + if wants_write && permissions.on_read_only_fs { + return Err(OpenError::ReadOnlyFileSystem); + } + if (read_allowed && !context.can_read(permissions)) + || (wants_write && !context.can_write(permissions)) + { + return Err(OpenError::AccessNotAllowed); + } + } + let file = self.backend.open_file(file, flags)?; + let seek_behavior = self.backend.seek_behavior(&file); + Ok(entry(Handle::File(file), seek_behavior)) } - Err(WalkError::PathError(PathError::NoSuchFileOrDirectory)) - if flags.contains(OFlags::CREAT) => - { - let Some((parent_components, name)) = path.parent_and_name() else { - unreachable!("root path was handled above") + Resolution::MissingFinal { parent } if flags.contains(OFlags::CREAT) => { + let Some(name) = components.last() else { + // components is empty => `/` => cannot be missing final + unreachable!() }; - let parent = self - .walk_to_directory( - &context, - self.backend.root(), - &parent_components, - &parent_components, - ) - .map_err(|error| match error { - WalkError::Io => OpenError::Io, - WalkError::PathError(error) => error.into(), - })?; if !Self::can_change_entries_in_dir(&context, &parent) { return Err(OpenError::NoWritePerms); } - let parent = self - .owned_parent_dir(parent.handle) - .map_err(|error| match error { - WalkError::Io => OpenError::Io, - WalkError::PathError(error) => error.into(), - })?; let file = self.backend.create_file_at( parent, name, @@ -796,10 +584,7 @@ impl Resolver match error { - WalkError::Io => Err(OpenError::Io), - WalkError::PathError(error) => Err(error.into()), - }, + Resolution::MissingFinal { .. } => Err(PathError::NoSuchFileOrDirectory.into()), } } @@ -1019,7 +804,11 @@ impl Resolver bool { + fn may_change_metadata(context: &Context, target: &ResolvedTarget<'_>) -> bool { + let permissions = match target { + ResolvedTarget::Dir(dir) => &dir.permissions, + ResolvedTarget::File(file) => &file.permissions, + }; let PermissionCheck::ByResolver(permissions) = permissions else { return true; }; @@ -1036,16 +825,16 @@ impl Resolver ChmodError::Io, WalkError::PathError(error) => error.into(), })?; - let handle = self - .path_handle(&context, &path) + let target = self + .resolve_target(&context, &path) .map_err(|error| match error { WalkError::Io => ChmodError::Io, WalkError::PathError(error) => error.into(), })?; - if !Self::may_change_metadata(&context, &handle.permissions) { + if !Self::may_change_metadata(&context, &target) { return Err(ChmodError::NotTheOwner); } - self.backend.chmod(handle.item.as_ref(), mode) + self.backend.chmod(target, mode) } /// Change the owner of a file @@ -1063,16 +852,16 @@ impl Resolver ChownError::Io, WalkError::PathError(error) => error.into(), })?; - let handle = self - .path_handle(&context, &path) + let target = self + .resolve_target(&context, &path) .map_err(|error| match error { WalkError::Io => ChownError::Io, WalkError::PathError(error) => error.into(), })?; - if !Self::may_change_metadata(&context, &handle.permissions) { + if !Self::may_change_metadata(&context, &target) { return Err(ChownError::NotTheOwner); } - self.backend.chown(handle.item.as_ref(), user, group) + self.backend.chown(target, user, group) } /// Unlink a file @@ -1094,25 +883,24 @@ impl Resolver { - UnlinkError::IsADirectory + match self.backend.resolve(parent, &[&name], &|permissions| { + Self::check_search_permission(&context, permissions) + }) { + Ok(Resolution::Found(ResolvedTarget::Dir(_))) => UnlinkError::IsADirectory, + Ok(Resolution::Found(ResolvedTarget::File(_))) => { + PathError::ComponentNotADirectory.into() } - Ok(_) => PathError::ComponentNotADirectory.into(), - Err(WalkError::Io) => UnlinkError::Io, - Err(WalkError::PathError(error)) => error.into(), + Ok(Resolution::MissingFinal { .. }) => PathError::NoSuchFileOrDirectory.into(), + Err(error) => match error.error { + WalkError::Io => UnlinkError::Io, + WalkError::PathError(error) => error.into(), + }, }, ); } if !Self::can_change_entries_in_dir(&context, &parent) { return Err(UnlinkError::NoWritePerms); } - let parent = self - .owned_parent_dir(parent.handle) - .map_err(|error| match error { - WalkError::Io => UnlinkError::Io, - WalkError::PathError(error) => error.into(), - })?; self.backend.unlink_at(parent, &name) } @@ -1145,12 +933,6 @@ impl Resolver SymlinkError::Io, - WalkError::PathError(error) => error.into(), - })?; self.backend.symlink_at( parent, &name, @@ -1197,18 +979,34 @@ impl Resolver RenameError::PathError(error), + OpenError::ReadOnlyFileSystem => RenameError::ReadOnlyFileSystem, + OpenError::NoWritePerms | OpenError::AccessNotAllowed => { + RenameError::NoWritePerms + } + _ => RenameError::Io, + })?; // XXX(jayb): like unlink, this trailing-slash check isn't atomic with the rename; fix // as part of broader atomicity fixups. if require_directory { let walking = self.backend.walking_dir_at(&dir).ok_or(RenameError::Io)?; - let outcome = self + match self .backend - .walk_directories(walking, &[&name]) - .map_err(map_walk_error)?; - if outcome.stop_reason != WalkStopReason::CompleteDirectory { - return Err(PathError::ComponentNotADirectory.into()); + .resolve(walking, &[&name], &|permissions| { + Self::check_search_permission(&context, permissions) + }) + .map_err(|error| map_walk_error(error.error))? + { + Resolution::Found(ResolvedTarget::Dir(_)) => {} + Resolution::Found(ResolvedTarget::File(_)) => { + return Err(PathError::ComponentNotADirectory.into()); + } + Resolution::MissingFinal { .. } => { + return Err(PathError::NoSuchFileOrDirectory.into()); + } } } Ok((dir, name)) @@ -1242,12 +1040,6 @@ impl Resolver MkdirError::Io, - WalkError::PathError(error) => error.into(), - })?; self.backend .mkdir_at( parent, @@ -1280,12 +1072,6 @@ impl Resolver RmdirError::Io, - WalkError::PathError(error) => error.into(), - })?; self.backend.rmdir_at(parent, &name) } @@ -1329,25 +1115,21 @@ impl Resolver Result { - let flags = if follow_symlinks { - OFlags::PATH - } else { - OFlags::PATH | OFlags::NOFOLLOW - }; - let entry = self - .open(user, path, flags, Mode::empty()) + let context = Context::new(user); + let path = self + .resolve_path(&context, path, follow_symlinks, false) .map_err(|error| match error { - OpenError::PathError(error) => error.into(), - OpenError::Io - | OpenError::AccessNotAllowed - | OpenError::NoWritePerms - | OpenError::ReadOnlyFileSystem - | OpenError::AlreadyExists - | OpenError::TruncateError(_) - | OpenError::NoSuchDevice - | OpenError::NoSpace => FileStatusError::Io, + WalkError::Io => FileStatusError::Io, + WalkError::PathError(error) => error.into(), + })?; + let target = self + .resolve_target(&context, &path) + .map_err(|error| match error { + WalkError::Io => FileStatusError::Io, + WalkError::PathError(error) => error.into(), })?; - self.handle_status(&entry) + // Unlike an O_PATH open, this must not trigger backend open hooks. + self.backend.resolved_status(&target) } /// Equivalent to [`Self::file_status`], but on an open entry instead. diff --git a/litebox_broker_core/src/fs/tar_ro.rs b/litebox_broker_core/src/fs/tar_ro.rs index c501279ba..a8b0b5994 100644 --- a/litebox_broker_core/src/fs/tar_ro.rs +++ b/litebox_broker_core/src/fs/tar_ro.rs @@ -34,10 +34,14 @@ use super::{DirEntry, FileType}; use super::{ Mode, NodeInfo, OFlags, UserInfo, - backend::{CreationMetadata, DirHandle, FileHandle, HandleRef, WalkingDirHandle}, + backend::{ + CreationMetadata, DirHandle, FileHandle, HandleRef, PermissionCheck, PermissionInfo, + Resolution, ResolvedDir, ResolvedFile, ResolvedTarget, + }, errors::{ ChmodError, ChownError, MkdirError, OpenError, PathError, ReadDirError, ReadError, - RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, WriteError, + RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }, inode_allocator::InodeAllocator, }; @@ -52,6 +56,17 @@ pub struct TarRo { } impl TarRo { + fn resolved_dir<'a>(&self, dir: TarRoDirHandle) -> ResolvedDir<'a> { + let permissions = PermissionCheck::ByResolver(PermissionInfo { + mode: DEFAULT_DIR_MODE, + owner: self.tar_index.dirs[dir.idx] + .owner + .unwrap_or(DEFAULT_DIRECTORY_OWNER), + on_read_only_fs: true, + }); + ResolvedDir::from_typed::(dir, permissions) + } + /// Construct a tar backend using a caller-provided inode allocator. #[must_use] pub fn new( @@ -75,91 +90,85 @@ pub struct TarRoFileHandle { idx: usize, } impl super::backend::BackendHandles for TarRo { - type WalkingDirHandle<'a> = TarRoDirHandle; + type ResolvedDir<'a> = TarRoDirHandle; + type ResolvedFile<'a> = TarRoFileHandle; type FileHandle = TarRoFileHandle; type DirHandle = TarRoDirHandle; } impl super::backend::Backend for TarRo { - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::(TarRoDirHandle { idx: 0 }) + fn root(&self) -> Result, WalkError> { + Ok(self.resolved_dir(TarRoDirHandle { idx: 0 })) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { - let mut current = from.into_typed::(); - let mut walked_components = Vec::with_capacity(components.len()); - for component in components { - let child = self.tar_index.dirs[current.idx] - .children - .get(*component) - .ok_or(WalkError::PathError(PathError::NoSuchFileOrDirectory))?; - let IndexedChild::Dir(child_idx) = *child else { - return Ok(super::backend::WalkOutcome { - components: walked_components, - last: WalkingDirHandle::from_typed::(current), - stop_reason: super::backend::WalkStopReason::StoppedAtNonDirectory { - file_type: FileType::RegularFile, - }, - }); + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + let mut current = from; + for (index, component) in components.iter().enumerate() { + let fail = |error| ResolutionError { + component: Some(index), + error, }; - - let child = &self.tar_index.dirs[child_idx]; - walked_components.push(super::backend::WalkedComponent { - permissions: super::backend::PermissionCheck::ByResolver( - super::backend::PermissionInfo { - mode: DEFAULT_DIR_MODE, - owner: child.owner.unwrap_or(DEFAULT_DIRECTORY_OWNER), - }, - ), - }); - current = TarRoDirHandle { idx: child_idx }; + let PermissionCheck::ByResolver(permissions) = ¤t.permissions else { + unreachable!() + }; + authorize_dir_lookup(permissions).map_err(fail)?; + let dir = current.into_typed::(); + match self.tar_index.dirs[dir.idx].children.get(*component) { + Some(IndexedChild::Dir(idx)) => { + current = self.resolved_dir(TarRoDirHandle { idx: *idx }); + } + Some(IndexedChild::File(idx)) => { + if index + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + 1), + error: PathError::ComponentNotADirectory.into(), + }); + } + let file = &self.tar_index.files[*idx]; + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + TarRoFileHandle { idx: *idx }, + PermissionCheck::ByResolver(PermissionInfo { + mode: file.mode, + owner: file.owner, + on_read_only_fs: true, + }), + FileType::RegularFile, + ), + ))); + } + None if index + 1 == components.len() => { + return Ok(Resolution::MissingFinal { + parent: self.resolved_dir(dir), + }); + } + None => return Err(fail(PathError::NoSuchFileOrDirectory.into())), + } } - Ok(super::backend::WalkOutcome { - components: walked_components, - last: WalkingDirHandle::from_typed::(current), - stop_reason: super::backend::WalkStopReason::CompleteDirectory, - }) + Ok(Resolution::Found(ResolvedTarget::Dir(current))) } - fn read_link_at(&self, _dir: &WalkingDirHandle<'_>, _name: &str) -> Result { + fn read_link_at(&self, _dir: &ResolvedDir<'_>, _name: &str) -> Result { Err(PathError::InvalidPathname.into()) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result { if flags.intersects(OFlags::CREAT | OFlags::TRUNC | OFlags::WRONLY | OFlags::RDWR) { return Err(OpenError::ReadOnlyFileSystem); } Ok(DirHandle::from_typed::(dir.into_typed::())) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { - Some(WalkingDirHandle::from_typed::( - dir.get_typed::().clone(), - )) + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + Some(self.resolved_dir(dir.get_typed::().clone())) } - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { - let dir = dir.into_typed::(); - let child = self.tar_index.dirs[dir.idx] - .children - .get(name) - .ok_or(OpenError::PathError(PathError::NoSuchFileOrDirectory))?; - let IndexedChild::File(file_idx) = *child else { - return Err(OpenError::PathError(PathError::ComponentNotADirectory)); - }; + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { if flags.contains(OFlags::DIRECTORY) { return Err(OpenError::PathError(PathError::ComponentNotADirectory)); } @@ -171,16 +180,7 @@ impl super::backend::Backend for TarRo { { return Err(OpenError::ReadOnlyFileSystem); } - let file = &self.tar_index.files[file_idx]; - Ok(super::backend::Permissioned { - item: FileHandle::from_typed::(TarRoFileHandle { idx: file_idx }), - permissions: super::backend::PermissionCheck::ByResolver( - super::backend::PermissionInfo { - mode: file.mode, - owner: file.owner, - }, - ), - }) + Ok(FileHandle::from_typed::(file.into_typed::())) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { @@ -262,9 +262,23 @@ impl super::backend::Backend for TarRo { } } + fn resolved_status( + &self, + target: &ResolvedTarget<'_>, + ) -> Result { + match target { + ResolvedTarget::File(h) => self.status(HandleRef::File( + &FileHandle::from_typed::(h.get_typed::().clone()), + )), + ResolvedTarget::Dir(h) => self.status(HandleRef::Dir(&DirHandle::from_typed::( + h.get_typed::().clone(), + ))), + } + } + fn create_file_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { @@ -273,7 +287,7 @@ impl super::backend::Backend for TarRo { fn symlink_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _target: &str, _metadata: CreationMetadata, @@ -283,14 +297,14 @@ impl super::backend::Backend for TarRo { fn mkdir_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { Err(MkdirError::ReadOnlyFileSystem) } - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError> { let dir = dir.into_typed::(); match self.tar_index.dirs[dir.idx].children.get(name) { Some(IndexedChild::Dir(_)) => Err(UnlinkError::IsADirectory), @@ -299,7 +313,7 @@ impl super::backend::Backend for TarRo { } } - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError> { let dir = dir.into_typed::(); match self.tar_index.dirs[dir.idx].children.get(name) { Some(IndexedChild::Dir(_)) => Err(RmdirError::ReadOnlyFileSystem), @@ -318,13 +332,13 @@ impl super::backend::Backend for TarRo { Err(RenameError::ReadOnlyFileSystem) } - fn chmod(&self, _h: HandleRef<'_>, _mode: Mode) -> Result<(), ChmodError> { + fn chmod(&self, _h: ResolvedTarget<'_>, _mode: Mode) -> Result<(), ChmodError> { Err(ChmodError::ReadOnlyFileSystem) } fn chown( &self, - _h: HandleRef<'_>, + _h: ResolvedTarget<'_>, _user: Option, _group: Option, ) -> Result<(), ChownError> { diff --git a/litebox_broker_core/src/fs/tests.rs b/litebox_broker_core/src/fs/tests.rs index 9f46b3ab0..6f30aa96a 100644 --- a/litebox_broker_core/src/fs/tests.rs +++ b/litebox_broker_core/src/fs/tests.rs @@ -162,6 +162,39 @@ mod in_mem { )); } + #[test] + fn denied_open_does_not_truncate_the_file() { + let fs = in_mem_fs(); + world_writable_tmp(&fs); + let path = "/tmp/private"; + let mut fd = fs + .open( + USER, + path, + OFlags::CREAT | OFlags::WRONLY, + Mode::RUSR | Mode::WUSR, + ) + .unwrap(); + fs.write(&mut fd, b"keep this data", None).unwrap(); + drop(fd); + fs.chmod(USER, path, Mode::RUSR).unwrap(); + + for flags in [ + OFlags::WRONLY | OFlags::TRUNC, + OFlags::RDONLY | OFlags::TRUNC, + ] { + assert!(matches!( + fs.open(USER, path, flags, Mode::empty()), + Err(OpenError::AccessNotAllowed) + )); + assert_eq!(fs.file_status(USER, path, true).unwrap().size, 14); + } + let mut fd = fs.open(USER, path, OFlags::RDONLY, Mode::empty()).unwrap(); + let mut data = [0; 14]; + assert_eq!(fs.read(&mut fd, &mut data, None).unwrap(), data.len()); + assert_eq!(&data, b"keep this data"); + } + #[test] fn root_directory_creation_and_removal() { let fs = in_mem_fs(); @@ -553,7 +586,7 @@ mod in_mem { assert_eq!(stat.file_type, FileType::RegularFile); // TODO(jayb): Restore coverage of `O_RDWR | O_DIRECTORY` once `OpenError` can report - // `EISDIR`; see the matching TODO in `InMem::owned_dir_at`. The legacy in-memory file + // `EISDIR`; see the matching TODO in `InMem::open_dir`. The legacy in-memory file // system used to accept such an open, which Linux rejects. } @@ -963,6 +996,18 @@ mod in_mem { ("/dir/file-link", symlink("file")), ("/dir-link", symlink("/dir")), ("/dangling", symlink("dir/created")), + ( + "/blocked", + InitialNode::Directory { + mode: Mode::empty(), + owner: ROOT, + }, + ), + ("/dotlink", symlink("blocked/../dir/file")), + ("/dir/a", directory()), + ("/dir/a/b", directory()), + ("/dir/a/b/c", directory()), + ("/deep-link", symlink("dir/a/b/c")), ])); let path = |path: &str, flags: OFlags| -> String { let entry = fs @@ -975,6 +1020,16 @@ mod in_mem { assert_eq!(path("/dir-link", OFlags::RDONLY), "/dir"); assert_eq!(path("/dir-link/file-link", OFlags::RDONLY), "/dir/file"); assert_eq!(path("/dir/../dir-link/./file", OFlags::RDONLY), "/dir/file"); + assert_eq!( + path("/deep-link/../../../file", OFlags::RDONLY), + "/dir/file" + ); + for path in ["/dotlink", "/blocked/../dir/file", "/blocked/./../dir/file"] { + assert!(matches!( + fs.open(USER, path, OFlags::PATH, Mode::empty()), + Err(OpenError::PathError(PathError::NoSearchPerms)) + )); + } assert_eq!( path("/dir-link/file-link", OFlags::PATH | OFlags::NOFOLLOW), "/dir/file-link" @@ -1073,6 +1128,70 @@ mod tar_ro { )); } + #[test] + fn read_only_errors_precede_file_permission_denials() { + use crate::fs::UserInfo; + use crate::fs::backend::Backend; + use crate::fs::composer::Composer; + use crate::fs::inode_allocator::InodeAllocator; + use crate::fs::resolver::Resolver; + use crate::fs::tar_ro::TarRo; + use crate::test_platform::TestPlatform; + + fn check(backend: impl Backend, path: &str, user: UserInfo) { + let fs = Resolver::::new(backend); + for flags in [OFlags::WRONLY, OFlags::RDWR, OFlags::RDONLY | OFlags::TRUNC] { + assert!(matches!( + fs.open(user, path, flags, Mode::empty()), + Err(OpenError::ReadOnlyFileSystem) + )); + } + assert!(matches!( + fs.open( + user, + path, + OFlags::WRONLY | OFlags::DIRECTORY, + Mode::empty() + ), + Err(OpenError::PathError(PathError::ComponentNotADirectory)) + )); + assert!(matches!( + fs.open( + user, + path, + OFlags::CREAT | OFlags::EXCL | OFlags::WRONLY, + Mode::empty() + ), + Err(OpenError::AlreadyExists) + )); + drop(fs.open(user, path, OFlags::RDONLY, Mode::empty()).unwrap()); + drop( + fs.open( + user, + path, + OFlags::PATH | OFlags::WRONLY | OFlags::TRUNC, + Mode::empty(), + ) + .unwrap(), + ); + assert_eq!(fs.file_status(user, path, true).unwrap().size, 8); + } + let user = UserInfo { + user: 4242, + group: 4242, + }; + let tar = || TarRo::new(TEST_TAR_FILE.into(), InodeAllocator::standalone()); + check(tar(), "/foo", user); + check( + Composer::builder() + .mount("/tar", |_| tar()) + .build() + .unwrap(), + "/tar/foo", + user, + ); + } + #[test] fn write_or_truncate_open_of_directory_fails() { let fs = tar_ro_fs(TEST_TAR_FILE.into()); @@ -1837,6 +1956,88 @@ mod overlay { )); } + #[test] + fn nested_overlay_resolution_and_mutation_do_not_deadlock() { + use crate::fs::composer::Composer; + use crate::fs::inode_allocator::InodeAllocator; + use crate::fs::tar_ro::{EMPTY_TAR_FILE, TarRo}; + use std::sync::mpsc; + use std::thread; + use std::time::Duration; + + let (tx, rx) = mpsc::channel(); + thread::spawn(move || { + let lower = Overlay::::new( + upper([]), + TarRo::new(TEST_TAR_FILE.into(), InodeAllocator::standalone()), + InodeAllocator::standalone(), + ); + let nested_upper = Overlay::::new( + upper([( + "/link", + InitialNode::Symlink { + mode: ALL_PERMS, + owner: USER, + target: "/bar/baz".into(), + }, + )]), + TarRo::new(EMPTY_TAR_FILE.into(), InodeAllocator::standalone()), + InodeAllocator::standalone(), + ); + let fs = Resolver::::new( + Composer::builder() + .mount("/", |_| { + Overlay::::new( + nested_upper, + lower, + InodeAllocator::standalone(), + ) + }) + .build() + .unwrap(), + ); + assert_eq!(fs.file_status(USER, "/bar/baz", true).unwrap().size, 13); + assert_eq!( + fs.file_status(USER, "/bar", true).unwrap().file_type, + FileType::Directory + ); + drop( + fs.open(USER, "/", OFlags::RDONLY | OFlags::DIRECTORY, Mode::empty()) + .unwrap(), + ); + assert_eq!(fs.read_link(USER, "/link").unwrap(), "/bar/baz"); + drop( + fs.open(USER, "/link", OFlags::RDONLY, Mode::empty()) + .unwrap(), + ); + fs.chmod(USER, "/bar/baz", Mode::RUSR | Mode::WUSR).unwrap(); + drop( + fs.open( + USER, + "/bar/baz", + OFlags::WRONLY | OFlags::TRUNC, + Mode::empty(), + ) + .unwrap(), + ); + drop( + fs.open( + USER, + "/bar/new", + OFlags::CREAT | OFlags::WRONLY, + Mode::RUSR | Mode::WUSR, + ) + .unwrap(), + ); + fs.unlink(USER, "/bar/new").unwrap(); + fs.mkdir(USER, "/bar/sub", ALL_PERMS).unwrap(); + fs.rmdir(USER, "/bar/sub").unwrap(); + tx.send(()).unwrap(); + }); + rx.recv_timeout(Duration::from_secs(5)) + .expect("nested overlay operation failed or deadlocked"); + } + #[test] fn copy_up_does_not_deadlock() { use std::sync::mpsc; diff --git a/litebox_broker_userland/src/fs/mod.rs b/litebox_broker_userland/src/fs/mod.rs index 12f214625..0c7610199 100644 --- a/litebox_broker_userland/src/fs/mod.rs +++ b/litebox_broker_userland/src/fs/mod.rs @@ -44,12 +44,12 @@ use io_errors::{ use litebox_broker_core::fs::backend::{ Backend, BackendHandles, CreationMetadata, DirHandle, FileHandle, HandleRef, PermissionCheck, - Permissioned, SeekBehavior, WalkOutcome, WalkStopReason, WalkedComponent, WalkingDirHandle, + PermissionInfo, Resolution, ResolvedDir, ResolvedFile, ResolvedTarget, SeekBehavior, }; use litebox_broker_core::fs::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, PathError, ReadDirError, - ReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, WalkError, - WriteError, + ReadError, RenameError, ResolutionError, RmdirError, SymlinkError, TruncateError, UnlinkError, + WalkError, WriteError, }; use litebox_broker_core::fs::inode_allocator::InodeAllocator; use litebox_broker_core::fs::{DirEntry, FileStatus, FileType, Mode, NodeInfo, OFlags, UserInfo}; @@ -131,7 +131,9 @@ struct Located { } impl BackendHandles for LocalFs { - type WalkingDirHandle<'a> = LocalDir; + // XXX(jayb): LocalFs handles are path-based and can go stale on host rename. + type ResolvedDir<'a> = LocalDir; + type ResolvedFile<'a> = LocalFile; type FileHandle = LocalFile; type DirHandle = LocalDir; } @@ -322,11 +324,9 @@ fn set_permissions_path(path: &Path, mode: Mode) -> std::io::Result<()> { fs::set_permissions(path, permissions) } +#[cfg(windows)] fn set_permissions_file(file: &File, mode: Mode) -> std::io::Result<()> { let mut permissions = file.metadata()?.permissions(); - #[cfg(unix)] - std::os::unix::fs::PermissionsExt::set_mode(&mut permissions, host_mode(mode)); - #[cfg(windows)] permissions.set_readonly(readonly_for_mode(mode)); file.set_permissions(permissions) } @@ -404,52 +404,83 @@ impl LocalFs { } impl Backend for LocalFs { - fn root(&self) -> WalkingDirHandle<'_> { - WalkingDirHandle::from_typed::(self.root_dir()) + fn root(&self) -> Result, WalkError> { + Ok(ResolvedDir::from_typed::( + self.root_dir(), + PermissionCheck::ByBackend, + )) } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { + _authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { let mut dir = from.into_typed::(); if !self.valid_dir(&dir) { - return Err(WalkError::Io); + return Err(ResolutionError { + component: None, + error: WalkError::Io, + }); } - let mut walked = Vec::with_capacity(components.len()); - for &name in components { + for (index, &name) in components.iter().enumerate() { let Located { canonical_path, file_type, - } = Self::locate(&dir, name).map_err(walk_error)?; + } = match Self::locate(&dir, name) { + Ok(located) => located, + Err(error) + if error.kind() == std::io::ErrorKind::NotFound + && index + 1 == components.len() => + { + return Ok(Resolution::MissingFinal { + parent: ResolvedDir::from_typed::(dir, PermissionCheck::ByBackend), + }); + } + Err(error) => { + return Err(ResolutionError { + component: Some(index), + error: walk_error(error), + }); + } + }; if file_type == FileType::Directory { dir = LocalDir { instance_tag: self.instance_tag.clone(), canonical_path, }; // XXX: search on a dir followed by `..` goes unchecked (e.g. `blocked/../target`). - walked.push(WalkedComponent { - permissions: PermissionCheck::ByBackend, + } else if index + 1 != components.len() { + return Err(ResolutionError { + component: Some(index + 1), + error: PathError::ComponentNotADirectory.into(), }); } else { - return Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(dir), - stop_reason: WalkStopReason::StoppedAtNonDirectory { file_type }, - }); + return Ok(Resolution::Found(ResolvedTarget::File( + ResolvedFile::from_typed::( + LocalFile { + instance_tag: self.instance_tag.clone(), + canonical_path, + file_type, + file: None, + readable: false, + writable: false, + }, + PermissionCheck::ByBackend, + file_type, + ), + ))); } } - Ok(WalkOutcome { - components: walked, - last: WalkingDirHandle::from_typed::(dir), - stop_reason: WalkStopReason::CompleteDirectory, - }) + Ok(Resolution::Found(ResolvedTarget::Dir( + ResolvedDir::from_typed::(dir, PermissionCheck::ByBackend), + ))) } - fn read_link_at(&self, dir: &WalkingDirHandle<'_>, name: &str) -> Result { + fn read_link_at(&self, dir: &ResolvedDir<'_>, name: &str) -> Result { validate_name(name)?; - let dir = dir.as_typed::(); + let dir = dir.get_typed::(); if !self.valid_dir(dir) { return Err(WalkError::Io); } @@ -507,11 +538,7 @@ impl Backend for LocalFs { Ok(target.into()) } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result { check_flags(flags)?; let dir = dir.into_typed::(); if !self.valid_dir(&dir) { @@ -547,7 +574,7 @@ impl Backend for LocalFs { Ok(DirHandle::from_typed::(dir)) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { let dir = dir.get_typed::(); if !self.valid_dir(dir) { return None; @@ -559,24 +586,23 @@ impl Backend for LocalFs { ); return None; } - Some(WalkingDirHandle::from_typed::(dir.clone())) + Some(ResolvedDir::from_typed::( + dir.clone(), + PermissionCheck::ByBackend, + )) } - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { check_flags(flags)?; - let dir = dir.into_typed::(); - if !self.valid_dir(&dir) { + let file = file.into_typed::(); + if !self.valid_file(&file) { return Err(OpenError::Io); } - let Located { + let LocalFile { canonical_path, file_type, - } = Self::locate(&dir, name).map_err(open_error)?; + .. + } = file; if flags.contains(OFlags::CREAT | OFlags::EXCL) { return Err(OpenError::AlreadyExists); } @@ -630,17 +656,14 @@ impl Backend for LocalFs { } Some(Arc::new(opened)) }; - Ok(Permissioned { - item: FileHandle::from_typed::(LocalFile { - instance_tag: self.instance_tag.clone(), - canonical_path, - file_type, - file, - readable, - writable, - }), - permissions: PermissionCheck::ByBackend, - }) + Ok(FileHandle::from_typed::(LocalFile { + instance_tag: self.instance_tag.clone(), + canonical_path, + file_type, + file, + readable, + writable, + })) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { @@ -803,9 +826,30 @@ impl Backend for LocalFs { } } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + let (path, file_type) = match target { + ResolvedTarget::Dir(dir) => { + let dir = dir.get_typed::(); + if !self.valid_dir(dir) { + return Err(FileStatusError::Io); + } + (&dir.canonical_path, FileType::Directory) + } + ResolvedTarget::File(file) => { + let file = file.get_typed::(); + if !self.valid_file(file) { + return Err(FileStatusError::Io); + } + (&file.canonical_path, file.file_type) + } + }; + self.status_path_of_type(path, file_type) + .map_err(|_| FileStatusError::Io) + } + fn create_file_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { @@ -845,7 +889,7 @@ impl Backend for LocalFs { fn symlink_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _target: &str, _metadata: CreationMetadata, @@ -858,7 +902,7 @@ impl Backend for LocalFs { fn mkdir_at( &self, - dir: DirHandle, + dir: ResolvedDir<'_>, name: &str, metadata: CreationMetadata, ) -> Result { @@ -903,7 +947,7 @@ impl Backend for LocalFs { })) } - fn unlink_at(&self, dir: DirHandle, name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), UnlinkError> { validate_name(name)?; let dir = dir.into_typed::(); if !self.valid_dir(&dir) { @@ -938,7 +982,7 @@ impl Backend for LocalFs { } } - fn rmdir_at(&self, dir: DirHandle, name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, dir: ResolvedDir<'_>, name: &str) -> Result<(), RmdirError> { validate_name(name)?; let dir = dir.into_typed::(); if !self.valid_dir(&dir) { @@ -994,14 +1038,14 @@ impl Backend for LocalFs { fs::rename(old_path, new_path).map_err(rename_error) } - fn chmod(&self, h: HandleRef<'_>, mode: Mode) -> Result<(), ChmodError> { + fn chmod(&self, h: ResolvedTarget<'_>, mode: Mode) -> Result<(), ChmodError> { #[cfg(windows)] litebox_util_log::debug!( mode:? = mode; "Windows chmod keeps only whether any write bit is set (the readonly attribute)" ); let result = match h { - HandleRef::Dir(dir) => { + ResolvedTarget::Dir(dir) => { let dir = dir.get_typed::(); if !self.valid_dir(dir) || !fs::metadata(&dir.canonical_path).is_ok_and(|metadata| metadata.is_dir()) @@ -1010,7 +1054,7 @@ impl Backend for LocalFs { } set_permissions_path(&dir.canonical_path, mode) } - HandleRef::File(file) => { + ResolvedTarget::File(file) => { let file = file.get_typed::(); if !self.valid_file(file) { return Err(ChmodError::Io); @@ -1018,15 +1062,10 @@ impl Backend for LocalFs { if file.is_symlink() { return Err(ChmodError::IsSymlink); } - if let Some(open) = &file.file { - set_permissions_file(open, mode) - } else { - if !fs::metadata(&file.canonical_path).is_ok_and(|metadata| metadata.is_file()) - { - return Err(ChmodError::Io); - } - set_permissions_path(&file.canonical_path, mode) + if !fs::metadata(&file.canonical_path).is_ok_and(|metadata| metadata.is_file()) { + return Err(ChmodError::Io); } + set_permissions_path(&file.canonical_path, mode) } }; result.map_err(chmod_error) @@ -1034,26 +1073,19 @@ impl Backend for LocalFs { fn chown( &self, - h: HandleRef<'_>, + h: ResolvedTarget<'_>, user: Option, group: Option, ) -> Result<(), ChownError> { - #[derive(Clone, Copy)] - enum Target<'a> { - Directory(&'a Path), - PathFile(&'a Path), - OpenFile(&'a File), - } - - let target = match h { - HandleRef::Dir(dir) => { + let (path, file_type) = match &h { + ResolvedTarget::Dir(dir) => { let dir = dir.get_typed::(); if !self.valid_dir(dir) { return Err(ChownError::Io); } - Target::Directory(&dir.canonical_path) + (&dir.canonical_path, FileType::Directory) } - HandleRef::File(file) => { + ResolvedTarget::File(file) => { let file = file.get_typed::(); if !self.valid_file(file) { return Err(ChownError::Io); @@ -1061,18 +1093,12 @@ impl Backend for LocalFs { if file.is_symlink() { return Err(ChownError::IsSymlink); } - file.file - .as_deref() - .map_or(Target::PathFile(&file.canonical_path), Target::OpenFile) + (&file.canonical_path, FileType::RegularFile) } }; - let metadata = match target { - Target::Directory(path) | Target::PathFile(path) => fs::metadata(path), - Target::OpenFile(file) => file.metadata(), - } - .map_err(|_| ChownError::Io)?; - if matches!(target, Target::Directory(_)) && !metadata.is_dir() - || matches!(target, Target::PathFile(_)) && !metadata.is_file() + let metadata = fs::metadata(path).map_err(|_| ChownError::Io)?; + if file_type == FileType::Directory && !metadata.is_dir() + || file_type == FileType::RegularFile && !metadata.is_file() { return Err(ChownError::Io); } @@ -1095,13 +1121,7 @@ impl Backend for LocalFs { let user = user.map(u32::from); let group = group.map(u32::from); // The ids are unchanged, but the host call still applies its own permission check. - let result = match target { - Target::Directory(path) | Target::PathFile(path) => { - std::os::unix::fs::chown(path, user, group) - } - Target::OpenFile(file) => std::os::unix::fs::fchown(file, user, group), - }; - result.map_err(chown_error) + std::os::unix::fs::chown(path, user, group).map_err(chown_error) } #[cfg(windows)] { diff --git a/litebox_broker_userland/tests/local_fs.rs b/litebox_broker_userland/tests/local_fs.rs index 2be5ce70e..246b4bd62 100644 --- a/litebox_broker_userland/tests/local_fs.rs +++ b/litebox_broker_userland/tests/local_fs.rs @@ -15,21 +15,21 @@ use std::path::Path; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; -#[cfg(unix)] -use litebox_broker_core::fs::FileType; #[cfg(unix)] use litebox_broker_core::fs::SeekWhence; -use litebox_broker_core::fs::backend::{Backend, HandleRef}; +use litebox_broker_core::fs::backend::{ + Backend, HandleRef, Resolution, ResolvedFile, ResolvedTarget, +}; #[cfg(unix)] use litebox_broker_core::fs::composer::Composer; #[cfg(windows)] use litebox_broker_core::fs::errors::RmdirError; #[cfg(unix)] use litebox_broker_core::fs::errors::{ChownError, TruncateError, WriteError}; -use litebox_broker_core::fs::errors::{OpenError, PathError, WalkError}; +use litebox_broker_core::fs::errors::{OpenError, PathError, ResolutionError, WalkError}; use litebox_broker_core::fs::inode_allocator::InodeAllocator; use litebox_broker_core::fs::resolver::Resolver; -use litebox_broker_core::fs::{Mode, OFlags, UserInfo}; +use litebox_broker_core::fs::{FileType, Mode, OFlags, UserInfo}; use litebox_broker_protocol::shared_buffer::{ SharedBufferLayout, SharedBufferSequence, SharedBufferSlotIndex, }; @@ -48,6 +48,18 @@ fn backend(root: &Path) -> LocalFs { LocalFs::new(root, InodeAllocator::standalone()).unwrap() } +fn resolved_file<'a>(local: &'a LocalFs, name: &str) -> ResolvedFile<'a> { + match local + .resolve(local.root().unwrap(), &[name], &|_| { + panic!("LocalFs must enforce host permissions itself") + }) + .unwrap() + { + Resolution::Found(ResolvedTarget::File(file)) => file, + _ => panic!("expected a resolved file"), + } +} + #[cfg(unix)] #[test] fn canonical_root_live_io_and_open_file_handles() { @@ -59,13 +71,14 @@ fn canonical_root_live_io_and_open_file_handles() { symlink(&root, temp.path().join("root_link")).unwrap(); let local = backend(&temp.path().join("root_link")); assert_eq!( - local.read_link_at(&local.root(), "absolute").unwrap(), + local + .read_link_at(&local.root().unwrap(), "absolute") + .unwrap(), "file" ); let read_only = local - .open_file_at(local.root(), "file", OFlags::RDONLY) - .unwrap() - .item; + .open_file(resolved_file(&local, "file"), OFlags::RDONLY) + .unwrap(); let mut buf = [0; 7]; assert_eq!(local.read(&read_only, &mut buf, 0).unwrap(), 7); assert_eq!(&buf, b"initial"); @@ -78,11 +91,10 @@ fn canonical_root_live_io_and_open_file_handles() { Err(TruncateError::NotForWriting) )); - let dir = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); + let dir = local.root().unwrap(); let file = local - .open_file_at(local.root(), "file", OFlags::RDWR) - .unwrap() - .item; + .open_file(resolved_file(&local, "file"), OFlags::RDWR) + .unwrap(); fs::rename(root.join("file"), root.join("moved_file")).unwrap(); fs::write(root.join("file"), b"decoy").unwrap(); local.write(&file, b"live", 0).unwrap(); @@ -92,7 +104,12 @@ fn canonical_root_live_io_and_open_file_handles() { assert_eq!(local.read(&file, &mut buf, 0).unwrap(), 7); local.truncate(&file, 3).unwrap(); assert_eq!(local.status(HandleRef::File(&file)).unwrap().size, 3); - local.chmod(HandleRef::File(&file), Mode::RUSR).unwrap(); + local + .chmod( + ResolvedTarget::File(resolved_file(&local, "moved_file")), + Mode::RUSR, + ) + .unwrap(); assert_eq!( local.status(HandleRef::File(&file)).unwrap().mode, Mode::RUSR @@ -135,9 +152,8 @@ fn shared_buffer_io_is_direct_when_the_buffer_is_contiguous_raw_bytes() { fs::write(temp.path().join("file"), b"0123456789").unwrap(); let local = backend(temp.path()); let file = local - .open_file_at(local.root(), "file", OFlags::RDWR) - .unwrap() - .item; + .open_file(resolved_file(&local, "file"), OFlags::RDWR) + .unwrap(); let memory = Arc::new(CountingMemory { bytes: HeapSharedMemory::new(16), raw, @@ -187,13 +203,11 @@ fn native_identity_and_listing_agree_even_for_hard_links() { fs::hard_link(temp.path().join("one"), temp.path().join("two")).unwrap(); let local = backend(temp.path()); let one = local - .open_file_at(local.root(), "one", OFlags::PATH) - .unwrap() - .item; + .open_file(resolved_file(&local, "one"), OFlags::PATH) + .unwrap(); let two = local - .open_file_at(local.root(), "two", OFlags::PATH) - .unwrap() - .item; + .open_file(resolved_file(&local, "two"), OFlags::PATH) + .unwrap(); let expected = local.status(HandleRef::File(&one)).unwrap().node_info; assert_eq!( expected, @@ -202,7 +216,11 @@ fn native_identity_and_listing_agree_even_for_hard_links() { let bad_name = std::ffi::OsStr::from_bytes(b"bad\xffname"); fs::write(temp.path().join(bad_name), b"hidden").unwrap(); let entries = local - .list_dir_at(local.owned_dir_at(local.root(), OFlags::RDONLY).unwrap()) + .list_dir_at( + local + .open_dir(local.root().unwrap(), OFlags::RDONLY) + .unwrap(), + ) .unwrap(); assert_eq!(entries.len(), 2, "non-UTF-8 entries must be hidden"); assert_eq!( @@ -239,22 +257,22 @@ fn invalid_components_are_rejected() { for &bad in rejected { assert!( matches!( - local.open_file_at(local.root(), bad, OFlags::PATH), - Err(OpenError::PathError(PathError::InvalidPathname)) + local.resolve(local.root().unwrap(), &[bad], &|_| unreachable!()), + Err(ResolutionError { + component: Some(0), + error: WalkError::PathError(PathError::InvalidPathname), + }) ), "{bad:?}" ); assert!( - matches!( - local.walk_directories(local.root(), &[bad]), - Err(WalkError::PathError(PathError::InvalidPathname)) - ), + local.unlink_at(local.root().unwrap(), bad).is_err(), + "{bad:?}" + ); + assert!( + local.rmdir_at(local.root().unwrap(), bad).is_err(), "{bad:?}" ); - let root = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); - assert!(local.unlink_at(root, bad).is_err(), "{bad:?}"); - let root = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); - assert!(local.rmdir_at(root, bad).is_err(), "{bad:?}"); } // Win32-reserved spellings are ordinary names on Unix hosts. @@ -264,14 +282,18 @@ fn invalid_components_are_rejected() { fs::write(temp.path().join(name), b"visible").unwrap(); } let entries = local - .list_dir_at(local.owned_dir_at(local.root(), OFlags::RDONLY).unwrap()) + .list_dir_at( + local + .open_dir(local.root().unwrap(), OFlags::RDONLY) + .unwrap(), + ) .unwrap(); assert_eq!(entries.len(), win32_special.len()); for name in win32_special { assert!(entries.iter().any(|e| e.name == name), "{name:?}"); assert!( local - .open_file_at(local.root(), name, OFlags::RDONLY) + .open_file(resolved_file(&local, name), OFlags::RDONLY) .is_ok(), "{name:?}" ); @@ -279,15 +301,72 @@ fn invalid_components_are_rejected() { } } +#[test] +fn resolution_is_unopened_and_reports_component_failures() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir(temp.path().join("dir")).unwrap(); + fs::write(temp.path().join("file"), b"intact").unwrap(); + let local = backend(temp.path()); + assert!(matches!( + local.resolve(local.root().unwrap(), &[], &|_| unreachable!()), + Ok(Resolution::Found(ResolvedTarget::Dir(_))) + )); + let target = ResolvedTarget::File(resolved_file(&local, "file")); + assert_eq!(local.resolved_status(&target).unwrap().size, 6); + assert_eq!( + local.resolved_status(&target).unwrap().file_type, + FileType::RegularFile + ); + #[cfg(unix)] + { + fs::set_permissions(temp.path().join("file"), fs::Permissions::from_mode(0o000)).unwrap(); + assert_eq!(local.resolved_status(&target).unwrap().mode, Mode::empty()); + local.chmod(target, Mode::RUSR | Mode::WUSR).unwrap(); + } + assert_eq!(fs::read(temp.path().join("file")).unwrap(), b"intact"); + for (components, component, non_directory) in [ + (["absent", "child"], 0, false), + (["file", "child"], 1, true), + ] { + let error = local + .resolve(local.root().unwrap(), &components, &|_| unreachable!()) + .err() + .expect("an intermediate component must fail"); + assert_eq!(error.component, Some(component)); + assert!(matches!( + (error.error, non_directory), + ( + WalkError::PathError(PathError::NoSuchFileOrDirectory), + false + ) | ( + WalkError::PathError(PathError::ComponentNotADirectory), + true + ) + )); + } + let Resolution::MissingFinal { parent } = local + .resolve( + local.root().unwrap(), + &["dir", "missing"], + &|_| unreachable!(), + ) + .unwrap() + else { + panic!("only the final component is missing"); + }; + let parent = local.open_dir(parent, OFlags::RDONLY).unwrap(); + assert_eq!(local.list_dir_at(parent).unwrap().len(), 0); + assert!(!temp.path().join("dir/missing").exists()); +} + #[test] fn exclusive_open_never_truncates_existing_file() { let temp = tempfile::tempdir().unwrap(); fs::write(temp.path().join("file"), b"intact").unwrap(); let local = backend(temp.path()); assert!(matches!( - local.open_file_at( - local.root(), - "file", + local.open_file( + resolved_file(&local, "file"), OFlags::CREAT | OFlags::EXCL | OFlags::TRUNC | OFlags::RDWR, ), Err(OpenError::AlreadyExists) @@ -301,9 +380,8 @@ fn path_handles_report_live_files() { fs::write(temp.path().join("file"), b"content").unwrap(); let local = backend(temp.path()); let file = local - .open_file_at(local.root(), "file", OFlags::PATH) - .unwrap() - .item; + .open_file(resolved_file(&local, "file"), OFlags::PATH) + .unwrap(); assert_eq!(local.status(HandleRef::File(&file)).unwrap().size, 7); fs::write(temp.path().join("file"), b"longer content").unwrap(); assert_eq!(local.status(HandleRef::File(&file)).unwrap().size, 14); @@ -316,9 +394,8 @@ fn direct_backend_mutations_and_flags() { let local = backend(temp.path()); fs::write(temp.path().join("new"), b"").unwrap(); let file = local - .open_file_at(local.root(), "new", OFlags::RDWR) - .unwrap() - .item; + .open_file(resolved_file(&local, "new"), OFlags::RDWR) + .unwrap(); local.write(&file, b"x", 4096).unwrap(); let mut hole = [1; 4]; assert_eq!(local.read(&file, &mut hole, 64).unwrap(), 4); @@ -329,35 +406,25 @@ fn direct_backend_mutations_and_flags() { local.truncate(&file, 4097).unwrap(); assert!( local - .open_file_at( - local.root(), - "new", + .open_file( + resolved_file(&local, "new"), OFlags::WRONLY | OFlags::DIRECTORY | OFlags::TRUNC ) .is_err() ); assert_eq!(fs::metadata(temp.path().join("new")).unwrap().len(), 4097); fs::create_dir(temp.path().join("dir")).unwrap(); - let dir = local - .owned_dir_at( - local.walk_directories(local.root(), &["dir"]).unwrap().last, - OFlags::PATH, - ) - .unwrap(); + let Resolution::Found(ResolvedTarget::Dir(dir)) = local + .resolve(local.root().unwrap(), &["dir"], &|_| unreachable!()) + .unwrap() + else { + panic!("expected directory"); + }; + let dir = local.open_dir(dir, OFlags::PATH).unwrap(); assert!(local.status(HandleRef::Dir(&dir)).is_ok()); assert!(local.walking_dir_at(&dir).is_some()); - local - .unlink_at( - local.owned_dir_at(local.root(), OFlags::PATH).unwrap(), - "new", - ) - .unwrap(); - local - .rmdir_at( - local.owned_dir_at(local.root(), OFlags::PATH).unwrap(), - "dir", - ) - .unwrap(); + local.unlink_at(local.root().unwrap(), "new").unwrap(); + local.rmdir_at(local.root().unwrap(), "dir").unwrap(); assert!(!temp.path().join("new").exists()); assert!(!temp.path().join("dir").exists()); } @@ -405,7 +472,10 @@ fn read_only_truncate_requires_host_write_permission() { let local = backend(temp.path()); assert!( local - .open_file_at(local.root(), "file", OFlags::RDONLY | OFlags::TRUNC) + .open_file( + resolved_file(&local, "file"), + OFlags::RDONLY | OFlags::TRUNC + ) .is_err() ); assert_eq!(fs::read(&path).unwrap(), b"intact"); @@ -538,7 +608,7 @@ fn unreadable_directory_is_not_an_empty_listing() { } fs::write(temp.path().join("entry"), b"data").unwrap(); let local = backend(temp.path()); - let dir = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); + let dir = local.open_dir(local.root().unwrap(), OFlags::PATH).unwrap(); fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o000)).unwrap(); assert!(local.list_dir_at(dir).is_err()); fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o700)).unwrap(); @@ -553,8 +623,7 @@ fn final_symlink_removal_does_not_follow_the_link() { symlink("missing", temp.path().join("file_link")).unwrap(); let local = backend(temp.path()); - let dir = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); - assert!(local.rmdir_at(dir, "dir_link").is_err()); + assert!(local.rmdir_at(local.root().unwrap(), "dir_link").is_err()); assert!(temp.path().join("directory").is_dir()); assert!( fs::symlink_metadata(temp.path().join("dir_link")) @@ -563,8 +632,7 @@ fn final_symlink_removal_does_not_follow_the_link() { .is_symlink() ); - let dir = local.owned_dir_at(local.root(), OFlags::PATH).unwrap(); - local.unlink_at(dir, "file_link").unwrap(); + local.unlink_at(local.root().unwrap(), "file_link").unwrap(); assert!(fs::symlink_metadata(temp.path().join("file_link")).is_err()); } @@ -637,24 +705,53 @@ fn foreign_handles_are_not_usable_and_host_modes_are_live() { fs::write(a.path().join("f"), b"one").unwrap(); let first = backend(a.path()); let second = backend(b.path()); - assert!(second.walk_directories(first.root(), &["f"]).is_err()); - let foreign = first.owned_dir_at(first.root(), OFlags::PATH).unwrap(); + assert!( + second + .resolve(first.root().unwrap(), &["f"], &|_| unreachable!()) + .is_err() + ); + let foreign = first.open_dir(first.root().unwrap(), OFlags::PATH).unwrap(); assert!(second.list_dir_at(foreign.clone()).is_err()); assert!(second.walking_dir_at(&foreign).is_none()); - assert!(second.unlink_at(foreign, "f").is_err()); + assert!(second.unlink_at(first.root().unwrap(), "f").is_err()); assert_eq!(fs::read(a.path().join("f")).unwrap(), b"one"); let file = first - .open_file_at(first.root(), "f", OFlags::RDONLY) - .unwrap() - .item; + .open_file(resolved_file(&first, "f"), OFlags::RDONLY) + .unwrap(); assert!(second.status(HandleRef::File(&file)).is_err()); + assert!( + second + .open_file(resolved_file(&first, "f"), OFlags::PATH) + .is_err() + ); + assert!( + second + .resolved_status(&ResolvedTarget::File(resolved_file(&first, "f"))) + .is_err() + ); + assert!( + second + .chmod( + ResolvedTarget::File(resolved_file(&first, "f")), + Mode::empty() + ) + .is_err() + ); + assert!( + second + .chown(ResolvedTarget::File(resolved_file(&first, "f")), None, None) + .is_err() + ); fs::set_permissions(a.path().join("f"), fs::Permissions::from_mode(0o600)).unwrap(); assert_eq!( first.status(HandleRef::File(&file)).unwrap().mode.bits() & 0o777, 0o600 ); first - .chmod(HandleRef::File(&file), Mode::RUSR | Mode::SUID | Mode::SGID) + .chmod( + ResolvedTarget::File(resolved_file(&first, "f")), + Mode::RUSR | Mode::SUID | Mode::SGID, + ) .unwrap(); assert_eq!( first.status(HandleRef::File(&file)).unwrap().mode & (Mode::SUID | Mode::SGID), @@ -674,10 +771,18 @@ fn foreign_handles_are_not_usable_and_host_modes_are_live() { u16::try_from(host_metadata.gid()), ) { first - .chown(HandleRef::File(&file), Some(uid), Some(gid)) + .chown( + ResolvedTarget::File(resolved_file(&first, "f")), + Some(uid), + Some(gid), + ) .unwrap(); assert!(matches!( - first.chown(HandleRef::File(&file), Some(uid.wrapping_add(1)), None), + first.chown( + ResolvedTarget::File(resolved_file(&first, "f")), + Some(uid.wrapping_add(1)), + None + ), Err(ChownError::NotTheOwner) )); } diff --git a/litebox_broker_vm_kernel/src/share.rs b/litebox_broker_vm_kernel/src/share.rs index 76215c4a6..492581e7e 100644 --- a/litebox_broker_vm_kernel/src/share.rs +++ b/litebox_broker_vm_kernel/src/share.rs @@ -11,13 +11,13 @@ use alloc::string::String; use alloc::sync::Arc; use alloc::vec::Vec; use litebox_broker_core::fs::backend::{ - Backend, CreationMetadata, DirHandle, FileHandle, HandleRef, Permissioned, SeekBehavior, - WalkOutcome, WalkingDirHandle, + Backend, CreationMetadata, DirHandle, FileHandle, HandleRef, PermissionInfo, Resolution, + ResolvedDir, ResolvedFile, ResolvedTarget, SeekBehavior, }; use litebox_broker_core::fs::errors::{ ChmodError, ChownError, FileStatusError, MkdirError, OpenError, ReadDirError, - ReadError as FsReadError, RenameError, RmdirError, SymlinkError, TruncateError, UnlinkError, - WalkError, WriteError as FsWriteError, + ReadError as FsReadError, RenameError, ResolutionError, RmdirError, SymlinkError, + TruncateError, UnlinkError, WalkError, WriteError as FsWriteError, }; use litebox_broker_core::fs::inode_allocator::InodeAllocator; use litebox_broker_core::fs::nine_p::transport::{Read, ReadError, Write, WriteError}; @@ -164,52 +164,42 @@ const WRITE_FLAGS: OFlags = OFlags::CREAT .union(OFlags::RDWR); impl Backend for ShareFs { - fn root(&self) -> WalkingDirHandle<'_> { + fn root(&self) -> Result, WalkError> { self.0.root() } - fn walk_directories<'a>( + fn resolve<'a>( &'a self, - from: WalkingDirHandle<'a>, + from: ResolvedDir<'a>, components: &[&str], - ) -> Result>, WalkError> { - self.0.walk_directories(from, components) + authorize_dir_lookup: &dyn Fn(&PermissionInfo) -> Result<(), WalkError>, + ) -> Result, ResolutionError> { + self.0.resolve(from, components, authorize_dir_lookup) } - fn read_link_at(&self, _dir: &WalkingDirHandle<'_>, _name: &str) -> Result { - // The 9P backend does not yet report symbolic links, so walks never stop at one. - unimplemented!("symbolic links on the VM share") + fn read_link_at(&self, _dir: &ResolvedDir<'_>, _name: &str) -> Result { + unimplemented!("reading VM-share symlinks") } - fn owned_dir_at( - &self, - dir: WalkingDirHandle<'_>, - flags: OFlags, - ) -> Result { + fn open_dir(&self, dir: ResolvedDir<'_>, flags: OFlags) -> Result { if flags.intersects(WRITE_FLAGS) { return Err(OpenError::ReadOnlyFileSystem); } - self.0.owned_dir_at(dir, flags) + self.0.open_dir(dir, flags) } - fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { + fn walking_dir_at<'a>(&'a self, dir: &DirHandle) -> Option> { self.0.walking_dir_at(dir) } - /// An exclusive creation of an existing file must succeed, read-only, for - /// the resolver to report `EEXIST` rather than `EROFS`, as `TarRo` does. - fn open_file_at( - &self, - dir: WalkingDirHandle<'_>, - name: &str, - flags: OFlags, - ) -> Result, OpenError> { - let exclusive = flags.contains(OFlags::CREAT | OFlags::EXCL); - if !exclusive && flags.intersects(WRITE_FLAGS) { + fn open_file(&self, file: ResolvedFile<'_>, flags: OFlags) -> Result { + if flags.contains(OFlags::CREAT | OFlags::EXCL) { + return Err(OpenError::AlreadyExists); + } + if flags.intersects(WRITE_FLAGS) { return Err(OpenError::ReadOnlyFileSystem); } - self.0 - .open_file_at(dir, name, flags - WRITE_FLAGS - OFlags::EXCL) + self.0.open_file(file, flags) } fn list_dir_at(&self, handle: DirHandle) -> Result, ReadDirError> { @@ -254,13 +244,17 @@ impl Backend for ShareFs { self.0.status(h) } + fn resolved_status(&self, target: &ResolvedTarget<'_>) -> Result { + self.0.resolved_status(target) + } + fn readiness_source(&self, h: &FileHandle) -> Option> { self.0.readiness_source(h) } fn create_file_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { @@ -269,7 +263,7 @@ impl Backend for ShareFs { fn symlink_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _target: &str, _metadata: CreationMetadata, @@ -279,14 +273,14 @@ impl Backend for ShareFs { fn mkdir_at( &self, - _dir: DirHandle, + _dir: ResolvedDir<'_>, _name: &str, _metadata: CreationMetadata, ) -> Result { Err(MkdirError::ReadOnlyFileSystem) } - fn unlink_at(&self, _dir: DirHandle, _name: &str) -> Result<(), UnlinkError> { + fn unlink_at(&self, _dir: ResolvedDir<'_>, _name: &str) -> Result<(), UnlinkError> { Err(UnlinkError::ReadOnlyFileSystem) } @@ -300,17 +294,17 @@ impl Backend for ShareFs { Err(RenameError::ReadOnlyFileSystem) } - fn rmdir_at(&self, _dir: DirHandle, _name: &str) -> Result<(), RmdirError> { + fn rmdir_at(&self, _dir: ResolvedDir<'_>, _name: &str) -> Result<(), RmdirError> { Err(RmdirError::ReadOnlyFileSystem) } - fn chmod(&self, _h: HandleRef<'_>, _mode: Mode) -> Result<(), ChmodError> { + fn chmod(&self, _h: ResolvedTarget<'_>, _mode: Mode) -> Result<(), ChmodError> { Err(ChmodError::ReadOnlyFileSystem) } fn chown( &self, - _h: HandleRef<'_>, + _h: ResolvedTarget<'_>, _user: Option, _group: Option, ) -> Result<(), ChownError> {