Skip to content

Commit

Permalink
use_file: Use AtomicI32 instead of AtomicUsize to avoid conversio…
Browse files Browse the repository at this point in the history
…ns (#480)

All the targets that use `use_file` support `AtomicI32`. Using
`AtomicI32` eliminates `as` conversions and thus avoids any possibility
of truncation or confusion between `FD_UNINIT` and a valid file
descriptor.

Use -1 as the sentinel value for `FD_UNINIT` since libstd (only)
guarantees that -1 is not a valid file descriptor value.

Minimize the scope of `FD_UNINIT`.
  • Loading branch information
briansmith authored Jun 17, 2024
1 parent 15c6be8 commit 480d6b0
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions src/use_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::{
cell::UnsafeCell,
ffi::c_void,
mem::MaybeUninit,
sync::atomic::{AtomicUsize, Ordering::Relaxed},
sync::atomic::{AtomicI32, Ordering::Relaxed},
};

/// For all platforms, we use `/dev/urandom` rather than `/dev/random`.
Expand All @@ -17,7 +17,6 @@ use core::{
/// - 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 FD_UNINIT: usize = usize::MAX;

// 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.
Expand All @@ -33,12 +32,21 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// 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> {
static FD: AtomicUsize = AtomicUsize::new(FD_UNINIT);
// 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.
static FD: AtomicI32 = AtomicI32::new(FD_UNINIT);

fn get_fd() -> Option<libc::c_int> {
match FD.load(Relaxed) {
FD_UNINIT => None,
val => Some(val as libc::c_int),
val => Some(val),
}
}

Expand All @@ -59,9 +67,8 @@ fn get_rng_fd() -> Result<libc::c_int, Error> {
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 != FD_UNINIT);
FD.store(fd, Relaxed);

Ok(fd)
}
Expand Down

0 comments on commit 480d6b0

Please sign in to comment.