Skip to content

add more explicit I/O safety documentation #114780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 22, 2023
42 changes: 41 additions & 1 deletion library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! the [`Read`] and [`Write`] traits, which provide the
//! most general interface for reading and writing input and output.
//!
//! # Read and Write
//! ## Read and Write
//!
//! Because they are traits, [`Read`] and [`Write`] are implemented by a number
//! of other types, and you can implement them for your types too. As such,
Expand Down Expand Up @@ -238,13 +238,53 @@
//! contract. The implementation of many of these functions are subject to change over
//! time and may call fewer or more syscalls/library functions.
//!
//! ## I/O Safety
//!
//! Rust follows an [I/O safety] discipline that is comparable to its memory safety discipline. This
//! means that file descriptors can be *exclusively owned*. (Here, "file descriptor" is meant to
//! subsume similar concepts that exist across a wide range of operating systems even if they might
//! use a different name, such as "handle".) An exclusively owned file descriptor is one that no
//! other code is allowed to close, but the owner is allowed to close it any time. A type that owns
//! its file descriptor should usually close it in its `drop` function. Types like [`File`] generally own
//! their file descriptor. Similarly, file descriptors can be *borrowed*. This indicates that the
//! file descriptor will not be closed for the lifetime of the borrow, but it does *not* imply any
//! right to close this file descriptor, since it will likely be owned by someone else.
//!
//! The platform-specific parts of the Rust standard library expose types that reflect these
//! concepts, see [`os::unix`] and [`os::windows`].
//!
//! To uphold I/O safety, it is crucial that no code closes file descriptors it does not own. In
//! other words, a safe function that takes a regular integer, treats it as a file descriptor, and
//! closes it, is *unsound*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intent with I/O safety is a stronger rule than just "only the owner is allowed to close". I also intend "and no one can do any operation, including read, write, or anything else, with a file descriptor that is closed".

Without this stronger rule, it would be impossible to ever encapsulate a file description, because things like dup(rand()) would become permitted, and that would preclude some useful use cases.

Copy link
Member Author

@RalfJung RalfJung Aug 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this stronger rule, it would be impossible to ever encapsulate a file description, because things like dup(rand()) would become permitted, and that would preclude some useful use cases.

Given that I can dup a file descriptor that I have merely borrowed, most of these usecases will already not work -- at least they won't work with OwnedFd. I was hoping we'd have that "you cannot write an FD that you do not own", but we do not have that guarantee with OwnedFd and it is unclear if there is any way to protect an FD against being written-to by other parts of the code.

The OS will reliably return EBADFD on any operation if the FD is closed, so there's not really any point in disallowing this that I can see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File description encapsulation requires encapsulating the OwnedFd and not implementing AsFd or handing out a BorrowedFd. So it takes some deliberate doing, but it is doable, as long as we disallow things like dup(rand()) and read(rand(), ...) and so on.

The OS is documented to return EBADF, however any time you get that error on a closed fd, it only means you got lucky that no code, perhaps on another thread racing with you, did eg. File::open and reused the fd number.

Copy link
Member Author

@RalfJung RalfJung Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(let's merge this with the other thread about file object encapsulation)

//!
//! Not upholding I/O safety and closing a file descriptor without proof of ownership can lead to
//! misbehavior and even Undefined Behavior in code that relies on ownership of its file
//! descriptors: the closed file descriptor could be re-allocated to some other library (such as the
//! allocator or a memory mapping library) and now accessing the file descriptor will interfere in
//! arbitrarily destructive ways with that other library.
//!
//! Note that this does not talk about performing other operations on the file descriptor, such as
//! reading or writing. For example, on Unix, the [`OwnedFd`] and [`BorrowedFd`] types from the
//! standard library do *not* exclude that there is other code that reads or writes the same
//! underlying object, and indeed there exist safe functions like `BorrowedFd::try_clone_to_owned`
//! that can be used to read or write an object even after the end of the borrow. However, user code
//! might want to rely on keeping the object behind a file descriptor completely private and
//! protected against reads or writes from other parts of the program. Whether that is sound is
//! [currently unclear](https://github.com/rust-lang/rust/issues/114167). Certainly, `OwnedFd` as a
//! type does not provide any promise that the underlying file descriptor has not been cloned.
//!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: can we also list some of the risks of violating IO safety?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally, exhaustively if possible

roughly: if you are accessing an fd after it has been closed, it might have been reallocated to belong to someone else, which might be the allocator, or memory mapping libraries, or something else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a paragraph. However, without full file description encapsulation, I am not sure if there even is an example where this can truly lead to UB?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//!
//!
//! Note that functions like `pidfd_getfd` or debugging interfaces are out of scope.
//!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably say a bit about what that means, too, similar to https://doc.rust-lang.org/nightly/std/os/unix/io/index.html#procselfmem-and-similar-os-features? Basically we are saying Rust programs must not abuse these APIs, so "out of scope" is not a free pass to use them to circumvent everything. They can be used for debugging/tracing only.

//! [`File`]: crate::fs::File
//! [`TcpStream`]: crate::net::TcpStream
//! [`io::stdout`]: stdout
//! [`io::Result`]: self::Result
//! [`?` operator]: ../../book/appendix-02-operators.html
//! [`Result`]: crate::result::Result
//! [`.unwrap()`]: crate::result::Result::unwrap
//! [I/O safety]: https://rust-lang.github.io/rfcs/3128-io-safety.html
//! [`os::unix`]: ../os/unix/io/index.html
//! [`os::windows`]: ../os/windows/io/index.html
//! [`OwnedFd`]: ../os/fd/struct.OwnedFd.html
//! [`BorrowedFd`]: ../os/fd/struct.BorrowedFd.html

#![stable(feature = "rust1", since = "1.0.0")]

Expand Down
12 changes: 8 additions & 4 deletions library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use crate::sys_common::{AsInner, FromInner, IntoInner};

/// A borrowed file descriptor.
///
/// This has a lifetime parameter to tie it to the lifetime of something that
/// owns the file descriptor.
/// This has a lifetime parameter to tie it to the lifetime of something that owns the file
/// descriptor. For the duration of that lifetime, it is guaranteed that nobody will close the file
/// descriptor.
///
/// This uses `repr(transparent)` and has the representation of a host file
/// descriptor, so it can be used in FFI in places where a file descriptor is
Expand All @@ -42,7 +43,8 @@ pub struct BorrowedFd<'fd> {

/// An owned file descriptor.
///
/// This closes the file descriptor on drop.
/// This closes the file descriptor on drop. It is guaranteed that nobody else will close the file
/// descriptor.
///
/// This uses `repr(transparent)` and has the representation of a host file
/// descriptor, so it can be used in FFI in places where a file descriptor is
Expand Down Expand Up @@ -155,7 +157,9 @@ impl FromRawFd for OwnedFd {
/// # Safety
///
/// The resource pointed to by `fd` must be open and suitable for assuming
/// ownership. The resource must not require any cleanup other than `close`.
/// [ownership][io-safety]. The resource must not require any cleanup other than `close`.
///
/// [io-safety]: io#io-safety
#[inline]
unsafe fn from_raw_fd(fd: RawFd) -> Self {
assert_ne!(fd, u32::MAX as RawFd);
Expand Down
5 changes: 4 additions & 1 deletion library/std/src/os/fd/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ pub trait FromRawFd {
///
/// # Safety
///
/// The `fd` passed in must be a valid and open file descriptor.
/// The `fd` passed in must be an [owned file descriptor][io-safety];
/// in particular, it must be open.
///
/// [io-safety]: io#io-safety
///
/// # Example
///
Expand Down
23 changes: 15 additions & 8 deletions library/std/src/os/fortanix_sgx/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,22 @@ pub trait FromRawFd {
/// Constructs a new instance of `Self` from the given raw file
/// descriptor and metadata.
///
/// This function **consumes ownership** of the specified file
/// descriptor. The returned object will take responsibility for closing
/// it when the object goes out of scope.
/// This function is typically used to **consume ownership** of the
/// specified file descriptor. When used in this way, the returned object
/// will take responsibility for closing it when the object goes out of
/// scope.
///
/// This function is also unsafe as the primitives currently returned
/// have the contract that they are the sole owner of the file
/// descriptor they are wrapping. Usage of this function could
/// accidentally allow violating this contract which can cause memory
/// unsafety in code that relies on it being true.
/// However, consuming ownership is not strictly required. Use a
/// [`From<OwnedFd>::from`] implementation for an API which strictly
/// consumes ownership.
///
/// # Safety
///
/// The `fd` passed in must be an [owned file descriptor][io-safety];
/// in particular, it must be open.
// FIXME: say something about `metadata`.
///
/// [io-safety]: io#io-safety
#[unstable(feature = "sgx_platform", issue = "56975")]
unsafe fn from_raw_fd(fd: RawFd, metadata: Self::Metadata) -> Self;
}
Expand Down
22 changes: 14 additions & 8 deletions library/std/src/os/solid/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,21 @@ pub trait FromRawFd {
/// Constructs a new instance of `Self` from the given raw file
/// descriptor.
///
/// This function **consumes ownership** of the specified file
/// descriptor. The returned object will take responsibility for closing
/// it when the object goes out of scope.
/// This function is typically used to **consume ownership** of the
/// specified file descriptor. When used in this way, the returned object
/// will take responsibility for closing it when the object goes out of
/// scope.
///
/// This function is also unsafe as the primitives currently returned
/// have the contract that they are the sole owner of the file
/// descriptor they are wrapping. Usage of this function could
/// accidentally allow violating this contract which can cause memory
/// unsafety in code that relies on it being true.
/// However, consuming ownership is not strictly required. Use a
/// [`From<OwnedFd>::from`] implementation for an API which strictly
/// consumes ownership.
///
/// # Safety
///
/// The `fd` passed in must be an [owned file descriptor][io-safety];
/// in particular, it must be open.
///
/// [io-safety]: io#io-safety
unsafe fn from_raw_fd(fd: RawFd) -> Self;
}

Expand Down
9 changes: 5 additions & 4 deletions library/std/src/os/unix/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//!
//! This module provides three types for representing file descriptors,
//! with different ownership properties: raw, borrowed, and owned, which are
//! analogous to types used for representing pointers:
//! analogous to types used for representing pointers. These types reflect the Unix version of [I/O safety].
//!
//! | Type | Analogous to |
//! | ------------------ | ------------ |
Expand Down Expand Up @@ -65,15 +65,16 @@
//! to be opened and read from or written must be `unsafe`. Rust's safety guarantees
//! only cover what the program itself can do, and not what entities outside
//! the program can do to it. `/proc/self/mem` is considered to be such an
//! external entity, along with debugging interfaces, and people with physical access to
//! the hardware. This is true even in cases where the program is controlling
//! the external entity.
//! external entity, along with `/proc/self/fd/*`, debugging interfaces, and people with physical
//! access to the hardware. This is true even in cases where the program is controlling the external
//! entity.
//!
//! If you desire to comprehensively prevent programs from reaching out and
//! causing external entities to reach back in and violate memory safety, it's
//! necessary to use *sandboxing*, which is outside the scope of `std`.
//!
//! [`BorrowedFd<'a>`]: crate::os::unix::io::BorrowedFd
//! [I/O safety]: crate::io#io-safety

#![stable(feature = "rust1", since = "1.0.0")]

Expand Down
3 changes: 2 additions & 1 deletion library/std/src/os/windows/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//!
//! This module provides three types for representing raw handles and sockets
//! with different ownership properties: raw, borrowed, and owned, which are
//! analogous to types used for representing pointers:
//! analogous to types used for representing pointers. These types reflect the Windows version of [I/O safety].
//!
//! | Type | Analogous to |
//! | ---------------------- | ------------ |
Expand Down Expand Up @@ -47,6 +47,7 @@
//!
//! [`BorrowedHandle<'a>`]: crate::os::windows::io::BorrowedHandle
//! [`BorrowedSocket<'a>`]: crate::os::windows::io::BorrowedSocket
//! [I/O safety]: crate::io#io-safety

#![stable(feature = "rust1", since = "1.0.0")]

Expand Down
6 changes: 4 additions & 2 deletions library/std/src/os/windows/io/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub trait FromRawHandle {
/// # Safety
///
/// The `handle` passed in must:
/// - be a valid an open handle,
/// - be an [owned handle][io-safety]; in particular, it must be open.
/// - be a handle for a resource that may be freed via [`CloseHandle`]
/// (as opposed to `RegCloseKey` or other close functions).
///
Expand All @@ -71,6 +71,7 @@ pub trait FromRawHandle {
///
/// [`CloseHandle`]: https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle
/// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
/// [io-safety]: io#io-safety
#[stable(feature = "from_raw_os", since = "1.1.0")]
unsafe fn from_raw_handle(handle: RawHandle) -> Self;
}
Expand Down Expand Up @@ -207,10 +208,11 @@ pub trait FromRawSocket {
/// # Safety
///
/// The `socket` passed in must:
/// - be a valid an open socket,
/// - be an [owned socket][io-safety]; in particular, it must be open.
/// - be a socket that may be freed via [`closesocket`].
///
/// [`closesocket`]: https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-closesocket
/// [io-safety]: io#io-safety
#[stable(feature = "from_raw_os", since = "1.1.0")]
unsafe fn from_raw_socket(sock: RawSocket) -> Self;
}
Expand Down