From 38ded129236f112a7421f311aeb8ca750b661443 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Thu, 18 Apr 2024 20:03:45 +0200 Subject: [PATCH] Abort a process when FD ownership is violated When an EBADF happens then something else already 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. --- library/std/src/os/fd/owned.rs | 15 ++++++++++++++- library/std/src/sys/pal/unix/fs.rs | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 010ce4e5076..8c421540af4 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -176,7 +176,20 @@ fn drop(&mut self) { // 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); + { + use crate::sys::os::errno; + // ideally this would use assert_unsafe_precondition!, but that's only in core + if cfg!(debug_assertions) { + // close() can bubble up error codes from FUSE which can send semantically + // inappropriate error codes including EBADF. + // 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. + if libc::fcntl(self.fd, libc::F_GETFD) == -1 && errno() == libc::EBADF { + rtabort!("IO Safety violation: owned file descriptor already closed"); + } + } + let _ = libc::close(self.fd); + } #[cfg(target_os = "hermit")] let _ = hermit_abi::close(self.fd); } diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index 3456155509e..2d57a9d81c9 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -870,6 +870,27 @@ fn next(&mut self) -> Option> { impl Drop for Dir { fn drop(&mut self) { + // ideally this would use assert_unsafe_precondition!, but that's only in core + #[cfg(all( + debug_assertions, + not(any( + target_os = "redox", + target_os = "nto", + target_os = "vita", + target_os = "hurd", + )) + ))] + { + use crate::sys::os::errno; + // close() can bubble up error codes from FUSE which can send semantically + // inappropriate error codes including EBADF. + // 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. + let fd = unsafe { libc::dirfd(self.0) }; + if unsafe { libc::fcntl(fd, libc::F_GETFD) } == -1 && errno() == libc::EBADF { + rtabort!("IO Safety violation: DIR*'s owned file descriptor already closed"); + } + } let r = unsafe { libc::closedir(self.0) }; assert!( r == 0 || crate::io::Error::last_os_error().is_interrupted(),