Auto merge of rust-lang#124210 - the8472:consign-ebadf-to-the-fire, r=Mark-Simulacrum

Abort a process when FD ownership is violated

When an owned FD has already been closed before it's dropped that means something else touched an FD in ways it is not allowed to. At that point things can already be arbitrarily bad, e.g. clobbered mmaps. Recovery is not possible.
All we can do is hasten the fire.

Unlike the previous attempt in rust-lang#124130 this shouldn't suffer from the possibility that FUSE filesystems can return arbitrary errors.

Author: bors

library/core/src/intrinsics.rs

@@ -2767,7 +2767,7 @@ pub const unsafe fn typed_swap<T>(x: *mut T, y: *mut T) {
 #[unstable(feature = "core_intrinsics", issue = "none")]
 #[inline(always)]
 #[cfg_attr(not(bootstrap), rustc_intrinsic)] // just make it a regular fn in bootstrap
-pub(crate) const fn ub_checks() -> bool {
+pub const fn ub_checks() -> bool {
     cfg!(debug_assertions)
 }
 

library/core/src/lib.rs

@@ -193,6 +193,7 @@
 #![feature(str_split_inclusive_remainder)]
 #![feature(str_split_remainder)]
 #![feature(strict_provenance)]
+#![feature(ub_checks)]
 #![feature(unchecked_shifts)]
 #![feature(utf16_extra)]
 #![feature(utf16_extra_const)]
@@ -370,7 +371,8 @@ pub mod hint;
 pub mod intrinsics;
 pub mod mem;
 pub mod ptr;
-mod ub_checks;
+#[unstable(feature = "ub_checks", issue = "none")]
+pub mod ub_checks;
 
 /* Core language traits */
 

library/core/src/ub_checks.rs

@@ -46,6 +46,8 @@ use crate::intrinsics::{self, const_eval_select};
 /// variables cannot be optimized out in MIR, an innocent-looking `let` can produce enough
 /// debuginfo to have a measurable compile-time impact on debug builds.
 #[allow_internal_unstable(const_ub_checks)] // permit this to be called in stably-const fn
+#[macro_export]
+#[unstable(feature = "ub_checks", issue = "none")]
 macro_rules! assert_unsafe_precondition {
     ($kind:ident, $message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => {
         {
@@ -75,11 +77,13 @@ macro_rules! assert_unsafe_precondition {
         }
     };
 }
-pub(crate) use assert_unsafe_precondition;
+#[unstable(feature = "ub_checks", issue = "none")]
+pub use assert_unsafe_precondition;
 
 /// Checking library UB is always enabled when UB-checking is done
 /// (and we use a reexport so that there is no unnecessary wrapper function).
-pub(crate) use intrinsics::ub_checks as check_library_ub;
+#[unstable(feature = "ub_checks", issue = "none")]
+pub use intrinsics::ub_checks as check_library_ub;
 
 /// Determines whether we should check for language UB.
 ///

library/std/src/lib.rs

@@ -356,6 +356,7 @@
 #![feature(str_internals)]
 #![feature(strict_provenance)]
 #![feature(strict_provenance_atomic_ptr)]
+#![feature(ub_checks)]
 // tidy-alphabetical-end
 //
 // Library features (alloc):

library/std/src/os/fd/owned.rs

@@ -176,7 +176,12 @@ impl Drop for OwnedFd {
             // something like EINTR), we might close another valid file descriptor
             // opened after we closed ours.
             #[cfg(not(target_os = "hermit"))]
-            let _ = libc::close(self.fd);
+            {
+                #[cfg(unix)]
+                crate::sys::fs::debug_assert_fd_is_open(self.fd);
+
+                let _ = libc::close(self.fd);
+            }
             #[cfg(target_os = "hermit")]
             let _ = hermit_abi::close(self.fd);
         }

library/std/src/sys/pal/unix/fs.rs

@@ -864,8 +864,40 @@ impl Iterator for ReadDir {
     }
 }
 
+/// Aborts the process if a file desceriptor is not open, if debug asserts are enabled
+///
+/// Many IO syscalls can't be fully trusted about EBADF error codes because those
+/// might get bubbled up from a remote FUSE server rather than the file descriptor
+/// in the current process being invalid.
+///
+/// So we check file flags instead which live on the file descriptor and not the underlying file.
+/// The downside is that it costs an extra syscall, so we only do it for debug.
+#[inline]
+pub(crate) fn debug_assert_fd_is_open(fd: RawFd) {
+    use crate::sys::os::errno;
+
+    // this is similar to assert_unsafe_precondition!() but it doesn't require const
+    if core::ub_checks::check_library_ub() {
+        if unsafe { libc::fcntl(fd, libc::F_GETFD) } == -1 && errno() == libc::EBADF {
+            rtabort!("IO Safety violation: owned file descriptor already closed");
+        }
+    }
+}
+
 impl Drop for Dir {
     fn drop(&mut self) {
+        // dirfd isn't supported everywhere
+        #[cfg(not(any(
+            miri,
+            target_os = "redox",
+            target_os = "nto",
+            target_os = "vita",
+            target_os = "hurd",
+        )))]
+        {
+            let fd = unsafe { libc::dirfd(self.0) };
+            debug_assert_fd_is_open(fd);
+        }
         let r = unsafe { libc::closedir(self.0) };
         assert!(
             r == 0 || crate::io::Error::last_os_error().is_interrupted(),