use_file: Use std::fs::File instead of libc and once_cell::sync::OnceCell instead of pthreads mutex

.github/workflows/workspace.yml

@@ -56,7 +56,7 @@ jobs:
     - name: SOLID (solid.rs)
       run: cargo clippy -Zbuild-std=core --target aarch64-kmc-solid_asp3
     - name: Redox (use_file.rs)
-      run: cargo clippy -Zbuild-std=core --target x86_64-unknown-redox
+      run: cargo clippy -Zbuild-std=std --target x86_64-unknown-redox
     - name: VxWorks (vxworks.rs)
       run: cargo clippy -Zbuild-std=core --target x86_64-wrs-vxworks
     - name: WASI (wasi.rs)

Cargo.toml

@@ -27,6 +27,9 @@ wasi = { version = "0.11", default-features = false }
 [target.'cfg(all(windows, not(target_vendor = "win7")))'.dependencies]
 windows-targets = "0.52"
 
+[target.'cfg(any(target_os = "aix", target_os = "android", target_os = "haiku", target_os = "linux", target_os = "nto", target_os = "redox"))'.dependencies]
+once_cell = { version = "1.19.0", default-features = false, features = ["std"]}
+
 [target.'cfg(all(any(target_arch = "wasm32", target_arch = "wasm64"), target_os = "unknown"))'.dependencies]
 wasm-bindgen = { version = "0.2.62", default-features = false, optional = true }
 js-sys = { version = "0.3", optional = true }

src/use_file.rs

@@ -1,97 +1,60 @@
 //! Implementations that just need to read from a file
-use crate::{
-    util_libc::{open_readonly, sys_fill_exact},
-    Error,
-};
-use core::{
-    cell::UnsafeCell,
-    ffi::c_void,
-    mem::MaybeUninit,
-    sync::atomic::{AtomicI32, Ordering},
-};
+
+extern crate std;
+
+use crate::{util_libc::sys_fill_exact, Error};
+use core::{ffi::c_void, mem::MaybeUninit};
+use std::{fs::File, io, os::unix::io::AsRawFd as _};
+// TODO(MSRV feature(once_cell_try)): Use std::sync::OnceLock instead.
+use once_cell::sync::OnceCell as OnceLock;
 
 /// For all platforms, we use `/dev/urandom` rather than `/dev/random`.
 /// For more information see the linked man pages in lib.rs.
 ///   - On Linux, "/dev/urandom is preferred and sufficient in all use cases".
 ///   - On Redox, only /dev/urandom is provided.
 ///   - On AIX, /dev/urandom will "provide cryptographically secure output".
 ///   - On Haiku and QNX Neutrino they are identical.
-const FILE_PATH: &[u8] = b"/dev/urandom\0";
+const FILE_PATH: &str = "/dev/urandom";
 
 // Do not inline this when it is the fallback implementation, but don't mark it
 // `#[cold]` because it is hot when it is actually used.
 #[cfg_attr(any(target_os = "android", target_os = "linux"), inline(never))]
 pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
-    let fd = get_rng_fd()?;
-    sys_fill_exact(dest, |buf| unsafe {
-        libc::read(fd, buf.as_mut_ptr().cast::<c_void>(), buf.len())
-    })
-}
-
-// Returns the file descriptor for the device file used to retrieve random
-// bytes. The file will be opened exactly once. All subsequent calls will
-// return the same file descriptor. This file descriptor is never closed.
-fn get_rng_fd() -> Result<libc::c_int, Error> {
-    // std::os::fd::{BorrowedFd, OwnedFd} guarantee that -1 is not a valid file descriptor.
-    const FD_UNINIT: libc::c_int = -1;
-
-    // In theory `libc::c_int` could be something other than `i32`, but for the
-    // targets we currently support that use `use_file`, it is always `i32`.
-    // If/when we add support for a target where that isn't the case, we may
-    // need to use a different atomic type or make other accomodations. The
-    // compiler will let us know if/when that is the case, because the
-    // `FD.store(fd)` would fail to compile.
-    //
     // The opening of the file, by libc/libstd/etc. may write some unknown
     // state into in-process memory. (Such state may include some sanitizer
     // bookkeeping, or we might be operating in a unikernal-like environment
     // where all the "kernel" file descriptor bookkeeping is done in our
-    // process.) `get_fd_locked` stores into FD using `Ordering::Release` to
-    // ensure any such state is synchronized. `get_fd` loads from `FD` with
-    // `Ordering::Acquire` to synchronize with it.
-    static FD: AtomicI32 = AtomicI32::new(FD_UNINIT);
-
-    fn get_fd() -> Option<libc::c_int> {
-        match FD.load(Ordering::Acquire) {
-            FD_UNINIT => None,
-            val => Some(val),
-        }
-    }
-
-    #[cold]
-    fn get_fd_locked() -> Result<libc::c_int, Error> {
-        // This mutex is used to prevent multiple threads from opening file
-        // descriptors concurrently, which could run into the limit on the
-        // number of open file descriptors. Our goal is to have no more than one
-        // file descriptor open, ever.
-        //
-        // SAFETY: We use the mutex only in this method, and we always unlock it
-        // before returning, making sure we don't violate the pthread_mutex_t API.
-        static MUTEX: Mutex = Mutex::new();
-        unsafe { MUTEX.lock() };
-        let _guard = DropGuard(|| unsafe { MUTEX.unlock() });
-
-        if let Some(fd) = get_fd() {
-            return Ok(fd);
-        }
-
-        // On Linux, /dev/urandom might return insecure values.
-        #[cfg(any(target_os = "android", target_os = "linux"))]
-        wait_until_rng_ready()?;
-
-        let fd = open_readonly(FILE_PATH)?;
-        debug_assert!(fd != FD_UNINIT);
-        FD.store(fd, Ordering::Release);
+    // process.) Thus we avoid using (relaxed) atomics like we use in other
+    // parts of the library.
+    //
+    // We prevent multiple threads from opening file descriptors concurrently,
+    // which could run into the limit on the number of open file descriptors.
+    // Our goal is to have no more than one file descriptor open, ever.
+    //
+    // We assume any call to `OnceLock::get_or_try_init` synchronizes-with
+    // (Ordering::Acquire) the preceding call to `OnceLock::get_or_try_init`
+    // after `init()` returns an `Ok` result (Ordering::Release). See
+    // https://github.com/rust-lang/rust/issues/126239.
+    static FILE: OnceLock<File> = OnceLock::new();
+    let file = FILE.get_or_try_init(init)?;
+
+    // TODO(MSRV feature(read_buf)): Use `std::io::Read::read_buf`
+    sys_fill_exact(dest, |buf| unsafe {
+        libc::read(
+            file.as_raw_fd(),
+            buf.as_mut_ptr().cast::<c_void>(),
+            buf.len(),
+        )
+    })
+}
 
-        Ok(fd)
-    }
+#[cold]
+fn init() -> Result<File, Error> {
+    // On Linux, /dev/urandom might return insecure values.
+    #[cfg(any(target_os = "android", target_os = "linux"))]
+    wait_until_rng_ready()?;
 
-    // Use double-checked locking to avoid acquiring the lock if possible.
-    if let Some(fd) = get_fd() {
-        Ok(fd)
-    } else {
-        get_fd_locked()
-    }
+    File::open(FILE_PATH).map_err(map_io_error)
 }
 
 // Polls /dev/random to make sure it is ok to read from /dev/urandom.
@@ -124,15 +87,12 @@ fn get_rng_fd() -> Result<libc::c_int, Error> {
 // libsodium uses `libc::poll` similarly to this.
 #[cfg(any(target_os = "android", target_os = "linux"))]
 fn wait_until_rng_ready() -> Result<(), Error> {
-    let fd = open_readonly(b"/dev/random\0")?;
+    let file = File::open("/dev/random").map_err(map_io_error)?;
     let mut pfd = libc::pollfd {
-        fd,
+        fd: file.as_raw_fd(),
         events: libc::POLLIN,
         revents: 0,
     };
-    let _guard = DropGuard(|| unsafe {
-        libc::close(fd);
-    });
 
     loop {
         // A negative timeout means an infinite timeout.
@@ -149,28 +109,20 @@ fn wait_until_rng_ready() -> Result<(), Error> {
     }
 }
 
-struct Mutex(UnsafeCell<libc::pthread_mutex_t>);
-
-impl Mutex {
-    const fn new() -> Self {
-        Self(UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER))
-    }
-    unsafe fn lock(&self) {
-        let r = libc::pthread_mutex_lock(self.0.get());
-        debug_assert_eq!(r, 0);
-    }
-    unsafe fn unlock(&self) {
-        let r = libc::pthread_mutex_unlock(self.0.get());
-        debug_assert_eq!(r, 0);
-    }
-}
-
-unsafe impl Sync for Mutex {}
-
-struct DropGuard<F: FnMut()>(F);
-
-impl<F: FnMut()> Drop for DropGuard<F> {
-    fn drop(&mut self) {
-        self.0()
-    }
+fn map_io_error(err: io::Error) -> Error {
+    // TODO(MSRV feature(raw_os_error_ty)): Use `std::io::RawOsError`.
+    type RawOsError = i32;
+
+    err.raw_os_error()
+        .map_or(Error::UNEXPECTED, |errno: RawOsError| {
+            // RawOsError-to-u32 conversion is lossless for nonnegative values
+            // if they are the same size.
+            const _: () =
+                assert!(core::mem::size_of::<RawOsError>() == core::mem::size_of::<u32>());
+
+            match u32::try_from(errno) {
+                Ok(code) if code != 0 => Error::from_os_error(code),
+                _ => Error::ERRNO_NOT_POSITIVE,
+            }
+        })
 }

src/util_libc.rs

@@ -72,30 +72,3 @@ pub fn sys_fill_exact(
     }
     Ok(())
 }
-
-/// Open a file in read-only mode.
-///
-/// # Panics
-/// If `path` does not contain any zeros.
-// TODO: Move `path` to `CStr` and use `CStr::from_bytes_until_nul` (MSRV 1.69)
-// or C-string literals (MSRV 1.77) for statics
-#[inline(always)]
-pub fn open_readonly(path: &[u8]) -> Result<libc::c_int, Error> {
-    assert!(path.iter().any(|&b| b == 0));
-    loop {
-        let fd = unsafe {
-            libc::open(
-                path.as_ptr().cast::<libc::c_char>(),
-                libc::O_RDONLY | libc::O_CLOEXEC,
-            )
-        };
-        if fd >= 0 {
-            return Ok(fd);
-        }
-        let err = last_os_error();
-        // We should try again if open() was interrupted.
-        if err.raw_os_error() != Some(libc::EINTR) {
-            return Err(err);
-        }
-    }
-}