Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: Fix #50619 (again) and add a regression test #105638

Merged
merged 3 commits into from
Dec 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
57 changes: 19 additions & 38 deletions library/std/src/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down Expand Up @@ -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
Expand All @@ -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))),
Expand Down Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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};

Expand Down Expand Up @@ -1827,21 +1821,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(
Expand Down