Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase MSRV to 1.57 & completely check null termination of C strings at compile time. #426

Closed
wants to merge 9 commits into from
2 changes: 1 addition & 1 deletion .clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1 @@
msrv = "1.36"
msrv = "1.57"
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ 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 [#425], [#426]

[#425]: https://github.com/rust-random/getrandom/pull/425
[#426]: https://github.com/rust-random/getrandom/pull/426

## [0.2.15] - 2024-05-06
### Added
- Apple visionOS support [#410]
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/apple-other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let ret = unsafe { CCRandomGenerateBytes(dest.as_mut_ptr() as *mut c_void, dest.len()) };
let ret = unsafe { CCRandomGenerateBytes(dest.as_mut_ptr().cast::<c_void>(), dest.len()) };
// kCCSuccess (from CommonCryptoError.h) is always zero.
if ret != 0 {
Err(Error::IOS_SEC_RANDOM)
Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Error {
cfg_if! {
if #[cfg(unix)] {
fn os_err(errno: i32, buf: &mut [u8]) -> Option<&str> {
let buf_ptr = buf.as_mut_ptr() as *mut libc::c_char;
let buf_ptr = buf.as_mut_ptr().cast::<libc::c_char>();
if unsafe { libc::strerror_r(errno, buf_ptr, buf.len()) } != 0 {
return None;
}
Expand Down
2 changes: 1 addition & 1 deletion src/fuchsia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
unsafe { zx_cprng_draw(dest.as_mut_ptr() as *mut u8, dest.len()) }
unsafe { zx_cprng_draw(dest.as_mut_ptr().cast::<u8>(), dest.len()) }
Ok(())
}
4 changes: 2 additions & 2 deletions src/getentropy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
//!
//! For these targets, we use getentropy(2) because getrandom(2) doesn't exist.
use crate::{util_libc::last_os_error, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
for chunk in dest.chunks_mut(256) {
let ret = unsafe { libc::getentropy(chunk.as_mut_ptr() as *mut libc::c_void, chunk.len()) };
let ret = unsafe { libc::getentropy(chunk.as_mut_ptr().cast::<c_void>(), chunk.len()) };
if ret != 0 {
return Err(last_os_error());
}
Expand Down
4 changes: 2 additions & 2 deletions src/getrandom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
//! nothing. On illumos, the default pool is used to implement getentropy(2),
//! so we assume it is acceptable here.
use crate::{util_libc::sys_fill_exact, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
sys_fill_exact(dest, |buf| unsafe {
libc::getrandom(buf.as_mut_ptr() as *mut libc::c_void, buf.len(), 0)
libc::getrandom(buf.as_mut_ptr().cast::<c_void>(), buf.len(), 0)
})
}
2 changes: 1 addition & 1 deletion src/hermit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extern "C" {

pub fn getrandom_inner(mut dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
while !dest.is_empty() {
let res = unsafe { sys_read_entropy(dest.as_mut_ptr() as *mut u8, dest.len(), 0) };
let res = unsafe { sys_read_entropy(dest.as_mut_ptr().cast::<u8>(), dest.len(), 0) };
// Positive `isize`s can be safely casted to `usize`
if res > 0 && (res as usize) <= dest.len() {
dest = &mut dest[res as usize..];
Expand Down
4 changes: 2 additions & 2 deletions src/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error>
// have a notion of "uninitialized memory", this is purely
// a Rust/C/C++ concept.
let res = n.random_fill_sync(unsafe {
Uint8Array::view_mut_raw(chunk.as_mut_ptr() as *mut u8, chunk.len())
Uint8Array::view_mut_raw(chunk.as_mut_ptr().cast::<u8>(), chunk.len())
});
if res.is_err() {
return Err(Error::NODE_RANDOM_FILL_SYNC);
Expand All @@ -60,7 +60,7 @@ pub(crate) fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error>
}

// SAFETY: `sub_buf`'s length is the same length as `chunk`
unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr() as *mut u8) };
unsafe { sub_buf.raw_copy_to_ptr(chunk.as_mut_ptr().cast::<u8>()) };
}
}
};
Expand Down
10 changes: 5 additions & 5 deletions src/netbsd.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Implementation for NetBSD
use crate::{
util_libc::{sys_fill_exact, Weak},
util_libc::{cstr, sys_fill_exact, Weak},
Error,
};
use core::{mem::MaybeUninit, ptr};
use core::{ffi::c_void, mem::MaybeUninit, ptr};

fn kern_arnd(buf: &mut [MaybeUninit<u8>]) -> libc::ssize_t {
static MIB: [libc::c_int; 2] = [libc::CTL_KERN, libc::KERN_ARND];
Expand All @@ -12,7 +12,7 @@ fn kern_arnd(buf: &mut [MaybeUninit<u8>]) -> libc::ssize_t {
libc::sysctl(
MIB.as_ptr(),
MIB.len() as libc::c_uint,
buf.as_mut_ptr() as *mut _,
buf.as_mut_ptr().cast::<c_void>(),
&mut len,
ptr::null(),
0,
Expand All @@ -29,11 +29,11 @@ type GetRandomFn = unsafe extern "C" fn(*mut u8, libc::size_t, libc::c_uint) ->

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// getrandom(2) was introduced in NetBSD 10.0
static GETRANDOM: Weak = unsafe { Weak::new("getrandom\0") };
static GETRANDOM: Weak = Weak::new(cstr::unwrap_const_from_bytes_with_nul(b"getrandom\0"));
if let Some(fptr) = GETRANDOM.ptr() {
let func: GetRandomFn = unsafe { core::mem::transmute(fptr) };
return sys_fill_exact(dest, |buf| unsafe {
func(buf.as_mut_ptr() as *mut u8, buf.len(), 0)
func(buf.as_mut_ptr().cast::<u8>(), buf.len(), 0)
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/solaris.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
//! https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2
//! which also explains why this crate should not use getentropy(2).
use crate::{util_libc::last_os_error, Error};
use core::mem::MaybeUninit;
use core::{ffi::c_void, mem::MaybeUninit};

const MAX_BYTES: usize = 1024;

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
for chunk in dest.chunks_mut(MAX_BYTES) {
let ptr = chunk.as_mut_ptr() as *mut libc::c_void;
let ptr = chunk.as_mut_ptr().cast::<c_void>();
let ret = unsafe { libc::getrandom(ptr, chunk.len(), libc::GRND_RANDOM) };
// In case the man page has a typo, we also check for negative ret.
if ret <= 0 {
Expand Down
2 changes: 1 addition & 1 deletion src/solid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ extern "C" {
}

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
let ret = unsafe { SOLID_RNG_SampleRandomBytes(dest.as_mut_ptr() as *mut u8, dest.len()) };
let ret = unsafe { SOLID_RNG_SampleRandomBytes(dest.as_mut_ptr().cast::<u8>(), dest.len()) };
if ret >= 0 {
Ok(())
} else {
Expand Down
12 changes: 7 additions & 5 deletions src/use_file.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! 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::{
cell::UnsafeCell,
ffi::c_void,
mem::MaybeUninit,
sync::atomic::{AtomicUsize, Ordering::Relaxed},
};
Expand All @@ -15,13 +16,13 @@ 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> {
let fd = get_rng_fd()?;
sys_fill_exact(dest, |buf| unsafe {
libc::read(fd, buf.as_mut_ptr() as *mut libc::c_void, buf.len())
libc::read(fd, buf.as_mut_ptr().cast::<c_void>(), buf.len())
})
}

Expand Down Expand Up @@ -56,7 +57,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);
Expand All @@ -67,8 +68,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,
Expand Down
20 changes: 13 additions & 7 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,25 @@ pub fn uninit_slice_fill_zero(slice: &mut [MaybeUninit<u8>]) -> &mut [u8] {
}

#[inline(always)]
pub fn slice_as_uninit<T>(slice: &[T]) -> &[MaybeUninit<T>] {
// SAFETY: `MaybeUninit<T>` is guaranteed to be layout-compatible with `T`.
// There is no risk of writing a `MaybeUninit<T>` into the result since
pub fn slice_as_uninit(slice: &[u8]) -> &[MaybeUninit<u8>] {
// TODO: MSRV(1.76): Use `core::ptr::from_ref`.
let ptr: *const [u8] = slice;
// SAFETY: `MaybeUninit<u8>` is guaranteed to be layout-compatible with `u8`.
// There is no risk of writing a `MaybeUninit<u8>` into the result since
// the result isn't mutable.
unsafe { &*(slice as *const [T] as *const [MaybeUninit<T>]) }
// FIXME: Avoid relying on this assumption and eliminate this cast.
unsafe { &*(ptr as *const [MaybeUninit<u8>]) }
}

/// View an mutable initialized array as potentially-uninitialized.
///
/// This is unsafe because it allows assigning uninitialized values into
/// `slice`, which would be undefined behavior.
#[inline(always)]
pub unsafe fn slice_as_uninit_mut<T>(slice: &mut [T]) -> &mut [MaybeUninit<T>] {
// SAFETY: `MaybeUninit<T>` is guaranteed to be layout-compatible with `T`.
&mut *(slice as *mut [T] as *mut [MaybeUninit<T>])
pub unsafe fn slice_as_uninit_mut(slice: &mut [u8]) -> &mut [MaybeUninit<u8>] {
// TODO: MSRV(1.76): Use `core::ptr::from_mut`.
let ptr: *mut [u8] = slice;
// SAFETY: `MaybeUninit<u8>` is guaranteed to be layout-compatible with `u8`.
// FIXME: Avoid relying on this assumption and eliminate this cast.
&mut *(ptr as *mut [MaybeUninit<u8>])
}
77 changes: 77 additions & 0 deletions src/util_cstr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//! 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 {
#[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());
}
Loading
Loading