Skip to content

Commit

Permalink
put FD validity behind late debug_asserts checking
Browse files Browse the repository at this point in the history
uses the same machinery as assert_unsafe_precondition
  • Loading branch information
the8472 committed Apr 22, 2024
1 parent 25babe9 commit 072c32d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 29 deletions.
12 changes: 1 addition & 11 deletions library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
47 changes: 29 additions & 18 deletions library/std/src/sys/pal/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down

0 comments on commit 072c32d

Please sign in to comment.