From 13070137aad6b22e018c3f41de7211be0925fb29 Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 18:54:24 -0700 Subject: [PATCH 1/9] Return file creation times on Linux (using statx) Rust's standard library returns file creation times on Linux (at least with glibc), and now rust-std will too. File creation times are available on Linux via the `statx` syscall (not `fstatat`), introduced in kernel 4.11, which was released in April 2017. The Rust standard library already uses `statx` on Linux with glibc. Making cap-std support file creation times on Linux required two changes: 1. `File::metadata()` uses the standard library (which uses `statx`), but this wouldn't ever copy over the created field on Linux. Now, any time the created field is set in the std Metadata struct, it's also set in the cap-primitives Metadata, regardless of platform. 2. `stat_unchecked` is used in several places, including fetching DirEntry metadata. Before, it called `fstatat` directly. Now, it calls `statx` on Linux when available, and it falls back to `fstatat` otherwise. Fortunately, Dan Gohman (@sunfishcode) had already added a method to convert `statx` results in commit d1fa735 (PR #105) in 2020. This commit also adds a new test to make sure file creation times are set in cap-std Metadata if they are set in std Metadata. --- cap-primitives/src/fs/metadata.rs | 22 -------- cap-primitives/src/rustix/fs/metadata_ext.rs | 4 +- .../src/rustix/fs/stat_unchecked.rs | 29 +++++++++++ tests/metadata-ext.rs | 52 +++++++++++++++++++ 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/cap-primitives/src/fs/metadata.rs b/cap-primitives/src/fs/metadata.rs index 1bc7b457..e8703702 100644 --- a/cap-primitives/src/fs/metadata.rs +++ b/cap-primitives/src/fs/metadata.rs @@ -48,35 +48,13 @@ impl Metadata { #[inline] fn from_parts(std: fs::Metadata, ext: MetadataExt, file_type: FileType) -> Self { - // TODO: Initialize `created` on Linux with `std.created().ok()` once we - // make use of `statx`. Self { file_type, len: std.len(), permissions: Permissions::from_std(std.permissions()), modified: std.modified().ok().map(SystemTime::from_std), accessed: std.accessed().ok().map(SystemTime::from_std), - - #[cfg(any( - target_os = "freebsd", - target_os = "openbsd", - target_os = "macos", - target_os = "ios", - target_os = "netbsd", - windows, - ))] created: std.created().ok().map(SystemTime::from_std), - - #[cfg(not(any( - target_os = "freebsd", - target_os = "openbsd", - target_os = "macos", - target_os = "ios", - target_os = "netbsd", - windows, - )))] - created: None, - ext, } } diff --git a/cap-primitives/src/rustix/fs/metadata_ext.rs b/cap-primitives/src/rustix/fs/metadata_ext.rs index 6087121a..20507f27 100644 --- a/cap-primitives/src/rustix/fs/metadata_ext.rs +++ b/cap-primitives/src/rustix/fs/metadata_ext.rs @@ -3,6 +3,9 @@ use crate::fs::PermissionsExt; use crate::fs::{FileTypeExt, Metadata}; use crate::time::{Duration, SystemClock, SystemTime}; +// TODO: update all these to +// #[cfg(any(target_os = "android", target_os = "linux"))] +// once we're on restix >= v0.34.3. #[cfg(all(target_os = "linux", target_env = "gnu"))] use rustix::fs::{makedev, Statx}; use rustix::fs::{RawMode, Stat}; @@ -222,7 +225,6 @@ impl MetadataExt { /// Constructs a new instance of `Metadata` from the given `Statx`. #[cfg(all(target_os = "linux", target_env = "gnu"))] #[inline] - #[allow(dead_code)] // TODO: use `statx` when possible. pub(crate) fn from_rustix_statx(statx: Statx) -> Metadata { Metadata { file_type: FileTypeExt::from_raw_mode(RawMode::from(statx.stx_mode)), diff --git a/cap-primitives/src/rustix/fs/stat_unchecked.rs b/cap-primitives/src/rustix/fs/stat_unchecked.rs index 1dacc4de..25bb9812 100644 --- a/cap-primitives/src/rustix/fs/stat_unchecked.rs +++ b/cap-primitives/src/rustix/fs/stat_unchecked.rs @@ -3,6 +3,14 @@ use rustix::fs::{statat, AtFlags}; use std::path::Path; use std::{fs, io}; +// TODO: update all these to +// #[cfg(any(target_os = "android", target_os = "linux"))] +// once we're on restix >= v0.34.3. +#[cfg(all(target_os = "linux", target_env = "gnu"))] +use rustix::fs::{statx, StatxFlags}; +#[cfg(all(target_os = "linux", target_env = "gnu"))] +use std::sync::atomic::{AtomicBool, Ordering}; + /// *Unsandboxed* function similar to `stat`, but which does not perform /// sandboxing. pub(crate) fn stat_unchecked( @@ -15,5 +23,26 @@ pub(crate) fn stat_unchecked( FollowSymlinks::No => AtFlags::SYMLINK_NOFOLLOW, }; + // stax is preferred on Linux because it can return creation times. Linux + // kernels prior to 4.11 don't have statx and return ENOSYS. We store the + // availability in a global to avoid unnecessary syscalls. + #[cfg(all(target_os = "linux", target_env = "gnu"))] + static HAS_STATX: AtomicBool = AtomicBool::new(true); + + #[cfg(all(target_os = "linux", target_env = "gnu"))] + if HAS_STATX.load(Ordering::Relaxed) { + let statx_result = statx( + start, + path, + atflags, + StatxFlags::BASIC_STATS | StatxFlags::BTIME, + ); + if let Err(rustix::io::Error::NOSYS) = statx_result { + HAS_STATX.store(false, Ordering::Relaxed); + } else { + return Ok(statx_result.map(MetadataExt::from_rustix_statx)?); + } + } + Ok(statat(start, path, atflags).map(MetadataExt::from_rustix)?) } diff --git a/tests/metadata-ext.rs b/tests/metadata-ext.rs index 0aad8cc5..6d487358 100644 --- a/tests/metadata-ext.rs +++ b/tests/metadata-ext.rs @@ -83,3 +83,55 @@ fn test_metadata_ext() { ); } } + +#[test] +fn test_metadata_ext_created() { + let tmpdir = tmpdir(); + let a = check!(tmpdir.create("a")); + let a_metadata = check!(a.metadata()); + + let modified = check!(a_metadata.modified()); + let tolerance = std::time::Duration::from_secs(10); + let expected = (modified - tolerance)..(modified + tolerance); + + // If the standard library supports file creation times, then cap-std + // should too. + let std_supports_created = matches!( + a.into_std().metadata(), + Ok(m) if m.created().is_ok(), + ); + + if std_supports_created { + let created = check!(a_metadata.created()); + assert!( + expected.contains(&created), + "expected File creation time near {:#?} but got {:#?}", + modified, + created, + ); + + let tmpdir_metadata = check!(tmpdir.dir_metadata()); + let created = check!(tmpdir_metadata.created()); + assert!( + expected.contains(&created), + "expected Dir creation time near {:#?} but got {:#?}", + modified, + created, + ); + + let mut entries = check!(tmpdir.entries()); + if let Some(a) = entries.next() { + let a = check!(a); + assert_eq!(a.file_name(), "a"); + let metadata = check!(a.metadata()); + let created = check!(metadata.created()); + assert!( + expected.contains(&created), + "expected DirEntry creation time near {:#?} but got {:#?}", + modified, + created, + ); + } + assert!(entries.next().is_none(), "unexpected dir entry"); + } +} From 0b7dc95907482672bb3697a7c2bed767d72043cc Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 22:16:02 -0700 Subject: [PATCH 2/9] stat_unchecked: Add backticks to comments from PR suggestions Co-authored-by: Dan Gohman --- cap-primitives/src/rustix/fs/stat_unchecked.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cap-primitives/src/rustix/fs/stat_unchecked.rs b/cap-primitives/src/rustix/fs/stat_unchecked.rs index 25bb9812..ce96d524 100644 --- a/cap-primitives/src/rustix/fs/stat_unchecked.rs +++ b/cap-primitives/src/rustix/fs/stat_unchecked.rs @@ -23,8 +23,8 @@ pub(crate) fn stat_unchecked( FollowSymlinks::No => AtFlags::SYMLINK_NOFOLLOW, }; - // stax is preferred on Linux because it can return creation times. Linux - // kernels prior to 4.11 don't have statx and return ENOSYS. We store the + // `statx` is preferred on Linux because it can return creation times. Linux + // kernels prior to 4.11 don't have `statx` and return `ENOSYS`. We store the // availability in a global to avoid unnecessary syscalls. #[cfg(all(target_os = "linux", target_env = "gnu"))] static HAS_STATX: AtomicBool = AtomicBool::new(true); From 26270cd5a062f08b757a0d107d201ecb22954d02 Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 22:17:53 -0700 Subject: [PATCH 3/9] stat_unchecked: Reflow comment --- cap-primitives/src/rustix/fs/stat_unchecked.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cap-primitives/src/rustix/fs/stat_unchecked.rs b/cap-primitives/src/rustix/fs/stat_unchecked.rs index ce96d524..b57b32c7 100644 --- a/cap-primitives/src/rustix/fs/stat_unchecked.rs +++ b/cap-primitives/src/rustix/fs/stat_unchecked.rs @@ -23,9 +23,9 @@ pub(crate) fn stat_unchecked( FollowSymlinks::No => AtFlags::SYMLINK_NOFOLLOW, }; - // `statx` is preferred on Linux because it can return creation times. Linux - // kernels prior to 4.11 don't have `statx` and return `ENOSYS`. We store the - // availability in a global to avoid unnecessary syscalls. + // `statx` is preferred on Linux because it can return creation times. + // Linux kernels prior to 4.11 don't have `statx` and return `ENOSYS`. We + // store the availability in a global to avoid unnecessary syscalls. #[cfg(all(target_os = "linux", target_env = "gnu"))] static HAS_STATX: AtomicBool = AtomicBool::new(true); From c7f65a26156d88542bff627f313ca6899ef2f281 Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 22:22:13 -0700 Subject: [PATCH 4/9] stat_unchecked: Rewrite match statement for clarity --- cap-primitives/src/rustix/fs/stat_unchecked.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cap-primitives/src/rustix/fs/stat_unchecked.rs b/cap-primitives/src/rustix/fs/stat_unchecked.rs index b57b32c7..1a3b270e 100644 --- a/cap-primitives/src/rustix/fs/stat_unchecked.rs +++ b/cap-primitives/src/rustix/fs/stat_unchecked.rs @@ -37,10 +37,10 @@ pub(crate) fn stat_unchecked( atflags, StatxFlags::BASIC_STATS | StatxFlags::BTIME, ); - if let Err(rustix::io::Error::NOSYS) = statx_result { - HAS_STATX.store(false, Ordering::Relaxed); - } else { - return Ok(statx_result.map(MetadataExt::from_rustix_statx)?); + match statx_result { + Ok(statx) => return Ok(MetadataExt::from_rustix_statx(statx)), + Err(rustix::io::Error::NOSYS) => HAS_STATX.store(false, Ordering::Relaxed), + Err(e) => return Err(e.into()), } } From 6112c8875ab7eb95d58589081c5b16ed1974de81 Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 23:16:15 -0700 Subject: [PATCH 5/9] Rewrite file created times test to do exact comparisons against std --- tests/metadata-ext.rs | 66 ++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 42 deletions(-) diff --git a/tests/metadata-ext.rs b/tests/metadata-ext.rs index 6d487358..47c4ec10 100644 --- a/tests/metadata-ext.rs +++ b/tests/metadata-ext.rs @@ -87,51 +87,33 @@ fn test_metadata_ext() { #[test] fn test_metadata_ext_created() { let tmpdir = tmpdir(); - let a = check!(tmpdir.create("a")); - let a_metadata = check!(a.metadata()); + check!(tmpdir.create_dir("dir")); + let dir = check!(tmpdir.open_dir("dir")); + let file = check!(dir.create("file")); + + let cap_std_dir = check!(dir.dir_metadata()); + let cap_std_file = check!(file.metadata()); + let cap_std_dir_entry = { + let mut entries = check!(dir.entries()); + let entry = check!(entries.next().unwrap()); + assert_eq!(entry.file_name(), "file"); + assert!(entries.next().is_none(), "unexpected dir entry"); + check!(entry.metadata()) + }; - let modified = check!(a_metadata.modified()); - let tolerance = std::time::Duration::from_secs(10); - let expected = (modified - tolerance)..(modified + tolerance); + let std_dir = check!(dir.into_std_file().metadata()); + let std_file = check!(file.into_std().metadata()); // If the standard library supports file creation times, then cap-std // should too. - let std_supports_created = matches!( - a.into_std().metadata(), - Ok(m) if m.created().is_ok(), - ); - - if std_supports_created { - let created = check!(a_metadata.created()); - assert!( - expected.contains(&created), - "expected File creation time near {:#?} but got {:#?}", - modified, - created, - ); - - let tmpdir_metadata = check!(tmpdir.dir_metadata()); - let created = check!(tmpdir_metadata.created()); - assert!( - expected.contains(&created), - "expected Dir creation time near {:#?} but got {:#?}", - modified, - created, - ); - - let mut entries = check!(tmpdir.entries()); - if let Some(a) = entries.next() { - let a = check!(a); - assert_eq!(a.file_name(), "a"); - let metadata = check!(a.metadata()); - let created = check!(metadata.created()); - assert!( - expected.contains(&created), - "expected DirEntry creation time near {:#?} but got {:#?}", - modified, - created, - ); - } - assert!(entries.next().is_none(), "unexpected dir entry"); + if let Ok(expected) = std_dir.created() { + println!("std::fs supports file created times"); + assert_eq!(expected, check!(cap_std_dir.created()).into_std()); + } else { + println!("std::fs doesn't support file created times"); + } + if let Ok(expected) = std_file.created() { + assert_eq!(expected, check!(cap_std_file.created()).into_std()); + assert_eq!(expected, check!(cap_std_dir_entry.created()).into_std()); } } From 29cd8dd96e46e49e2857045bb5cd850286da373c Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Thu, 2 Jun 2022 23:20:04 -0700 Subject: [PATCH 6/9] Move file created times test into fs_additional --- tests/fs_additional.rs | 34 ++++++++++++++++++++++++++++++++++ tests/metadata-ext.rs | 34 ---------------------------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/tests/fs_additional.rs b/tests/fs_additional.rs index c8865810..5d68e82f 100644 --- a/tests/fs_additional.rs +++ b/tests/fs_additional.rs @@ -872,3 +872,37 @@ fn reopen_fd() { let tmpdir2 = check!(cap_std::fs::Dir::reopen_dir(&tmpdir.as_filelike())); assert!(tmpdir2.exists("subdir")); } + +#[test] +fn metadata_created() { + let tmpdir = tmpdir(); + check!(tmpdir.create_dir("dir")); + let dir = check!(tmpdir.open_dir("dir")); + let file = check!(dir.create("file")); + + let cap_std_dir = check!(dir.dir_metadata()); + let cap_std_file = check!(file.metadata()); + let cap_std_dir_entry = { + let mut entries = check!(dir.entries()); + let entry = check!(entries.next().unwrap()); + assert_eq!(entry.file_name(), "file"); + assert!(entries.next().is_none(), "unexpected dir entry"); + check!(entry.metadata()) + }; + + let std_dir = check!(dir.into_std_file().metadata()); + let std_file = check!(file.into_std().metadata()); + + // If the standard library supports file creation times, then cap-std + // should too. + if let Ok(expected) = std_dir.created() { + println!("std::fs supports file created times"); + assert_eq!(expected, check!(cap_std_dir.created()).into_std()); + } else { + println!("std::fs doesn't support file created times"); + } + if let Ok(expected) = std_file.created() { + assert_eq!(expected, check!(cap_std_file.created()).into_std()); + assert_eq!(expected, check!(cap_std_dir_entry.created()).into_std()); + } +} diff --git a/tests/metadata-ext.rs b/tests/metadata-ext.rs index 47c4ec10..0aad8cc5 100644 --- a/tests/metadata-ext.rs +++ b/tests/metadata-ext.rs @@ -83,37 +83,3 @@ fn test_metadata_ext() { ); } } - -#[test] -fn test_metadata_ext_created() { - let tmpdir = tmpdir(); - check!(tmpdir.create_dir("dir")); - let dir = check!(tmpdir.open_dir("dir")); - let file = check!(dir.create("file")); - - let cap_std_dir = check!(dir.dir_metadata()); - let cap_std_file = check!(file.metadata()); - let cap_std_dir_entry = { - let mut entries = check!(dir.entries()); - let entry = check!(entries.next().unwrap()); - assert_eq!(entry.file_name(), "file"); - assert!(entries.next().is_none(), "unexpected dir entry"); - check!(entry.metadata()) - }; - - let std_dir = check!(dir.into_std_file().metadata()); - let std_file = check!(file.into_std().metadata()); - - // If the standard library supports file creation times, then cap-std - // should too. - if let Ok(expected) = std_dir.created() { - println!("std::fs supports file created times"); - assert_eq!(expected, check!(cap_std_dir.created()).into_std()); - } else { - println!("std::fs doesn't support file created times"); - } - if let Ok(expected) = std_file.created() { - assert_eq!(expected, check!(cap_std_file.created()).into_std()); - assert_eq!(expected, check!(cap_std_dir_entry.created()).into_std()); - } -} From 2eca023ac81313241b6cbea36bdd286b5eb1a12f Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Fri, 3 Jun 2022 14:11:56 -0700 Subject: [PATCH 7/9] Expand file created times test to check entire Metadata --- tests/fs_additional.rs | 99 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 88 insertions(+), 11 deletions(-) diff --git a/tests/fs_additional.rs b/tests/fs_additional.rs index 5d68e82f..90e204a1 100644 --- a/tests/fs_additional.rs +++ b/tests/fs_additional.rs @@ -874,7 +874,7 @@ fn reopen_fd() { } #[test] -fn metadata_created() { +fn metadata_vs_std_fs() { let tmpdir = tmpdir(); check!(tmpdir.create_dir("dir")); let dir = check!(tmpdir.open_dir("dir")); @@ -893,16 +893,93 @@ fn metadata_created() { let std_dir = check!(dir.into_std_file().metadata()); let std_file = check!(file.into_std().metadata()); - // If the standard library supports file creation times, then cap-std - // should too. - if let Ok(expected) = std_dir.created() { - println!("std::fs supports file created times"); - assert_eq!(expected, check!(cap_std_dir.created()).into_std()); - } else { - println!("std::fs doesn't support file created times"); + match std_dir.created() { + Ok(_) => println!("std::fs supports file created times"), + Err(e) => println!("std::fs doesn't support file created times: {}", e), + } + + check_metadata(&std_dir, &cap_std_dir); + check_metadata(&std_file, &cap_std_file); + check_metadata(&std_file, &cap_std_dir_entry); +} + +fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) { + assert_eq!(std.is_dir(), cap.is_dir()); + assert_eq!(std.is_file(), cap.is_file()); + assert_eq!(std.is_symlink(), cap.is_symlink()); + assert_eq!(std.file_type().is_dir(), cap.file_type().is_dir()); + assert_eq!(std.file_type().is_file(), cap.file_type().is_file()); + assert_eq!(std.file_type().is_symlink(), cap.file_type().is_symlink()); + if cfg!(unix) { + use std::os::unix::fs::FileTypeExt; + assert_eq!( + std.file_type().is_block_device(), + cap.file_type().is_block_device() + ); + assert_eq!( + std.file_type().is_char_device(), + cap.file_type().is_char_device() + ); + assert_eq!(std.file_type().is_fifo(), cap.file_type().is_fifo()); + assert_eq!(std.file_type().is_socket(), cap.file_type().is_socket()); + } + + assert_eq!(std.len(), cap.len()); + + assert_eq!(std.permissions().readonly(), cap.permissions().readonly()); + if cfg!(unix) { + use std::os::unix::fs::PermissionsExt; + // The standard library returns file format bits with `mode()`, whereas + // cap-std currently doesn't. + assert_eq!(std.permissions().mode() & 0o7777, cap.permissions().mode()); } - if let Ok(expected) = std_file.created() { - assert_eq!(expected, check!(cap_std_file.created()).into_std()); - assert_eq!(expected, check!(cap_std_dir_entry.created()).into_std()); + + // If the standard library supports file modified/accessed/created times, + // then cap-std should too. + if let Ok(expected) = std.modified() { + assert_eq!(expected, check!(cap.modified()).into_std()); + } + // The access times might be a little different due to either our own + // or concurrent accesses. + const ACCESS_TOLERANCE_SEC: u32 = 60; + if let Ok(expected) = std.accessed() { + let access_tolerance = std::time::Duration::from_secs(ACCESS_TOLERANCE_SEC.into()); + assert!( + ((expected - access_tolerance)..(expected + access_tolerance)) + .contains(&check!(cap.accessed()).into_std()), + "std accessed {:#?}, cap accessed {:#?}", + expected, + cap.accessed() + ); + } + if let Ok(expected) = std.created() { + assert_eq!(expected, check!(cap.created()).into_std()); + } + + if cfg!(unix) { + use std::os::unix::fs::MetadataExt; + assert_eq!(std.dev(), cap.dev()); + assert_eq!(std.ino(), cap.ino()); + assert_eq!(std.mode(), cap.mode()); + assert_eq!(std.nlink(), cap.nlink()); + assert_eq!(std.uid(), cap.uid()); + assert_eq!(std.gid(), cap.gid()); + assert_eq!(std.rdev(), cap.rdev()); + assert_eq!(std.size(), cap.size()); + assert!( + ((std.atime() - i64::from(ACCESS_TOLERANCE_SEC)) + ..(std.atime() + i64::from(ACCESS_TOLERANCE_SEC))) + .contains(&cap.atime()), + "std atime {}, cap atime {}", + std.atime(), + cap.atime() + ); + assert!((0..1_000_000_000).contains(&cap.atime_nsec())); + assert_eq!(std.mtime(), cap.mtime()); + assert_eq!(std.mtime_nsec(), cap.mtime_nsec()); + assert_eq!(std.ctime(), cap.ctime()); + assert_eq!(std.ctime_nsec(), cap.ctime_nsec()); + assert_eq!(std.blksize(), cap.blksize()); + assert_eq!(std.blocks(), cap.blocks()); } } From 5aa6a7763d330532f195374a83e922b60da5a906 Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Fri, 3 Jun 2022 16:44:23 -0700 Subject: [PATCH 8/9] stat_unchecked: Handle EPERM errors from statx --- .../src/rustix/fs/stat_unchecked.rs | 62 ++++++++++++++----- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/cap-primitives/src/rustix/fs/stat_unchecked.rs b/cap-primitives/src/rustix/fs/stat_unchecked.rs index 1a3b270e..efaf1373 100644 --- a/cap-primitives/src/rustix/fs/stat_unchecked.rs +++ b/cap-primitives/src/rustix/fs/stat_unchecked.rs @@ -9,7 +9,7 @@ use std::{fs, io}; #[cfg(all(target_os = "linux", target_env = "gnu"))] use rustix::fs::{statx, StatxFlags}; #[cfg(all(target_os = "linux", target_env = "gnu"))] -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicU8, Ordering}; /// *Unsandboxed* function similar to `stat`, but which does not perform /// sandboxing. @@ -24,23 +24,51 @@ pub(crate) fn stat_unchecked( }; // `statx` is preferred on Linux because it can return creation times. - // Linux kernels prior to 4.11 don't have `statx` and return `ENOSYS`. We - // store the availability in a global to avoid unnecessary syscalls. + // Linux kernels prior to 4.11 don't have `statx` and return `ENOSYS`. + // Older versions of Docker/seccomp would return `EPERM` for `statx`; see + // . We store the + // availability in a global to avoid unnecessary syscalls. #[cfg(all(target_os = "linux", target_env = "gnu"))] - static HAS_STATX: AtomicBool = AtomicBool::new(true); - - #[cfg(all(target_os = "linux", target_env = "gnu"))] - if HAS_STATX.load(Ordering::Relaxed) { - let statx_result = statx( - start, - path, - atflags, - StatxFlags::BASIC_STATS | StatxFlags::BTIME, - ); - match statx_result { - Ok(statx) => return Ok(MetadataExt::from_rustix_statx(statx)), - Err(rustix::io::Error::NOSYS) => HAS_STATX.store(false, Ordering::Relaxed), - Err(e) => return Err(e.into()), + { + // 0: Unknown + // 1: Not available + // 2: Available + static STATX_STATE: AtomicU8 = AtomicU8::new(0); + let state = STATX_STATE.load(Ordering::Relaxed); + if state != 1 { + let statx_result = statx( + start, + path, + atflags, + StatxFlags::BASIC_STATS | StatxFlags::BTIME, + ); + match statx_result { + Ok(statx) => { + if state == 0 { + STATX_STATE.store(2, Ordering::Relaxed); + } + return Ok(MetadataExt::from_rustix_statx(statx)); + } + Err(rustix::io::Error::NOSYS) => STATX_STATE.store(1, Ordering::Relaxed), + Err(rustix::io::Error::PERM) if state == 0 => { + // This is an unlikely case, as `statx` doesn't normally + // return `PERM` errors. One way this can happen is when + // running on old versions of seccomp/Docker. If `statx` on + // the current working directory returns a similar error, + // then stop using `statx`. + if let Err(rustix::io::Error::PERM) = statx( + rustix::fs::cwd(), + "", + AtFlags::EMPTY_PATH, + StatxFlags::empty(), + ) { + STATX_STATE.store(1, Ordering::Relaxed); + } else { + return Err(rustix::io::Error::PERM.into()); + } + } + Err(e) => return Err(e.into()), + } } } From 7cee463bdc59c5e548d1b64ad37460b7ed15228a Mon Sep 17 00:00:00 2001 From: Diego Ongaro Date: Fri, 3 Jun 2022 16:54:07 -0700 Subject: [PATCH 9/9] fs_additional: Fix non-Unix test build in metadata test I had used `cfg!(unix)` instead of `#[cfg(unix)]` in 2eca023, which doesn't conditionally compile out the Unix-specific code. --- tests/fs_additional.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/fs_additional.rs b/tests/fs_additional.rs index 90e204a1..0e6fb610 100644 --- a/tests/fs_additional.rs +++ b/tests/fs_additional.rs @@ -910,7 +910,8 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) { assert_eq!(std.file_type().is_dir(), cap.file_type().is_dir()); assert_eq!(std.file_type().is_file(), cap.file_type().is_file()); assert_eq!(std.file_type().is_symlink(), cap.file_type().is_symlink()); - if cfg!(unix) { + #[cfg(unix)] + { use std::os::unix::fs::FileTypeExt; assert_eq!( std.file_type().is_block_device(), @@ -927,7 +928,8 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) { assert_eq!(std.len(), cap.len()); assert_eq!(std.permissions().readonly(), cap.permissions().readonly()); - if cfg!(unix) { + #[cfg(unix)] + { use std::os::unix::fs::PermissionsExt; // The standard library returns file format bits with `mode()`, whereas // cap-std currently doesn't. @@ -956,7 +958,8 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) { assert_eq!(expected, check!(cap.created()).into_std()); } - if cfg!(unix) { + #[cfg(unix)] + { use std::os::unix::fs::MetadataExt; assert_eq!(std.dev(), cap.dev()); assert_eq!(std.ino(), cap.ino());