Skip to content

Commit

Permalink
Abort a process when FD ownership is violated
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
the8472 committed Apr 20, 2024
1 parent c5de414 commit 2e09bf1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
15 changes: 14 additions & 1 deletion library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
20 changes: 20 additions & 0 deletions library/std/src/sys/pal/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 2e09bf1

Please sign in to comment.