From 4e2528d4801031a2a73f211a230a528c6dd31591 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 5 Jun 2024 14:12:40 -0700 Subject: [PATCH 1/2] Move `wait_until_rng_ready` to `linux_android_with_fallback.rs`. It is only used in one configuration, so put it with the other stuff from that configuration. --- src/linux_android_with_fallback.rs | 64 +++++++++++++++++++++++++++++- src/use_file.rs | 57 +------------------------- 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/src/linux_android_with_fallback.rs b/src/linux_android_with_fallback.rs index 0f5ea8a9..3015f1c9 100644 --- a/src/linux_android_with_fallback.rs +++ b/src/linux_android_with_fallback.rs @@ -1,7 +1,7 @@ //! Implementation for Linux / Android with `/dev/urandom` fallback use crate::{ lazy::LazyBool, - util_libc::{getrandom_syscall, last_os_error, sys_fill_exact}, + util_libc::{getrandom_syscall, last_os_error, open_readonly, sys_fill_exact}, {use_file, Error}, }; use core::mem::MaybeUninit; @@ -31,3 +31,65 @@ fn is_getrandom_available() -> bool { true } } + +// Polls /dev/random to make sure it is ok to read from /dev/urandom. +// +// Polling avoids draining the estimated entropy from /dev/random; +// short-lived processes reading even a single byte from /dev/random could +// be problematic if they are being executed faster than entropy is being +// collected. +// +// OTOH, reading a byte instead of polling is more compatible with +// sandboxes that disallow `poll()` but which allow reading /dev/random, +// e.g. sandboxes that assume that `poll()` is for network I/O. This way, +// fewer applications will have to insert pre-sandbox-initialization logic. +// Often (blocking) file I/O is not allowed in such early phases of an +// application for performance and/or security reasons. +// +// It is hard to write a sandbox policy to support `libc::poll()` because +// it may invoke the `poll`, `ppoll`, `ppoll_time64` (since Linux 5.1, with +// newer versions of glibc), and/or (rarely, and probably only on ancient +// systems) `select`. depending on the libc implementation (e.g. glibc vs +// musl), libc version, potentially the kernel version at runtime, and/or +// the target architecture. +// +// BoringSSL and libstd don't try to protect against insecure output from +// `/dev/urandom'; they don't open `/dev/random` at all. +// +// OpenSSL uses `libc::select()` unless the `dev/random` file descriptor +// is too large; if it is too large then it does what we do here. +// +// libsodium uses `libc::poll` similarly to this. +pub fn wait_until_rng_ready() -> Result<(), Error> { + let fd = open_readonly(b"/dev/random\0")?; + let mut pfd = libc::pollfd { + fd, + events: libc::POLLIN, + revents: 0, + }; + let _guard = DropGuard(|| unsafe { + libc::close(fd); + }); + + loop { + // A negative timeout means an infinite timeout. + let res = unsafe { libc::poll(&mut pfd, 1, -1) }; + if res >= 0 { + debug_assert_eq!(res, 1); // We only used one fd, and cannot timeout. + return Ok(()); + } + let err = crate::util_libc::last_os_error(); + match err.raw_os_error() { + Some(libc::EINTR) | Some(libc::EAGAIN) => continue, + _ => return Err(err), + } + } +} + +struct DropGuard(F); + +impl Drop for DropGuard { + fn drop(&mut self) { + self.0() + } +} diff --git a/src/use_file.rs b/src/use_file.rs index 36210dee..9eb06de3 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -56,7 +56,7 @@ fn get_rng_fd() -> Result { // On Linux, /dev/urandom might return insecure values. #[cfg(any(target_os = "android", target_os = "linux"))] - wait_until_rng_ready()?; + crate::imp::wait_until_rng_ready()?; let fd = open_readonly(FILE_PATH)?; // The fd always fits in a usize without conflicting with FD_UNINIT. @@ -74,61 +74,6 @@ fn get_rng_fd() -> Result { } } -// Polls /dev/random to make sure it is ok to read from /dev/urandom. -// -// Polling avoids draining the estimated entropy from /dev/random; -// short-lived processes reading even a single byte from /dev/random could -// be problematic if they are being executed faster than entropy is being -// collected. -// -// OTOH, reading a byte instead of polling is more compatible with -// sandboxes that disallow `poll()` but which allow reading /dev/random, -// e.g. sandboxes that assume that `poll()` is for network I/O. This way, -// fewer applications will have to insert pre-sandbox-initialization logic. -// Often (blocking) file I/O is not allowed in such early phases of an -// application for performance and/or security reasons. -// -// It is hard to write a sandbox policy to support `libc::poll()` because -// it may invoke the `poll`, `ppoll`, `ppoll_time64` (since Linux 5.1, with -// newer versions of glibc), and/or (rarely, and probably only on ancient -// systems) `select`. depending on the libc implementation (e.g. glibc vs -// musl), libc version, potentially the kernel version at runtime, and/or -// the target architecture. -// -// BoringSSL and libstd don't try to protect against insecure output from -// `/dev/urandom'; they don't open `/dev/random` at all. -// -// OpenSSL uses `libc::select()` unless the `dev/random` file descriptor -// is too large; if it is too large then it does what we do here. -// -// 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 mut pfd = libc::pollfd { - fd, - events: libc::POLLIN, - revents: 0, - }; - let _guard = DropGuard(|| unsafe { - libc::close(fd); - }); - - loop { - // A negative timeout means an infinite timeout. - let res = unsafe { libc::poll(&mut pfd, 1, -1) }; - if res >= 0 { - debug_assert_eq!(res, 1); // We only used one fd, and cannot timeout. - return Ok(()); - } - let err = crate::util_libc::last_os_error(); - match err.raw_os_error() { - Some(libc::EINTR) | Some(libc::EAGAIN) => continue, - _ => return Err(err), - } - } -} - struct Mutex(UnsafeCell); impl Mutex { From ef29db5a7b2be8ec4f8ae13584d22a6e2d31f025 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 4 Jun 2024 22:55:19 -0700 Subject: [PATCH 2/2] use_file: `std::sync::Mutex`, dropping all libpthread use. pthreads mutexes are not safe to move. While it is very unlikely that the mutex we create will ever be moved, we don't actively do anything to actively prevent it from being moved. (libstd, when it used/uses pthreads mutexes, would box them to prevent them from being moved.) Also, now on Linux and Android (and many other targets for which we don't use use_std), libstd uses futexes instead of pthreads mutexes. Thus using libstd's Mutex will be more efficient and avoid adding an often-otherwise-unnecessary libpthreads dependency on these targets. * Linux, Android: Futex [1]. * Haiku, Redox, NTO, AIX: pthreads [2]. * others: not using `use_file`. This will not affect our plans for *-*-linux-none, since we don't plan to use `use_file` for it. OnceLock This breaks 32-bit x86 QNX Neutrino, which doesn't have libstd because the target itself is abandoned [3]. the other QNX Neutrino targets didn't get libstd support until Rust 1.69, so this effectively raises the MSRV for them to 1.69. Otherwise, the MSRV increases to 1.63 for the above-mentioned targets, as that's when `Mutex::new()` became a `const fn`. I tried to use `Once` to avoid the MSRV increase but it doesn't support fallible initialization even in Nightly. `OnceLock` wasn't added until 1.70. On x86_64 Linux, this change removes all libpthreads dependencies: ```diff - pthread_mutex_lock - pthread_mutex_unlock ``` and adds these libstd dependencies: ```diff + std::panicking::panic_count::GLOBAL_PANIC_COUNT + std::panicking::panic_count::is_zero_slow_path + std::sys::sync::mutex::futex::Mutex::lock_contended + std::sys::sync::mutex::futex::Mutex::wake ``` as measured using `cargo asm`. [1] https://github.com/rust-lang/rust/blob/c1dba09f263cbff6170f130aa418e28bdf22bd96/library/std/src/sys/sync/mutex/mod.rs#L4-L10 [2] https://github.com/rust-lang/rust/blob/c1dba09f263cbff6170f130aa418e28bdf22bd96/library/std/src/sys/sync/mutex/mod.rs#L17-L20 [3] https://github.com/rust-random/getrandom/issues/453#issuecomment-2148124364 --- .clippy.toml | 2 +- .github/workflows/tests.yml | 4 +++- Cargo.toml | 2 +- README.md | 2 +- src/error.rs | 3 +++ src/use_file.rs | 38 ++++++------------------------------- 6 files changed, 15 insertions(+), 36 deletions(-) diff --git a/.clippy.toml b/.clippy.toml index 5cccb362..550d4759 100644 --- a/.clippy.toml +++ b/.clippy.toml @@ -1 +1 @@ -msrv = "1.57" +msrv = "1.63" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f3831d5c..e36d4d90 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,7 +44,7 @@ jobs: strategy: matrix: os: [ubuntu-22.04, windows-2022] - toolchain: [nightly, beta, stable, 1.57] + toolchain: [nightly, beta, stable, 1.63] # Only Test macOS on stable to reduce macOS CI jobs include: # x86_64-apple-darwin. @@ -348,6 +348,8 @@ jobs: ] include: # Supported tier 3 targets with libstd support + - target: aarch64-unknown-nto-qnx710 + features: ["std"] - target: x86_64-unknown-openbsd features: ["std"] - target: x86_64-unknown-dragonfly diff --git a/Cargo.toml b/Cargo.toml index b9357bce..6899c03b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "getrandom" version = "0.2.15" # Also update html_root_url in lib.rs when bumping this edition = "2021" -rust-version = "1.57" # Sync .clippy.toml, tests.yml, and README.md. +rust-version = "1.63" # Sync .clippy.toml, tests.yml, and README.md. authors = ["The Rand Project Developers"] license = "MIT OR Apache-2.0" description = "A small cross-platform library for retrieving random data from system source" diff --git a/README.md b/README.md index 56af89dd..bbab4f28 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ crate features, WASM support and Custom RNGs see the ## Minimum Supported Rust Version -This crate requires Rust 1.57.0 or later. +This crate requires Rust 1.63.0 or later. ## Platform Support diff --git a/src/error.rs b/src/error.rs index 5eff99eb..5c4f1c9e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -58,6 +58,8 @@ impl Error { pub const NODE_ES_MODULE: Error = internal_error(14); /// Calling Windows ProcessPrng failed. pub const WINDOWS_PROCESS_PRNG: Error = internal_error(15); + /// The mutex used when opening the random file was poisoned. + pub const UNEXPECTED_FILE_MUTEX_POISONED: Error = internal_error(16); /// Codes below this point represent OS Errors (i.e. positive i32 values). /// Codes at or above this point, but below [`Error::CUSTOM_START`] are @@ -175,6 +177,7 @@ fn internal_desc(error: Error) -> Option<&'static str> { Error::NODE_RANDOM_FILL_SYNC => Some("Calling Node.js API crypto.randomFillSync failed"), Error::NODE_ES_MODULE => Some("Node.js ES modules are not directly supported, see https://docs.rs/getrandom#nodejs-es-module-support"), Error::WINDOWS_PROCESS_PRNG => Some("ProcessPrng: Windows system function failure"), + Error::UNEXPECTED_FILE_MUTEX_POISONED => Some("File: Initialization panicked, poisoning the mutex"), _ => None, } } diff --git a/src/use_file.rs b/src/use_file.rs index 9eb06de3..bae1f0fa 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -4,11 +4,12 @@ use crate::{ Error, }; use core::{ - cell::UnsafeCell, ffi::c_void, mem::MaybeUninit, sync::atomic::{AtomicUsize, Ordering::Relaxed}, }; +extern crate std; +use std::sync::{Mutex, PoisonError}; /// For all platforms, we use `/dev/urandom` rather than `/dev/random`. /// For more information see the linked man pages in lib.rs. @@ -44,11 +45,10 @@ fn get_rng_fd() -> Result { #[cold] fn get_fd_locked() -> Result { - // 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() }); + static MUTEX: Mutex<()> = Mutex::new(()); + let _guard = MUTEX + .lock() + .map_err(|_: PoisonError<_>| Error::UNEXPECTED_FILE_MUTEX_POISONED)?; if let Some(fd) = get_fd() { return Ok(fd); @@ -73,29 +73,3 @@ fn get_rng_fd() -> Result { get_fd_locked() } } - -struct Mutex(UnsafeCell); - -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); - -impl Drop for DropGuard { - fn drop(&mut self) { - self.0() - } -}