about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-12-18 05:04:04 +0000
committerbors <bors@rust-lang.org>2022-12-18 05:04:04 +0000
commit48b3c4612681ba7828880aa9e675452c62714bbf (patch)
tree37dabe79aeb55bb603c8768455128a87ea06632d
parentff016a55c24d061a479f9116f9cd8edc5594eb9e (diff)
parent9fb7c5ae5e66703240ce4242f55b66a066e0a93c (diff)
downloadrust-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.rs28
-rw-r--r--library/std/src/sys/unix/fs.rs57
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(