From 072c32d467b3afbd8738e34958ef487ca00b69c2 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sun, 21 Apr 2024 17:19:15 +0200 Subject: [PATCH] put FD validity behind late debug_asserts checking uses the same machinery as assert_unsafe_precondition --- library/std/src/os/fd/owned.rs | 12 +------- library/std/src/sys/pal/unix/fs.rs | 47 ++++++++++++++++++------------ 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs index 8c421540af40e..9970827fec8cc 100644 --- a/library/std/src/os/fd/owned.rs +++ b/library/std/src/os/fd/owned.rs @@ -177,17 +177,7 @@ impl Drop for OwnedFd { // opened after we closed ours. #[cfg(not(target_os = "hermit"))] { - 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"); - } - } + crate::sys::fs::debug_assert_fd_is_open(self.fd); let _ = libc::close(self.fd); } #[cfg(target_os = "hermit")] diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index 2d57a9d81c911..0a86d28c2e3b3 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -868,28 +868,39 @@ 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) { - // 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", - )) - ))] + // dirfd isn't supported everywhere + #[cfg(not(any( + miri, + 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"); - } + debug_assert_fd_is_open(fd); } let r = unsafe { libc::closedir(self.0) }; assert!(