From 2e09bf1a1861c7ba7e437c8564c777212e75744e 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 | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 010ce4e5076ba..8c421540af40e 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -176,7 +176,20 @@ 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); + { + 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 3456155509ef3..b63749e7d2ad6 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -870,6 +870,26 @@ impl Iterator for ReadDir { 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", + )) + ))] + { + // 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 = libc::dirfd(self.0); + if 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(),