From a56faa8ca51566a39abc0059b95a0739dc0038de Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 2 Feb 2023 14:09:52 -0800 Subject: [PATCH 1/6] Use correct flag name The flag name is +rdrand not +rdrnd Signed-off-by: Joe Richey --- src/rdrand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 910b5350..547cee11 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -45,10 +45,10 @@ unsafe fn rdrand() -> Result<[u8; WORD_SIZE], Error> { Err(Error::FAILED_RDRAND) } -// "rdrand" target feature requires "+rdrnd" flag, see https://github.com/rust-lang/rust/issues/49653. +// "rdrand" target feature requires "+rdrand" flag, see https://github.com/rust-lang/rust/issues/49653. #[cfg(all(target_env = "sgx", not(target_feature = "rdrand")))] compile_error!( - "SGX targets require 'rdrand' target feature. Enable by using -C target-feature=+rdrnd." + "SGX targets require 'rdrand' target feature. Enable by using -C target-feature=+rdrand." ); #[cfg(target_feature = "rdrand")] From c4f7f45351d959c49f770de934776eb36fe62921 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 2 Feb 2023 14:12:09 -0800 Subject: [PATCH 2/6] rdrand: Remove check for 0 and !0 values Also makes rdrand() return an integer instead of an array of bytes. This will make the self-test implementation easier. As we can just return a `usize` we no longer need the `WORD_SIZE` constant. Signed-off-by: Joe Richey --- src/rdrand.rs | 46 ++++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 547cee11..41280019 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -7,8 +7,11 @@ // except according to those terms. //! Implementation for SGX using RDRAND instruction -use crate::{util::slice_as_uninit, Error}; -use core::mem::{self, MaybeUninit}; +use crate::{ + util::{slice_as_uninit, LazyBool}, + Error, +}; +use core::mem::{size_of, MaybeUninit}; cfg_if! { if #[cfg(target_arch = "x86_64")] { @@ -24,25 +27,16 @@ cfg_if! { // Implementation Guide" - Section 5.2.1 and "Intel® 64 and IA-32 Architectures // Software Developer’s Manual" - Volume 1 - Section 7.3.17.1. const RETRY_LIMIT: usize = 10; -const WORD_SIZE: usize = mem::size_of::(); #[target_feature(enable = "rdrand")] -unsafe fn rdrand() -> Result<[u8; WORD_SIZE], Error> { +unsafe fn rdrand() -> Option { for _ in 0..RETRY_LIMIT { - let mut el = mem::zeroed(); - if rdrand_step(&mut el) == 1 { - // AMD CPUs from families 14h to 16h (pre Ryzen) sometimes fail to - // set CF on bogus random data, so we check these values explicitly. - // See https://github.com/systemd/systemd/issues/11810#issuecomment-489727505 - // We perform this check regardless of target to guard against - // any implementation that incorrectly fails to set CF. - if el != 0 && el != !0 { - return Ok(el.to_ne_bytes()); - } - // Keep looping in case this was a false positive. + let mut val = 0; + if rdrand_step(&mut val) == 1 { + return Some(val as usize); } } - Err(Error::FAILED_RDRAND) + None } // "rdrand" target feature requires "+rdrand" flag, see https://github.com/rust-lang/rust/issues/49653. @@ -73,25 +67,25 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { if !is_rdrand_supported() { return Err(Error::NO_RDRAND); } - - // SAFETY: After this point, rdrand is supported, so calling the rdrand - // functions is not undefined behavior. - unsafe { rdrand_exact(dest) } + rdrand_exact(dest).ok_or(Error::FAILED_RDRAND) } -#[target_feature(enable = "rdrand")] -unsafe fn rdrand_exact(dest: &mut [MaybeUninit]) -> Result<(), Error> { +fn rdrand_exact(dest: &mut [MaybeUninit]) -> Option<()> { // We use chunks_exact_mut instead of chunks_mut as it allows almost all // calls to memcpy to be elided by the compiler. - let mut chunks = dest.chunks_exact_mut(WORD_SIZE); + let mut chunks = dest.chunks_exact_mut(size_of::()); for chunk in chunks.by_ref() { - chunk.copy_from_slice(slice_as_uninit(&rdrand()?)); + // SAFETY: After this point, we know rdrand is supported, so calling + // rdrand is not undefined behavior. + let src = unsafe { rdrand() }?.to_ne_bytes(); + chunk.copy_from_slice(slice_as_uninit(&src)); } let tail = chunks.into_remainder(); let n = tail.len(); if n > 0 { - tail.copy_from_slice(slice_as_uninit(&rdrand()?[..n])); + let src = unsafe { rdrand() }?.to_ne_bytes(); + tail.copy_from_slice(slice_as_uninit(&src[..n])); } - Ok(()) + Some(()) } From 82c3cb890175b37644a5259d2f91d30770ba5af3 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 2 Feb 2023 14:16:08 -0800 Subject: [PATCH 3/6] rdrand: Add CPUID feature detection and self-test Signed-off-by: Joe Richey --- src/rdrand.rs | 68 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 41280019..14dd8ed2 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -45,26 +45,66 @@ compile_error!( "SGX targets require 'rdrand' target feature. Enable by using -C target-feature=+rdrand." ); -#[cfg(target_feature = "rdrand")] -fn is_rdrand_supported() -> bool { - true +// Run a small self-test to make sure we aren't repeating values +// Adapted from Linux's test in arch/x86/kernel/cpu/rdrand.c +// Fails with probability < 2^(-90) on 32-bit systems +#[target_feature(enable = "rdrand")] +unsafe fn self_test() -> bool { + // On AMD, RDRAND returns usize::MAX on failure, count it as a collision. + let mut prev = usize::MAX; + let mut fails = 0; + for _ in 0..8 { + match rdrand() { + Some(val) if val == prev => fails += 1, + Some(val) => prev = val, + None => return false, + }; + } + fails <= 2 } -// TODO use is_x86_feature_detected!("rdrand") when that works in core. See: -// https://github.com/rust-lang-nursery/stdsimd/issues/464 -#[cfg(not(target_feature = "rdrand"))] -fn is_rdrand_supported() -> bool { - use crate::util::LazyBool; +fn is_rdrand_good() -> bool { + #[cfg(not(target_feature = "rdrand"))] + { + // SAFETY: All Rust x86 targets are new enough to have CPUID, and we + // check that leaf 1 is supported before using it. + let cpuid0 = unsafe { arch::__cpuid(0) }; + if cpuid0.eax < 1 { + return false; + } + let cpuid1 = unsafe { arch::__cpuid(1) }; + + let vendor_id = [ + cpuid0.ebx.to_le_bytes(), + cpuid0.edx.to_le_bytes(), + cpuid0.ecx.to_le_bytes(), + ]; + if vendor_id == [*b"Auth", *b"enti", *b"cAMD"] { + let mut family = (cpuid1.eax >> 8) & 0xF; + if family == 0xF { + family += (cpuid1.eax >> 20) & 0xFF; + } + // AMD CPUs families before 17h (Zen) sometimes fail to set CF when + // RDRAND fails after suspend. Don't use RDRAND on those families. + // See https://bugzilla.redhat.com/show_bug.cgi?id=1150286 + if family < 0x17 { + return false; + } + } + + const RDRAND_FLAG: u32 = 1 << 30; + if cpuid1.ecx & RDRAND_FLAG == 0 { + return false; + } + } - // SAFETY: All Rust x86 targets are new enough to have CPUID, and if CPUID - // is supported, CPUID leaf 1 is always supported. - const FLAG: u32 = 1 << 30; - static HAS_RDRAND: LazyBool = LazyBool::new(); - HAS_RDRAND.unsync_init(|| unsafe { (arch::__cpuid(1).ecx & FLAG) != 0 }) + // SAFETY: We have already checked that rdrand is available. + unsafe { self_test() } } pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { - if !is_rdrand_supported() { + static RDRAND_GOOD: LazyBool = LazyBool::new(); + if !RDRAND_GOOD.unsync_init(is_rdrand_good) { return Err(Error::NO_RDRAND); } rdrand_exact(dest).ok_or(Error::FAILED_RDRAND) From f39423084cf1f7958aea0c088b15c83a19b1c677 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 6 Feb 2023 13:11:42 -0800 Subject: [PATCH 4/6] Use !0 instead of usize::MAX We could use core::usize::MAX, but that is deprecated. Signed-off-by: Joe Richey --- src/rdrand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 14dd8ed2..c2e52971 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -50,8 +50,8 @@ compile_error!( // Fails with probability < 2^(-90) on 32-bit systems #[target_feature(enable = "rdrand")] unsafe fn self_test() -> bool { - // On AMD, RDRAND returns usize::MAX on failure, count it as a collision. - let mut prev = usize::MAX; + // On AMD, RDRAND returns 0xFF...FF on failure, count it as a collision. + let mut prev = !0; // TODO(MSRV 1.43): Move to usize::MAX let mut fails = 0; for _ in 0..8 { match rdrand() { From 6a3fac8a01ceb6e17d9f33a2fb452b7c78b1ffac Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 8 Feb 2023 14:36:43 -0800 Subject: [PATCH 5/6] Remove comment implying this code only runs on SGX Signed-off-by: Joe Richey --- src/rdrand.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index c2e52971..64ae38a3 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -5,8 +5,6 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. - -//! Implementation for SGX using RDRAND instruction use crate::{ util::{slice_as_uninit, LazyBool}, Error, From 5f7aadf0ea90c202b41a911447b869444d6b56dc Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 8 Feb 2023 14:37:21 -0800 Subject: [PATCH 6/6] Make rdrand_exact #[target_feature(enable = "rdrand")] Signed-off-by: Joe Richey --- src/rdrand.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/rdrand.rs b/src/rdrand.rs index 64ae38a3..69f6a5d1 100644 --- a/src/rdrand.rs +++ b/src/rdrand.rs @@ -105,24 +105,25 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit]) -> Result<(), Error> { if !RDRAND_GOOD.unsync_init(is_rdrand_good) { return Err(Error::NO_RDRAND); } - rdrand_exact(dest).ok_or(Error::FAILED_RDRAND) + // SAFETY: After this point, we know rdrand is supported. + unsafe { rdrand_exact(dest) }.ok_or(Error::FAILED_RDRAND) } -fn rdrand_exact(dest: &mut [MaybeUninit]) -> Option<()> { +// TODO: make this function safe when we have feature(target_feature_11) +#[target_feature(enable = "rdrand")] +unsafe fn rdrand_exact(dest: &mut [MaybeUninit]) -> Option<()> { // We use chunks_exact_mut instead of chunks_mut as it allows almost all // calls to memcpy to be elided by the compiler. let mut chunks = dest.chunks_exact_mut(size_of::()); for chunk in chunks.by_ref() { - // SAFETY: After this point, we know rdrand is supported, so calling - // rdrand is not undefined behavior. - let src = unsafe { rdrand() }?.to_ne_bytes(); + let src = rdrand()?.to_ne_bytes(); chunk.copy_from_slice(slice_as_uninit(&src)); } let tail = chunks.into_remainder(); let n = tail.len(); if n > 0 { - let src = unsafe { rdrand() }?.to_ne_bytes(); + let src = rdrand()?.to_ne_bytes(); tail.copy_from_slice(slice_as_uninit(&src[..n])); } Some(())