From 557314e19837d8b66f498817fccceff9d4416b3e Mon Sep 17 00:00:00 2001 From: iansm Date: Thu, 2 Dec 2021 21:59:56 -0500 Subject: [PATCH 1/4] Generalize random arrays --- src/rand.rs | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/rand.rs b/src/rand.rs index e7ebece216..a03b7f0da6 100644 --- a/src/rand.rs +++ b/src/rand.rs @@ -72,6 +72,7 @@ where pub(crate) mod sealed { use crate::error; + use core::{mem, slice}; pub trait SecureRandom: core::fmt::Debug { /// Fills `dest` with random bytes. @@ -83,21 +84,43 @@ pub(crate) mod sealed { fn as_mut_bytes(&mut self) -> &mut [u8]; // `AsMut<[u8]>::as_mut` } - macro_rules! impl_random_arrays { - [ $($len:expr)+ ] => { - $( - impl RandomlyConstructable for [u8; $len] { - #[inline] - fn zero() -> Self { [0; $len] } + pub unsafe trait Pod: Sized + Copy { + fn zero() -> Self; + } + macro_rules! impl_pod { + ($($type:ty),*) => { + $( + unsafe impl Pod for $type { #[inline] - fn as_mut_bytes(&mut self) -> &mut [u8] { &mut self[..] } + fn zero() -> Self { 0 } } - )+ - } + )* + }; } - impl_random_arrays![4 8 16 32 48 64 128 256]; + impl_pod!(i8, u8, i16, u16, i32, u32, i64, u64, i128, u128, isize, usize); + + impl RandomlyConstructable for [T; N] { + #[inline] + fn zero() -> Self { + [T::zero(); N] + } + + #[inline] + fn as_mut_bytes(&mut self) -> &mut [u8] { + assert( + N == 0 || (isize::MAX as usize) / N < mem::size_of::(), + "Array too large to be converted to slice of u8" + ); + + let data = self.as_mut_ptr(); + let len = N * mem::size_of::(); + unsafe { + slice::from_raw_parts_mut(data as *mut u8, len) + } + } + } } /// A type that can be returned by `ring::rand::generate()`. From ac30c20f7fef6ff2401514e067d0aad35ac16c7d Mon Sep 17 00:00:00 2001 From: iansm Date: Mon, 6 Dec 2021 21:27:08 -0500 Subject: [PATCH 2/4] Improve rand API --- src/rand.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/rand.rs b/src/rand.rs index a03b7f0da6..852d002be6 100644 --- a/src/rand.rs +++ b/src/rand.rs @@ -84,7 +84,12 @@ pub(crate) mod sealed { fn as_mut_bytes(&mut self) -> &mut [u8]; // `AsMut<[u8]>::as_mut` } - pub unsafe trait Pod: Sized + Copy { + // Safety: in order for a type to be POD, it must satisfy the following criteria: + // - The type must be inhabited (ie not the never type) + // - All bit patterns must be valid + // - The type must have no padding + // - Any fields within the type must also be Pod + pub unsafe trait Pod: Sized { fn zero() -> Self; } @@ -101,21 +106,37 @@ pub(crate) mod sealed { impl_pod!(i8, u8, i16, u16, i32, u32, i64, u64, i128, u128, isize, usize); - impl RandomlyConstructable for [T; N] { + impl RandomlyConstructable for [T; N] { #[inline] fn zero() -> Self { + // Note that the Copy bound on T is required in order to initialize this array + // without unsafe code. [T::zero(); N] } #[inline] fn as_mut_bytes(&mut self) -> &mut [u8] { - assert( - N == 0 || (isize::MAX as usize) / N < mem::size_of::(), + // Since all values in this expression are constant, the compiler should optimize this + // assertion out. + assert!( + N == 0 || (isize::MAX as usize) / N >= mem::size_of::(), "Array too large to be converted to slice of u8" ); let data = self.as_mut_ptr(); let len = N * mem::size_of::(); + + // Safety: + // - `data` is valid (aligned, non-null, non-dangling) for reads and writes of size + // `len` bytes + // - The entire memory range of this slice is in a single allocated object because + // it came from another mutable slice + // - `data` is not dangling and properly aligned for all types because, again, it was + // derived from another mutable slice + // - Because T is POD, and because `len` is calculated based on the size of T, + // `data` points to `len` initialized u8s + // - We co-opt the lifetime of `self`, so no borrow checker rules are broken + // - `len` is less than `isize::MAX` by the assertion at the beginning of this function unsafe { slice::from_raw_parts_mut(data as *mut u8, len) } From 42672c512f2eb685b52e24ec27ea000b3eec0f3e Mon Sep 17 00:00:00 2001 From: Cassy343 Date: Sat, 18 Dec 2021 15:43:32 -0500 Subject: [PATCH 3/4] Remove unnecessary assertion --- src/rand.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/rand.rs b/src/rand.rs index 852d002be6..57c69ff356 100644 --- a/src/rand.rs +++ b/src/rand.rs @@ -90,16 +90,17 @@ pub(crate) mod sealed { // - The type must have no padding // - Any fields within the type must also be Pod pub unsafe trait Pod: Sized { - fn zero() -> Self; + #[inline] + fn zero() -> Self { + // Safe by the safety guarantees on the trait, notably that all bit patterns are valid + unsafe { mem::zeroed() } + } } macro_rules! impl_pod { ($($type:ty),*) => { $( - unsafe impl Pod for $type { - #[inline] - fn zero() -> Self { 0 } - } + unsafe impl Pod for $type {} )* }; } @@ -116,13 +117,6 @@ pub(crate) mod sealed { #[inline] fn as_mut_bytes(&mut self) -> &mut [u8] { - // Since all values in this expression are constant, the compiler should optimize this - // assertion out. - assert!( - N == 0 || (isize::MAX as usize) / N >= mem::size_of::(), - "Array too large to be converted to slice of u8" - ); - let data = self.as_mut_ptr(); let len = N * mem::size_of::(); @@ -136,7 +130,7 @@ pub(crate) mod sealed { // - Because T is POD, and because `len` is calculated based on the size of T, // `data` points to `len` initialized u8s // - We co-opt the lifetime of `self`, so no borrow checker rules are broken - // - `len` is less than `isize::MAX` by the assertion at the beginning of this function + // - `len` is less than `isize::MAX` since it was derived from the size of `self` unsafe { slice::from_raw_parts_mut(data as *mut u8, len) } From 89f8bc7a4aa1955a8bac8b3e4bf05c1925a5affd Mon Sep 17 00:00:00 2001 From: Cassy343 Date: Sat, 18 Dec 2021 15:44:19 -0500 Subject: [PATCH 4/4] Add rand construction test --- tests/rand_tests.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/rand_tests.rs b/tests/rand_tests.rs index 0685a6dcee..a129cf73f5 100644 --- a/tests/rand_tests.rs +++ b/tests/rand_tests.rs @@ -66,6 +66,31 @@ fn test_system_random_lengths() { } } +#[test] +fn test_randomly_constructable() { + // Picked arbitrarily + const BY: u8 = 0x5A; + + let rng = test::rand::FixedByteRandom { + byte: BY + }; + + assert!(rand::generate::<[u8; 0]>(&rng).is_ok()); + assert!(rand::generate::<[u64; 0]>(&rng).is_ok()); + assert!(rand::generate::<[usize; 0]>(&rng).is_ok()); + + let arr: [u8; 256] = rand::generate(&rng) + .unwrap() + .expose(); + assert!(arr.iter().all(|&x| x == BY)); + + let arr: [u32; 256] = rand::generate(&rng) + .unwrap() + .expose(); + // Endianness doesn't matter since all the bytes are the same + assert!(arr.iter().all(|&x| x.to_le_bytes() == [BY; 4])); +} + #[test] fn test_system_random_traits() { test::compile_time_assert_clone::();