Bump MSRV to 1.57 & improve the memory safety guarantees around open_readonly.

.clippy.toml

@@ -1 +1 @@
-msrv = "1.36"
+msrv = "1.57"

.github/workflows/tests.yml

@@ -44,7 +44,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-22.04, windows-2022]
-        toolchain: [nightly, beta, stable, 1.36]
+        toolchain: [nightly, beta, stable, 1.57]
         # Only Test macOS on stable to reduce macOS CI jobs
         include:
           # x86_64-apple-darwin.

CHANGELOG.md

@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Breaking Changes
+- Update MSRV to 1.57 [#???]
+
+[#???]: https://github.com/rust-random/getrandom/pull/???
+
 ## [0.2.15] - 2024-05-06
 ### Added
 - Apple visionOS support [#410]

README.md

@@ -52,7 +52,7 @@ crate features, WASM support and Custom RNGs see the
 
 ## Minimum Supported Rust Version
 
-This crate requires Rust 1.36.0 or later.
+This crate requires Rust 1.57.0 or later.
 
 ## Platform Support
 

src/use_file.rs

@@ -1,6 +1,6 @@
 //! Implementations that just need to read from a file
 use crate::{
-    util_libc::{open_readonly, sys_fill_exact},
+    util_libc::{cstr, open_readonly, sys_fill_exact},
     Error,
 };
 use core::{
@@ -15,7 +15,7 @@ use core::{
 ///   - 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: &str = "/dev/urandom\0";
+const FILE_PATH: cstr::Ref = cstr::unwrap_const_from_bytes_with_nul(b"/dev/urandom\0");
 const FD_UNINIT: usize = usize::max_value();
 
 pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
@@ -56,7 +56,7 @@ fn get_rng_fd() -> Result<libc::c_int, Error> {
     #[cfg(any(target_os = "android", target_os = "linux"))]
     wait_until_rng_ready()?;
 
-    let fd = unsafe { open_readonly(FILE_PATH)? };
+    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);
@@ -67,8 +67,9 @@ fn get_rng_fd() -> Result<libc::c_int, Error> {
 // Succeeds once /dev/urandom is safe to read from
 #[cfg(any(target_os = "android", target_os = "linux"))]
 fn wait_until_rng_ready() -> Result<(), Error> {
+    const DEV_RANDOM: cstr::Ref = cstr::unwrap_const_from_bytes_with_nul(b"/dev/random\0");
     // Poll /dev/random to make sure it is ok to read from /dev/urandom.
-    let fd = unsafe { open_readonly("/dev/random\0")? };
+    let fd = open_readonly(DEV_RANDOM)?;
     let mut pfd = libc::pollfd {
         fd,
         events: libc::POLLIN,

src/util_cstr.rs

@@ -0,0 +1,78 @@
+//! Work around lack of `core::ffi::CStr` prior to Rust 1.64, and the lack of
+//! `const fn` support for `CStr` in later versions.
+
+// TODO(MSRV 1.64): Use `core::ffi::c_char`.
+use libc::c_char;
+
+// TODO(MSRV 1.64): Replace with `&core::ffi::CStr`.
+pub struct Ref(&'static [u8]);
+
+impl Ref {
+    // The result is guaranteed to be non-null and NUL-terminated.
+    #[inline(always)]
+    pub fn as_ptr(&self) -> *const c_char {
+        const _SAME_ALIGNMENT: () =
+            assert!(core::mem::align_of::<u8>() == core::mem::align_of::<c_char>());
+        const _SAME_SIZE: () =
+            assert!(core::mem::size_of::<u8>() == core::mem::size_of::<c_char>());
+
+        // It is safe to cast a `*const u8` to a `const c_char` as they are the
+        // same size and alignment.
+        self.0.as_ptr().cast()
+    }
+
+    // SAFETY: Same as `CStr::from_bytes_with_nul_unchecked`.
+    const unsafe fn from_bytes_with_nul_unchecked(value: &'static [u8]) -> Self {
+        Self(value)
+    }
+}
+
+pub const fn unwrap_const_from_bytes_with_nul(value: &'static [u8]) -> Ref {
+    // XXX: We cannot use `unwrap_const` since `Ref`/`CStr` is not `Copy`.
+    match const_from_bytes_with_nul(value) {
+        Some(r) => r,
+        None => panic!("const_from_bytes_with_nul failed"),
+    }
+}
+
+// TODO(MSRV 1.72): Replace with `CStr::from_bytes_with_nul`.
+#[inline(always)]
+const fn const_from_bytes_with_nul(value: &'static [u8]) -> Option<Ref> {
+    const fn const_contains(mut value: &[u8], needle: &u8) -> bool {
+        while let [head, tail @ ..] = value {
+            if *head == *needle {
+                return true;
+            }
+            value = tail;
+        }
+        false
+    }
+
+    // TODO(MSRV 1.69): Use `core::ffi::CStr::from_bytes_until_nul`
+    match value {
+        [before_nul @ .., 0] if !const_contains(before_nul, &0) => {
+            // SAFETY:
+            //   * `value` is nul-terminated according to the slice pattern.
+            //   * `value` doesn't contain any interior null, by the guard.
+            // TODO(MSRV 1.64): Use `CStr::from_bytes_with_nul_unchecked`
+            Some(unsafe { Ref::from_bytes_with_nul_unchecked(value) })
+        }
+        _ => None,
+    }
+}
+
+mod tests {
+    use super::const_from_bytes_with_nul;
+
+    // Bad.
+    const _EMPTY_UNTERMINATED: () = assert!(const_from_bytes_with_nul(b"").is_none());
+    const _EMPTY_DOUBLE_TERMINATED: () = assert!(const_from_bytes_with_nul(b"\0\0").is_none());
+    const _DOUBLE_NUL: () = assert!(const_from_bytes_with_nul(b"\0\0").is_none());
+    const _LEADINGL_NUL: () = assert!(const_from_bytes_with_nul(b"\0a\0").is_none());
+    const _INTERNAL_NUL_UNTERMINATED: () = assert!(const_from_bytes_with_nul(b"\0a").is_none());
+
+    // Good.
+    const EMPTY_TERMINATED: () = assert!(const_from_bytes_with_nul(b"\0").is_some());
+    const _NONEMPTY: () = assert!(const_from_bytes_with_nul(b"asdf\0").is_some());
+    const _1_CHAR: () = assert!(const_from_bytes_with_nul(b"a\0").is_some());
+}

src/util_libc.rs

@@ -2,6 +2,9 @@
 use crate::Error;
 use core::{mem::MaybeUninit, num::NonZeroU32};
 
+#[path = "util_cstr.rs"]
+pub(crate) mod cstr;
+
 cfg_if! {
     if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android"))] {
         use libc::__errno as errno_location;
@@ -70,11 +73,12 @@ pub fn sys_fill_exact(
     Ok(())
 }
 
-// SAFETY: path must be null terminated, FD must be manually closed.
-pub unsafe fn open_readonly(path: &str) -> Result<libc::c_int, Error> {
-    debug_assert_eq!(path.as_bytes().last(), Some(&0));
+// Memory leak hazard: The returned file descriptor must be manually closed to
+// avoid a file descriptor leak.
+pub fn open_readonly(path: cstr::Ref) -> Result<libc::c_int, Error> {
     loop {
-        let fd = libc::open(path.as_ptr() as *const _, libc::O_RDONLY | libc::O_CLOEXEC);
+        // SAFETY: path.as_ptr() is guaranteed to be a non-NULL && NULL-terminated.
+        let fd = unsafe { libc::open(path.as_ptr(), libc::O_RDONLY | libc::O_CLOEXEC) };
         if fd >= 0 {
             return Ok(fd);
         }