From 175eb187c7fe893463877060f0e92698c3bc58aa Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 20 Aug 2020 10:41:30 -0700 Subject: [PATCH 1/6] Refactor some code out of open_manually.rs into separate files. --- cap-primitives/src/fs/cow_component.rs | 32 +++++++++ cap-primitives/src/fs/mod.rs | 5 ++ cap-primitives/src/fs/open_manually.rs | 66 +++---------------- cap-primitives/src/fs/open_unchecked_error.rs | 18 +++++ 4 files changed, 64 insertions(+), 57 deletions(-) create mode 100644 cap-primitives/src/fs/cow_component.rs create mode 100644 cap-primitives/src/fs/open_unchecked_error.rs diff --git a/cap-primitives/src/fs/cow_component.rs b/cap-primitives/src/fs/cow_component.rs new file mode 100644 index 00000000..db991071 --- /dev/null +++ b/cap-primitives/src/fs/cow_component.rs @@ -0,0 +1,32 @@ +use std::{borrow::Cow, ffi::OsStr, path::Component}; + +/// Like `std::path::Component` except we combine `Prefix` and `RootDir` since +/// we don't support absolute paths, and `Normal` has a `Cow` instead of a plain +/// `OsStr` reference, so it can optionally own its own string. +#[derive(Debug)] +pub(super) enum CowComponent<'borrow> { + PrefixOrRootDir, + CurDir, + ParentDir, + Normal(Cow<'borrow, OsStr>), +} + +/// Convert a `Component` into a `CowComponent` which borrows strings. +pub(super) fn to_borrowed_component(component: Component) -> CowComponent { + match component { + Component::Prefix(_) | Component::RootDir => CowComponent::PrefixOrRootDir, + Component::CurDir => CowComponent::CurDir, + Component::ParentDir => CowComponent::ParentDir, + Component::Normal(os_str) => CowComponent::Normal(os_str.into()), + } +} + +/// Convert a `Component` into a `CowComponent` which owns strings. +pub(super) fn to_owned_component<'borrow>(component: Component) -> CowComponent<'borrow> { + match component { + Component::Prefix(_) | Component::RootDir => CowComponent::PrefixOrRootDir, + Component::CurDir => CowComponent::CurDir, + Component::ParentDir => CowComponent::ParentDir, + Component::Normal(os_str) => CowComponent::Normal(os_str.to_os_string().into()), + } +} diff --git a/cap-primitives/src/fs/mod.rs b/cap-primitives/src/fs/mod.rs index 88c29e90..7794fc68 100644 --- a/cap-primitives/src/fs/mod.rs +++ b/cap-primitives/src/fs/mod.rs @@ -3,6 +3,7 @@ mod canonicalize; mod canonicalize_manually; mod copy; +mod cow_component; mod dir_builder; mod dir_entry; mod dir_options; @@ -24,6 +25,7 @@ mod open_entry_manually; mod open_manually; mod open_options; mod open_parent; +mod open_unchecked_error; mod permissions; mod read_dir; mod readlink; @@ -48,6 +50,8 @@ mod unlink_via_parent; pub(crate) mod errors; +use cow_component::*; + pub(crate) use canonicalize_manually::*; #[cfg(not(feature = "no_racy_asserts"))] pub(crate) use get_path::*; @@ -58,6 +62,7 @@ pub(crate) use mkdir_via_parent::*; pub(crate) use open_entry_manually::*; pub(crate) use open_manually::*; pub(crate) use open_parent::*; +pub(crate) use open_unchecked_error::*; pub(crate) use readlink_one::*; #[cfg(not(windows))] // doesn't work on windows; use a windows-specific impl pub(crate) use readlink_via_parent::*; diff --git a/cap-primitives/src/fs/open_manually.rs b/cap-primitives/src/fs/open_manually.rs index bf6eb808..2a30438f 100644 --- a/cap-primitives/src/fs/open_manually.rs +++ b/cap-primitives/src/fs/open_manually.rs @@ -4,47 +4,16 @@ #[cfg(not(feature = "no_racy_asserts"))] use crate::fs::is_same_file; use crate::fs::{ - dir_options, errors, open_unchecked, path_requires_dir, readlink_one, FollowSymlinks, - MaybeOwnedFile, OpenOptions, + dir_options, errors, open_unchecked, path_requires_dir, readlink_one, to_borrowed_component, + to_owned_component, CowComponent, FollowSymlinks, MaybeOwnedFile, OpenOptions, + OpenUncheckedError, }; use std::{ - borrow::Cow, ffi::OsStr, fs, io, path::{Component, Path, PathBuf}, }; -/// Like `std::path::Component` except we combine `Prefix` and `RootDir` since -/// we don't support absolute paths, and `Normal` has a `Cow` instead of a plain -/// `OsStr` reference, so it can optionally own its own string. -#[derive(Debug)] -enum CowComponent<'borrow> { - PrefixOrRootDir, - CurDir, - ParentDir, - Normal(Cow<'borrow, OsStr>), -} - -/// Convert a `Component` into a `CowComponent` which borrows strings. -fn to_borrowed_component(component: Component) -> CowComponent { - match component { - Component::Prefix(_) | Component::RootDir => CowComponent::PrefixOrRootDir, - Component::CurDir => CowComponent::CurDir, - Component::ParentDir => CowComponent::ParentDir, - Component::Normal(os_str) => CowComponent::Normal(os_str.into()), - } -} - -/// Convert a `Component` into a `CowComponent` which owns strings. -fn to_owned_component<'borrow>(component: Component) -> CowComponent<'borrow> { - match component { - Component::Prefix(_) | Component::RootDir => CowComponent::PrefixOrRootDir, - Component::CurDir => CowComponent::CurDir, - Component::ParentDir => CowComponent::ParentDir, - Component::Normal(os_str) => CowComponent::Normal(os_str.to_os_string().into()), - } -} - /// Utility for collecting the canonical path components. struct CanonicalPath<'path_buf> { /// If the user requested a canonical path, a reference to the `PathBuf` to @@ -122,8 +91,8 @@ pub(crate) fn open_manually_wrapper( ) -> io::Result { let mut symlink_count = 0; let start = MaybeOwnedFile::borrowed(start); - open_manually(start, path, options, &mut symlink_count, None) - .and_then(|maybe_owned| maybe_owned.into_file(options)) + let maybe_owned = open_manually(start, path, options, &mut symlink_count, None)?; + maybe_owned.into_file(options) } /// Implement `open` by breaking up the path into components, resolving each @@ -171,7 +140,7 @@ pub(crate) fn open_manually<'start>( match c { CowComponent::PrefixOrRootDir => return Err(errors::escape_attempt()), CowComponent::CurDir => { - // If the path ends in `.` and we want write access, fail. + // If the path ends in `.` and we can't open a directory, fail. if components.is_empty() { if dir_precluded { return Err(errors::is_directory()); @@ -204,7 +173,7 @@ pub(crate) fn open_manually<'start>( assert!(canonical_path.pop()); } CowComponent::Normal(one) => { - // If the path requires a directory and we'd open it for writing, fail. + // If the path requires a directory and we can't open a directory, fail. if components.is_empty() && dir_required && dir_precluded { return Err(errors::is_directory()); } @@ -275,7 +244,7 @@ pub(crate) fn open_manually<'start>( } #[cfg(not(feature = "no_racy_asserts"))] - check_open(&start_clone, path, options, &canonical_path, &base); + check_open_manually(&start_clone, path, options, &canonical_path, &base); canonical_path.complete(); Ok(base) @@ -290,7 +259,7 @@ fn should_emulate_o_path(use_options: &OpenOptions) -> bool { } #[cfg(not(feature = "no_racy_asserts"))] -fn check_open( +fn check_open_manually( start: &fs::File, path: &Path, options: &OpenOptions, @@ -332,20 +301,3 @@ fn check_open( } } } - -#[derive(Debug)] -pub(crate) enum OpenUncheckedError { - Other(io::Error), - Symlink(io::Error), - NotFound(io::Error), -} - -impl From for io::Error { - fn from(error: OpenUncheckedError) -> Self { - match error { - OpenUncheckedError::Other(err) - | OpenUncheckedError::Symlink(err) - | OpenUncheckedError::NotFound(err) => err, - } - } -} diff --git a/cap-primitives/src/fs/open_unchecked_error.rs b/cap-primitives/src/fs/open_unchecked_error.rs new file mode 100644 index 00000000..08f6652d --- /dev/null +++ b/cap-primitives/src/fs/open_unchecked_error.rs @@ -0,0 +1,18 @@ +use std::io; + +#[derive(Debug)] +pub(crate) enum OpenUncheckedError { + Other(io::Error), + Symlink(io::Error), + NotFound(io::Error), +} + +impl From for io::Error { + fn from(error: OpenUncheckedError) -> Self { + match error { + OpenUncheckedError::Other(err) + | OpenUncheckedError::Symlink(err) + | OpenUncheckedError::NotFound(err) => err, + } + } +} From f4b781e1e222e6cc04c7c7001d5d509f5217f4af Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 20 Aug 2020 12:56:40 -0700 Subject: [PATCH 2/6] Fix `stat` when the last path component is a symlink to `..`. Replace `stat_via_parent` with `stat_manually` which similarly to `open_manually`. Factor the `open_manually` implemenation to allow parts to be reused. --- .../src/fs/canonicalize_manually.rs | 4 +- cap-primitives/src/fs/metadata.rs | 22 + cap-primitives/src/fs/mod.rs | 2 - cap-primitives/src/fs/open_entry_manually.rs | 4 +- cap-primitives/src/fs/open_manually.rs | 403 +++++++++++------- cap-primitives/src/fs/open_parent.rs | 23 +- cap-primitives/src/fs/stat_via_parent.rs | 37 -- cap-primitives/src/winx/fs/mod.rs | 4 +- cap-primitives/src/yanix/fs/metadata_ext.rs | 109 ++++- cap-primitives/src/yanix/fs/mod.rs | 4 +- .../src/yanix/linux/fs/open_impl.rs | 6 +- .../src/yanix/linux/fs/stat_impl.rs | 6 +- tests/fs_additional.rs | 69 ++- 13 files changed, 447 insertions(+), 246 deletions(-) delete mode 100644 cap-primitives/src/fs/stat_via_parent.rs diff --git a/cap-primitives/src/fs/canonicalize_manually.rs b/cap-primitives/src/fs/canonicalize_manually.rs index e036aff9..89161148 100644 --- a/cap-primitives/src/fs/canonicalize_manually.rs +++ b/cap-primitives/src/fs/canonicalize_manually.rs @@ -1,7 +1,7 @@ //! Manual path canonicalization, one component at a time, with manual symlink //! resolution, in order to enforce sandboxing. -use crate::fs::{canonicalize_options, open_manually, FollowSymlinks, MaybeOwnedFile}; +use crate::fs::{canonicalize_options, open_manually_impl, FollowSymlinks, MaybeOwnedFile}; use std::{ fs, io, path::{Path, PathBuf}, @@ -28,7 +28,7 @@ pub(crate) fn canonicalize_manually( let mut canonical_path = PathBuf::new(); let start = MaybeOwnedFile::borrowed(start); - if let Err(e) = open_manually( + if let Err(e) = open_manually_impl( start, path, canonicalize_options().follow(follow), diff --git a/cap-primitives/src/fs/metadata.rs b/cap-primitives/src/fs/metadata.rs index 411b9302..bd401876 100644 --- a/cap-primitives/src/fs/metadata.rs +++ b/cap-primitives/src/fs/metadata.rs @@ -27,13 +27,35 @@ impl Metadata { /// Constructs a new instance of `Self` from the given `std::fs::Metadata`. #[inline] pub fn from_std(std: fs::Metadata) -> Self { + // TODO: Initialize `created` on Linux with `std.created().ok()` once yanix + // has `statx` and we make use of it. Self { file_type: FileType::from_std(std.file_type()), len: std.len(), permissions: Permissions::from_std(std.permissions()), modified: std.modified().ok(), accessed: std.accessed().ok(), + + #[cfg(any( + target_os = "freebsd", + target_os = "openbsd", + target_os = "macos", + target_os = "ios", + target_os = "netbsd", + windows, + ))] created: std.created().ok(), + + #[cfg(not(any( + target_os = "freebsd", + target_os = "openbsd", + target_os = "macos", + target_os = "ios", + target_os = "netbsd", + windows, + )))] + created: None, + ext: MetadataExt::from_std(std), } } diff --git a/cap-primitives/src/fs/mod.rs b/cap-primitives/src/fs/mod.rs index 7794fc68..e2b31d21 100644 --- a/cap-primitives/src/fs/mod.rs +++ b/cap-primitives/src/fs/mod.rs @@ -42,7 +42,6 @@ mod set_permissions; #[cfg(not(target_os = "linux"))] // doesn't work reliably on linux mod set_permissions_via_parent; mod stat; -mod stat_via_parent; mod symlink; mod symlink_via_parent; mod unlink; @@ -70,7 +69,6 @@ pub(crate) use rename_via_parent::*; pub(crate) use rmdir_via_parent::*; #[cfg(not(target_os = "linux"))] // doesn't work reliably on linux pub(crate) use set_permissions_via_parent::*; -pub(crate) use stat_via_parent::*; pub(crate) use symlink_via_parent::*; pub(crate) use unlink_via_parent::*; diff --git a/cap-primitives/src/fs/open_entry_manually.rs b/cap-primitives/src/fs/open_entry_manually.rs index 5a1ef5ea..a3244030 100644 --- a/cap-primitives/src/fs/open_entry_manually.rs +++ b/cap-primitives/src/fs/open_entry_manually.rs @@ -1,5 +1,5 @@ use crate::fs::{ - open_manually, open_unchecked, readlink_one, FollowSymlinks, MaybeOwnedFile, OpenOptions, + open_manually_impl, open_unchecked, readlink_one, FollowSymlinks, MaybeOwnedFile, OpenOptions, OpenUncheckedError, }; use std::{ffi::OsStr, fs, io}; @@ -19,7 +19,7 @@ pub(crate) fn open_entry_manually( let mut symlink_count = 0; let destination = readlink_one(start, path, &mut symlink_count)?; let maybe = MaybeOwnedFile::borrowed(start); - open_manually(maybe, &destination, options, &mut symlink_count, None) + open_manually_impl(maybe, &destination, options, &mut symlink_count, None) .map(MaybeOwnedFile::unwrap_owned) } Err(OpenUncheckedError::NotFound(err)) | Err(OpenUncheckedError::Other(err)) => Err(err), diff --git a/cap-primitives/src/fs/open_manually.rs b/cap-primitives/src/fs/open_manually.rs index 2a30438f..c01112ea 100644 --- a/cap-primitives/src/fs/open_manually.rs +++ b/cap-primitives/src/fs/open_manually.rs @@ -4,9 +4,9 @@ #[cfg(not(feature = "no_racy_asserts"))] use crate::fs::is_same_file; use crate::fs::{ - dir_options, errors, open_unchecked, path_requires_dir, readlink_one, to_borrowed_component, - to_owned_component, CowComponent, FollowSymlinks, MaybeOwnedFile, OpenOptions, - OpenUncheckedError, + dir_options, errors, open_unchecked, path_requires_dir, readlink_one, stat_unchecked, + to_borrowed_component, to_owned_component, CowComponent, FollowSymlinks, MaybeOwnedFile, + Metadata, OpenOptions, OpenUncheckedError, }; use std::{ ffi::OsStr, @@ -81,173 +81,290 @@ impl<'path_buf> Drop for CanonicalPath<'path_buf> { } } -/// A wrapper around `open_manually` which starts with a `symlink_count` of 0 -/// and does not return the canonical path, so it has the signature needed to be -/// used as `open_impl`. -pub(crate) fn open_manually_wrapper( +/// Implement `open` by breaking up the path into components, resolving each +/// component individually, and resolving symbolic links manually. +pub(crate) fn open_manually( start: &fs::File, path: &Path, options: &OpenOptions, ) -> io::Result { let mut symlink_count = 0; let start = MaybeOwnedFile::borrowed(start); - let maybe_owned = open_manually(start, path, options, &mut symlink_count, None)?; + let maybe_owned = open_manually_impl(start, path, options, &mut symlink_count, None)?; maybe_owned.into_file(options) } -/// Implement `open` by breaking up the path into components, resolving each -/// component individually, and resolving symbolic links manually. If requested, -/// also produce the canonical path along the way. +/// Context for performing manual component-at-a-time path resolution. +struct Context<'start> { + /// The current base directory handle for path lookups. + base: MaybeOwnedFile<'start>, + + /// The stack of directory handles below the base. + dirs: Vec>, + + /// The current worklist stack of path components to process. + components: Vec>, + + /// If requested, the canonical path is constructed here. + canonical_path: CanonicalPath<'start>, + + /// Does the path end in `/` or similar, so it requires a directory? + dir_required: bool, + + /// Are we requesting write permissions, so we can't open a directory? + dir_precluded: bool, + + #[cfg(not(feature = "no_racy_asserts"))] + start_clone: MaybeOwnedFile<'start>, +} + +impl<'start> Context<'start> { + /// Construct a new instance of `Self`. + fn new( + start: MaybeOwnedFile<'start>, + path: &'start Path, + options: &OpenOptions, + canonical_path: Option<&'start mut PathBuf>, + ) -> Self { + let components = path + .components() + .rev() + .map(to_borrowed_component) + .collect::>(); + + #[cfg(not(feature = "no_racy_asserts"))] + let start_clone = MaybeOwnedFile::owned(start.try_clone().unwrap()); + + Self { + base: start, + dirs: Vec::with_capacity(components.len()), + components, + canonical_path: CanonicalPath::new(canonical_path), + dir_required: path_requires_dir(path), + dir_precluded: options.write || options.append, + + #[cfg(not(feature = "no_racy_asserts"))] + start_clone, + } + } + + /// Handle a "." path component. + fn cur_dir(&mut self) -> io::Result<()> { + // If the path ends in `.` and we can't open a directory, fail. + if self.components.is_empty() { + if self.dir_precluded { + return Err(errors::is_directory()); + } + // TODO: This metdata call is unneeded in a common case of calling + // `read_dir` on `.`. + if !self.base.metadata()?.is_dir() { + return Err(errors::is_not_directory()); + } + self.canonical_path.push(Component::CurDir.as_os_str()); + } + + // Otherwise do nothing. + Ok(()) + } + + /// Handle a ".." path component. + fn parent_dir(&mut self) -> io::Result<()> { + #[cfg(not(feature = "no_racy_asserts"))] + assert!(self.dirs.is_empty() || !is_same_file(&self.start_clone, &self.base)?); + + if self.components.is_empty() && self.dir_precluded { + return Err(errors::is_directory()); + } + + // We hold onto all the parent directory descriptors so that we + // don't have to re-open anything when we encounter a `..`. This + // way, even if the directory is concurrently moved, we don't have + // to worry about `..` leaving the sandbox. + match self.dirs.pop() { + Some(dir) => self.base = dir, + None => return Err(errors::escape_attempt()), + } + assert!(self.canonical_path.pop()); + + Ok(()) + } + + /// Handle a "normal" path component. + fn normal( + &mut self, + one: &OsStr, + options: &OpenOptions, + symlink_count: &mut u8, + ) -> io::Result<()> { + // If the path requires a directory and we can't open a directory, fail. + if self.components.is_empty() && self.dir_required && self.dir_precluded { + return Err(errors::is_directory()); + } + + // Otherwise we're doing an open. + let mut use_options = if self.components.is_empty() { + options.clone() + } else { + dir_options() + }; + let dir_required = self.dir_required || use_options.dir_required; + match open_unchecked( + &self.base, + one.as_ref(), + use_options + .follow(FollowSymlinks::No) + .dir_required(dir_required), + ) { + Ok(file) => { + // Emulate `O_PATH` + `FollowSymlinks::Yes` on Linux. If `file` is a + // symlink, follow it. + #[cfg(target_os = "linux")] + if should_emulate_o_path(&use_options) { + match readlink_one(&file, Default::default(), symlink_count) { + Ok(destination) => { + self.components + .extend(destination.components().rev().map(to_owned_component)); + self.dir_required |= path_requires_dir(&destination); + return Ok(()); + } + // If it isn't a symlink, handle it as normal. `readlinkat` returns + // `ENOENT` if the file isn't a symlink in this situation. + Err(err) if err.kind() == io::ErrorKind::NotFound => (), + // If `readlinkat` fails any other way, pass it on. + Err(err) => return Err(err), + } + } + + // Normal case + let prev_base = self.base.descend_to(MaybeOwnedFile::owned(file)); + self.dirs.push(prev_base); + self.canonical_path.push(one); + Ok(()) + } + Err(OpenUncheckedError::Symlink(err)) => { + if options.follow == FollowSymlinks::No && self.components.is_empty() { + self.canonical_path.push(one); + self.canonical_path.complete(); + return Err(err); + } + self.symlink(one, symlink_count) + } + Err(OpenUncheckedError::NotFound(err)) => Err(err), + Err(OpenUncheckedError::Other(err)) => { + // An error occurred. If this was the last component, and the error wasn't + // due to invalid inputs (eg. the path has an embedded NUL), record it as + // the last component of the canonical path, even if we couldn't open it. + if self.components.is_empty() && err.kind() != io::ErrorKind::InvalidInput { + self.canonical_path.push(one); + self.canonical_path.complete(); + } + Err(err) + } + } + } + + /// Dereference one symlink level. + fn symlink(&mut self, one: &OsStr, symlink_count: &mut u8) -> io::Result<()> { + let destination = readlink_one(&self.base, one, symlink_count)?; + self.components + .extend(destination.components().rev().map(to_owned_component)); + self.dir_required |= path_requires_dir(&destination); + Ok(()) + } +} + +/// Internal implementation of `open_manually`, exposing some additional +/// parameters. /// /// Callers can request the canonical path by passing `Some` to /// `canonical_path`. If the complete canonical path is processed, even if /// `open_manually` returns an `Err`, it will be stored in the provided /// `&mut PathBuf`. If an error occurs before the complete canonical path is /// processed, the provided `&mut PathBuf` is cleared to empty. -pub(crate) fn open_manually<'start>( +/// +/// A note on lifetimes: `path` and `canonical_path` here don't strictly +/// need `'start`, but using them makes it easier to store them in the +/// `Context` struct. +pub(crate) fn open_manually_impl<'start>( start: MaybeOwnedFile<'start>, - path: &Path, + path: &'start Path, options: &OpenOptions, symlink_count: &mut u8, - canonical_path: Option<&mut PathBuf>, + canonical_path: Option<&'start mut PathBuf>, ) -> io::Result> { - #[cfg(not(feature = "no_racy_asserts"))] - let start_clone = MaybeOwnedFile::owned(start.try_clone().unwrap()); - // POSIX returns `ENOENT` on an empty path. TODO: On Windows, we should // be compatible with what Windows does instead. if path.as_os_str().is_empty() { return Err(errors::no_such_file_or_directory()); } - let mut components = path - .components() - .map(to_borrowed_component) - .rev() - .collect::>(); - let mut base = start; - let mut dirs = Vec::with_capacity(components.len()); - let mut canonical_path = CanonicalPath::new(canonical_path); - let dir_options = dir_options(); + let mut ctx = Context::new(start, path, options, canonical_path); - // Does the path end in `/` or similar, so it requires a directory? - let mut dir_required = path_requires_dir(path); - - // Are we requesting write permissions, so we can't open a directory? - let dir_precluded = options.write || options.append; - - while let Some(c) = components.pop() { + while let Some(c) = ctx.components.pop() { match c { CowComponent::PrefixOrRootDir => return Err(errors::escape_attempt()), - CowComponent::CurDir => { - // If the path ends in `.` and we can't open a directory, fail. - if components.is_empty() { - if dir_precluded { - return Err(errors::is_directory()); - } - if !base.metadata()?.is_dir() { - return Err(errors::is_not_directory()); - } - canonical_path.push(Component::CurDir.as_os_str()); - } + CowComponent::CurDir => ctx.cur_dir()?, + CowComponent::ParentDir => ctx.parent_dir()?, + CowComponent::Normal(one) => ctx.normal(&one, options, symlink_count)?, + } + } - // Otherwise just skip `.`. - continue; - } - CowComponent::ParentDir => { - #[cfg(not(feature = "no_racy_asserts"))] - assert!(dirs.is_empty() || !is_same_file(&start_clone, &base)?); + #[cfg(not(feature = "no_racy_asserts"))] + check_open_manually_impl(&ctx, path, options); - if components.is_empty() && dir_precluded { - return Err(errors::is_directory()); - } + ctx.canonical_path.complete(); + Ok(ctx.base) +} - // We hold onto all the parent directory descriptors so that we - // don't have to re-open anything when we encounter a `..`. This - // way, even if the directory is concurrently moved, we don't have - // to worry about `..` leaving the sandbox. - match dirs.pop() { - Some(dir) => base = dir, - None => return Err(errors::escape_attempt()), - } - assert!(canonical_path.pop()); - } - CowComponent::Normal(one) => { - // If the path requires a directory and we can't open a directory, fail. - if components.is_empty() && dir_required && dir_precluded { - return Err(errors::is_directory()); - } +/// Implement `stat` in a similar manner as `open_manually`. +pub(crate) fn stat_manually<'start>( + start: &fs::File, + path: &'start Path, + follow: FollowSymlinks, +) -> io::Result { + // POSIX returns `ENOENT` on an empty path. TODO: On Windows, we should + // be compatible with what Windows does instead. + if path.as_os_str().is_empty() { + return Err(errors::no_such_file_or_directory()); + } - let use_options = if components.is_empty() { - options - } else { - &dir_options - }; - match open_unchecked( - &base, - one.as_ref(), - use_options - .clone() - .follow(FollowSymlinks::No) - .dir_required(dir_required), - ) { - Ok(file) => { - // Emulate `O_PATH` + `FollowSymlinks::Yes` on Linux. If `file` is a - // symlink, follow it. - #[cfg(target_os = "linux")] - if should_emulate_o_path(use_options) { - match readlink_one(&file, Default::default(), symlink_count) { - Ok(destination) => { - components.extend( - destination.components().map(to_owned_component).rev(), - ); - dir_required |= path_requires_dir(&destination); - continue; - } - // If it isn't a symlink, handle it as normal. `readlinkat` returns - // `ENOENT` if the file isn't a symlink in this situation. - Err(err) if err.kind() == io::ErrorKind::NotFound => (), - // If `readlinkat` fails any other way, pass it on. - Err(err) => return Err(err), - } - } + let mut options = OpenOptions::new(); + options.follow(follow); + let mut symlink_count = 0; + let mut ctx = Context::new(MaybeOwnedFile::borrowed(start), path, &options, None); + assert!(!ctx.dir_precluded); - // Normal case - let prev_base = base.descend_to(MaybeOwnedFile::owned(file)); - dirs.push(prev_base); - canonical_path.push(&one); - } - Err(OpenUncheckedError::Symlink(err)) => { - if use_options.follow == FollowSymlinks::No && components.is_empty() { - canonical_path.push(&one); - canonical_path.complete(); - return Err(err); - } - let destination = readlink_one(&base, &one, symlink_count)?; - components.extend(destination.components().map(to_owned_component).rev()); - dir_required |= path_requires_dir(&destination); - } - Err(OpenUncheckedError::NotFound(err)) => return Err(err), - Err(OpenUncheckedError::Other(err)) => { - // An error occurred. If this was the last component, and the error wasn't - // due to invalid inputs (eg. the path has an embedded NUL), record it as - // the last component of the canonical path, even if we couldn't open it. - if components.is_empty() && err.kind() != io::ErrorKind::InvalidInput { - canonical_path.push(&one); - canonical_path.complete(); + while let Some(c) = ctx.components.pop() { + match c { + CowComponent::PrefixOrRootDir => return Err(errors::escape_attempt()), + CowComponent::CurDir => ctx.cur_dir()?, + CowComponent::ParentDir => ctx.parent_dir()?, + CowComponent::Normal(one) => { + if ctx.components.is_empty() { + // If this is the last component, do a non-following `stat_unchecked` on it. + let stat = stat_unchecked(&ctx.base, one.as_ref(), FollowSymlinks::No)?; + + // If we weren't asked to follow symlinks, or it wasn't a symlink, we're done. + if options.follow == FollowSymlinks::No || !stat.file_type().is_symlink() { + if ctx.dir_required && !stat.is_dir() { + return Err(errors::is_not_directory()); } - return Err(err); + return Ok(stat); } + + // If it was a symlink and we're asked to follow symlinks, dereference it. + ctx.symlink(&one, &mut symlink_count)? + } else { + // Otherwise open the path component normally. + ctx.normal(&one, &options, &mut symlink_count)? } } } } - #[cfg(not(feature = "no_racy_asserts"))] - check_open_manually(&start_clone, path, options, &canonical_path, &base); - - canonical_path.complete(); - Ok(base) + // If the path ended in `.` or `..`, we already have it open, so just do + // `.metadata()` on it. + ctx.base.metadata().map(Metadata::from_std) } /// Test whether the given options imply that we should treat an open file as @@ -259,16 +376,10 @@ fn should_emulate_o_path(use_options: &OpenOptions) -> bool { } #[cfg(not(feature = "no_racy_asserts"))] -fn check_open_manually( - start: &fs::File, - path: &Path, - options: &OpenOptions, - canonical_path: &CanonicalPath, - base: &MaybeOwnedFile, -) { +fn check_open_manually_impl(ctx: &Context, path: &Path, options: &OpenOptions) { match open_unchecked( - start, - canonical_path.debug.as_ref(), + &ctx.start_clone, + ctx.canonical_path.debug.as_ref(), options .clone() .create(false) @@ -277,13 +388,13 @@ fn check_open_manually( ) { Ok(unchecked_file) => { assert!( - is_same_file(base, &unchecked_file).unwrap(), + is_same_file(&ctx.base, &unchecked_file).unwrap(), "path resolution inconsistency: start='{:?}', path='{}'; canonical_path='{}'; \ got='{:?}' expected='{:?}'", - start, + ctx.start_clone, path.display(), - canonical_path.debug.display(), - base, + ctx.canonical_path.debug.display(), + ctx.base, &unchecked_file, ); } @@ -292,10 +403,10 @@ fn check_open_manually( panic!( "unexpected success opening result={:?} start='{:?}', path='{}'; canonical_path='{}'; \ expected {:?}", - base, - start, + ctx.base, + ctx.start_clone, path.display(), - canonical_path.debug.display(), + ctx.canonical_path.debug.display(), unchecked_error, */ } diff --git a/cap-primitives/src/fs/open_parent.rs b/cap-primitives/src/fs/open_parent.rs index ea840468..8b37900d 100644 --- a/cap-primitives/src/fs/open_parent.rs +++ b/cap-primitives/src/fs/open_parent.rs @@ -2,7 +2,7 @@ /// It opens the parent directory of the given path, and returns the basename, /// so that all the `*_via_parent` routines need to do is make sure they /// don't follow symlinks in the basename. -use crate::fs::{dir_options, errors, open, open_manually, path_requires_dir, MaybeOwnedFile}; +use crate::fs::{dir_options, errors, open, path_requires_dir, MaybeOwnedFile}; use std::{ ffi::OsStr, io, @@ -29,27 +29,6 @@ pub(crate) fn open_parent<'path, 'borrow>( Ok((dir, basename.as_os_str())) } -/// Similar to `open_parent`, but with a `symlink_count` argument which allows it -/// to be part of a multi-part lookup that operates under a single symlink count. -/// -/// To do this, it uses `open_manually`, so it doesn't benefit from the same -/// optimizations that using plain `open` does. -pub(crate) fn open_parent_manually<'path, 'borrow>( - start: MaybeOwnedFile<'borrow>, - path: &'path Path, - symlink_count: &mut u8, -) -> io::Result<(MaybeOwnedFile<'borrow>, &'path OsStr)> { - let (dirname, basename) = split_parent(path).ok_or_else(errors::no_such_file_or_directory)?; - - let dir = if dirname.as_os_str().is_empty() { - start - } else { - open_manually(start, dirname, &dir_options(), symlink_count, None)? - }; - - Ok((dir, basename.as_os_str())) -} - /// Split `path` into parent and basename parts. Return `None` if `path` /// is empty. /// diff --git a/cap-primitives/src/fs/stat_via_parent.rs b/cap-primitives/src/fs/stat_via_parent.rs deleted file mode 100644 index a665c7fc..00000000 --- a/cap-primitives/src/fs/stat_via_parent.rs +++ /dev/null @@ -1,37 +0,0 @@ -//! `stat` by resolving the parent directory and calling `fstatat`. - -use crate::fs::{ - open_parent_manually, readlink_one, stat_unchecked, FollowSymlinks, MaybeOwnedFile, Metadata, -}; -use std::{borrow::Cow, fs, io, path::Path}; - -/// Implement `stat` by `open`ing up the parent component of the path and then -/// calling `stat_unchecked` on the last component. If it's a symlink, repeat this -/// process. -pub(crate) fn stat_via_parent( - start: &fs::File, - path: &Path, - follow: FollowSymlinks, -) -> io::Result { - let mut symlink_count = 0; - let mut dir = MaybeOwnedFile::borrowed(start); - let mut path = Cow::Borrowed(path); - - loop { - // Split `path` into parent and basename and open the parent. - let (opened_dir, basename) = open_parent_manually(dir, &path, &mut symlink_count)?; - dir = opened_dir; - - // Do the stat. - let metadata = stat_unchecked(&dir, basename.as_ref(), FollowSymlinks::No)?; - - // If the user didn't want us to follow a symlink in the last component, or we didn't - // find a symlink, we're done. - if !metadata.file_type().is_symlink() || follow == FollowSymlinks::No { - return Ok(metadata); - } - - // Dereference the symlink and iterate. - path = Cow::Owned(readlink_one(&dir, basename, &mut symlink_count)?); - } -} diff --git a/cap-primitives/src/winx/fs/mod.rs b/cap-primitives/src/winx/fs/mod.rs index 5fc35915..ce6995a9 100644 --- a/cap-primitives/src/winx/fs/mod.rs +++ b/cap-primitives/src/winx/fs/mod.rs @@ -35,13 +35,13 @@ pub(crate) use crate::fs::{ rename_via_parent as rename_impl, rmdir_via_parent as rmdir_impl, set_permissions_via_parent as set_permissions_impl, - stat_via_parent as stat_impl, + stat_manually as stat_impl, symlink_dir_via_parent as symlink_dir_impl, symlink_file_via_parent as symlink_file_impl, unlink_via_parent as unlink_impl, }; -pub(crate) use crate::fs::open_manually_wrapper as open_impl; +pub(crate) use crate::fs::open_manually as open_impl; pub(crate) use copy::*; pub(crate) use dir_entry_inner::*; pub(crate) use dir_options_ext::*; diff --git a/cap-primitives/src/yanix/fs/metadata_ext.rs b/cap-primitives/src/yanix/fs/metadata_ext.rs index e68868e8..4bf9687b 100644 --- a/cap-primitives/src/yanix/fs/metadata_ext.rs +++ b/cap-primitives/src/yanix/fs/metadata_ext.rs @@ -54,32 +54,95 @@ impl MetadataExt { /// Constructs a new instance of `Metadata` from the given `libc::stat`. #[inline] - pub(crate) fn from_libc(mode: libc::stat) -> Metadata { + pub(crate) fn from_libc(stat: libc::stat) -> Metadata { Metadata { - file_type: FileTypeExt::from_libc(mode.st_mode), - len: u64::try_from(mode.st_size).unwrap(), - permissions: PermissionsExt::from_libc(mode.st_mode), - modified: system_time_from_libc(mode.st_mtime, mode.st_mtime_nsec), - accessed: system_time_from_libc(mode.st_atime, mode.st_atime_nsec), - created: system_time_from_libc(mode.st_ctime, mode.st_ctime_nsec), + file_type: FileTypeExt::from_libc(stat.st_mode), + len: u64::try_from(stat.st_size).unwrap(), + permissions: PermissionsExt::from_libc(stat.st_mode), + modified: system_time_from_libc(stat.st_mtime, stat.st_mtime_nsec), + accessed: system_time_from_libc(stat.st_atime, stat.st_atime_nsec), + + #[cfg(any( + target_os = "freebsd", + target_os = "openbsd", + target_os = "macos", + target_os = "ios" + ))] + created: system_time_from_libc(stat.st_birthtime, stat.st_birthtime_nsec), + + #[cfg(target_os = "netbsd")] + created: system_time_from_libc(stat.st_birthtime, stat.st_birthtimensec), + + // `stat.st_ctime` is the latest status change; we want the creation. + #[cfg(not(any( + target_os = "freebsd", + target_os = "openbsd", + target_os = "macos", + target_os = "ios", + target_os = "netbsd" + )))] + created: None, ext: Self { - dev: u64::try_from(mode.st_dev).unwrap(), - ino: mode.st_ino.into(), - mode: u32::from(mode.st_mode), - nlink: u64::from(mode.st_nlink), - uid: mode.st_uid, - gid: mode.st_gid, - rdev: u64::try_from(mode.st_rdev).unwrap(), - size: u64::try_from(mode.st_size).unwrap(), - atime: i64::from(mode.st_atime), - atime_nsec: i64::from(mode.st_atime_nsec), - mtime: i64::from(mode.st_mtime), - mtime_nsec: i64::from(mode.st_mtime_nsec), - ctime: i64::from(mode.st_ctime), - ctime_nsec: i64::from(mode.st_ctime_nsec), - blksize: u64::try_from(mode.st_blksize).unwrap(), - blocks: u64::try_from(mode.st_blocks).unwrap(), + dev: u64::try_from(stat.st_dev).unwrap(), + ino: stat.st_ino.into(), + mode: u32::from(stat.st_mode), + nlink: u64::from(stat.st_nlink), + uid: stat.st_uid, + gid: stat.st_gid, + rdev: u64::try_from(stat.st_rdev).unwrap(), + size: u64::try_from(stat.st_size).unwrap(), + atime: i64::from(stat.st_atime), + atime_nsec: i64::from(stat.st_atime_nsec), + mtime: i64::from(stat.st_mtime), + mtime_nsec: i64::from(stat.st_mtime_nsec), + ctime: i64::from(stat.st_ctime), + ctime_nsec: i64::from(stat.st_ctime_nsec), + blksize: u64::try_from(stat.st_blksize).unwrap(), + blocks: u64::try_from(stat.st_blocks).unwrap(), + }, + } + } + + /// Constructs a new instance of `Metadata` from the given `libc::statx`. + #[cfg(all(target_os = "linux", target_env = "gnu"))] + #[inline] + #[allow(dead_code)] // TODO: Add `statx` to yanix and use this when possible. + pub(crate) fn from_libc_statx(statx: libc::statx) -> Metadata { + Metadata { + file_type: FileTypeExt::from_libc(libc::mode_t::from(statx.stx_mode)), + len: u64::try_from(statx.stx_size).unwrap(), + permissions: PermissionsExt::from_libc(libc::mode_t::from(statx.stx_mode)), + modified: system_time_from_libc( + statx.stx_mtime.tv_sec, + i64::from(statx.stx_mtime.tv_nsec), + ), + accessed: system_time_from_libc( + statx.stx_atime.tv_sec, + i64::from(statx.stx_atime.tv_nsec), + ), + created: system_time_from_libc( + statx.stx_btime.tv_sec, + i64::from(statx.stx_btime.tv_nsec), + ), + + ext: Self { + dev: unsafe { libc::makedev(statx.stx_dev_major, statx.stx_dev_minor) }, + ino: statx.stx_ino.into(), + mode: u32::from(statx.stx_mode), + nlink: u64::from(statx.stx_nlink), + uid: statx.stx_uid, + gid: statx.stx_gid, + rdev: unsafe { libc::makedev(statx.stx_rdev_major, statx.stx_rdev_minor) }, + size: statx.stx_size, + atime: i64::from(statx.stx_atime.tv_sec), + atime_nsec: i64::from(statx.stx_atime.tv_nsec), + mtime: i64::from(statx.stx_mtime.tv_sec), + mtime_nsec: i64::from(statx.stx_mtime.tv_nsec), + ctime: i64::from(statx.stx_ctime.tv_sec), + ctime_nsec: i64::from(statx.stx_ctime.tv_nsec), + blksize: u64::from(statx.stx_blksize), + blocks: statx.stx_blocks, }, } } diff --git a/cap-primitives/src/yanix/fs/mod.rs b/cap-primitives/src/yanix/fs/mod.rs index a13f45d3..82787a32 100644 --- a/cap-primitives/src/yanix/fs/mod.rs +++ b/cap-primitives/src/yanix/fs/mod.rs @@ -41,8 +41,8 @@ pub(crate) use crate::yanix::linux::fs::*; #[rustfmt::skip] pub(crate) use crate::fs::{ open_entry_manually as open_entry_impl, - open_manually_wrapper as open_impl, - stat_via_parent as stat_impl, + open_manually as open_impl, + stat_manually as stat_impl, canonicalize_manually_and_follow as canonicalize_impl, set_permissions_via_parent as set_permissions_impl, }; diff --git a/cap-primitives/src/yanix/linux/fs/open_impl.rs b/cap-primitives/src/yanix/linux/fs/open_impl.rs index a0ff21e2..f0100ad1 100644 --- a/cap-primitives/src/yanix/linux/fs/open_impl.rs +++ b/cap-primitives/src/yanix/linux/fs/open_impl.rs @@ -10,7 +10,7 @@ use super::super::super::fs::compute_oflags; #[cfg(not(feature = "no_racy_asserts"))] use crate::fs::is_same_file; -use crate::fs::{errors, open_manually_wrapper, OpenOptions}; +use crate::fs::{errors, open_manually, OpenOptions}; use std::{ ffi::CString, fs, io, @@ -46,7 +46,7 @@ pub(crate) fn open_impl( // If that returned `ENOSYS`, use a fallback strategy. if let Err(e) = &result { if let Some(libc::ENOSYS) = e.raw_os_error() { - return open_manually_wrapper(start, path, options); + return open_manually(start, path, options); } } @@ -133,7 +133,7 @@ fn other_error(errno: i32) -> io::Result { #[cfg(not(feature = "no_racy_asserts"))] fn check_open(start: &fs::File, path: &Path, options: &OpenOptions, file: &fs::File) { - let check = open_manually_wrapper( + let check = open_manually( start, path, options diff --git a/cap-primitives/src/yanix/linux/fs/stat_impl.rs b/cap-primitives/src/yanix/linux/fs/stat_impl.rs index f9685030..0ae9c69a 100644 --- a/cap-primitives/src/yanix/linux/fs/stat_impl.rs +++ b/cap-primitives/src/yanix/linux/fs/stat_impl.rs @@ -3,11 +3,11 @@ //! `fstat` to perform a fast sandboxed `stat`. use super::file_metadata; -use crate::fs::{open_beneath, stat_via_parent, FollowSymlinks, Metadata, OpenOptions}; +use crate::fs::{open_beneath, stat_manually, FollowSymlinks, Metadata, OpenOptions}; use std::{fs, io, path::Path}; /// Use `openat2` with `O_PATH` and `fstat`. If that's not available, fallback -/// to `stat_via_parent`. +/// to `stat_manually`. pub(crate) fn stat_impl( start: &fs::File, path: &Path, @@ -33,7 +33,7 @@ pub(crate) fn stat_impl( Err(err) => match err.raw_os_error() { // `ENOSYS` from `open_beneath` means `openat2` is unavailable // and we should use a fallback. - Some(libc::ENOSYS) => stat_via_parent(start, path, follow), + Some(libc::ENOSYS) => stat_manually(start, path, follow), _ => Err(err), }, } diff --git a/tests/fs_additional.rs b/tests/fs_additional.rs index 9d646f96..be6a71b3 100644 --- a/tests/fs_additional.rs +++ b/tests/fs_additional.rs @@ -7,10 +7,36 @@ mod sys_common; use cap_std::fs::{DirBuilder, OpenOptions}; use std::{ - io::{Read, Write}, + io::{self, Read, Write}, + path::Path, str, }; -use sys_common::io::tmpdir; +use sys_common::io::{tmpdir, TempDir}; + +#[cfg(not(windows))] +fn symlink_dir, Q: AsRef>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> { + tmpdir.symlink(src, dst) +} +#[cfg(not(windows))] +fn symlink_file, Q: AsRef>( + src: P, + tmpdir: &TempDir, + dst: Q, +) -> io::Result<()> { + tmpdir.symlink(src, dst) +} +#[cfg(windows)] +fn symlink_dir, Q: AsRef>(src: P, tmpdir: &TempDir, dst: Q) -> io::Result<()> { + tmpdir.symlink_dir(src, dst) +} +#[cfg(windows)] +fn symlink_file, Q: AsRef>( + src: P, + tmpdir: &TempDir, + dst: Q, +) -> io::Result<()> { + tmpdir.symlink_file(src, dst) +} #[test] fn recursive_mkdir() { @@ -183,3 +209,42 @@ fn file_test_directoryinfo_readdir() { } check!(tmpdir.remove_dir(dir)); } + +#[test] +fn follow_dotdot_symlink() { + let tmpdir = tmpdir(); + check!(tmpdir.create_dir_all("a/b")); + check!(symlink_dir("..", &tmpdir, "a/b/c")); + check!(symlink_dir("../..", &tmpdir, "a/b/d")); + check!(symlink_dir("../../..", &tmpdir, "a/b/e")); + check!(symlink_dir("../../../..", &tmpdir, "a/b/f")); + + check!(tmpdir.open_dir("a/b/c")); + assert!(check!(tmpdir.metadata("a/b/c")).is_dir()); + + check!(tmpdir.open_dir("a/b/d")); + assert!(check!(tmpdir.metadata("a/b/d")).is_dir()); + + assert!(tmpdir.open_dir("a/b/e").is_err()); + assert!(tmpdir.metadata("a/b/e").is_err()); + + assert!(tmpdir.open_dir("a/b/f").is_err()); + assert!(tmpdir.metadata("a/b/f").is_err()); +} + +#[test] +fn follow_file_symlink() { + let tmpdir = tmpdir(); + + check!(tmpdir.create("file")); + + check!(symlink_file("file", &tmpdir, "link")); + check!(symlink_file("file/", &tmpdir, "link_slash")); + check!(symlink_file("file/.", &tmpdir, "link_slashdot")); + check!(symlink_file("file/..", &tmpdir, "link_slashdotdot")); + + check!(tmpdir.open("link")); + assert!(tmpdir.open("link_slash").is_err()); + assert!(tmpdir.open("link_slashdot").is_err()); + assert!(dbg!(tmpdir.open("link_slashdotdot")).is_err()); +} From 1d01ed7dd54e484e4631ad09a50e8ecb0b61770a Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 20 Aug 2020 16:31:25 -0700 Subject: [PATCH 3/6] Fix the delete portion of the `recursive_create_delete` benchmarks. Call `remove_dir_all` on the outermost directory, rather than the innermost one, so that it removes the whole directory. --- benches/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/benches/mod.rs b/benches/mod.rs index 0a2da337..fb99bee2 100644 --- a/benches/mod.rs +++ b/benches/mod.rs @@ -381,13 +381,13 @@ fn recursive_create_delete(b: &mut test::Bencher) { let dir = unsafe { cap_tempfile::tempdir().unwrap() }; let mut path = PathBuf::new(); - for _ in 0..256 { - path.push("abc"); + for depth in 0..256 { + path.push(format!("depth{}", depth)); } b.iter(|| { dir.create_dir_all(&path).unwrap(); - dir.remove_dir_all(&path).unwrap(); + dir.remove_dir_all("depth0").unwrap(); }); } @@ -397,13 +397,13 @@ fn recursive_create_delete_baseline(b: &mut test::Bencher) { let mut path = PathBuf::new(); path.push(dir); - for _ in 0..256 { - path.push("abc"); + for depth in 0..256 { + path.push(format!("depth{}", depth)); } b.iter(|| { fs::create_dir_all(&path).unwrap(); - fs::remove_dir_all(&path).unwrap(); + fs::remove_dir_all("depth0").unwrap(); }); } From fd1a3b333829c28f7f3714833872bcfda5c36ea9 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 20 Aug 2020 16:48:26 -0700 Subject: [PATCH 4/6] Factor out `CanonicalPath` into its own file. And make more things `pub(super)` instead of `pub(crate)`. --- cap-primitives/src/fs/canonical_path.rs | 71 ++++++++++++++++++++++ cap-primitives/src/fs/maybe_owned_file.rs | 12 ++-- cap-primitives/src/fs/mod.rs | 10 ++-- cap-primitives/src/fs/open_manually.rs | 73 +---------------------- cap-primitives/src/fs/open_parent.rs | 2 +- cap-primitives/src/fs/readlink_one.rs | 2 +- 6 files changed, 88 insertions(+), 82 deletions(-) create mode 100644 cap-primitives/src/fs/canonical_path.rs diff --git a/cap-primitives/src/fs/canonical_path.rs b/cap-primitives/src/fs/canonical_path.rs new file mode 100644 index 00000000..dad783a9 --- /dev/null +++ b/cap-primitives/src/fs/canonical_path.rs @@ -0,0 +1,71 @@ +use std::{ + ffi::OsStr, + path::{Component, PathBuf}, +}; + +/// Utility for collecting the canonical path components. +pub(super) struct CanonicalPath<'path_buf> { + /// If the user requested a canonical path, a reference to the `PathBuf` to + /// write it to. + path: Option<&'path_buf mut PathBuf>, + + /// Our own private copy of the canonical path, for assertion checking. + #[cfg(not(feature = "no_racy_asserts"))] + pub(super) debug: PathBuf, +} + +impl<'path_buf> CanonicalPath<'path_buf> { + pub(super) fn new(path: Option<&'path_buf mut PathBuf>) -> Self { + Self { + #[cfg(not(feature = "no_racy_asserts"))] + debug: PathBuf::new(), + + path, + } + } + + pub(super) fn push(&mut self, one: &OsStr) { + #[cfg(not(feature = "no_racy_asserts"))] + self.debug.push(one); + + if let Some(path) = &mut self.path { + path.push(one) + } + } + + pub(super) fn pop(&mut self) -> bool { + #[cfg(not(feature = "no_racy_asserts"))] + self.debug.pop(); + + if let Some(path) = &mut self.path { + path.pop() + } else { + true + } + } + + /// The complete canonical path has been scanned. Set `path` to `None` + /// so that it isn't cleared when `self` is dropped. + pub(super) fn complete(&mut self) { + // Replace "" with ".", since "" as a relative path is interpreted as an error. + if let Some(path) = &mut self.path { + if path.as_os_str().is_empty() { + path.push(Component::CurDir); + } + self.path = None; + } + } +} + +impl<'path_buf> Drop for CanonicalPath<'path_buf> { + fn drop(&mut self) { + // If `self.path` is still `Some` here, it means that we haven't called + // `complete()` yet, meaning the `CanonicalPath` is being dropped before + // the complete path has been processed. In that case, clear `path` to + // indicate that we weren't able to obtain a complete path. + if let Some(path) = &mut self.path { + path.clear(); + self.path = None; + } + } +} diff --git a/cap-primitives/src/fs/maybe_owned_file.rs b/cap-primitives/src/fs/maybe_owned_file.rs index bca19823..f442f58c 100644 --- a/cap-primitives/src/fs/maybe_owned_file.rs +++ b/cap-primitives/src/fs/maybe_owned_file.rs @@ -19,7 +19,7 @@ enum Inner<'borrow> { /// /// And, this type has the special `descend_to`, which just does an assignment, /// but also some useful assertion checks. -pub(crate) struct MaybeOwnedFile<'borrow> { +pub(super) struct MaybeOwnedFile<'borrow> { inner: Inner<'borrow>, #[cfg(not(feature = "no_racy_asserts"))] @@ -28,7 +28,7 @@ pub(crate) struct MaybeOwnedFile<'borrow> { impl<'borrow> MaybeOwnedFile<'borrow> { /// Constructs a new `MaybeOwnedFile` which is not owned. - pub(crate) fn borrowed(file: &'borrow fs::File) -> Self { + pub(super) fn borrowed(file: &'borrow fs::File) -> Self { #[cfg(not(feature = "no_racy_asserts"))] let path = get_path(file); @@ -41,7 +41,7 @@ impl<'borrow> MaybeOwnedFile<'borrow> { } /// Constructs a new `MaybeOwnedFile` which is owned. - pub(crate) fn owned(file: fs::File) -> Self { + pub(super) fn owned(file: fs::File) -> Self { #[cfg(not(feature = "no_racy_asserts"))] let path = get_path(&file); @@ -56,7 +56,7 @@ impl<'borrow> MaybeOwnedFile<'borrow> { /// Set this `MaybeOwnedFile` to a new owned file which is from a subtree /// of the current file. Return a `MaybeOwnedFile` representing the previous /// state. - pub(crate) fn descend_to(&mut self, to: MaybeOwnedFile<'borrow>) -> Self { + pub(super) fn descend_to(&mut self, to: MaybeOwnedFile<'borrow>) -> Self { #[cfg(not(feature = "no_racy_asserts"))] let path = self.path.clone(); @@ -83,7 +83,7 @@ impl<'borrow> MaybeOwnedFile<'borrow> { /// Produce an owned `File`. This uses `open` on "." if needed to convert a /// borrowed `File` to an owned one. - pub(crate) fn into_file(self, options: &OpenOptions) -> io::Result { + pub(super) fn into_file(self, options: &OpenOptions) -> io::Result { match self.inner { Inner::Owned(file) => Ok(file), Inner::Borrowed(file) => { @@ -97,7 +97,7 @@ impl<'borrow> MaybeOwnedFile<'borrow> { /// Assuming `self` holds an owned `File`, return it. #[cfg_attr(windows, allow(dead_code))] - pub(crate) fn unwrap_owned(self) -> fs::File { + pub(super) fn unwrap_owned(self) -> fs::File { match self.inner { Inner::Owned(file) => file, Inner::Borrowed(_) => panic!("expected owned file"), diff --git a/cap-primitives/src/fs/mod.rs b/cap-primitives/src/fs/mod.rs index e2b31d21..064cea22 100644 --- a/cap-primitives/src/fs/mod.rs +++ b/cap-primitives/src/fs/mod.rs @@ -1,5 +1,6 @@ //! Filesystem utilities. +mod canonical_path; mod canonicalize; mod canonicalize_manually; mod copy; @@ -49,20 +50,21 @@ mod unlink_via_parent; pub(crate) mod errors; -use cow_component::*; +use canonical_path::CanonicalPath; +use cow_component::{to_borrowed_component, to_owned_component, CowComponent}; +use maybe_owned_file::MaybeOwnedFile; +use open_parent::open_parent; +use readlink_one::readlink_one; pub(crate) use canonicalize_manually::*; #[cfg(not(feature = "no_racy_asserts"))] pub(crate) use get_path::*; pub(crate) use link_via_parent::*; -pub(crate) use maybe_owned_file::*; pub(crate) use mkdir_via_parent::*; #[cfg(not(windows))] // not needed on windows pub(crate) use open_entry_manually::*; pub(crate) use open_manually::*; -pub(crate) use open_parent::*; pub(crate) use open_unchecked_error::*; -pub(crate) use readlink_one::*; #[cfg(not(windows))] // doesn't work on windows; use a windows-specific impl pub(crate) use readlink_via_parent::*; pub(crate) use rename_via_parent::*; diff --git a/cap-primitives/src/fs/open_manually.rs b/cap-primitives/src/fs/open_manually.rs index c01112ea..4b45eb96 100644 --- a/cap-primitives/src/fs/open_manually.rs +++ b/cap-primitives/src/fs/open_manually.rs @@ -5,8 +5,8 @@ use crate::fs::is_same_file; use crate::fs::{ dir_options, errors, open_unchecked, path_requires_dir, readlink_one, stat_unchecked, - to_borrowed_component, to_owned_component, CowComponent, FollowSymlinks, MaybeOwnedFile, - Metadata, OpenOptions, OpenUncheckedError, + to_borrowed_component, to_owned_component, CanonicalPath, CowComponent, FollowSymlinks, + MaybeOwnedFile, Metadata, OpenOptions, OpenUncheckedError, }; use std::{ ffi::OsStr, @@ -14,73 +14,6 @@ use std::{ path::{Component, Path, PathBuf}, }; -/// Utility for collecting the canonical path components. -struct CanonicalPath<'path_buf> { - /// If the user requested a canonical path, a reference to the `PathBuf` to - /// write it to. - path: Option<&'path_buf mut PathBuf>, - - /// Our own private copy of the canonical path, for assertion checking. - #[cfg(not(feature = "no_racy_asserts"))] - debug: PathBuf, -} - -impl<'path_buf> CanonicalPath<'path_buf> { - fn new(path: Option<&'path_buf mut PathBuf>) -> Self { - Self { - #[cfg(not(feature = "no_racy_asserts"))] - debug: PathBuf::new(), - - path, - } - } - - fn push(&mut self, one: &OsStr) { - #[cfg(not(feature = "no_racy_asserts"))] - self.debug.push(one); - - if let Some(path) = &mut self.path { - path.push(one) - } - } - - fn pop(&mut self) -> bool { - #[cfg(not(feature = "no_racy_asserts"))] - self.debug.pop(); - - if let Some(path) = &mut self.path { - path.pop() - } else { - true - } - } - - /// The complete canonical path has been scanned. Set `path` to `None` - /// so that it isn't cleared when `self` is dropped. - fn complete(&mut self) { - // Replace "" with ".", since "" as a relative path is interpreted as an error. - if let Some(path) = &mut self.path { - if path.as_os_str().is_empty() { - path.push(Component::CurDir); - } - self.path = None; - } - } -} - -impl<'path_buf> Drop for CanonicalPath<'path_buf> { - fn drop(&mut self) { - // If `self.path` is still `Some` here, it means that we haven't called - // `complete()` yet, meaning the `CanonicalPath` is being dropped before - // the complete path has been processed. In that case, clear `path` to - // indicate that we weren't able to obtain a complete path. - if let Some(path) = &mut self.path { - path.clear(); - self.path = None; - } - } -} - /// Implement `open` by breaking up the path into components, resolving each /// component individually, and resolving symbolic links manually. pub(crate) fn open_manually( @@ -285,7 +218,7 @@ impl<'start> Context<'start> { /// A note on lifetimes: `path` and `canonical_path` here don't strictly /// need `'start`, but using them makes it easier to store them in the /// `Context` struct. -pub(crate) fn open_manually_impl<'start>( +pub(super) fn open_manually_impl<'start>( start: MaybeOwnedFile<'start>, path: &'start Path, options: &OpenOptions, diff --git a/cap-primitives/src/fs/open_parent.rs b/cap-primitives/src/fs/open_parent.rs index 8b37900d..e84a527f 100644 --- a/cap-primitives/src/fs/open_parent.rs +++ b/cap-primitives/src/fs/open_parent.rs @@ -14,7 +14,7 @@ use std::{ /// the single remaining path component. This last component will not be `..`, /// though it may be `.` or a symbolic link to anywhere (possibly /// including `..` or an absolute path). -pub(crate) fn open_parent<'path, 'borrow>( +pub(super) fn open_parent<'path, 'borrow>( start: MaybeOwnedFile<'borrow>, path: &'path Path, ) -> io::Result<(MaybeOwnedFile<'borrow>, &'path OsStr)> { diff --git a/cap-primitives/src/fs/readlink_one.rs b/cap-primitives/src/fs/readlink_one.rs index 5961263c..81b46d17 100644 --- a/cap-primitives/src/fs/readlink_one.rs +++ b/cap-primitives/src/fs/readlink_one.rs @@ -8,7 +8,7 @@ use std::{ /// This is a wrapper around `readlink_unchecked` which performs a single /// symlink expansion on a single path component, and which enforces the /// recursion limit. -pub(crate) fn readlink_one( +pub(super) fn readlink_one( base: &fs::File, name: &OsStr, symlink_count: &mut u8, From 184fedf7c4a7eb2a46c1bcb2dcdc9ee4f468f45f Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 21 Aug 2020 05:40:49 -0700 Subject: [PATCH 5/6] Implement enough of `Catalog` to avoid compiler warnings. --- cap-primitives/src/net/catalog.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/cap-primitives/src/net/catalog.rs b/cap-primitives/src/net/catalog.rs index 9fe78dec..6679378b 100644 --- a/cap-primitives/src/net/catalog.rs +++ b/cap-primitives/src/net/catalog.rs @@ -11,11 +11,26 @@ enum AddrSet { NameWildcard(String), } +impl AddrSet { + fn contains(&self, addr: net::IpAddr) -> bool { + match self { + Self::Net(ip_net) => ip_net.contains(&addr), + Self::NameWildcard(name) => false, + } + } +} + struct IpGrant { set: AddrSet, port: u16, // TODO: IANA port names, TODO: range } +impl IpGrant { + fn contains(&self, addr: &net::SocketAddr) -> bool { + self.set.contains(addr.ip()) && addr.port() == self.port + } +} + /// A representation of a set of network resources that may be accessed. /// This is presently a very incomplete concept. /// @@ -27,9 +42,14 @@ pub struct Catalog { impl Catalog { pub fn check_addr(&self, addr: &net::SocketAddr) -> io::Result<()> { - todo!("Catalog::check_addr({:?})", addr) - //self.grants.iter().any(|grant| grant. - //PermissionDenied + if self.grants.iter().any(|grant| grant.contains(addr)) { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "An address led outside the catalog", + )) + } } } From 4dcf166775a1bfa54298290f9cca338b78d4cff4 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 21 Aug 2020 05:41:17 -0700 Subject: [PATCH 6/6] Add a test for Windows filesystem behavior that we depend on. --- cap-async-std/src/fs/dir.rs | 5 + cap-async-std/src/fs_utf8/dir.rs | 5 + cap-primitives/src/fs/open_options.rs | 2 + cap-primitives/src/winx/fs/dir_utils.rs | 5 + .../src/winx/fs/open_options_ext.rs | 2 + .../src/winx/fs/remove_open_dir_impl.rs | 17 ++++ .../yanix/fs/remove_open_dir_by_searching.rs | 1 - cap-std/src/fs/dir.rs | 5 + cap-std/src/fs_utf8/dir.rs | 5 + cap-tempfile/src/lib.rs | 2 +- tests/windows-open.rs | 94 +++++++++++++++++++ 11 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tests/windows-open.rs diff --git a/cap-async-std/src/fs/dir.rs b/cap-async-std/src/fs/dir.rs index 9a472938..ebf7190e 100644 --- a/cap-async-std/src/fs/dir.rs +++ b/cap-async-std/src/fs/dir.rs @@ -46,6 +46,9 @@ pub struct Dir { impl Dir { /// Constructs a new instance of `Self` from the given `async_std::fs::File`. + /// + /// To prevent race conditions on Windows, the file must be opened without + /// `FILE_SHARE_DELETE`. #[inline] pub fn from_std_file(std_file: fs::File) -> Self { Self { std_file } @@ -624,6 +627,8 @@ impl FromRawFd for Dir { #[cfg(windows)] impl FromRawHandle for Dir { + /// To prevent race conditions on Windows, the handle must be opened without + /// `FILE_SHARE_DELETE`. #[inline] unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self::from_std_file(fs::File::from_raw_handle(handle)) diff --git a/cap-async-std/src/fs_utf8/dir.rs b/cap-async-std/src/fs_utf8/dir.rs index d81a12f9..f072e00c 100644 --- a/cap-async-std/src/fs_utf8/dir.rs +++ b/cap-async-std/src/fs_utf8/dir.rs @@ -31,6 +31,9 @@ pub struct Dir { impl Dir { /// Constructs a new instance of `Self` from the given `async_std::fs::File`. + /// + /// To prevent race conditions on Windows, the file must be opened without + /// `FILE_SHARE_DELETE`. #[inline] pub fn from_std_file(std_file: fs::File) -> Self { Self::from_cap_std(crate::fs::Dir::from_std_file(std_file)) @@ -543,6 +546,8 @@ impl FromRawFd for Dir { #[cfg(windows)] impl FromRawHandle for Dir { + /// To prevent race conditions on Windows, the handle must be opened without + /// `FILE_SHARE_DELETE`. #[inline] unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self::from_std_file(fs::File::from_raw_handle(handle)) diff --git a/cap-primitives/src/fs/open_options.rs b/cap-primitives/src/fs/open_options.rs index 326e36e7..3dc89b4e 100644 --- a/cap-primitives/src/fs/open_options.rs +++ b/cap-primitives/src/fs/open_options.rs @@ -174,6 +174,8 @@ impl std::os::windows::fs::OpenOptionsExt for OpenOptions { self } + /// To prevent race conditions on Windows, handles for directories must be opened + /// without `FILE_SHARE_DELETE`. #[inline] fn share_mode(&mut self, val: u32) -> &mut Self { self.ext.share_mode(val); diff --git a/cap-primitives/src/winx/fs/dir_utils.rs b/cap-primitives/src/winx/fs/dir_utils.rs index cde09bea..baff8ad5 100644 --- a/cap-primitives/src/winx/fs/dir_utils.rs +++ b/cap-primitives/src/winx/fs/dir_utils.rs @@ -9,6 +9,7 @@ use std::{ }, path::{Path, PathBuf}, }; +use winapi::um::winnt; use winx::file::Flags; /// Rust's `Path` implicitly strips redundant slashes and `.` components, however @@ -46,10 +47,14 @@ pub(crate) fn strip_dir_suffix(path: &Path) -> impl Deref + '_ { /// Return an `OpenOptions` for opening directories. pub(crate) fn dir_options() -> OpenOptions { + // Set `FILE_FLAG_BACKUP_SEMANTICS` so that we can open directories. Unset + // `FILE_SHARE_DELETE` so that directories can't be renamed or deleted + // underneath us, since we use paths to implement many directory operations. OpenOptions::new() .read(true) .dir_required(true) .custom_flags(Flags::FILE_FLAG_BACKUP_SEMANTICS.bits()) + .share_mode(winnt::FILE_SHARE_READ | winnt::FILE_SHARE_WRITE) .clone() } diff --git a/cap-primitives/src/winx/fs/open_options_ext.rs b/cap-primitives/src/winx/fs/open_options_ext.rs index 57eb28e6..c65dffd3 100644 --- a/cap-primitives/src/winx/fs/open_options_ext.rs +++ b/cap-primitives/src/winx/fs/open_options_ext.rs @@ -29,6 +29,8 @@ impl std::os::windows::fs::OpenOptionsExt for OpenOptionsExt { self } + /// To prevent race conditions on Windows, handles must be opened without + /// `FILE_SHARE_DELETE`. fn share_mode(&mut self, share: u32) -> &mut Self { self.share_mode = share; self diff --git a/cap-primitives/src/winx/fs/remove_open_dir_impl.rs b/cap-primitives/src/winx/fs/remove_open_dir_impl.rs index a7ee221c..393b8de4 100644 --- a/cap-primitives/src/winx/fs/remove_open_dir_impl.rs +++ b/cap-primitives/src/winx/fs/remove_open_dir_impl.rs @@ -3,5 +3,22 @@ use std::{fs, io}; pub(crate) fn remove_open_dir_impl(dir: fs::File) -> io::Result<()> { let path = get_path(&dir)?; + + // Drop the directory before removing it, since we open directories without + // `FILE_SHARE_DELETE`, and removing it requires accessing via its name + // rather than its handle. + // + // There is a window here in which another process could remove or rename + // a directory with this path after the handle is dropped, however it's + // unlikely to happen by accident, and unlikely to cause major problems. + // It may cause spurious failures, or failures with different error codes, + // but this appears to be unaoidable. + // + // Even if we did have `FILE_SHARE_DELETE` and we kept the handle open + // while doing the `remove_dir, `FILE_SHARE_DELETE` would grant other + // processes the right to remove or rename the directory. So there + // doesn't seem to be a race-free way of removing opened directories. + drop(dir); + fs::remove_dir(path) } diff --git a/cap-primitives/src/yanix/fs/remove_open_dir_by_searching.rs b/cap-primitives/src/yanix/fs/remove_open_dir_by_searching.rs index 002192cc..fb7678f5 100644 --- a/cap-primitives/src/yanix/fs/remove_open_dir_by_searching.rs +++ b/cap-primitives/src/yanix/fs/remove_open_dir_by_searching.rs @@ -10,7 +10,6 @@ pub(crate) fn remove_open_dir_by_searching(dir: fs::File) -> io::Result<()> { while let Some(child) = iter.next() { let child = child?; if child.is_same_file(&metadata)? { - drop(dir); return child.remove_dir(); } } diff --git a/cap-std/src/fs/dir.rs b/cap-std/src/fs/dir.rs index 25f99c2c..90b0ad9b 100644 --- a/cap-std/src/fs/dir.rs +++ b/cap-std/src/fs/dir.rs @@ -46,6 +46,9 @@ pub struct Dir { impl Dir { /// Constructs a new instance of `Self` from the given `std::fs::File`. + /// + /// To prevent race conditions on Windows, the file must be opened without + /// `FILE_SHARE_DELETE`. #[inline] pub fn from_std_file(std_file: fs::File) -> Self { Self { std_file } @@ -602,6 +605,8 @@ impl FromRawFd for Dir { #[cfg(windows)] impl FromRawHandle for Dir { + /// To prevent race conditions on Windows, the handle must be opened without + /// `FILE_SHARE_DELETE`. #[inline] unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self::from_std_file(fs::File::from_raw_handle(handle)) diff --git a/cap-std/src/fs_utf8/dir.rs b/cap-std/src/fs_utf8/dir.rs index 150699ae..5428454e 100644 --- a/cap-std/src/fs_utf8/dir.rs +++ b/cap-std/src/fs_utf8/dir.rs @@ -30,6 +30,9 @@ pub struct Dir { impl Dir { /// Constructs a new instance of `Self` from the given `std::fs::File`. + /// + /// To prevent race conditions on Windows, the file must be opened without + /// `FILE_SHARE_DELETE`. #[inline] pub fn from_std_file(std_file: fs::File) -> Self { Self::from_cap_std(crate::fs::Dir::from_std_file(std_file)) @@ -556,6 +559,8 @@ impl FromRawFd for Dir { #[cfg(windows)] impl FromRawHandle for Dir { + /// To prevent race conditions on Windows, the handle must be opened without + /// `FILE_SHARE_DELETE`. #[inline] unsafe fn from_raw_handle(handle: RawHandle) -> Self { Self::from_std_file(fs::File::from_raw_handle(handle)) diff --git a/cap-tempfile/src/lib.rs b/cap-tempfile/src/lib.rs index a3c2755f..4844c7fe 100644 --- a/cap-tempfile/src/lib.rs +++ b/cap-tempfile/src/lib.rs @@ -191,7 +191,7 @@ fn close_outer() { #[cfg(windows)] assert_eq!( t.close().unwrap_err().raw_os_error(), - Some(winapi::shared::winerror::ERROR_DIR_NOT_EMPTY as i32) + Some(winapi::shared::winerror::ERROR_SHARING_VIOLATION as i32) ); #[cfg(not(windows))] t.close().unwrap(); diff --git a/tests/windows-open.rs b/tests/windows-open.rs new file mode 100644 index 00000000..9f333a88 --- /dev/null +++ b/tests/windows-open.rs @@ -0,0 +1,94 @@ +//! On Windows, cap-std uses the technique of looking up absolute paths for +//! directory handles. This would be racy, except that cap-std uses Windows' +//! sharing modes to prevent open directories from being removed or renamed. +//! Test that this works. + +#[cfg(windows)] +#[macro_use] +mod sys_common; + +#[cfg(windows)] +use sys_common::io::tmpdir; + +#[test] +#[cfg(windows)] +fn windows_open_one() { + let tmpdir = tmpdir(); + check!(tmpdir.create_dir("aaa")); + + let dir = tmpdir.open_dir("aaa"); + + // Attempts to remove or rename the open directory should fail. + tmpdir.remove_dir("aaa").unwrap_err(); + tmpdir.rename("aaa", &tmpdir, "zzz").unwrap_err(); + + drop(dir); + + // Now that we've droped the handle, the same operations should succeed. + check!(tmpdir.rename("aaa", &tmpdir, "xxx")); + check!(tmpdir.remove_dir("xxx")); +} + +#[test] +#[cfg(windows)] +fn windows_open_multiple() { + let tmpdir = tmpdir(); + check!(tmpdir.create_dir_all("aaa/bbb")); + + let dir = tmpdir.open_dir("aaa/bbb"); + + // Attempts to remove or rename any component of the open directory should fail. + tmpdir.remove_dir("aaa/bbb").unwrap_err(); + tmpdir.remove_dir("aaa").unwrap_err(); + tmpdir.rename("aaa/bbb", &tmpdir, "aaa/yyy").unwrap_err(); + tmpdir.rename("aaa", &tmpdir, "zzz").unwrap_err(); + + drop(dir); + + // Now that we've droped the handle, the same operations should succeed. + check!(tmpdir.rename("aaa/bbb", &tmpdir, "aaa/www")); + check!(tmpdir.rename("aaa", &tmpdir, "xxx")); + check!(tmpdir.remove_dir("xxx/www")); + check!(tmpdir.remove_dir("xxx")); +} + +/// Like `windows_open_multiple`, but does so within a directory that we +/// can close and then independently mutate. +#[test] +#[cfg(windows)] +fn windows_open_tricky() { + let tmpdir = tmpdir(); + check!(tmpdir.create_dir("qqq")); + + let qqq = check!(tmpdir.open_dir("qqq")); + check!(qqq.create_dir_all("aaa/bbb")); + + let dir = check!(qqq.open_dir("aaa/bbb")); + + // Now drop `qqq`. + drop(qqq); + + // Attempts to remove or rename any component of the open directory should fail. + dir.remove_dir("aaa/bbb").unwrap_err(); + dir.remove_dir("aaa").unwrap_err(); + dir.rename("aaa/bbb", &tmpdir, "aaa/yyy").unwrap_err(); + dir.rename("aaa", &tmpdir, "zzz").unwrap_err(); + tmpdir.remove_dir("qqq/aaa/bbb").unwrap_err(); + tmpdir.remove_dir("qqq/aaa").unwrap_err(); + tmpdir.remove_dir("qqq").unwrap_err(); + tmpdir + .rename("qqq/aaa/bbb", &tmpdir, "qqq/aaa/yyy") + .unwrap_err(); + tmpdir.rename("qqq/aaa", &tmpdir, "qqq/zzz").unwrap_err(); + tmpdir.rename("qqq", &tmpdir, "vvv").unwrap_err(); + + drop(dir); + + // Now that we've droped the handle, the same operations should succeed. + check!(tmpdir.rename("qqq/aaa/bbb", &tmpdir, "qqq/aaa/www")); + check!(tmpdir.rename("qqq/aaa", &tmpdir, "qqq/xxx")); + check!(tmpdir.rename("qqq", &tmpdir, "uuu")); + check!(tmpdir.remove_dir("uuu/xxx/www")); + check!(tmpdir.remove_dir("uuu/xxx")); + check!(tmpdir.remove_dir("uuu")); +}