Skip to content

Commit 67a004a

Browse files
committed
put FD validity behind late debug_asserts checking
uses the same machinery as assert_unsafe_precondition
1 parent ae183cf commit 67a004a

File tree

2 files changed

+30
-29
lines changed

2 files changed

+30
-29
lines changed

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

+1-11
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,7 @@ impl Drop for OwnedFd {
177177
// opened after we closed ours.
178178
#[cfg(not(target_os = "hermit"))]
179179
{
180-
use crate::sys::os::errno;
181-
// ideally this would use assert_unsafe_precondition!, but that's only in core
182-
if cfg!(debug_assertions) {
183-
// close() can bubble up error codes from FUSE which can send semantically
184-
// inappropriate error codes including EBADF.
185-
// So we check file flags instead which live on the file descriptor and not the underlying file.
186-
// The downside is that it costs an extra syscall, so we only do it for debug.
187-
if libc::fcntl(self.fd, libc::F_GETFD) == -1 && errno() == libc::EBADF {
188-
rtabort!("IO Safety violation: owned file descriptor already closed");
189-
}
190-
}
180+
crate::sys::fs::debug_assert_fd_is_open(self.fd);
191181
let _ = libc::close(self.fd);
192182
}
193183
#[cfg(target_os = "hermit")]

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

+29-18
Original file line numberDiff line numberDiff line change
@@ -868,28 +868,39 @@ impl Iterator for ReadDir {
868868
}
869869
}
870870

871+
/// Aborts the process if a file desceriptor is not open, if debug asserts are enabled
872+
///
873+
/// Many IO syscalls can't be fully trusted about EBADF error codes because those
874+
/// might get bubbled up from a remote FUSE server rather than the file descriptor
875+
/// in the current process being invalid.
876+
///
877+
/// So we check file flags instead which live on the file descriptor and not the underlying file.
878+
/// The downside is that it costs an extra syscall, so we only do it for debug.
879+
#[inline]
880+
pub(crate) fn debug_assert_fd_is_open(fd: RawFd) {
881+
use crate::sys::os::errno;
882+
883+
// this is similar to assert_unsafe_precondition!() but it doesn't require const
884+
if core::ub_checks::check_library_ub() {
885+
if unsafe { libc::fcntl(fd, libc::F_GETFD) } == -1 && errno() == libc::EBADF {
886+
rtabort!("IO Safety violation: owned file descriptor already closed");
887+
}
888+
}
889+
}
890+
871891
impl Drop for Dir {
872892
fn drop(&mut self) {
873-
// ideally this would use assert_unsafe_precondition!, but that's only in core
874-
#[cfg(all(
875-
debug_assertions,
876-
not(any(
877-
target_os = "redox",
878-
target_os = "nto",
879-
target_os = "vita",
880-
target_os = "hurd",
881-
))
882-
))]
893+
// dirfd isn't supported everywhere
894+
#[cfg(not(any(
895+
miri,
896+
target_os = "redox",
897+
target_os = "nto",
898+
target_os = "vita",
899+
target_os = "hurd",
900+
)))]
883901
{
884-
use crate::sys::os::errno;
885-
// close() can bubble up error codes from FUSE which can send semantically
886-
// inappropriate error codes including EBADF.
887-
// So we check file flags instead which live on the file descriptor and not the underlying file.
888-
// The downside is that it costs an extra syscall, so we only do it for debug.
889902
let fd = unsafe { libc::dirfd(self.0) };
890-
if unsafe { libc::fcntl(fd, libc::F_GETFD) } == -1 && errno() == libc::EBADF {
891-
rtabort!("IO Safety violation: DIR*'s owned file descriptor already closed");
892-
}
903+
debug_assert_fd_is_open(fd);
893904
}
894905
let r = unsafe { libc::closedir(self.0) };
895906
assert!(

0 commit comments

Comments
 (0)