From aa72a293095303308d575a629a654d53e507f813 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 18 Jul 2024 18:50:10 +1000 Subject: [PATCH] deps: update to snafu v0.8 Most of the changes are from the v0.7 update we missed and were mechanically applied with the snafu migration tool. This switch also requires moving to std::backtrace::Backtrace, which required a few other changes (including removing the global setting for whether we want to capture backtraces). If we really wanted to keep the global setting knob, we could wrap std::backtrace::Backtrace in our own type but I don't think the global setting really made sense in the first place (and now that there are no backtraces in the C FFI, the standard RUST_* environment variables can still be used for Rust programs). Signed-off-by: Aleksa Sarai --- Cargo.toml | 2 +- src/capi/core.rs | 8 ++--- src/error.rs | 61 ++++++------------------------------- src/lib.rs | 1 - src/resolvers/kernel.rs | 4 +-- src/resolvers/user.rs | 18 +++++------ src/root.rs | 14 ++++----- src/syscalls.rs | 66 ++++++++++++++++++++--------------------- src/utils.rs | 10 +++---- 9 files changed, 69 insertions(+), 115 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f0415be6..38774f17 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,4 +48,4 @@ bitflags = "^2" lazy_static = "^1" libc = "^0.2" rand = "^0.8" -snafu = { version = "^0.6", features = ["backtraces-impl-backtrace-crate"] } +snafu = "^0.8" diff --git a/src/capi/core.rs b/src/capi/core.rs index 768af583..c2086fc7 100644 --- a/src/capi/core.rs +++ b/src/capi/core.rs @@ -41,7 +41,7 @@ use snafu::ResultExt; fn parse_path<'a>(path: *const c_char) -> Result<&'a Path, Error> { ensure!( !path.is_null(), - error::InvalidArgument { + error::InvalidArgumentSnafu { name: "path", description: "cannot be NULL", } @@ -105,7 +105,7 @@ pub extern "C" fn pathrs_reopen(fd: RawFd, flags: c_int) -> RawFd { // Rust sets O_CLOEXEC by default, without an opt-out. We need to // disable it if we weren't asked to do O_CLOEXEC. if flags.0 & libc::O_CLOEXEC == 0 { - syscalls::fcntl_unset_cloexec(file.as_raw_fd()).context(error::RawOsError { + syscalls::fcntl_unset_cloexec(file.as_raw_fd()).context(error::RawOsSnafu { operation: "clear O_CLOEXEC on fd", })?; } @@ -248,11 +248,11 @@ pub extern "C" fn pathrs_mknod( libc::S_IFBLK => InodeType::BlockDevice(&perms, dev), libc::S_IFCHR => InodeType::CharacterDevice(&perms, dev), libc::S_IFIFO => InodeType::Fifo(&perms), - libc::S_IFSOCK => error::NotImplemented { + libc::S_IFSOCK => error::NotImplementedSnafu { feature: "mknod(S_IFSOCK)", } .fail()?, - _ => error::InvalidArgument { + _ => error::InvalidArgumentSnafu { name: "mode", description: "invalid S_IFMT mask", } diff --git a/src/error.rs b/src/error.rs index 64ab501b..cac3fa01 100644 --- a/src/error.rs +++ b/src/error.rs @@ -24,58 +24,15 @@ // resolved: // // * https://github.com/shepmaster/snafu/issues/188. -// * `std::error::Backtrace` is stabilised. // * `std::error::Error::chain` is stabilised. // * I figure out a nice way to implement GlobalBacktrace... #[doc(inline)] pub use crate::syscalls::{Error as SyscallError, FrozenFd}; -use std::{ - error::Error as StdError, - io::Error as IOError, - sync::atomic::{AtomicBool, Ordering}, -}; +use std::{backtrace::Backtrace, error::Error as StdError, io::Error as IOError}; -use snafu::{GenerateBacktrace, ResultExt}; - -/// A wrapper around [`backtrace::Backtrace`]. -/// -/// The primary reason for this is that it allows for custom configuration of -/// whether backtraces are generated by libpathrs. You may configure this by -/// modifying [`BACKTRACES_ENABLED`]. -/// -/// # Stability -/// Note that this interface will drastically change once -/// `std::error::Backtrace` is stabilised. -/// -/// [`backtrace::Backtrace`]: https://docs.rs/backtrace/*/backtrace/struct.Backtrace.html -/// [`BACKTRACES_ENABLED`]: static.BACKTRACES_ENABLED.html -// NOTE: Once std's Backtrace is finalised this will need to be changed. -#[derive(Debug)] -pub struct Backtrace(pub Option); - -/// Controls whether backtraces will be generated during error handling within -/// libpathrs. -/// -/// By default, backtraces are disabled for release builds and enabled otherwise. -// TODO: This should probably be a getter+setter setup but I couldn't figure out -// nice names for the getter and setter. -pub static BACKTRACES_ENABLED: AtomicBool = AtomicBool::new(cfg!(debug_assertions)); - -impl GenerateBacktrace for Backtrace { - fn generate() -> Self { - if BACKTRACES_ENABLED.load(Ordering::SeqCst) { - Backtrace(Some(backtrace::Backtrace::new())) - } else { - Backtrace(None) - } - } - - fn as_backtrace(&self) -> Option<&snafu::Backtrace> { - self.0.as_ref() - } -} +use snafu::ResultExt; /// The primary error type returned by libpathrs. /// @@ -90,7 +47,7 @@ impl GenerateBacktrace for Backtrace { /// [`BACKTRACES_ENABLED`]: static.BACKTRACES_ENABLED.html /// [`Error::chain`]: https://doc.rust-lang.org/nightly/std/error/trait.Error.html#method.chain #[derive(Snafu, Debug)] -#[snafu(visibility = "pub(crate)")] +#[snafu(visibility(pub(crate)))] pub enum Error { /// The requested feature is not yet implemented. #[snafu(display("feature '{}' not implemented", feature))] @@ -98,7 +55,7 @@ pub enum Error { /// Feature which is not implemented. feature: String, /// Backtrace captured at time of error. - backtrace: Backtrace, + backtrace: Option, }, /// The requested feature is not supported by this kernel. @@ -107,7 +64,7 @@ pub enum Error { /// Feature which is not supported. feature: String, /// Backtrace captured at time of error. - backtrace: Backtrace, + backtrace: Option, }, /// One of the provided arguments in invalid. @@ -118,7 +75,7 @@ pub enum Error { /// Description of what makes the argument invalid. description: String, /// Backtrace captured at time of error. - backtrace: Backtrace, + backtrace: Option, }, /// libpathrs has detected some form of safety requirement violation. @@ -129,7 +86,7 @@ pub enum Error { /// Description of safety requirement which was violated. description: String, /// Backtrace captured at time of error. - backtrace: Backtrace, + backtrace: Option, }, /// The requested libpathrs operation resulted in an [`IOError`]. This @@ -146,7 +103,7 @@ pub enum Error { /// Underlying error. source: IOError, /// Backtrace captured at time of error. - backtrace: Backtrace, + backtrace: Option, }, /// The requested libpathrs operation resulted in a [`SyscallError`] by @@ -190,7 +147,7 @@ pub(crate) trait ErrorExt { impl ErrorExt for Result { fn wrap>(self, context: S) -> Self { - self.context(Wrapped { + self.context(WrappedSnafu { context: context.into(), }) } diff --git a/src/lib.rs b/src/lib.rs index fee23888..146bea6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -119,7 +119,6 @@ // libpathrs only supports Linux at the moment. #![cfg(target_os = "linux")] -extern crate backtrace; #[macro_use] extern crate bitflags; #[macro_use] diff --git a/src/resolvers/kernel.rs b/src/resolvers/kernel.rs index 0a2c713a..7f7a9392 100644 --- a/src/resolvers/kernel.rs +++ b/src/resolvers/kernel.rs @@ -37,7 +37,7 @@ pub(crate) fn resolve>( path: P, flags: ResolverFlags, ) -> Result { - ensure!(*IS_SUPPORTED, error::NotSupported { feature: "openat2" }); + ensure!(*IS_SUPPORTED, error::NotSupportedSnafu { feature: "openat2" }); let mut how = syscalls::OpenHow::default(); how.flags = libc::O_PATH as u64; @@ -64,7 +64,7 @@ pub(crate) fn resolve>( // TODO: Add wrapper for known-bad openat2 return codes. //Some(libc::EXDEV) | Some(libc::ELOOP) => { ... } _ => { - return Err(err).context(error::RawOsError { + return Err(err).context(error::RawOsSnafu { operation: "openat2 subpath", })? } diff --git a/src/resolvers/user.rs b/src/resolvers/user.rs index 51952c3c..76a4fd1e 100644 --- a/src/resolvers/user.rs +++ b/src/resolvers/user.rs @@ -96,7 +96,7 @@ fn check_current>(current: &File, root: &File, expected: P) -> Re // The paths should be identical. ensure!( current_path == full_path, - error::SafetyViolation { + error::SafetyViolationSnafu { description: "fd doesn't match expected path" } ); @@ -111,7 +111,7 @@ fn check_current>(current: &File, root: &File, expected: P) -> Re .wrap("get root path to double-check it hasn't moved")?; ensure!( root_path == new_root_path, - error::SafetyViolation { + error::SafetyViolationSnafu { description: "root moved during lookup" } ); @@ -168,7 +168,7 @@ pub(crate) fn resolve>( // ever happen, but it's better to be safe. ensure!( !part.as_bytes().contains(&b'/'), - error::SafetyViolation { + error::SafetyViolationSnafu { description: "component of path resolution contains '/'", } ); @@ -187,7 +187,7 @@ pub(crate) fn resolve>( // Get our next element. let next = syscalls::openat(current.as_raw_fd(), part, libc::O_PATH, 0).context( - error::RawOsError { + error::RawOsSnafu { operation: "open next component of resolution", }, )?; @@ -211,7 +211,7 @@ pub(crate) fn resolve>( // NOTE: File::metadata definitely does an fstat(2) here. let next_type = next .metadata() - .context(error::OsError { + .context(error::OsSnafu { operation: "fstat of next component", })? .file_type(); @@ -225,7 +225,7 @@ pub(crate) fn resolve>( // Don't continue walking if user asked for no symlinks. if flags.contains(ResolverFlags::NO_SYMLINKS) { - return error::SafetyViolation { + return error::SafetyViolationSnafu { description: "next is a symlink and symlink resolution disabled", } .fail(); @@ -238,7 +238,7 @@ pub(crate) fn resolve>( .is_dangerous() .wrap("check if next is on a dangerous filesystem")? { - return error::SafetyViolation { + return error::SafetyViolationSnafu { description: "next is a symlink on a dangerous filesystem", } .fail(); @@ -248,7 +248,7 @@ pub(crate) fn resolve>( // hitting filesystem loops and DoSing. symlink_traversals += 1; if symlink_traversals >= MAX_SYMLINK_TRAVERSALS { - return Err(IOError::from_raw_os_error(libc::ELOOP)).context(error::OsError { + return Err(IOError::from_raw_os_error(libc::ELOOP)).context(error::OsSnafu { operation: "emulated symlink resolution", })?; } @@ -258,7 +258,7 @@ pub(crate) fn resolve>( // path to the symlink. However, since readlink(2) doesn't follow // symlink components we can just do it manually safely. let contents = - syscalls::readlinkat(current.as_raw_fd(), part).context(error::RawOsError { + syscalls::readlinkat(current.as_raw_fd(), part).context(error::RawOsSnafu { operation: "readlink next symlink component", })?; diff --git a/src/root.rs b/src/root.rs index 85386bf6..18a48b7b 100644 --- a/src/root.rs +++ b/src/root.rs @@ -103,7 +103,7 @@ fn path_split(path: &'_ Path) -> Result<(&'_ Path, &'_ Path), Error> { let parent = path.parent().unwrap_or_else(|| "/".as_ref()); // Now construct the trailing portion of the target. - let name = path.file_name().context(error::InvalidArgument { + let name = path.file_name().context(error::InvalidArgumentSnafu { name: "path", description: "no trailing component", })?; @@ -112,7 +112,7 @@ fn path_split(path: &'_ Path) -> Result<(&'_ Path, &'_ Path), Error> { // If there are any other path components we must bail. ensure!( !name.as_bytes().contains(&b'/'), - error::SafetyViolation { + error::SafetyViolationSnafu { description: "trailing component of split pathname contains '/'", } ); @@ -201,7 +201,7 @@ impl Root { /// [`Resolver`]: struct.Resolver.html pub fn open>(path: P) -> Result { let file = syscalls::openat(libc::AT_FDCWD, path, libc::O_PATH | libc::O_DIRECTORY, 0) - .context(error::RawOsError { + .context(error::RawOsSnafu { operation: "open root handle", })?; Ok(Root::from_file_unchecked(file)) @@ -336,7 +336,7 @@ impl Root { syscalls::mknodat(dirfd, name, libc::S_IFBLK | mode, *dev) } } - .context(error::RawOsError { + .context(error::RawOsSnafu { operation: "pathrs create", }) } @@ -390,7 +390,7 @@ impl Root { // can't be done with the emulated backend that might be a bad idea. let flags = flags.0 | libc::O_CREAT; let file = - syscalls::openat(dirfd, name, flags, perm.mode()).context(error::RawOsError { + syscalls::openat(dirfd, name, flags, perm.mode()).context(error::RawOsSnafu { operation: "pathrs create_file", })?; @@ -458,7 +458,7 @@ impl Root { // If we ever are here, then last_error must be Some. Err(last_error.expect("unlinkat loop failed so last_error must exist")).context( - error::RawOsError { + error::RawOsSnafu { operation: "pathrs remove", }, ) @@ -497,7 +497,7 @@ impl Root { let dst_dirfd = dst_dir.as_raw_fd(); syscalls::renameat2(src_dirfd, src_name, dst_dirfd, dst_name, flags.0).context( - error::RawOsError { + error::RawOsSnafu { operation: "pathrs rename", }, ) diff --git a/src/syscalls.rs b/src/syscalls.rs index 039816d0..20483a0a 100644 --- a/src/syscalls.rs +++ b/src/syscalls.rs @@ -19,12 +19,10 @@ // We need to permit unsafe code because we are interacting with libc APIs. #![allow(unsafe_code)] -use crate::{ - error::Backtrace, - utils::{RawFdExt, ToCString}, -}; +use crate::utils::{RawFdExt, ToCString}; use std::{ + backtrace::Backtrace, ffi::OsStr, fmt, fs::File, @@ -89,14 +87,14 @@ pub enum Error { FcntlDup { fd: FrozenFd, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("fcntl({}, F_GETFD)", fd))] FcntlGetFlags { fd: FrozenFd, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("fcntl({}, F_SETFD, 0x{:x})", fd, flags))] @@ -104,7 +102,7 @@ pub enum Error { fd: FrozenFd, flags: i32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("openat({}, {:?}, 0x{:x}, 0o{:o})", dirfd, path, flags, mode))] @@ -114,7 +112,7 @@ pub enum Error { flags: i32, mode: u32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("openat2({}, {:?}, {}, {})", dirfd, path, how, size))] @@ -124,7 +122,7 @@ pub enum Error { how: OpenHow, size: usize, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("readlinkat({}, {:?})", dirfd, path))] @@ -132,7 +130,7 @@ pub enum Error { dirfd: FrozenFd, path: PathBuf, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("mkdirat({}, {:?}, 0o{:o})", dirfd, path, mode))] @@ -141,7 +139,7 @@ pub enum Error { path: PathBuf, mode: u32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("mknodat({}, {:?}, 0o{:o}, {}:{})", dirfd, path, mode, major, minor))] @@ -152,7 +150,7 @@ pub enum Error { major: u32, minor: u32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("unlinkat({}, {:?}, 0x{:x})", dirfd, path, flags))] @@ -161,7 +159,7 @@ pub enum Error { path: PathBuf, flags: i32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display( @@ -179,7 +177,7 @@ pub enum Error { newpath: PathBuf, flags: i32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("symlinkat({:?}, {}, {:?})", target, dirfd, path))] @@ -188,7 +186,7 @@ pub enum Error { path: PathBuf, target: PathBuf, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("renameat({}, {:?}, {}, {:?})", olddirfd, oldpath, newdirfd, newpath))] @@ -198,7 +196,7 @@ pub enum Error { newdirfd: FrozenFd, newpath: PathBuf, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display( @@ -216,14 +214,14 @@ pub enum Error { newpath: PathBuf, flags: u32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("fstatfs({})", fd))] Fstatfs { fd: FrozenFd, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, #[snafu(display("fstatat({}, {:?}, 0x{:x})", dirfd, path, flags))] @@ -232,7 +230,7 @@ pub enum Error { path: PathBuf, flags: i32, source: IOError, - backtrace: Backtrace, + backtrace: Option, }, } @@ -280,7 +278,7 @@ pub(crate) fn fcntl_dupfd_cloxec(fd: RawFd) -> Result { // SAFETY: We know it's a real file descriptor. Ok(unsafe { File::from_raw_fd(newfd) }) } else { - Err(err).context(FcntlDup { fd }) + Err(err).context(FcntlDupSnafu { fd }) } } @@ -296,7 +294,7 @@ pub(crate) fn fcntl_unset_cloexec(fd: RawFd) -> Result<(), Error> { let err = IOError::last_os_error(); if old < 0 { - return Err(err).context(FcntlGetFlags { fd }); + return Err(err).context(FcntlGetFlagsSnafu { fd }); } let new = old & !libc::FD_CLOEXEC; @@ -311,7 +309,7 @@ pub(crate) fn fcntl_unset_cloexec(fd: RawFd) -> Result<(), Error> { if ret >= 0 { Ok(()) } else { - Err(err).context(FcntlSetFlags { fd, flags: new }) + Err(err).context(FcntlSetFlagsSnafu { fd, flags: new }) } } @@ -336,7 +334,7 @@ pub(crate) fn openat_follow>( // SAFETY: We know it's a real file descriptor. Ok(unsafe { File::from_raw_fd(fd) }) } else { - Err(err).context(Openat { + Err(err).context(OpenatSnafu { dirfd, path, flags, @@ -385,7 +383,7 @@ pub(crate) fn readlinkat>(dirfd: RawFd, path: P) -> Result>(dirfd: RawFd, path: P, mode: mode_t) -> Re if ret >= 0 { Ok(()) } else { - Err(err).context(Mkdirat { dirfd, path, mode }) + Err(err).context(MkdiratSnafu { dirfd, path, mode }) } } @@ -427,7 +425,7 @@ pub(crate) fn mknodat>( if ret >= 0 { Ok(()) } else { - Err(err).context(Mknodat { + Err(err).context(MknodatSnafu { dirfd, path, mode, @@ -452,7 +450,7 @@ pub(crate) fn unlinkat>(dirfd: RawFd, path: P, flags: c_int) -> R if ret >= 0 { Ok(()) } else { - Err(err).context(Unlinkat { dirfd, path, flags }) + Err(err).context(UnlinkatSnafu { dirfd, path, flags }) } } @@ -483,7 +481,7 @@ pub(crate) fn linkat>( if ret >= 0 { Ok(()) } else { - Err(err).context(Linkat { + Err(err).context(LinkatSnafu { olddirfd, oldpath, newdirfd, @@ -513,7 +511,7 @@ pub(crate) fn symlinkat>(target: P, dirfd: RawFd, path: P) -> Res if ret >= 0 { Ok(()) } else { - Err(err).context(Symlinkat { + Err(err).context(SymlinkatSnafu { dirfd, path, target, @@ -546,7 +544,7 @@ pub(crate) fn renameat>( if ret >= 0 { Ok(()) } else { - Err(err).context(Renameat { + Err(err).context(RenameatSnafu { olddirfd, oldpath, newdirfd, @@ -604,7 +602,7 @@ pub(crate) fn renameat2>( // Fall back to renameat(2) if possible. return renameat(olddirfd, oldpath, newdirfd, newpath); } - Err(err).context(Renameat2 { + Err(err).context(Renameat2Snafu { olddirfd, oldpath, newdirfd, @@ -628,7 +626,7 @@ pub(crate) fn fstatfs(fd: RawFd) -> Result { if ret >= 0 { Ok(buf) } else { - Err(err).context(Fstatfs { fd }) + Err(err).context(FstatfsSnafu { fd }) } } @@ -657,7 +655,7 @@ pub(crate) fn fstatat>(dirfd: RawFd, path: P) -> Result= 0 { Ok(buf) } else { - Err(err).context(Fstatat { dirfd, path, flags }) + Err(err).context(FstatatSnafu { dirfd, path, flags }) } } @@ -710,7 +708,7 @@ pub fn openat2>(dirfd: RawFd, path: P, how: &OpenHow) -> Result Result { } else if fd.is_positive() { Ok(format!("self/fd/{}", fd)) } else { - error::InvalidArgument { + error::InvalidArgumentSnafu { name: "fd", description: "must be positive or AT_FDCWD", } @@ -182,21 +182,21 @@ impl RawFdExt for RawFd { // avoid the /proc dependency -- though then again, as_unsafe_path // necessarily requires /proc. syscalls::openat_follow(PROCFS_HANDLE.as_raw_fd(), proc_subpath(*self)?, flags.0, 0) - .context(error::RawOsError { + .context(error::RawOsSnafu { operation: "reopen fd through procfs", }) } fn as_unsafe_path(&self) -> Result { syscalls::readlinkat(PROCFS_HANDLE.as_raw_fd(), proc_subpath(*self)?).context( - error::RawOsError { + error::RawOsSnafu { operation: "get fd's path through procfs", }, ) } fn try_clone_hotfix(&self) -> Result { - syscalls::fcntl_dupfd_cloxec(*self).context(error::RawOsError { + syscalls::fcntl_dupfd_cloxec(*self).context(error::RawOsSnafu { operation: "clone fd", }) } @@ -247,7 +247,7 @@ impl FileExt for File { // nd_jump_link() is used internally. So, we just have to make an // educated guess based on which mainline filesystems expose // magic-links. - let stat = syscalls::fstatfs(self.as_raw_fd()).context(error::RawOsError { + let stat = syscalls::fstatfs(self.as_raw_fd()).context(error::RawOsSnafu { operation: "check fstype of fd", })?; Ok(DANGEROUS_FILESYSTEMS.contains(&stat.f_type))