Add safety comments for mmap etc. (#1188)

* Add safety comments for `mmap` etc.

Add proper safety comments for `mmap` and related functions. Many of
the tricky things around `mmap` are about handing out Rust references
to mapped memory, and that technically isn't rustix's responsibility
to worry about, so these safety comments can be pretty simple.

And, several other miscellaneous documentation and comment cleanups.

* Document the new safety precondition for `split_init`.

Author: sunfishcode

.github/workflows/test-users.yml

@@ -112,7 +112,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -281,7 +281,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -437,7 +437,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -622,7 +622,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -773,7 +773,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -924,7 +924,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -1075,7 +1075,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -1226,7 +1226,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -1382,7 +1382,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -1538,7 +1538,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched
@@ -1759,7 +1759,7 @@ jobs:
         rustup target add ${{ matrix.target }}
       if: matrix.target != ''
 
-    - uses: actions/cache@v3
+    - uses: actions/cache@v4
       with:
         path: ${{ runner.tool_cache }}/qemu
         key: qemu-${{ matrix.target }}-${{ env.QEMU_BUILD_VERSION }}-patched

Cargo.toml

@@ -21,7 +21,7 @@ itoa = { version = "1.0.1", default-features = false, optional = true }
 
 # Special dependencies used in rustc-dep-of-std mode.
 core = { version = "1.0.0", optional = true, package = "rustc-std-workspace-core" }
-rustc-std-workspace-alloc = { version = "1.0.0", optional = true } # not aliased here but in lib.rs casuse of name collision with the alloc feature
+rustc-std-workspace-alloc = { version = "1.0.0", optional = true } # not aliased here but in lib.rs because of name collision with the alloc feature
 compiler_builtins = { version = '0.1.49', optional = true }
 
 # The procfs feature needs once_cell.

src/backend/libc/mod.rs

@@ -78,7 +78,7 @@ pub(crate) mod fd {
     ///
     /// [`AsFd`]: https://doc.rust-lang.org/stable/std/os/fd/trait.AsFd.html
     pub trait AsFd {
-        /// An `as_fd` function for Winsock, where a `Fd` is a `Socket`.
+        /// An `as_fd` function for Winsock, where an `Fd` is a `Socket`.
         fn as_fd(&self) -> BorrowedFd;
     }
     impl<T: AsSocket> AsFd for T {

src/backend/libc/net/addr.rs

@@ -74,7 +74,7 @@ impl SocketAddrUnix {
         })
     }
 
-    fn init() -> c::sockaddr_un {
+    const fn init() -> c::sockaddr_un {
         c::sockaddr_un {
             #[cfg(any(
                 bsd,

src/backend/linux_raw/vdso_wrappers.rs

@@ -530,9 +530,9 @@ fn init() {
             if ok {
                 assert!(!ptr.is_null());
 
-                // Store the computed function addresses in static
-                // storage so that we don't need to compute it again (but if
-                // we do, it doesn't hurt anything).
+                // Store the computed function addresses in static storage so
+                // that we don't need to compute them again (but if we do, it
+                // doesn't hurt anything).
                 CLOCK_GETTIME.store(ptr.cast(), Relaxed);
             }
         }
@@ -583,9 +583,9 @@ fn init() {
             if ok {
                 assert!(!ptr.is_null());
 
-                // Store the computed function addresses in static
-                // storage so that we don't need to compute it again (but if
-                // we do, it doesn't hurt anything).
+                // Store the computed function addresses in static storage so
+                // that we don't need to compute them again (but if we do, it
+                // doesn't hurt anything).
                 GETCPU.store(ptr.cast(), Relaxed);
             }
         }

src/buffer.rs

@@ -9,7 +9,8 @@ use core::slice;
 ///
 /// # Safety
 ///
-/// At least `init_len` bytes must be initialized.
+/// `init_len` must not be greater than `buf.len()`, and at least `init_len`
+/// bytes must be initialized.
 #[inline]
 pub(super) unsafe fn split_init(
     buf: &mut [MaybeUninit<u8>],

src/event/poll.rs

@@ -7,7 +7,7 @@ pub use backend::event::poll_fd::{PollFd, PollFlags};
 /// On macOS, `poll` doesn't work on fds for /dev/tty or /dev/null, however
 /// [`select`] is available and does work on these fds.
 ///
-/// [`select`]: crate::event::select
+/// [`select`]: crate::event::select()
 ///
 /// # References
 ///  - [Beej's Guide to Network Programming]

src/mm/mmap.rs

@@ -3,7 +3,7 @@
 //! # Safety
 //!
 //! `mmap` and related functions manipulate raw pointers and have special
-//! semantics and are wildly unsafe.
+//! semantics.
 #![allow(unsafe_code)]
 
 use crate::{backend, io};
@@ -52,7 +52,16 @@ impl MapFlags {
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// If `ptr` is not null, it must be aligned to the applicable page size, and
+/// the range of memory starting at `ptr` and extending for `len` bytes,
+/// rounded up to the applicable page size, must be valid to mutate using
+/// `ptr`'s provenance.
+///
+/// If there exist any Rust references referring to the memory region, or if
+/// you subsequently create a Rust reference referring to the resulting region,
+/// it is your responsibility to ensure that the Rust reference invariants are
+/// preserved, including ensuring that the memory is not mutated in a way that
+/// a Rust reference would not expect.
 ///
 /// # References
 ///  - [POSIX]
@@ -93,7 +102,10 @@ pub unsafe fn mmap<Fd: AsFd>(
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// If `ptr` is not null, it must be aligned to the applicable page size, and
+/// the range of memory starting at `ptr` and extending for `len` bytes,
+/// rounded up to the applicable page size, must be valid to mutate with
+/// `ptr`'s provenance.
 ///
 /// # References
 ///  - [POSIX]
@@ -130,7 +142,10 @@ pub unsafe fn mmap_anonymous(
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// `ptr` must be aligned to the applicable page size, and the range of memory
+/// starting at `ptr` and extending for `len` bytes, rounded up to the
+/// applicable page size, must be valid to mutate with `ptr`'s provenance. And
+/// there must be no Rust references referring to that memory.
 ///
 /// # References
 ///  - [POSIX]
@@ -165,7 +180,15 @@ pub unsafe fn munmap(ptr: *mut c_void, len: usize) -> io::Result<()> {
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// `old_address` must be aligned to the applicable page size, and the range of
+/// memory starting at `old_address` and extending for `old_size` bytes,
+/// rounded up to the applicable page size, must be valid to mutate with
+/// `old_address`'s provenance. If `MremapFlags::MAY_MOVE` is set in `flags`,
+/// there must be no Rust references referring to that the memory.
+///
+/// If `new_size` is less than `old_size`, than there must be no Rust
+/// references referring to the memory starting at offset `new_size` and ending
+/// at `old_size`.
 ///
 /// # References
 ///  - [Linux]
@@ -190,7 +213,16 @@ pub unsafe fn mremap(
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// `old_address` and `new_address` must be aligned to the applicable page
+/// size, the range of memory starting at `old_address` and extending for
+/// `old_size` bytes, rounded up to the applicable page size, must be valid to
+/// mutate with `old_address`'s provenance, and the range of memory starting at
+/// `new_address` and extending for `new_size` bytes, rounded up to the
+/// applicable page size, must be valid to mutate with `new_address`'s
+/// provenance.
+///
+/// There must be no Rust references referring to either of those memory
+/// regions.
 ///
 /// # References
 ///  - [Linux]
@@ -214,7 +246,9 @@ pub unsafe fn mremap_fixed(
 ///
 /// # Safety
 ///
-/// Raw pointers and lots of special semantics.
+/// The range of memory starting at `ptr` and extending for `len` bytes,
+/// rounded up to the applicable page size, must be valid to read with `ptr`'s
+/// provenance.
 ///
 /// # References
 ///  - [POSIX]
@@ -241,18 +275,17 @@ pub unsafe fn mprotect(ptr: *mut c_void, len: usize, flags: MprotectFlags) -> io
 
 /// `mlock(ptr, len)`—Lock memory into RAM.
 ///
-/// # Safety
-///
-/// This function operates on raw pointers, but it should only be used on
-/// memory which the caller owns. Technically, locking memory shouldn't violate
-/// any invariants, but since unlocking it can violate invariants, this
-/// function is also unsafe for symmetry.
-///
 /// Some implementations implicitly round the memory region out to the nearest
 /// page boundaries, so this function may lock more memory than explicitly
 /// requested if the memory isn't page-aligned. Other implementations fail if
 /// the memory isn't page-aligned.
 ///
+/// # Safety
+///
+/// The range of memory starting at `ptr`, rounded down to the applicable page
+/// boundary, and extending for `len` bytes, rounded up to the applicable page
+/// size, must be valid to read with `ptr`'s provenance.
+///
 /// # References
 ///  - [POSIX]
 ///  - [Linux]
@@ -282,17 +315,16 @@ pub unsafe fn mlock(ptr: *mut c_void, len: usize) -> io::Result<()> {
 ///
 /// `mlock_with` is the same as [`mlock`] but adds an additional flags operand.
 ///
-/// # Safety
-///
-/// This function operates on raw pointers, but it should only be used on
-/// memory which the caller owns. Technically, locking memory shouldn't violate
-/// any invariants, but since unlocking it can violate invariants, this
-/// function is also unsafe for symmetry.
-///
 /// Some implementations implicitly round the memory region out to the nearest
 /// page boundaries, so this function may lock more memory than explicitly
 /// requested if the memory isn't page-aligned.
 ///
+/// # Safety
+///
+/// The range of memory starting at `ptr`, rounded down to the applicable page
+/// boundary, and extending for `len` bytes, rounded up to the applicable page
+/// size, must be valid to read with `ptr`'s provenance.
+///
 /// # References
 ///  - [Linux]
 ///  - [glibc]
@@ -308,16 +340,16 @@ pub unsafe fn mlock_with(ptr: *mut c_void, len: usize, flags: MlockFlags) -> io:
 
 /// `munlock(ptr, len)`—Unlock memory.
 ///
-/// # Safety
-///
-/// This function operates on raw pointers, but it should only be used on
-/// memory which the caller owns, to avoid compromising the `mlock` invariants
-/// of other unrelated code in the process.
-///
 /// Some implementations implicitly round the memory region out to the nearest
 /// page boundaries, so this function may unlock more memory than explicitly
 /// requested if the memory isn't page-aligned.
 ///
+/// # Safety
+///
+/// The range of memory starting at `ptr`, rounded down to the applicable page
+/// boundary, and extending for `len` bytes, rounded up to the applicable page
+/// size, must be valid to read with `ptr`'s provenance.
+///
 /// # References
 ///  - [POSIX]
 ///  - [Linux]

src/net/send_recv/msg.rs

@@ -593,6 +593,11 @@ impl FusedIterator for AncillaryDrain<'_> {}
 
 /// `sendmsg(msghdr)`—Sends a message on a socket.
 ///
+/// This function is for use on connected sockets, as it doesn't have
+/// a way to specify an address. See the [`sendmsg_v4`], [`sendmsg_v6`]
+/// [`sendmsg_unix`], [`sendmsg_xdp`], and [`sendmsg_any`] to send
+/// messages on unconnected sockets.
+///
 /// # References
 ///  - [POSIX]
 ///  - [Linux]

src/process/procctl.rs

@@ -134,9 +134,11 @@ pub enum DumpableBehavior {
 }
 
 /// Set the state of the `dumpable` attribute for the process indicated by
-/// `idtype` and `id`. This determines whether the process can be traced and
-/// whether core dumps are produced for the process upon delivery of a signal
-/// whose default behavior is to produce a core dump.
+/// `idtype` and `id`.
+///
+/// This determines whether the process can be traced and whether core dumps
+/// are produced for the process upon delivery of a signal whose default
+/// behavior is to produce a core dump.
 ///
 /// This is similar to `set_dumpable_behavior` on Linux, with the exception
 /// that on FreeBSD there is an extra argument `process`. When `process` is set
@@ -453,6 +455,7 @@ pub enum TrapCapBehavior {
 }
 
 /// Set the current value of the capability mode violation trapping behavior.
+///
 /// If this behavior is enabled, the kernel would deliver a [`Signal::Trap`]
 /// signal on any return from a system call that would result in a
 /// [`io::Errno::NOTCAPABLE`] or [`io::Errno::CAPMODE`] error.