Auto merge of #114780 - RalfJung:io-safety, r=Amanieu

add more explicit I/O safety documentation

Fixes https://github.com/rust-lang/unsafe-code-guidelines/issues/434
Cc https://github.com/rust-lang/rust/issues/114167
Cc `@Manishearth` `@sunfishcode` `@joshtriplett`

8 files changed, 99 insertions, 29 deletions

diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index eca64fb6063..7582c7444f0 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -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,
@@ -238,6 +238,47 @@
 //! 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 access in any way, but the owner is allowed to access and even close
+//! it any time. A type that owns its file descriptor should usually close it in its `drop`
+//! function. Types like [`File`] own their file descriptor. Similarly, file descriptors
+//! can be *borrowed*, granting the temporary right to perform operations on this file descriptor.
+//! 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 acts on file descriptors it does not own or
+//! borrow, and 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 acts on it, is *unsound*.
+//!
+//! Not upholding I/O safety and acting on 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: a closed file descriptor could be re-allocated, so the original owner of that file
+//! descriptor is now working on the wrong file. Some code might even rely on fully encapsulating
+//! its file descriptors with no operations being performed by any other part of the program.
+//!
+//! Note that exclusive ownership of a file descriptor does *not* imply exclusive ownership of the
+//! underlying kernel object that the file descriptor references (also called "file description" on
+//! some operating systems). File descriptors basically work like [`Arc`]: when you receive an owned
+//! file descriptor, you cannot know whether there are any other file descriptors that reference the
+//! same kernel object. However, when you create a new kernel object, you know that you are holding
+//! the only reference to it. Just be careful not to lend it to anyone, since they can obtain a
+//! clone and then you can no longer know what the reference count is! In that sense, [`OwnedFd`] is
+//! like `Arc` and [`BorrowedFd<'a>`] is like `&'a Arc` (and similar for the Windows types). In
+//! particular, given a `BorrowedFd<'a>`, you are not allowed to close the file descriptor -- just
+//! like how, given a `&'a Arc`, you are not allowed to decrement the reference count and
+//! potentially free the underlying object. There is no equivalent to `Box` for file descriptors in
+//! the standard library (that would be a type that guarantees that the reference count is `1`),
+//! however, it would be possible for a crate to define a type with those semantics.
+//!
 //! [`File`]: crate::fs::File
 //! [`TcpStream`]: crate::net::TcpStream
 //! [`io::stdout`]: stdout
@@ -245,6 +286,11 @@
 //! [`?` operator]: ../../book/appendix-02-operators.html
 //! [`Result`]: crate::result::Result
 //! [`.unwrap()`]: crate::result::Result::unwrap
+//! [`os::unix`]: ../os/unix/io/index.html
+//! [`os::windows`]: ../os/windows/io/index.html
+//! [`OwnedFd`]: ../os/fd/struct.OwnedFd.html
+//! [`BorrowedFd<'a>`]: ../os/fd/struct.BorrowedFd.html
+//! [`Arc`]: crate::sync::Arc
 
 #![stable(feature = "rust1", since = "1.0.0")]
 
diff --git a/library/std/src/os/fd/owned.rs b/library/std/src/os/fd/owned.rs
index 2180d2974d5..81106d6c62c 100644
--- a/library/std/src/os/fd/owned.rs
+++ b/library/std/src/os/fd/owned.rs
@@ -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
@@ -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
@@ -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);
diff --git a/library/std/src/os/fd/raw.rs b/library/std/src/os/fd/raw.rs
index 592e072ad90..ef896ea95c9 100644
--- a/library/std/src/os/fd/raw.rs
+++ b/library/std/src/os/fd/raw.rs
@@ -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
     ///
diff --git a/library/std/src/os/fortanix_sgx/io.rs b/library/std/src/os/fortanix_sgx/io.rs
index 7223ade6815..7e57435b65c 100644
--- a/library/std/src/os/fortanix_sgx/io.rs
+++ b/library/std/src/os/fortanix_sgx/io.rs
@@ -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;
 }
diff --git a/library/std/src/os/solid/io.rs b/library/std/src/os/solid/io.rs
index 33cc5a015b5..f82034663d4 100644
--- a/library/std/src/os/solid/io.rs
+++ b/library/std/src/os/solid/io.rs
@@ -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;
 }
 
diff --git a/library/std/src/os/unix/io/mod.rs b/library/std/src/os/unix/io/mod.rs
index 25b5dbff14f..c12d89ed637 100644
--- a/library/std/src/os/unix/io/mod.rs
+++ b/library/std/src/os/unix/io/mod.rs
@@ -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 |
 //! | ------------------ | ------------ |
@@ -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")]
 
diff --git a/library/std/src/os/windows/io/mod.rs b/library/std/src/os/windows/io/mod.rs
index e2a401fb696..3d4bb96d458 100644
--- a/library/std/src/os/windows/io/mod.rs
+++ b/library/std/src/os/windows/io/mod.rs
@@ -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 |
 //! | ---------------------- | ------------ |
@@ -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")]
 
diff --git a/library/std/src/os/windows/io/raw.rs b/library/std/src/os/windows/io/raw.rs
index 1759e2e7f3f..770583a9ce3 100644
--- a/library/std/src/os/windows/io/raw.rs
+++ b/library/std/src/os/windows/io/raw.rs
@@ -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).
     ///
@@ -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;
 }
@@ -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;
 }