From 3b0b67b86755643837e1ea3134f399c594b85b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Sun, 16 Jun 2024 18:19:57 +0300 Subject: [PATCH 1/3] use_file: replace mutex with `nanosleep`-based loop --- src/use_file.rs | 112 ++++++++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/src/use_file.rs b/src/use_file.rs index 0764766be..f5fdeeca8 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -4,10 +4,9 @@ use crate::{ Error, }; use core::{ - cell::UnsafeCell, ffi::c_void, mem::MaybeUninit, - sync::atomic::{AtomicUsize, Ordering::Relaxed}, + sync::atomic::{AtomicUsize, Ordering}, }; /// For all platforms, we use `/dev/urandom` rather than `/dev/random`. @@ -18,6 +17,7 @@ use core::{ /// - On Haiku and QNX Neutrino they are identical. const FILE_PATH: &[u8] = b"/dev/urandom\0"; const FD_UNINIT: usize = usize::MAX; +const FD_ONGOING_INIT: usize = usize::MAX - 1; // 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. @@ -35,42 +35,70 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { fn get_rng_fd() -> Result { static FD: AtomicUsize = AtomicUsize::new(FD_UNINIT); - fn get_fd() -> Option { - match FD.load(Relaxed) { - FD_UNINIT => None, - val => Some(val as libc::c_int), - } - } - #[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() }); - - if let Some(fd) = get_fd() { - return Ok(fd); + fn init_or_wait_fd() -> Result { + // Maximum sleep time (~268 milliseconds) + let max_sleep_ns = 1 << 28; + // Starting sleep time (~4 microseconds) + let mut timeout_ns = 1 << 12; + loop { + match FD.load(Ordering::Acquire) { + FD_UNINIT => {} + FD_ONGOING_INIT => { + let rqtp = libc::timespec { + tv_sec: 0, + tv_nsec: timeout_ns, + }; + let mut rmtp = libc::timespec { + tv_sec: 0, + tv_nsec: 0, + }; + unsafe { + libc::nanosleep(&rqtp, &mut rmtp); + } + if timeout_ns < max_sleep_ns { + timeout_ns *= 2; + } + continue; + } + val => return Ok(val as libc::c_int), + } + + let xch_res = FD.compare_exchange_weak( + FD_UNINIT, + FD_ONGOING_INIT, + Ordering::AcqRel, + Ordering::Relaxed, + ); + if xch_res.is_err() { + continue; + } + + let res = open_fd(); + let val = match res { + Ok(fd) => fd as usize, + Err(_) => FD_UNINIT, + }; + FD.store(val, Ordering::Release); + return res; } + } + fn open_fd() -> Result { // 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)?; // The fd always fits in a usize without conflicting with FD_UNINIT. - debug_assert!(fd >= 0 && (fd as usize) < FD_UNINIT); - FD.store(fd as usize, Relaxed); + debug_assert!(fd >= 0 && (fd as usize) < FD_ONGOING_INIT); Ok(fd) } - // Use double-checked locking to avoid acquiring the lock if possible. - if let Some(fd) = get_fd() { - Ok(fd) - } else { - get_fd_locked() + match FD.load(Ordering::Relaxed) { + FD_UNINIT | FD_ONGOING_INIT => init_or_wait_fd(), + val => Ok(val as libc::c_int), } } @@ -104,6 +132,14 @@ fn get_rng_fd() -> Result { // libsodium uses `libc::poll` similarly to this. #[cfg(any(target_os = "android", target_os = "linux"))] fn wait_until_rng_ready() -> Result<(), Error> { + struct DropGuard(F); + + impl Drop for DropGuard { + fn drop(&mut self) { + self.0() + } + } + let fd = open_readonly(b"/dev/random\0")?; let mut pfd = libc::pollfd { fd, @@ -128,29 +164,3 @@ fn wait_until_rng_ready() -> Result<(), Error> { } } } - -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() - } -} From 8339958dcd93ca02c518c7038e73087ae4e114c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Sun, 16 Jun 2024 19:51:52 +0300 Subject: [PATCH 2/3] Tweak code --- src/use_file.rs | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/use_file.rs b/src/use_file.rs index f5fdeeca8..1be0e5bd4 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -6,7 +6,10 @@ use crate::{ use core::{ ffi::c_void, mem::MaybeUninit, - sync::atomic::{AtomicUsize, Ordering}, + sync::atomic::{ + AtomicUsize, + Ordering::{AcqRel, Acquire, Relaxed, Release}, + }, }; /// For all platforms, we use `/dev/urandom` rather than `/dev/random`. @@ -41,9 +44,15 @@ fn get_rng_fd() -> Result { let max_sleep_ns = 1 << 28; // Starting sleep time (~4 microseconds) let mut timeout_ns = 1 << 12; + loop { - match FD.load(Ordering::Acquire) { - FD_UNINIT => {} + match FD.load(Acquire) { + FD_UNINIT => { + let res = FD.compare_exchange_weak(FD_UNINIT, FD_ONGOING_INIT, AcqRel, Relaxed); + if res.is_ok() { + break; + } + } FD_ONGOING_INIT => { let rqtp = libc::timespec { tv_sec: 0, @@ -53,35 +62,25 @@ fn get_rng_fd() -> Result { tv_sec: 0, tv_nsec: 0, }; - unsafe { - libc::nanosleep(&rqtp, &mut rmtp); - } if timeout_ns < max_sleep_ns { timeout_ns *= 2; } + unsafe { + libc::nanosleep(&rqtp, &mut rmtp); + } continue; } val => return Ok(val as libc::c_int), } - - let xch_res = FD.compare_exchange_weak( - FD_UNINIT, - FD_ONGOING_INIT, - Ordering::AcqRel, - Ordering::Relaxed, - ); - if xch_res.is_err() { - continue; - } - - let res = open_fd(); - let val = match res { - Ok(fd) => fd as usize, - Err(_) => FD_UNINIT, - }; - FD.store(val, Ordering::Release); - return res; } + + let res = open_fd(); + let val = match res { + Ok(fd) => fd as usize, + Err(_) => FD_UNINIT, + }; + FD.store(val, Release); + res } fn open_fd() -> Result { @@ -96,7 +95,7 @@ fn get_rng_fd() -> Result { Ok(fd) } - match FD.load(Ordering::Relaxed) { + match FD.load(Relaxed) { FD_UNINIT | FD_ONGOING_INIT => init_or_wait_fd(), val => Ok(val as libc::c_int), } From 3c33e6a82227355384305088a493132c228d3ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D1=80=D1=82=D1=91=D0=BC=20=D0=9F=D0=B0=D0=B2=D0=BB?= =?UTF-8?q?=D0=BE=D0=B2=20=5BArtyom=20Pavlov=5D?= Date: Sun, 16 Jun 2024 19:57:33 +0300 Subject: [PATCH 3/3] Replace DropGuard with explicit close --- src/use_file.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/use_file.rs b/src/use_file.rs index 1be0e5bd4..ffe3a6b05 100644 --- a/src/use_file.rs +++ b/src/use_file.rs @@ -131,35 +131,29 @@ fn get_rng_fd() -> Result { // libsodium uses `libc::poll` similarly to this. #[cfg(any(target_os = "android", target_os = "linux"))] fn wait_until_rng_ready() -> Result<(), Error> { - struct DropGuard(F); - - impl Drop for DropGuard { - fn drop(&mut self) { - self.0() - } - } - 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 { + let res = 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(()); + break Ok(()); } let err = crate::util_libc::last_os_error(); match err.raw_os_error() { Some(libc::EINTR) | Some(libc::EAGAIN) => continue, - _ => return Err(err), + _ => break Err(err), } + }; + + unsafe { + libc::close(fd); } + res }