From dc89211bcac956025de0a4cf02da49ba29064ffa Mon Sep 17 00:00:00 2001 From: Artyom Pavlov Date: Thu, 19 Dec 2024 00:28:03 +0300 Subject: [PATCH 1/2] Allow `Error::raw_os_error` to return non-`i32` codes (#569) This allows us to remove some UEFI-specific exceptions from the code. This PR only partially resolves #568 since we still use `NonZeroU32` on UEFI targets, but it should be sufficient to future-proof ourselves for the v0.3 release. --- src/error.rs | 41 ++++++++++++++++++++++++++++------------- src/error_std_impls.rs | 5 ----- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/error.rs b/src/error.rs index 125051ee..4004af57 100644 --- a/src/error.rs +++ b/src/error.rs @@ -3,6 +3,16 @@ extern crate std; use core::{fmt, num::NonZeroU32}; +// This private alias mirrors `std::io::RawOsError`: +// https://doc.rust-lang.org/std/io/type.RawOsError.html) +cfg_if::cfg_if!( + if #[cfg(target_os = "uefi")] { + type RawOsError = usize; + } else { + type RawOsError = i32; + } +); + /// A small and `no_std` compatible error type /// /// The [`Error::raw_os_error()`] will indicate if the error is from the OS, and @@ -57,20 +67,25 @@ impl Error { /// Extract the raw OS error code (if this error came from the OS) /// /// This method is identical to [`std::io::Error::raw_os_error()`][1], except - /// that it works in `no_std` contexts. If this method returns `None`, the - /// error value can still be formatted via the `Display` implementation. + /// that it works in `no_std` contexts. On most targets this method returns + /// `Option`, but some platforms (e.g. UEFI) may use a different primitive + /// type like `usize`. Consult with the [`RawOsError`] docs for more information. + /// + /// If this method returns `None`, the error value can still be formatted via + /// the `Display` implementation. /// /// [1]: https://doc.rust-lang.org/std/io/struct.Error.html#method.raw_os_error + /// [`RawOsError`]: https://doc.rust-lang.org/std/io/type.RawOsError.html #[inline] - pub fn raw_os_error(self) -> Option { - i32::try_from(self.0.get()).ok().map(|errno| { - // On SOLID, negate the error code again to obtain the original error code. - if cfg!(target_os = "solid_asp3") { - -errno - } else { - errno - } - }) + pub fn raw_os_error(self) -> Option { + let code = self.0.get(); + if code >= Self::INTERNAL_START { + return None; + } + let errno = RawOsError::try_from(code).ok()?; + #[cfg(target_os = "solid_asp3")] + let errno = -errno; + Some(errno) } /// Creates a new instance of an `Error` from a particular custom error code. @@ -134,7 +149,7 @@ impl fmt::Debug for Error { let mut dbg = f.debug_struct("Error"); if let Some(errno) = self.raw_os_error() { dbg.field("os_error", &errno); - #[cfg(all(feature = "std", not(target_os = "uefi")))] + #[cfg(feature = "std")] dbg.field("description", &std::io::Error::from_raw_os_error(errno)); } else if let Some(desc) = self.internal_desc() { dbg.field("internal_code", &self.0.get()); @@ -150,7 +165,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(errno) = self.raw_os_error() { cfg_if! { - if #[cfg(all(feature = "std", not(target_os = "uefi")))] { + if #[cfg(feature = "std")] { std::io::Error::from_raw_os_error(errno).fmt(f) } else { write!(f, "OS Error: {}", errno) diff --git a/src/error_std_impls.rs b/src/error_std_impls.rs index 569165e3..2c326012 100644 --- a/src/error_std_impls.rs +++ b/src/error_std_impls.rs @@ -5,15 +5,10 @@ use std::io; impl From for io::Error { fn from(err: Error) -> Self { - #[cfg(not(target_os = "uefi"))] match err.raw_os_error() { Some(errno) => io::Error::from_raw_os_error(errno), None => io::Error::new(io::ErrorKind::Other, err), } - #[cfg(target_os = "uefi")] - { - io::Error::new(io::ErrorKind::Other, err) - } } } From 9b902af17175fc5bc7ed4ee835376622691ae7f1 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Wed, 18 Dec 2024 14:36:13 -0800 Subject: [PATCH 2/2] Detect use of MemorySanitizer without using Nightly-only features (#571) This allows msan detection to "just-work" whenever someone passes `-Zsanitizer=memory`. Users no longer need to do any `getrandom`-specific configuration. This will also continue working once https://github.com/rust-lang/rust/issues/123615 is merged, which stabilizes some sanitizers (but not MemorySanitizer). This is the approch taken by other low-level crates: - [`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs) - [`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42) The only downside is that this adds a build-script, but it's as small as possible, doesn't seem to impact build times, and is only a temporary workaround. --------- Signed-off-by: Joe Richey --- .github/workflows/tests.yml | 6 +++--- CHANGELOG.md | 3 ++- Cargo.toml | 2 +- README.md | 13 ++++++------- build.rs | 9 +++++++++ src/lib.rs | 7 ++----- 6 files changed, 23 insertions(+), 17 deletions(-) create mode 100644 build.rs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ae4910ea..673fd370 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -140,9 +140,9 @@ jobs: toolchain: nightly-2024-10-08 components: rust-src - env: - RUSTFLAGS: -Dwarnings -Zsanitizer=memory --cfg getrandom_sanitize - # `--all-targets` is used to skip doc tests which currently fail linking - run: cargo test -Zbuild-std --target=x86_64-unknown-linux-gnu --all-targets + RUSTFLAGS: -Dwarnings -Zsanitizer=memory + RUSTDOCFLAGS: -Dwarnings -Zsanitizer=memory + run: cargo test -Zbuild-std --target=x86_64-unknown-linux-gnu cross: name: Cross diff --git a/CHANGELOG.md b/CHANGELOG.md index 36cabb35..afdefce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Error::new_custom` method [#507] - `rndr` opt-in backend [#512] - `linux_rustix` opt-in backend [#520] -- Memory sanitizer support gated behind `getrandom_sanitize` configuration flag [#521] +- Automatic MemorySanitizer support [#521] [#571] - `u32` and `u64` functions for generating random values of the respective type [#544] ### Fixed @@ -61,6 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#554]: https://github.com/rust-random/getrandom/pull/554 [#555]: https://github.com/rust-random/getrandom/pull/555 [#557]: https://github.com/rust-random/getrandom/pull/557 +[#571]: https://github.com/rust-random/getrandom/pull/571 ## [0.2.15] - 2024-05-06 ### Added diff --git a/Cargo.toml b/Cargo.toml index e8c00a58..57fe2274 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,7 +81,7 @@ rustc-dep-of-std = ["dep:compiler_builtins", "dep:core"] level = "warn" check-cfg = [ 'cfg(getrandom_backend, values("custom", "rdrand", "rndr", "linux_getrandom", "linux_rustix", "wasm_js", "esp_idf"))', - 'cfg(getrandom_sanitize)', + 'cfg(getrandom_msan)', 'cfg(getrandom_test_linux_fallback)', 'cfg(getrandom_test_netbsd_fallback)', ] diff --git a/README.md b/README.md index ebd1b76a..7ece9ecf 100644 --- a/README.md +++ b/README.md @@ -267,15 +267,14 @@ our code should correctly handle it and return an error, e.g. ## Sanitizer support -If your code uses [`fill_uninit`] and you enable memory sanitization -(i.e. `-Zsanitizer=memory`), you need to pass the `getrandom_sanitize` -configuration flag to enable unpoisoning of the destination buffer -filled by `fill_uninit`. +If your code uses [`fill_uninit`] and you enable +[MemorySanitizer](https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html#memorysanitizer) +(i.e. `-Zsanitizer=memory`), we will automatically handle unpoisoning +of the destination buffer filled by `fill_uninit`. -For example, it can be done as follows (requires a Nightly compiler): +You can run sanitizer tests for your crate dependent on `getrandom` like this: ```sh -RUSTFLAGS="-Zsanitizer=memory --cfg getrandom_sanitize" \ - cargo test -Zbuild-std --target=x86_64-unknown-linux-gnu +RUSTFLAGS="-Zsanitizer=memory" cargo test -Zbuild-std --target=x86_64-unknown-linux-gnu ``` ## Minimum Supported Rust Version diff --git a/build.rs b/build.rs new file mode 100644 index 00000000..15d41919 --- /dev/null +++ b/build.rs @@ -0,0 +1,9 @@ +// Automatically detect cfg(sanitize = "memory") even if cfg(sanitize) isn't +// supported. Build scripts get cfg() info, even if the cfg is unstable. +fn main() { + println!("cargo:rerun-if-changed=build.rs"); + let santizers = std::env::var("CARGO_CFG_SANITIZE").unwrap_or_default(); + if santizers.contains("memory") { + println!("cargo:rustc-cfg=getrandom_msan"); + } +} diff --git a/src/lib.rs b/src/lib.rs index 5b0f47fd..2ac0ad0b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,6 @@ #![doc = include_str!("../README.md")] #![warn(rust_2018_idioms, unused_lifetimes, missing_docs)] #![cfg_attr(docsrs, feature(doc_auto_cfg))] -#![cfg_attr(getrandom_sanitize, feature(cfg_sanitize))] #![deny( clippy::cast_lossless, clippy::cast_possible_truncation, @@ -99,8 +98,7 @@ pub fn fill_uninit(dest: &mut [MaybeUninit]) -> Result<&mut [u8], Error> { backends::fill_inner(dest)?; } - #[cfg(getrandom_sanitize)] - #[cfg(sanitize = "memory")] + #[cfg(getrandom_msan)] extern "C" { fn __msan_unpoison(a: *mut core::ffi::c_void, size: usize); } @@ -108,8 +106,7 @@ pub fn fill_uninit(dest: &mut [MaybeUninit]) -> Result<&mut [u8], Error> { // SAFETY: `dest` has been fully initialized by `imp::fill_inner` // since it returned `Ok`. Ok(unsafe { - #[cfg(getrandom_sanitize)] - #[cfg(sanitize = "memory")] + #[cfg(getrandom_msan)] __msan_unpoison(dest.as_mut_ptr().cast(), dest.len()); util::slice_assume_init_mut(dest)