From 43847b24274c7dd8698ad32948016ae3ff216a77 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 2 Sep 2021 22:52:55 +0200 Subject: [PATCH 1/7] use openat when encountering ENAMETOOLONG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a fallback to various fs methods that handles ENAMETOOLONG by splitting the path into relative segments and then opening each segment with openat(dirfd, segment, O_PATH). Limitations: * not all fs methods are covered. the primary motivation is to get remove_dir_all working to delete a deep directory structure created by accident * requires O_PATH, so this can currently only be a fallback and not the default due to our current minimum kernel version. If you're not dealing with long paths it won't be used. * currently linux-only. this could be extended to platforms which have either O_PATH or O_EXEC but there's no CI coverage for the BSDs so I don't want to foist it on them * O(n²) performance if remove_dir_all has to use the fallback, albeit a small constant factor due to the long path segments used but ideally it should be rewritten to use openat in its recursion steps. But to do it properly we need a higher minimum kernel version. --- library/std/src/fs/tests.rs | 75 ++++-- library/std/src/sys/common/small_c_string.rs | 7 +- library/std/src/sys/unix/fs.rs | 255 +++++++++++++++--- library/std/src/sys/unix/mod.rs | 2 +- .../src/sys/unix/process/process_common.rs | 2 +- library/std/src/sys_common/fs.rs | 1 + 6 files changed, 267 insertions(+), 75 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index d74f0f00e46f4..708b6740fd633 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1658,19 +1658,19 @@ fn test_file_times() { let modified = SystemTime::UNIX_EPOCH + Duration::from_secs(54321); times = times.set_accessed(accessed).set_modified(modified); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] - let created = SystemTime::UNIX_EPOCH + Duration::from_secs(32123); + let created = SystemTime::UNIX_EPOCH + Duration::from_secs(32123); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] { times = times.set_created(created); @@ -1678,16 +1678,16 @@ fn test_file_times() { match file.set_times(times) { // Allow unsupported errors on platforms which don't support setting times. #[cfg(not(any( - windows, - all( - unix, - not(any( - target_os = "android", - target_os = "redox", - target_os = "espidf", - target_os = "horizon" - )) - ) + windows, + all( + unix, + not(any( + target_os = "android", + target_os = "redox", + target_os = "espidf", + target_os = "horizon" + )) + ) )))] Err(e) if e.kind() == ErrorKind::Unsupported => return, Err(e) => panic!("error setting file times: {e:?}"), @@ -1697,13 +1697,36 @@ fn test_file_times() { assert_eq!(metadata.accessed().unwrap(), accessed); assert_eq!(metadata.modified().unwrap(), modified); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] { assert_eq!(metadata.created().unwrap(), created); } } + +#[test] +#[cfg(any(target_os = "linux", target_os = "android"))] +fn deep_traversal() -> crate::io::Result<()> { + use crate::fs::{create_dir_all, metadata, remove_dir_all, write}; + use crate::iter::repeat; + + let tmpdir = tmpdir(); + + let segment = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + + let mut dir = tmpdir.join(segment); + repeat(segment).take(100).for_each(|name| dir.push(name)); + assert!(dir.as_os_str().len() > libc::PATH_MAX as usize); + let file = dir.join("b"); + + create_dir_all(&dir).expect("deep create tailed"); + write(&file, "foo").expect("deep write failed"); + metadata(&file).expect("deep stat failed"); + remove_dir_all(&dir).expect("deep remove failed"); + + Ok(()) +} diff --git a/library/std/src/sys/common/small_c_string.rs b/library/std/src/sys/common/small_c_string.rs index 963d17a47e4c0..1278e6f5beb14 100644 --- a/library/std/src/sys/common/small_c_string.rs +++ b/library/std/src/sys/common/small_c_string.rs @@ -1,6 +1,5 @@ -use crate::ffi::{CStr, CString}; +use crate::ffi::{CStr, CString, OsStr}; use crate::mem::MaybeUninit; -use crate::path::Path; use crate::slice; use crate::{io, ptr}; @@ -15,11 +14,11 @@ const NUL_ERR: io::Error = io::const_io_error!(io::ErrorKind::InvalidInput, "file name contained an unexpected NUL byte"); #[inline] -pub fn run_path_with_cstr(path: &Path, f: F) -> io::Result +pub fn run_path_with_cstr(path: &(impl AsRef + ?Sized), f: F) -> io::Result where F: FnOnce(&CStr) -> io::Result, { - run_with_cstr(path.as_os_str().as_os_str_bytes(), f) + run_with_cstr(path.as_ref().as_os_str_bytes(), f) } #[inline] diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index a5604c92a80ba..27a199ae6070a 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -3,7 +3,7 @@ use crate::os::unix::prelude::*; -use crate::ffi::{CStr, OsStr, OsString}; +use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::fmt; use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; @@ -141,7 +141,8 @@ cfg_has_statx! {{ // Default `stat64` contains no creation time and may have 32-bit `time_t`. unsafe fn try_statx( fd: c_int, - path: *const c_char, + raw_path: *const c_char, + path: &Path, flags: i32, mask: u32, ) -> Option> { @@ -169,19 +170,17 @@ cfg_has_statx! {{ } let mut buf: libc::statx = mem::zeroed(); - if let Err(err) = cvt(statx(fd, path, flags, mask, &mut buf)) { - if STATX_SAVED_STATE.load(Ordering::Relaxed) == STATX_STATE::Present as u8 { - return Some(Err(err)); - } + let result = match cvt(statx(fd, raw_path, flags, mask, &mut buf)) { + o @ Ok(_) => o, + e @ Err(_) if STATX_SAVED_STATE.load(Ordering::Relaxed) == STATX_STATE::Present as u8 => e, // Availability not checked yet. // // First try the cheap way. - if err.raw_os_error() == Some(libc::ENOSYS) { + Err(err) if err.raw_os_error() == Some(libc::ENOSYS) => { STATX_SAVED_STATE.store(STATX_STATE::Unavailable as u8, Ordering::Relaxed); - return None; - } - + return None + }, // Error other than `ENOSYS` is not a good enough indicator -- it is // known that `EPERM` can be returned as a result of using seccomp to // block the syscall. @@ -192,16 +191,27 @@ cfg_has_statx! {{ // previous iteration of the code checked it for all errors and for now // this is retained. // FIXME what about transient conditions like `ENOMEM`? - let err2 = cvt(statx(0, ptr::null(), 0, libc::STATX_ALL, ptr::null_mut())) - .err() - .and_then(|e| e.raw_os_error()); - if err2 == Some(libc::EFAULT) { - STATX_SAVED_STATE.store(STATX_STATE::Present as u8, Ordering::Relaxed); - return Some(Err(err)); - } else { - STATX_SAVED_STATE.store(STATX_STATE::Unavailable as u8, Ordering::Relaxed); - return None; + e @ Err(_) => { + let err2 = cvt(statx(0, ptr::null(), 0, libc::STATX_ALL, ptr::null_mut())) + .err() + .and_then(|e| e.raw_os_error()); + if err2 == Some(libc::EFAULT) { + STATX_SAVED_STATE.store(STATX_STATE::Present as u8, Ordering::Relaxed); + e + } else { + STATX_SAVED_STATE.store(STATX_STATE::Unavailable as u8, Ordering::Relaxed); + return None + } } + }; + + let result = long_filename_fallback!(path, result, |dirfd, file_name| { + // FIXME: use libc::AT_EMPTY_PATH + cvt(statx(dirfd.as_raw_fd(), file_name.as_ptr(), flags, mask, &mut buf)) + }); + + if let Err(err) = result { + return Some(Err(err)); } // We cannot fill `stat64` exhaustively because of private padding fields. @@ -820,6 +830,7 @@ impl DirEntry { if let Some(ret) = unsafe { try_statx( fd, name, + self.file_name_os_str().as_ref(), libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT, libc::STATX_ALL, ) } { @@ -1052,19 +1063,55 @@ impl OpenOptions { impl File { pub fn open(path: &Path, opts: &OpenOptions) -> io::Result { - run_path_with_cstr(path, |path| File::open_c(path, opts)) + let result = run_path_with_cstr(path, |path| File::open_c(None, &path, opts)); + + #[cfg(any(target_os = "linux", target_os = "android"))] + let result = { + use crate::io::ErrorKind; + match result { + Ok(file) => Ok(file), + Err(e) if e.kind() == ErrorKind::InvalidFilename => open_deep(path, opts), + Err(e) => Err(e), + } + }; + + result } - pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result { + pub fn open_c( + dirfd: Option>, + path: &CStr, + opts: &OpenOptions, + ) -> io::Result { let flags = libc::O_CLOEXEC | opts.get_access_mode()? | opts.get_creation_mode()? | (opts.custom_flags as c_int & !libc::O_ACCMODE); - // The third argument of `open64` is documented to have type `mode_t`. On - // some platforms (like macOS, where `open64` is actually `open`), `mode_t` is `u16`. - // However, since this is a variadic function, C integer promotion rules mean that on - // the ABI level, this still gets passed as `c_int` (aka `u32` on Unix platforms). - let fd = cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })?; + let fd = match dirfd { + None => { + // The third argument of `open64` is documented to have type `mode_t`. On + // some platforms (like macOS, where `open64` is actually `open`), `mode_t` is `u16`. + // However, since this is a variadic function, C integer promotion rules mean that on + // the ABI level, this still gets passed as `c_int` (aka `u32` on Unix platforms). + cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })? + } + Some(dirfd) => { + cfg_if::cfg_if! { + if #[cfg(any(target_os = "linux", target_os = "android"))] { + use libc::openat64; + cvt_r(|| unsafe { + openat64( + dirfd.as_raw_fd(), + path.as_ptr(), + flags + ) + })? + } else { + return super::unsupported::unsupported() + } + } + } + }; Ok(File(unsafe { FileDesc::from_raw_fd(fd) })) } @@ -1075,6 +1122,7 @@ impl File { if let Some(ret) = unsafe { try_statx( fd, b"\0" as *const _ as *const c_char, + "".as_ref(), libc::AT_EMPTY_PATH | libc::AT_STATX_SYNC_AS_STAT, libc::STATX_ALL, ) } { @@ -1322,7 +1370,13 @@ impl DirBuilder { } pub fn mkdir(&self, p: &Path) -> io::Result<()> { - run_path_with_cstr(p, |p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map(|_| ())) + let result = run_path_with_cstr(p, |p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) })); + + let result = long_filename_fallback!(p, result, |dirfd, file_name| { + cvt(unsafe { libc::mkdirat(dirfd.as_raw_fd(), file_name.as_ptr(), self.mode) }) + }); + + result.map(|_| ()) } pub fn set_mode(&mut self, mode: u32) { @@ -1500,18 +1554,49 @@ impl fmt::Debug for File { } pub fn readdir(path: &Path) -> io::Result { - let ptr = run_path_with_cstr(path, |p| unsafe { Ok(libc::opendir(p.as_ptr())) })?; - if ptr.is_null() { - Err(Error::last_os_error()) - } else { - let root = path.to_path_buf(); - let inner = InnerReadDir { dirp: Dir(ptr), root }; - Ok(ReadDir::new(inner)) + + fn cvt_p(ptr: *mut T) -> io::Result<*mut T> { + if ptr.is_null() { + return Err(Error::last_os_error()); + } + Ok(ptr) } + + let root = path.to_path_buf(); + let ptr = cvt_p(run_path_with_cstr(path, |p| unsafe { Ok(libc::opendir(p.as_ptr())) })?); + + let ptr = match ptr { + #[cfg(any(target_os = "linux", target_os = "android"))] + Err(e) if e.kind() == crate::io::ErrorKind::InvalidFilename => { + let mut opts = OpenOptions::new(); + opts.read(true); + opts.custom_flags(libc::O_DIRECTORY); + let fd = File::open(path, &opts)?.into_raw_fd(); + cvt_p(unsafe { libc::fdopendir(fd) }) + } + other @ _ => other, + }?; + + let inner = InnerReadDir { dirp: Dir(ptr), root }; + Ok(ReadDir { + inner: Arc::new(inner), + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox", + )))] + end_of_stream: false, + }) } -pub fn unlink(p: &Path) -> io::Result<()> { - run_path_with_cstr(p, |p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ())) +pub fn unlink(path: &Path) -> io::Result<()> { + let result = run_path_with_cstr(path, |p| cvt(unsafe { libc::unlink(p.as_ptr()) })); + let result = long_filename_fallback!(path, result, |dirfd, file_name| { + cvt(unsafe { libc::unlinkat(dirfd.as_raw_fd(), file_name.as_ptr(), 0) }) + }); + + result.map(|_| ()) } pub fn rename(old: &Path, new: &Path) -> io::Result<()> { @@ -1526,8 +1611,15 @@ pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> { run_path_with_cstr(p, |p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ())) } -pub fn rmdir(p: &Path) -> io::Result<()> { - run_path_with_cstr(p, |p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ())) +pub fn rmdir(path: &Path) -> io::Result<()> { + let result = run_path_with_cstr(path, |p| cvt(unsafe { libc::rmdir(p.as_ptr()) })); + + #[cfg(any(target_os = "linux", target_os = "android"))] + let result = long_filename_fallback!(path, result, |dirfd, file_name| { + cvt(unsafe { libc::unlinkat(dirfd.as_raw_fd(), file_name.as_ptr(), libc::AT_REMOVEDIR) }) + }); + + result.map(|_| ()) } pub fn readlink(p: &Path) -> io::Result { @@ -1602,12 +1694,13 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> { }) } -pub fn stat(p: &Path) -> io::Result { - run_path_with_cstr(p, |p| { +pub fn stat(path: &Path) -> io::Result { + run_path_with_cstr(path, |p| { cfg_has_statx! { if let Some(ret) = unsafe { try_statx( libc::AT_FDCWD, p.as_ptr(), + path, libc::AT_STATX_SYNC_AS_STAT, libc::STATX_ALL, ) } { @@ -1621,12 +1714,13 @@ pub fn stat(p: &Path) -> io::Result { }) } -pub fn lstat(p: &Path) -> io::Result { - run_path_with_cstr(p, |p| { +pub fn lstat(path: &Path) -> io::Result { + run_path_with_cstr(path, |p| { cfg_has_statx! { if let Some(ret) = unsafe { try_statx( libc::AT_FDCWD, p.as_ptr(), + path, libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT, libc::STATX_ALL, ) } { @@ -1635,7 +1729,12 @@ pub fn lstat(p: &Path) -> io::Result { } let mut stat: stat64 = unsafe { mem::zeroed() }; - cvt(unsafe { lstat64(p.as_ptr(), &mut stat) })?; + let result = cvt(unsafe { lstat64(p.as_ptr(), &mut stat) }); + long_filename_fallback!(path, result, |dirfd, file_name| { + cvt(unsafe { + fstatat64(dirfd.as_raw_fd(), file_name.as_ptr(), &mut stat, libc::AT_SYMLINK_NOFOLLOW) + }) + })?; Ok(FileAttr::from_stat64(stat)) }) } @@ -1654,6 +1753,76 @@ pub fn canonicalize(p: &Path) -> io::Result { }))) } +#[cfg(any(target_os = "linux", target_os = "android"))] +fn open_deep(path: &Path, opts: &OpenOptions) -> io::Result { + use super::path::is_sep_byte; + use libc::O_PATH; + const MAX_SLICE: usize = (libc::PATH_MAX - 1) as usize; + + let mut raw_path = path.as_os_str().as_bytes(); + let mut at_path = None; + + let mut dir_flags = OpenOptions::new(); + dir_flags.read(true); + dir_flags.custom_flags(O_PATH); + + while raw_path.len() > MAX_SLICE { + let sep_idx = match raw_path.iter().take(MAX_SLICE).rposition(|&byte| is_sep_byte(byte)) { + Some(idx) => idx, + _ => return Err(io::Error::from_raw_os_error(libc::ENAMETOOLONG)), + }; + + let (left, right) = raw_path.split_at(sep_idx + 1); + raw_path = right; + + let to_open = CString::new(left)?; + let dirfd = at_path.as_ref().map(AsFd::as_fd); + + at_path = Some(File::open_c(dirfd, &to_open, &dir_flags)?); + } + + let to_open = CString::new(raw_path)?; + let dirfd = at_path.as_ref().map(AsFd::as_fd); + + File::open_c(dirfd, &to_open, opts) +} + +macro long_filename_fallback { + ($path:expr, $result:expr, $fallback:expr) => { + { + cfg_if::cfg_if! { + if #[cfg(any(target_os = "linux", target_os = "android"))] { + fn deep_fallback(result: io::Result, path: &Path, mut fallback: impl FnMut(File, &CStr) -> io::Result) -> io::Result { + use crate::io::ErrorKind; + match result { + ok @ Ok(_) => ok, + Err(e) if e.kind() == ErrorKind::InvalidFilename => { + if let Some(parent) = path.parent() { + let mut options = OpenOptions::new(); + options.read(true); + options.custom_flags(libc::O_PATH); + let dirfd = open_deep(parent, &options)?; + let file_name = path.file_name().unwrap(); + return run_path_with_cstr(file_name, |file_name| { + fallback(dirfd, file_name) + }) + } + + Err(e) + }, + Err(e) => Err(e) + } + } + + deep_fallback($result, $path, $fallback) + } else { + $result + } + } + } + } +} + fn open_from(from: &Path) -> io::Result<(crate::fs::File, crate::fs::Metadata)> { use crate::fs::File; use crate::sys_common::fs::NOT_FILE_ERROR; diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index 77ef086f29b59..b322f1bb9ea52 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -420,7 +420,7 @@ cfg_if::cfg_if! { } } -#[cfg(any(target_os = "espidf", target_os = "horizon", target_os = "vita"))] +#[cfg(not(any(target_os = "linux", target_os = "android")))] mod unsupported { use crate::io; diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 640648e870748..ab3ffd9d84c51 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -475,7 +475,7 @@ impl Stdio { opts.read(readable); opts.write(!readable); let path = unsafe { CStr::from_ptr(DEV_NULL.as_ptr() as *const _) }; - let fd = File::open_c(&path, &opts)?; + let fd = File::open_c(None, &path, &opts)?; Ok((ChildStdio::Owned(fd.into_inner()), None)) } diff --git a/library/std/src/sys_common/fs.rs b/library/std/src/sys_common/fs.rs index 617ac52e51ca8..7641af654f4cf 100644 --- a/library/std/src/sys_common/fs.rs +++ b/library/std/src/sys_common/fs.rs @@ -30,6 +30,7 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> { if filetype.is_symlink() { fs::remove_file(path) } else { remove_dir_all_recursive(path) } } +// FIXME: this should have a unix-specific implementation using the -at version of syscalls and that has cycle detection fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { for child in fs::read_dir(path)? { let child = child?; From faab6298541acdbe7e81ee2e3661d3c01df6a21f Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 17 Sep 2021 21:51:59 +0200 Subject: [PATCH 2/7] also pass `mode` flag to openat() --- library/std/src/sys/unix/fs.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 27a199ae6070a..578348f2b8e47 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1103,7 +1103,9 @@ impl File { openat64( dirfd.as_raw_fd(), path.as_ptr(), - flags + flags, + // see previous comment why this cast is necessary + opts.mode as c_int ) })? } else { From 923ac9f3f43c90407d6aa7ac2d70649b55515df3 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sun, 10 Oct 2021 19:16:46 +0200 Subject: [PATCH 3/7] implement openat-based remove_dir_all in a separate module --- library/std/src/fs/tests.rs | 52 ++--- library/std/src/sys/unix/fs.rs | 150 ++++++--------- library/std/src/sys/unix/fs/dir_fd.rs | 265 ++++++++++++++++++++++++++ library/std/src/sys_common/fs.rs | 1 - 4 files changed, 344 insertions(+), 124 deletions(-) create mode 100644 library/std/src/sys/unix/fs/dir_fd.rs diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 708b6740fd633..6c52f4986b998 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1658,19 +1658,19 @@ fn test_file_times() { let modified = SystemTime::UNIX_EPOCH + Duration::from_secs(54321); times = times.set_accessed(accessed).set_modified(modified); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] - let created = SystemTime::UNIX_EPOCH + Duration::from_secs(32123); + let created = SystemTime::UNIX_EPOCH + Duration::from_secs(32123); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] { times = times.set_created(created); @@ -1678,16 +1678,16 @@ fn test_file_times() { match file.set_times(times) { // Allow unsupported errors on platforms which don't support setting times. #[cfg(not(any( - windows, - all( - unix, - not(any( - target_os = "android", - target_os = "redox", - target_os = "espidf", - target_os = "horizon" - )) - ) + windows, + all( + unix, + not(any( + target_os = "android", + target_os = "redox", + target_os = "espidf", + target_os = "horizon" + )) + ) )))] Err(e) if e.kind() == ErrorKind::Unsupported => return, Err(e) => panic!("error setting file times: {e:?}"), @@ -1697,11 +1697,11 @@ fn test_file_times() { assert_eq!(metadata.accessed().unwrap(), accessed); assert_eq!(metadata.modified().unwrap(), modified); #[cfg(any( - windows, - target_os = "macos", - target_os = "ios", - target_os = "watchos", - target_os = "tvos", + windows, + target_os = "macos", + target_os = "ios", + target_os = "watchos", + target_os = "tvos", ))] { assert_eq!(metadata.created().unwrap(), created); diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 578348f2b8e47..631d322378f47 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -3,7 +3,7 @@ use crate::os::unix::prelude::*; -use crate::ffi::{CStr, CString, OsStr, OsString}; +use crate::ffi::{CStr, OsStr, OsString}; use crate::fmt; use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom}; use crate::mem; @@ -86,7 +86,11 @@ use libc::{ lstat as lstat64, off_t as off64_t, open as open64, stat as stat64, }; #[cfg(any(target_os = "linux", target_os = "emscripten", target_os = "l4re"))] -use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, stat64}; +use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, stat64}; + +// FIXME: port this to other unices that support *at syscalls +#[cfg(any(target_os = "linux", target_os = "android"))] +mod dir_fd; pub use crate::sys_common::fs::try_exists; @@ -269,9 +273,25 @@ pub struct ReadDir { } impl ReadDir { + #[cfg(not(any(target_os = "android", target_os = "linux")))] fn new(inner: InnerReadDir) -> Self { Self { inner: Arc::new(inner), end_of_stream: false } } + + #[cfg(any(target_os = "android", target_os = "linux"))] + fn from_dirp(ptr: *mut libc::DIR, root: PathBuf) -> ReadDir { + let inner = InnerReadDir { dirp: Dir(ptr), root }; + ReadDir { + inner: Arc::new(inner), + #[cfg(not(any( + target_os = "solaris", + target_os = "illumos", + target_os = "fuchsia", + target_os = "redox", + )))] + end_of_stream: false, + } + } } struct Dir(*mut libc::DIR); @@ -1070,7 +1090,9 @@ impl File { use crate::io::ErrorKind; match result { Ok(file) => Ok(file), - Err(e) if e.kind() == ErrorKind::InvalidFilename => open_deep(path, opts), + Err(e) if e.kind() == ErrorKind::InvalidFilename => { + dir_fd::open_deep(None, path, opts) + } Err(e) => Err(e), } }; @@ -1078,6 +1100,7 @@ impl File { result } + #[cfg(not(any(target_os = "linux", target_os = "android")))] pub fn open_c( dirfd: Option>, path: &CStr, @@ -1095,24 +1118,7 @@ impl File { // the ABI level, this still gets passed as `c_int` (aka `u32` on Unix platforms). cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })? } - Some(dirfd) => { - cfg_if::cfg_if! { - if #[cfg(any(target_os = "linux", target_os = "android"))] { - use libc::openat64; - cvt_r(|| unsafe { - openat64( - dirfd.as_raw_fd(), - path.as_ptr(), - flags, - // see previous comment why this cast is necessary - opts.mode as c_int - ) - })? - } else { - return super::unsupported::unsupported() - } - } - } + Some(dirfd) => return super::unsupported::unsupported(), }; Ok(File(unsafe { FileDesc::from_raw_fd(fd) })) } @@ -1555,15 +1561,14 @@ impl fmt::Debug for File { } } -pub fn readdir(path: &Path) -> io::Result { - - fn cvt_p(ptr: *mut T) -> io::Result<*mut T> { - if ptr.is_null() { - return Err(Error::last_os_error()); - } - Ok(ptr) +fn cvt_p(ptr: *mut T) -> io::Result<*mut T> { + if ptr.is_null() { + return Err(Error::last_os_error()); } + Ok(ptr) +} +pub fn readdir(path: &Path) -> io::Result { let root = path.to_path_buf(); let ptr = cvt_p(run_path_with_cstr(path, |p| unsafe { Ok(libc::opendir(p.as_ptr())) })?); @@ -1734,7 +1739,12 @@ pub fn lstat(path: &Path) -> io::Result { let result = cvt(unsafe { lstat64(p.as_ptr(), &mut stat) }); long_filename_fallback!(path, result, |dirfd, file_name| { cvt(unsafe { - fstatat64(dirfd.as_raw_fd(), file_name.as_ptr(), &mut stat, libc::AT_SYMLINK_NOFOLLOW) + fstatat64( + dirfd.as_raw_fd(), + file_name.as_ptr(), + &mut stat, + libc::AT_SYMLINK_NOFOLLOW, + ) }) })?; Ok(FileAttr::from_stat64(stat)) @@ -1755,75 +1765,15 @@ pub fn canonicalize(p: &Path) -> io::Result { }))) } -#[cfg(any(target_os = "linux", target_os = "android"))] -fn open_deep(path: &Path, opts: &OpenOptions) -> io::Result { - use super::path::is_sep_byte; - use libc::O_PATH; - const MAX_SLICE: usize = (libc::PATH_MAX - 1) as usize; - - let mut raw_path = path.as_os_str().as_bytes(); - let mut at_path = None; - - let mut dir_flags = OpenOptions::new(); - dir_flags.read(true); - dir_flags.custom_flags(O_PATH); - - while raw_path.len() > MAX_SLICE { - let sep_idx = match raw_path.iter().take(MAX_SLICE).rposition(|&byte| is_sep_byte(byte)) { - Some(idx) => idx, - _ => return Err(io::Error::from_raw_os_error(libc::ENAMETOOLONG)), - }; - - let (left, right) = raw_path.split_at(sep_idx + 1); - raw_path = right; - - let to_open = CString::new(left)?; - let dirfd = at_path.as_ref().map(AsFd::as_fd); - - at_path = Some(File::open_c(dirfd, &to_open, &dir_flags)?); - } - - let to_open = CString::new(raw_path)?; - let dirfd = at_path.as_ref().map(AsFd::as_fd); - - File::open_c(dirfd, &to_open, opts) -} - -macro long_filename_fallback { - ($path:expr, $result:expr, $fallback:expr) => { - { - cfg_if::cfg_if! { - if #[cfg(any(target_os = "linux", target_os = "android"))] { - fn deep_fallback(result: io::Result, path: &Path, mut fallback: impl FnMut(File, &CStr) -> io::Result) -> io::Result { - use crate::io::ErrorKind; - match result { - ok @ Ok(_) => ok, - Err(e) if e.kind() == ErrorKind::InvalidFilename => { - if let Some(parent) = path.parent() { - let mut options = OpenOptions::new(); - options.read(true); - options.custom_flags(libc::O_PATH); - let dirfd = open_deep(parent, &options)?; - let file_name = path.file_name().unwrap(); - return run_path_with_cstr(file_name, |file_name| { - fallback(dirfd, file_name) - }) - } - - Err(e) - }, - Err(e) => Err(e) - } - } - - deep_fallback($result, $path, $fallback) - } else { - $result - } - } +macro long_filename_fallback($path:expr, $result:expr, $fallback:expr) {{ + cfg_if::cfg_if! { + if #[cfg(any(target_os = "linux", target_os = "android"))] { + dir_fd::long_filename_fallback($result, $path, $fallback) + } else { + $result } } -} +}} fn open_from(from: &Path) -> io::Result<(crate::fs::File, crate::fs::Metadata)> { use crate::fs::File; @@ -1854,7 +1804,6 @@ fn open_to_and_set_permissions( reader_metadata: crate::fs::Metadata, ) -> io::Result<(crate::fs::File, crate::fs::Metadata)> { use crate::fs::OpenOptions; - use crate::os::unix::fs::{OpenOptionsExt, PermissionsExt}; let perm = reader_metadata.permissions(); let writer = OpenOptions::new() @@ -2059,6 +2008,11 @@ mod remove_dir_impl { pub use crate::sys_common::fs::remove_dir_all; } +#[cfg(all(not(miri), any(target_os = "linux", target_os = "android")))] +mod remove_dir_impl { + pub use super::dir_fd::remove_dir_all; +} + // Modern implementation using openat(), unlinkat() and fdopendir() #[cfg(not(any( target_os = "redox", @@ -2066,6 +2020,8 @@ mod remove_dir_impl { target_os = "horizon", target_os = "vita", target_os = "nto", + target_os = "linux", + target_os = "android", miri )))] mod remove_dir_impl { diff --git a/library/std/src/sys/unix/fs/dir_fd.rs b/library/std/src/sys/unix/fs/dir_fd.rs new file mode 100644 index 0000000000000..9c47ca9e502c7 --- /dev/null +++ b/library/std/src/sys/unix/fs/dir_fd.rs @@ -0,0 +1,265 @@ +use super::super::path::is_sep_byte; +use super::{File, OpenOptions, ReadDir}; +use crate::collections::{HashSet, VecDeque}; +use crate::ffi::{CStr, CString}; +use crate::io; +use crate::os::unix::ffi::OsStrExt; +use crate::os::unix::fs::MetadataExt; +use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd}; +use crate::path::{Path, PathBuf}; +use crate::sys::common::small_c_string::run_path_with_cstr; +use crate::sys::fd::FileDesc; +use crate::sys::{cvt, cvt_r}; +use crate::sys_common::FromInner; +use libc::{c_int, openat64, O_DIRECTORY, O_PATH}; + +impl ReadDir { + pub fn from_dirfd(dirfd: OwnedFd, root: PathBuf) -> io::Result { + let dir_pointer = super::cvt_p(unsafe { libc::fdopendir(dirfd.as_raw_fd()) })?; + // fdopendir takes ownership on success + crate::mem::forget(dirfd); + Ok(Self::from_dirp(dir_pointer, root)) + } +} + +impl File { + pub fn open_c( + dirfd: Option>, + path: &CStr, + opts: &OpenOptions, + ) -> io::Result { + let flags = libc::O_CLOEXEC + | opts.get_access_mode()? + | opts.get_creation_mode()? + | (opts.custom_flags as c_int & !libc::O_ACCMODE); + + let dirfd = match dirfd { + None => libc::AT_FDCWD, + Some(dirfd) => dirfd.as_raw_fd(), + }; + let fd = cvt_r(|| unsafe { + openat64( + dirfd, + path.as_ptr(), + flags, + // see previous comment why this cast is necessary + opts.mode as c_int, + ) + })?; + + Ok(File(unsafe { FileDesc::from_raw_fd(fd) })) + } +} + +pub fn open_deep( + at_path: Option>, + path: &Path, + opts: &OpenOptions, +) -> io::Result { + const MAX_SLICE: usize = (libc::PATH_MAX - 1) as usize; + + enum AtPath<'a> { + None, + Borrowed(BorrowedFd<'a>), + File(File), + } + + impl<'a> AtPath<'a> { + fn as_fd(&'a self) -> Option> { + match self { + AtPath::Borrowed(borrowed) => Some(*borrowed), + AtPath::File(ref file) => Some(file.as_fd()), + AtPath::None => None, + } + } + } + + let mut raw_path = path.as_os_str().as_bytes(); + let mut at_path = match at_path { + Some(borrowed) => AtPath::Borrowed(borrowed), + None => AtPath::None, + }; + + let mut dir_flags = OpenOptions::new(); + dir_flags.read(true); + dir_flags.custom_flags(O_PATH); + + while raw_path.len() > MAX_SLICE { + let sep_idx = match raw_path.iter().take(MAX_SLICE).rposition(|&byte| is_sep_byte(byte)) { + Some(idx) => idx, + _ => return Err(io::Error::from_raw_os_error(libc::ENAMETOOLONG)), + }; + + let (left, right) = raw_path.split_at(sep_idx + 1); + raw_path = right; + + let to_open = CString::new(left)?; + let dirfd = at_path.as_fd(); + + at_path = AtPath::File(File::open_c(dirfd, &to_open, &dir_flags)?); + } + + let to_open = CString::new(raw_path)?; + let dirfd = at_path.as_fd(); + + File::open_c(dirfd, &to_open, opts) +} + +pub fn long_filename_fallback( + result: io::Result, + path: &Path, + mut fallback: impl FnMut(File, &CStr) -> io::Result, +) -> io::Result { + use crate::io::ErrorKind; + match result { + ok @ Ok(_) => ok, + Err(e) if e.kind() == ErrorKind::InvalidFilename => { + if let Some(parent) = path.parent() { + let mut options = OpenOptions::new(); + options.read(true); + options.custom_flags(libc::O_PATH); + let dirfd = open_deep(None, parent, &options)?; + let file_name = path.file_name().unwrap(); + return run_path_with_cstr(file_name, |file_name| fallback(dirfd, file_name)); + } + + Err(e) + } + Err(e) => Err(e), + } +} + +pub fn remove_dir_all(path: &Path) -> io::Result<()> { + let filetype = crate::fs::symlink_metadata(path)?.file_type(); + if filetype.is_symlink() { + crate::fs::remove_file(path) + } else { + remove_dir_all_recursive(path) + } +} + +fn unlinkat(dirfd: BorrowedFd<'_>, path: &CStr, rmdir: bool) -> io::Result<()> { + let flags = if rmdir { libc::AT_REMOVEDIR } else { 0 }; + cvt(unsafe { libc::unlinkat(dirfd.as_raw_fd(), path.as_ptr(), flags) })?; + Ok(()) +} + +fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { + let mut dir_open_opts = OpenOptions::new(); + dir_open_opts.read(true); + dir_open_opts.custom_flags(O_DIRECTORY); + let root_fd = crate::fs::File::from_inner(File::open(&path, &dir_open_opts)?); + // the current path relative to the root fd. + let mut current_path = PathBuf::from("."); + + let mut visited_stack: Vec<(u64, u64)> = Vec::new(); + let mut visited: HashSet<(u64, u64)> = HashSet::new(); + + let meta = root_fd.metadata()?; + let root_id = (meta.ino(), meta.dev()); + visited.insert(root_id); + visited_stack.push(root_id); + + const MAX_FDS: usize = 20; + let mut dir_stack: VecDeque<(RawFd, ReadDir)> = VecDeque::new(); + + let mut child_name_buf = Vec::new(); + let mut remove_current = false; + + 'tree_walk: loop { + if remove_current { + let name = current_path.file_name().expect("current path empty"); + child_name_buf.clear(); + child_name_buf.extend_from_slice(name.as_bytes()); + child_name_buf.push(0); + current_path.pop(); + let dir_id = visited_stack.pop().expect("visited stack empty"); + assert!(visited.remove(&dir_id)); + } + + let (current_dir_fd, mut current_reader) = match dir_stack.pop_back() { + Some(dir) => dir, + None => { + // when ascending out of a tree deeper than MAX_FDS the ReadDirs of the parents + // will have been dropped. reopen as needed. + let dirfd = OwnedFd::from(crate::fs::File::from_inner(open_deep( + Some(root_fd.as_fd()), + current_path.as_path(), + &dir_open_opts, + )?)); + // supply an empty pathbuf since we don't intend to use DirEntry::path() + let rawfd = dirfd.as_raw_fd(); + (rawfd, ReadDir::from_dirfd(dirfd, PathBuf::new())?) + } + }; + + let current_dir_fd = unsafe { BorrowedFd::borrow_raw(current_dir_fd) }; + + if remove_current { + let dir_name_c = + unsafe { CStr::from_bytes_with_nul_unchecked(child_name_buf.as_slice()) }; + unlinkat(current_dir_fd, dir_name_c, true)?; + remove_current = false; + } + + while let Some(child) = current_reader.next() { + let child = child?; + + child_name_buf.clear(); + child_name_buf.extend_from_slice(child.file_name_os_str().as_bytes()); + child_name_buf.push(0); + + let child_name_c = + unsafe { CStr::from_bytes_with_nul_unchecked(child_name_buf.as_slice()) }; + + if child.file_type()?.is_dir() { + current_path.push(child.file_name_os_str()); + + let dir = crate::fs::File::from_inner(File::open_c( + Some(current_dir_fd), + child_name_c, + &dir_open_opts, + )?); + + dir_stack.push_back((current_dir_fd.as_raw_fd(), current_reader)); + + // since we're traversing the directory tree via openat we have to do + // cycle-detection ourselves + let meta = dir.metadata()?; + let dir_id = (meta.ino(), meta.dev()); + if visited.contains(&dir_id) { + return Err(io::Error::from_raw_os_error(libc::ELOOP)); + } + visited_stack.push(dir_id); + visited.insert(dir_id); + + let dirfd = OwnedFd::from(dir); + let raw_fd = dirfd.as_raw_fd(); + let reader = ReadDir::from_dirfd(dirfd, PathBuf::new())?; + dir_stack.push_back((raw_fd, reader)); + + if dir_stack.len() > MAX_FDS { + dir_stack.pop_front(); + } + + continue 'tree_walk; + } + + unlinkat(current_dir_fd, child_name_c, false)?; + } + + // reached end of directory + if current_path.as_os_str().len() == 1 { + break; + } + + remove_current = true; + } + + // There's no dirfd to reuse here since unlinking the starting point requires unlinking relative to + // the parent directory. '..' also cannot be used because that is vulnerable to the directory + // being moved. And we couldn't have started with the parent directory either because the starting + // point may be '/' or '.' which can't be removed themselves but we still should be able to clean + // their descendants. + super::rmdir(path) +} diff --git a/library/std/src/sys_common/fs.rs b/library/std/src/sys_common/fs.rs index 7641af654f4cf..617ac52e51ca8 100644 --- a/library/std/src/sys_common/fs.rs +++ b/library/std/src/sys_common/fs.rs @@ -30,7 +30,6 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> { if filetype.is_symlink() { fs::remove_file(path) } else { remove_dir_all_recursive(path) } } -// FIXME: this should have a unix-specific implementation using the -at version of syscalls and that has cycle detection fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { for child in fs::read_dir(path)? { let child = child?; From 2b0bee2376398686831a1896f3ba9e1f7412df2b Mon Sep 17 00:00:00 2001 From: The8472 Date: Tue, 12 Oct 2021 22:10:33 +0200 Subject: [PATCH 4/7] implement long path fallback for stat() when statx isn't available --- library/std/src/sys/unix/fs.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 631d322378f47..e7db94288f661 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1701,6 +1701,13 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> { }) } +// On linux this is the default behavior for lstat and stat but it has to be set explicitly for fstatat +#[cfg(any(target_os = "linux", target_os = "android"))] +const DEFAULT_STATAT_FLAGS: c_int = libc::AT_NO_AUTOMOUNT; + +#[cfg(not(any(target_os = "linux", target_os = "android")))] +const DEFAULT_STATAT_FLAGS: c_int = 0; + pub fn stat(path: &Path) -> io::Result { run_path_with_cstr(path, |p| { cfg_has_statx! { @@ -1716,7 +1723,12 @@ pub fn stat(path: &Path) -> io::Result { } let mut stat: stat64 = unsafe { mem::zeroed() }; - cvt(unsafe { stat64(p.as_ptr(), &mut stat) })?; + let result = cvt(unsafe { stat64(p.as_ptr(), &mut stat) }); + long_filename_fallback!(path, result, |dirfd, file_name| { + cvt(unsafe { + fstatat64(dirfd.as_raw_fd(), file_name.as_ptr(), &mut stat, DEFAULT_STATAT_FLAGS) + }) + })?; Ok(FileAttr::from_stat64(stat)) }) } @@ -1743,7 +1755,7 @@ pub fn lstat(path: &Path) -> io::Result { dirfd.as_raw_fd(), file_name.as_ptr(), &mut stat, - libc::AT_SYMLINK_NOFOLLOW, + DEFAULT_STATAT_FLAGS | libc::AT_SYMLINK_NOFOLLOW, ) }) })?; From a603b07a4caacf28ff62549e984b1a1788162bd9 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 5 Aug 2023 16:58:43 +0200 Subject: [PATCH 5/7] disable long filename fallback under miri --- library/std/src/sys/unix/fs.rs | 11 +++++++---- library/std/src/sys/unix/mod.rs | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index e7db94288f661..645948fc9a622 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -43,6 +43,8 @@ use libc::c_char; use libc::dirfd; #[cfg(any(target_os = "linux", target_os = "emscripten"))] use libc::fstatat64; +#[cfg(all(miri, any(target_os = "linux")))] +use libc::open64; #[cfg(any( target_os = "android", target_os = "solaris", @@ -89,7 +91,7 @@ use libc::{ use libc::{dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, stat64}; // FIXME: port this to other unices that support *at syscalls -#[cfg(any(target_os = "linux", target_os = "android"))] +#[cfg(all(not(miri), any(target_os = "linux", target_os = "android")))] mod dir_fd; pub use crate::sys_common::fs::try_exists; @@ -1085,7 +1087,7 @@ impl File { pub fn open(path: &Path, opts: &OpenOptions) -> io::Result { let result = run_path_with_cstr(path, |path| File::open_c(None, &path, opts)); - #[cfg(any(target_os = "linux", target_os = "android"))] + #[cfg(all(not(miri), any(target_os = "linux", target_os = "android")))] let result = { use crate::io::ErrorKind; match result { @@ -1100,7 +1102,7 @@ impl File { result } - #[cfg(not(any(target_os = "linux", target_os = "android")))] + #[cfg(any(miri, not(any(target_os = "linux", target_os = "android"))))] pub fn open_c( dirfd: Option>, path: &CStr, @@ -1779,7 +1781,8 @@ pub fn canonicalize(p: &Path) -> io::Result { macro long_filename_fallback($path:expr, $result:expr, $fallback:expr) {{ cfg_if::cfg_if! { - if #[cfg(any(target_os = "linux", target_os = "android"))] { + // miri doesn't support the *at syscalls + if #[cfg(all(not(miri), any(target_os = "linux", target_os = "android")))] { dir_fd::long_filename_fallback($result, $path, $fallback) } else { $result diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index b322f1bb9ea52..e0a19080730d9 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -420,7 +420,7 @@ cfg_if::cfg_if! { } } -#[cfg(not(any(target_os = "linux", target_os = "android")))] +#[cfg(any(miri, not(any(target_os = "linux", target_os = "android"))))] mod unsupported { use crate::io; From 9aea939dfb6a31ad3328edd2182cbdaa7717f4cf Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 5 Aug 2023 18:54:18 +0200 Subject: [PATCH 6/7] bless miri test --- src/tools/miri/tests/fail/shims/fs/isolated_file.stderr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr b/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr index 2385439c8a5f7..f9b399eb88d1c 100644 --- a/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr +++ b/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr @@ -1,8 +1,8 @@ error: unsupported operation: `open` not available when isolation is enabled --> RUSTLIB/std/src/sys/PLATFORM/fs.rs:LL:CC | -LL | let fd = cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `open` not available when isolation is enabled +LL | cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })? + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `open` not available when isolation is enabled | = help: pass the flag `-Zmiri-disable-isolation` to disable isolation; = help: or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning @@ -12,7 +12,7 @@ LL | let fd = cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode a = note: inside `std::sys::PLATFORM::fs::File::open_c` at RUSTLIB/std/src/sys/PLATFORM/fs.rs:LL:CC = note: inside closure at RUSTLIB/std/src/sys/PLATFORM/fs.rs:LL:CC = note: inside `std::sys::PLATFORM::small_c_string::run_with_cstr::` at RUSTLIB/std/src/sys/PLATFORM/small_c_string.rs:LL:CC - = note: inside `std::sys::PLATFORM::small_c_string::run_path_with_cstr::` at RUSTLIB/std/src/sys/PLATFORM/small_c_string.rs:LL:CC + = note: inside `std::sys::PLATFORM::small_c_string::run_path_with_cstr::` at RUSTLIB/std/src/sys/PLATFORM/small_c_string.rs:LL:CC = note: inside `std::sys::PLATFORM::fs::File::open` at RUSTLIB/std/src/sys/PLATFORM/fs.rs:LL:CC = note: inside `std::fs::OpenOptions::_open` at RUSTLIB/std/src/fs.rs:LL:CC = note: inside `std::fs::OpenOptions::open::<&std::path::Path>` at RUSTLIB/std/src/fs.rs:LL:CC From 689f3d0aae56956797bf588b1de678a3dbf6eddf Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sat, 12 Aug 2023 22:17:46 +0200 Subject: [PATCH 7/7] refactor recursive deletion into a struct maintaining the dir stack it can remove arbitrarily-deep directory trees without exhausting the stack or file descriptor limits. The symlink attack/TOCTOU from CVE-2022-21658 that can occur when traversing the directory hierarchy more than level at a time is addressed by retracing the .. hierarchy after opening a descendant. Opening .. isn't subject to symlink attacks so we can reliably compare dev/ino numbers. --- library/std/src/lib.rs | 1 + library/std/src/sys/unix/fs/dir_fd.rs | 393 +++++++++++++++++++------- 2 files changed, 297 insertions(+), 97 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 9038e8fa9d7aa..0258326610be3 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -296,6 +296,7 @@ #![feature(int_roundings)] #![feature(ip)] #![feature(ip_in_core)] +#![feature(iter_advance_by)] #![feature(maybe_uninit_slice)] #![feature(maybe_uninit_uninit_array)] #![feature(maybe_uninit_write_slice)] diff --git a/library/std/src/sys/unix/fs/dir_fd.rs b/library/std/src/sys/unix/fs/dir_fd.rs index 9c47ca9e502c7..e319dd126bd00 100644 --- a/library/std/src/sys/unix/fs/dir_fd.rs +++ b/library/std/src/sys/unix/fs/dir_fd.rs @@ -1,17 +1,14 @@ use super::super::path::is_sep_byte; use super::{File, OpenOptions, ReadDir}; -use crate::collections::{HashSet, VecDeque}; use crate::ffi::{CStr, CString}; use crate::io; use crate::os::unix::ffi::OsStrExt; -use crate::os::unix::fs::MetadataExt; -use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd, RawFd}; +use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd}; use crate::path::{Path, PathBuf}; use crate::sys::common::small_c_string::run_path_with_cstr; use crate::sys::fd::FileDesc; use crate::sys::{cvt, cvt_r}; -use crate::sys_common::FromInner; -use libc::{c_int, openat64, O_DIRECTORY, O_PATH}; +use libc::{c_int, openat64, O_PATH}; impl ReadDir { pub fn from_dirfd(dirfd: OwnedFd, root: PathBuf) -> io::Result { @@ -134,7 +131,7 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> { if filetype.is_symlink() { crate::fs::remove_file(path) } else { - remove_dir_all_recursive(path) + rmdir::remove_dir_all_iter(path) } } @@ -144,122 +141,324 @@ fn unlinkat(dirfd: BorrowedFd<'_>, path: &CStr, rmdir: bool) -> io::Result<()> { Ok(()) } -fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { - let mut dir_open_opts = OpenOptions::new(); - dir_open_opts.read(true); - dir_open_opts.custom_flags(O_DIRECTORY); - let root_fd = crate::fs::File::from_inner(File::open(&path, &dir_open_opts)?); - // the current path relative to the root fd. - let mut current_path = PathBuf::from("."); +mod rmdir { + use crate::ffi::CStr; + + use crate::collections::HashSet; + use crate::ffi::OsStr; + use crate::fs::Metadata; + use crate::io; + use crate::io::Result; + use crate::os::unix::ffi::OsStrExt; + use crate::os::unix::fs::MetadataExt; + use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd}; + use crate::path::{Path, PathBuf}; + use crate::sys::fs::{File, OpenOptions, ReadDir}; + use crate::sys_common::FromInner; + use libc::{O_DIRECTORY, O_NOFOLLOW, O_PATH}; + + use super::{open_deep, unlinkat}; + + #[derive(PartialEq, Eq, Hash, Copy, Clone)] + struct DirId(u64, u64); + + impl From for DirId { + fn from(meta: Metadata) -> Self { + DirId(meta.dev(), meta.ino()) + } + } - let mut visited_stack: Vec<(u64, u64)> = Vec::new(); - let mut visited: HashSet<(u64, u64)> = HashSet::new(); + struct DirStack { + /// dirid specified the device id + inode number + /// for consistency checking. + /// The option carries the readdir and the file descriptor + /// from which it has been constructed. + /// Dropping the ReadDir invalidates the fd. + /// The first entry is the root directory whose + /// contents we want to delete. + dirs: Vec<(DirId, Option<(RawFd, ReadDir)>)>, + /// Each name component in the pathbuf represents + /// one entry in the `dirs` vec. + names: PathBuf, + /// Used for loop detection + visited: HashSet, + /// temporary buffer to construct a CStr for syscalls. + child_name_buffer: Vec, + } - let meta = root_fd.metadata()?; - let root_id = (meta.ino(), meta.dev()); - visited.insert(root_id); - visited_stack.push(root_id); + fn dir_open_options() -> OpenOptions { + let mut opts = OpenOptions::new(); + opts.read(true); + opts.custom_flags(O_DIRECTORY | O_NOFOLLOW); + opts + } const MAX_FDS: usize = 20; - let mut dir_stack: VecDeque<(RawFd, ReadDir)> = VecDeque::new(); - - let mut child_name_buf = Vec::new(); - let mut remove_current = false; - - 'tree_walk: loop { - if remove_current { - let name = current_path.file_name().expect("current path empty"); - child_name_buf.clear(); - child_name_buf.extend_from_slice(name.as_bytes()); - child_name_buf.push(0); - current_path.pop(); - let dir_id = visited_stack.pop().expect("visited stack empty"); - assert!(visited.remove(&dir_id)); + + impl DirStack { + fn new(root_fd: crate::fs::File) -> Result { + let mut stack = DirStack { + dirs: Vec::new(), + names: PathBuf::from("./"), + visited: HashSet::new(), + child_name_buffer: Vec::new(), + }; + + let meta = root_fd.metadata()?; + let root_id = meta.into(); + + let raw_fd = root_fd.as_raw_fd(); + let dirfd = OwnedFd::from(root_fd); + let reader = ReadDir::from_dirfd(dirfd, PathBuf::new())?; + + stack.visited.insert(root_id); + stack.dirs.push((root_id, Some((raw_fd, reader)))); + + stack.check_invariants(true); + + Ok(stack) } - let (current_dir_fd, mut current_reader) = match dir_stack.pop_back() { - Some(dir) => dir, - None => { - // when ascending out of a tree deeper than MAX_FDS the ReadDirs of the parents - // will have been dropped. reopen as needed. - let dirfd = OwnedFd::from(crate::fs::File::from_inner(open_deep( - Some(root_fd.as_fd()), - current_path.as_path(), - &dir_open_opts, - )?)); - // supply an empty pathbuf since we don't intend to use DirEntry::path() - let rawfd = dirfd.as_raw_fd(); - (rawfd, ReadDir::from_dirfd(dirfd, PathBuf::new())?) + fn ensure_open(&mut self) -> Result<()> { + self.check_invariants(false); + + // Don't refill ancestors until the last one has been popped off. + // Doing it in batches reduces traversal/checking costs. + if self.dirs.last().unwrap().1.is_none() { + let start = self.dirs.len().saturating_sub(MAX_FDS).max(1); + let end = self.dirs.len(); + for idx in start..end { + if self.dirs[idx].1.is_none() { + let nearest_ancestor_idx = self.dirs[..idx] + .iter() + .rposition(|(_, fd)| fd.is_some()) + .expect("some open ancestor"); + let (ancestor_id, ancestor_fd) = &self.dirs[nearest_ancestor_idx]; + let ancestor_id = *ancestor_id; + let mut path_components = self.names.components(); + path_components + .advance_by(nearest_ancestor_idx + 1) + .expect("advanced too far"); + path_components + .advance_back_by(self.dirs.len() - idx - 1) + .expect("advanced_back too far"); + let sub_path = path_components.as_path(); + let depth = idx - nearest_ancestor_idx; + + debug_assert_eq!(sub_path.components().count(), depth); + + let ancestor_fd = + unsafe { BorrowedFd::borrow_raw(ancestor_fd.as_ref().unwrap().0) }; + + // use open_deep since the relative path can be arbitrarily long + let dir = crate::fs::File::from_inner(open_deep( + Some(ancestor_fd), + sub_path, + &dir_open_options(), + )?); + let meta = dir.metadata()?; + let dir_id = DirId(meta.ino(), meta.dev()); + + let (expected_id, entry) = &mut self.dirs[idx]; + + // Security check to prevent TOCTOU attacks when re-traversing the hierarchy. + // `open_deep` follows symlinks, so verify that we arrive at the same spot + // we have been at before. + // But this leaves the possibility of a symlink + inode recycling race... + if dir_id != *expected_id { + return Err(io::const_io_error!( + io::ErrorKind::Uncategorized, + "directory with unexpected dev/ino", + )); + } + // ... so we make extra sure that the directory is a descendant by going back up via `../..` + // to the nearest still-open ancestor. The ancestor being open prevents inode recycling. + // This could be avoided if open_deep used used openat2(..., O_BENEATH) + // but that's only available in linux kernels >= 5.6 + if depth > 1 { + let mut buf = PathBuf::new(); + for _ in 0..depth { + buf.push("..") + } + let mut opts = OpenOptions::new(); + opts.read(true); + // open as path handle since we only want to stat it + opts.custom_flags(O_DIRECTORY | O_PATH); + + let ancestor_o_path = crate::fs::File::from_inner(open_deep( + Some(dir.as_fd()), + &buf, + &opts, + )?); + let meta = ancestor_o_path.metadata()?; + + if DirId(meta.dev(), meta.ino()) != ancestor_id { + return Err(io::const_io_error!( + io::ErrorKind::Uncategorized, + "unexpected ancestor dir dev/ino", + )); + } + } + + let dirfd = OwnedFd::from(dir); + let rawfd = dirfd.as_raw_fd(); + + // supply an empty pathbuf since we don't intend to use DirEntry::path() + *entry = Some((rawfd, ReadDir::from_dirfd(dirfd, PathBuf::new())?)); + } + } } - }; - let current_dir_fd = unsafe { BorrowedFd::borrow_raw(current_dir_fd) }; + self.check_invariants(true); - if remove_current { - let dir_name_c = - unsafe { CStr::from_bytes_with_nul_unchecked(child_name_buf.as_slice()) }; - unlinkat(current_dir_fd, dir_name_c, true)?; - remove_current = false; + Ok(()) } - while let Some(child) = current_reader.next() { - let child = child?; + fn sparsify(&mut self) { + self.check_invariants(true); + + // Keep the root fd and MAX_FDS tail entries open, close the rest. + // + // We could do something more clever here such as keeping a log(n) + // intermediate hops with expoenntial spacing to amortize reopening + // costs in very very deep directory trees. + for entry in self.dirs.iter_mut().skip(1).rev().skip(MAX_FDS) { + if entry.1.is_some() { + entry.1 = None; + } else { + break; + } + } + } - child_name_buf.clear(); - child_name_buf.extend_from_slice(child.file_name_os_str().as_bytes()); - child_name_buf.push(0); + fn check_invariants(&self, require_current_open: bool) { + debug_assert_eq!(self.names.components().count(), self.dirs.len()); + debug_assert!(self.dirs.len() > 0); + debug_assert!(self.dirs[0].1.is_some()); + if require_current_open { + debug_assert!(self.dirs.last().unwrap().1.is_some()); + } + } - let child_name_c = - unsafe { CStr::from_bytes_with_nul_unchecked(child_name_buf.as_slice()) }; + fn pop(&mut self) -> Result<()> { + self.check_invariants(true); + debug_assert!(self.dirs.len() > 1); - if child.file_type()?.is_dir() { - current_path.push(child.file_name_os_str()); + let (id, fd) = self.dirs.pop().unwrap(); + drop(fd); - let dir = crate::fs::File::from_inner(File::open_c( - Some(current_dir_fd), - child_name_c, - &dir_open_opts, - )?); + let name = self.names.file_name().expect("path should not be empty"); + let mut buf = crate::mem::take(&mut self.child_name_buffer); + buf.clear(); + buf.extend_from_slice(name.as_bytes()); + buf.push(0); - dir_stack.push_back((current_dir_fd.as_raw_fd(), current_reader)); + self.names.pop(); + self.visited.remove(&id); - // since we're traversing the directory tree via openat we have to do - // cycle-detection ourselves - let meta = dir.metadata()?; - let dir_id = (meta.ino(), meta.dev()); - if visited.contains(&dir_id) { - return Err(io::Error::from_raw_os_error(libc::ELOOP)); - } - visited_stack.push(dir_id); - visited.insert(dir_id); + self.ensure_open()?; - let dirfd = OwnedFd::from(dir); - let raw_fd = dirfd.as_raw_fd(); - let reader = ReadDir::from_dirfd(dirfd, PathBuf::new())?; - dir_stack.push_back((raw_fd, reader)); + let parent = self.dirs.last().expect("at least the root should still be open"); - if dir_stack.len() > MAX_FDS { - dir_stack.pop_front(); - } + let current_dir_fd = unsafe { BorrowedFd::borrow_raw(parent.1.as_ref().unwrap().0) }; - continue 'tree_walk; - } + let dir_name_c = unsafe { CStr::from_bytes_with_nul_unchecked(buf.as_slice()) }; + unlinkat(current_dir_fd, dir_name_c, true)?; + self.child_name_buffer = buf; - unlinkat(current_dir_fd, child_name_c, false)?; + Ok(()) } - // reached end of directory - if current_path.as_os_str().len() == 1 { - break; + fn push(&mut self, dir_name: &OsStr) -> Result<()> { + debug_assert!(dir_name.len() > 0); + self.check_invariants(true); + + let parent_fd = self.dirs.last().unwrap().1.as_ref().unwrap().0; + + let buf = &mut self.child_name_buffer; + buf.clear(); + buf.extend_from_slice(dir_name.as_bytes()); + buf.push(0); + let dir_name_c = unsafe { CStr::from_bytes_with_nul_unchecked(buf.as_slice()) }; + + let dir = crate::fs::File::from_inner(File::open_c( + Some(unsafe { BorrowedFd::borrow_raw(parent_fd) }), + dir_name_c, + &dir_open_options(), + )?); + let meta = dir.metadata()?; + let dir_id = DirId(meta.ino(), meta.dev()); + + if !self.visited.insert(dir_id) { + return Err(io::Error::from_raw_os_error(libc::ELOOP)); + } + + let dirfd = OwnedFd::from(dir); + let rawfd = dirfd.as_raw_fd(); + + let entry = (dir_id, Some((rawfd, ReadDir::from_dirfd(dirfd, PathBuf::new())?))); + + self.dirs.push(entry); + self.names.push(dir_name); + + self.sparsify(); + + Ok(()) } - remove_current = true; + fn walk_tree(&mut self) -> Result<()> { + loop { + self.check_invariants(true); + let (fd, current_dir) = self + .dirs + .last_mut() + .expect("at least the root should be open") + .1 + .as_mut() + .expect("the last entry shouldn't have its FD closed"); + + match current_dir.next() { + Some(child) => { + let child = child?; + let child_name = child.file_name(); + + if child.file_type()?.is_dir() { + self.push(&child_name)?; + continue; + } + + let buf = &mut self.child_name_buffer; + buf.clear(); + buf.extend_from_slice(child_name.as_os_str().as_bytes()); + buf.push(0); + let child_name_c = + unsafe { CStr::from_bytes_with_nul_unchecked(buf.as_slice()) }; + + let fd = unsafe { BorrowedFd::borrow_raw(*fd) }; + unlinkat(fd, child_name_c, false)?; + } + None if self.dirs.len() > 1 => { + self.pop()?; + } + None => break, + } + } + + Ok(()) + } } - // There's no dirfd to reuse here since unlinking the starting point requires unlinking relative to - // the parent directory. '..' also cannot be used because that is vulnerable to the directory - // being moved. And we couldn't have started with the parent directory either because the starting - // point may be '/' or '.' which can't be removed themselves but we still should be able to clean - // their descendants. - super::rmdir(path) + pub(super) fn remove_dir_all_iter(path: &Path) -> Result<()> { + let root_fd = crate::fs::File::from_inner(File::open(&path, &dir_open_options())?); + + let mut stack = DirStack::new(root_fd)?; + stack.walk_tree()?; + + // There's no dirfd to reuse here since unlinking the starting point requires unlinking relative to + // the parent directory. '..' also cannot be used because that is vulnerable to the directory + // being moved. And we couldn't have started with the parent directory either because the starting + // point may be '/' or '.' which can't be removed themselves but we still should be able to clean + // their descendants. + crate::sys::fs::rmdir(path) + } }