diff --git a/netstack/src/worker_pool.rs b/netstack/src/worker_pool.rs index 1910fcca72..4bda088469 100644 --- a/netstack/src/worker_pool.rs +++ b/netstack/src/worker_pool.rs @@ -50,21 +50,32 @@ pub struct OwnedFd { impl OwnedFd { /// Create an `OwnedFd` from a raw file descriptor. The fd is - /// duplicated via `/proc/self/fd/{n}` so the wrapper is - /// independent of the original handle's lifetime. - pub fn from_raw_fd(raw: usize) -> std::io::Result { + /// duplicated via `libc::dup` so the wrapper is independent of + /// the original handle's lifetime. + /// + /// The parameter is `RawFd` (the platform `c_int`), not `usize`, + /// so a garbage large integer cannot be cast to a valid fd + /// number. Callers that have a `File` should obtain its + /// `RawFd` via `IntoRawFd::into_raw_fd`; that round-trip via + /// the kernel's open-fd table ensures the value is a real fd. + pub fn from_raw_fd(raw: std::os::fd::RawFd) -> std::io::Result { use std::fs::File; - use std::os::fd::{FromRawFd, IntoRawFd}; - // SAFETY: the caller must guarantee that the raw fd is - // valid and that we have exclusive ownership. We do not - // own the original fd; the worker thread owns the dup. - let dup_fd = // SAFETY: caller must verify the safety contract for this operation -unsafe { libc::dup(raw as i32) }; + use std::os::fd::FromRawFd; + // SAFETY: the caller must guarantee that `raw` is a valid, + // open file descriptor and that we have exclusive ownership + // to dup it. Taking the parameter as `RawFd` (i32) rather + // than `usize` means a caller cannot pass a garbage 64-bit + // value that happens to cast to a valid i32 fd; the value + // can only have come from a previously-validated fd. + let dup_fd = unsafe { libc::dup(raw) }; if dup_fd < 0 { return Err(std::io::Error::last_os_error()); } - let f = // SAFETY: caller guarantees fd is valid, open, and not aliased -unsafe { File::from_raw_fd(dup_fd) }; + // SAFETY: `dup_fd` was just returned by `libc::dup` and is + // therefore valid and not aliased by any other `File`. The + // pre-`dup` raw fd is not aliased either, so the dup is + // independent of the original. + let f = unsafe { File::from_raw_fd(dup_fd) }; Ok(Self { inner: f }) } }