diff options
| author | bors <bors@rust-lang.org> | 2022-12-18 05:04:04 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-12-18 05:04:04 +0000 |
| commit | 48b3c4612681ba7828880aa9e675452c62714bbf (patch) | |
| tree | 37dabe79aeb55bb603c8768455128a87ea06632d | |
| parent | ff016a55c24d061a479f9116f9cd8edc5594eb9e (diff) | |
| parent | 9fb7c5ae5e66703240ce4242f55b66a066e0a93c (diff) | |
| download | rust-48b3c4612681ba7828880aa9e675452c62714bbf.tar.gz rust-48b3c4612681ba7828880aa9e675452c62714bbf.zip | |
Auto merge of #105638 - tavianator:fix-50619-again, r=Mark-Simulacrum
fs: Fix #50619 (again) and add a regression test Bug #50619 was fixed by adding an end_of_stream flag in #50630. Unfortunately, that fix only applied to the readdir_r() path. When I switched Linux to use readdir() in #92778, I inadvertently reintroduced the bug on that platform. Other platforms that had always used readdir() were presumably never fixed. This patch enables end_of_stream for all platforms, and adds a Linux-specific regression test that should hopefully prevent the bug from being reintroduced again.
| -rw-r--r-- | library/std/src/fs/tests.rs | 28 | ||||
| -rw-r--r-- | library/std/src/sys/unix/fs.rs | 57 |
2 files changed, 47 insertions, 38 deletions
diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 4748ac9d97e..b385ebde439 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1567,3 +1567,31 @@ fn test_eq_direntry_metadata() { assert_eq!(ft1, ft2); } } + +/// Regression test for https://github.com/rust-lang/rust/issues/50619. +#[test] +#[cfg(target_os = "linux")] +fn test_read_dir_infinite_loop() { + use crate::io::ErrorKind; + use crate::process::Command; + + // Create a zombie child process + let Ok(mut child) = Command::new("echo").spawn() else { return }; + + // Make sure the process is (un)dead + match child.kill() { + // InvalidInput means the child already exited + Err(e) if e.kind() != ErrorKind::InvalidInput => return, + _ => {} + } + + // open() on this path will succeed, but readdir() will fail + let id = child.id(); + let path = format!("/proc/{id}/net"); + + // Skip the test if we can't open the directory in the first place + let Ok(dir) = fs::read_dir(path) else { return }; + + // Check for duplicate errors + assert!(dir.filter(|e| e.is_err()).take(2).count() < 2); +} diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 818914a2df0..d5f50d77911 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -243,17 +243,15 @@ struct InnerReadDir { pub struct ReadDir { inner: Arc<InnerReadDir>, - #[cfg(not(any( - target_os = "android", - target_os = "linux", - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox", - )))] end_of_stream: bool, } +impl ReadDir { + fn new(inner: InnerReadDir) -> Self { + Self { inner: Arc::new(inner), end_of_stream: false } + } +} + struct Dir(*mut libc::DIR); unsafe impl Send for Dir {} @@ -594,6 +592,10 @@ impl Iterator for ReadDir { target_os = "illumos" ))] fn next(&mut self) -> Option<io::Result<DirEntry>> { + if self.end_of_stream { + return None; + } + unsafe { loop { // As of POSIX.1-2017, readdir() is not required to be thread safe; only @@ -604,8 +606,12 @@ impl Iterator for ReadDir { super::os::set_errno(0); let entry_ptr = readdir64(self.inner.dirp.0); if entry_ptr.is_null() { - // null can mean either the end is reached or an error occurred. - // So we had to clear errno beforehand to check for an error now. + // We either encountered an error, or reached the end. Either way, + // the next call to next() should return None. + self.end_of_stream = true; + + // To distinguish between errors and end-of-directory, we had to clear + // errno beforehand to check for an error now. return match super::os::errno() { 0 => None, e => Some(Err(Error::from_raw_os_error(e))), @@ -1363,18 +1369,7 @@ pub fn readdir(path: &Path) -> io::Result<ReadDir> { } else { let root = path.to_path_buf(); let inner = InnerReadDir { dirp: Dir(ptr), root }; - Ok(ReadDir { - inner: Arc::new(inner), - #[cfg(not(any( - target_os = "android", - target_os = "linux", - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox", - )))] - end_of_stream: false, - }) + Ok(ReadDir::new(inner)) } } @@ -1755,7 +1750,6 @@ mod remove_dir_impl { use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd}; use crate::os::unix::prelude::{OwnedFd, RawFd}; use crate::path::{Path, PathBuf}; - use crate::sync::Arc; use crate::sys::common::small_c_string::run_path_with_cstr; use crate::sys::{cvt, cvt_r}; @@ -1832,21 +1826,8 @@ mod remove_dir_impl { // a valid root is not needed because we do not call any functions involving the full path // of the DirEntrys. let dummy_root = PathBuf::new(); - Ok(( - ReadDir { - inner: Arc::new(InnerReadDir { dirp, root: dummy_root }), - #[cfg(not(any( - target_os = "android", - target_os = "linux", - target_os = "solaris", - target_os = "illumos", - target_os = "fuchsia", - target_os = "redox", - )))] - end_of_stream: false, - }, - new_parent_fd, - )) + let inner = InnerReadDir { dirp, root: dummy_root }; + Ok((ReadDir::new(inner), new_parent_fd)) } #[cfg(any( |
