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

Android/Linux: Support msan; unpoison output of getrandom syscall. #463

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ jobs:
- if: ${{ matrix.toolchain == 'nightly' }}
run: cargo test --benches

sanitizer-tests:
name: Sanitizer Tests
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-22.04]
target: [
x86_64-unknown-linux-gnu,
]
toolchain: [nightly]
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@master
with:
components: rust-src
targets: ${{ matrix.target }}
toolchain: ${{ matrix.toolchain }}
- uses: Swatinem/rust-cache@v2
- run: RUSTFLAGS="-Zsanitizer=memory" cargo test -Zbuild-std --target=${{ matrix.target }} --features=unstable-sanitize
- run: RUSTFLAGS="-Zsanitizer=memory" cargo test -Zbuild-std --target=${{ matrix.target }} --features=unstable-sanitize,std
- run: RUSTFLAGS="-Zsanitizer=memory" cargo test -Zbuild-std --target=${{ matrix.target }} --features=unstable-sanitize,linux_disable_fallback
# Doctests fail to link so use `--all-targets` to avoid them.
- run: RUSTFLAGS="-Zsanitizer=memory" cargo test -Zbuild-std --target=${{ matrix.target }} --features=unstable-sanitize,custom --all-targets

linux-tests:
name: Linux Test
runs-on: ubuntu-22.04
Expand Down
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ rustc-dep-of-std = [
"libc/rustc-dep-of-std",
"wasi/rustc-dep-of-std",
]
# Enable support for sanitizers; Requires Rust feature `cfg_sanitize`.
unstable-sanitize = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to use a configuration flag for this instead of this crate feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. The good thing about using a feature flag is that other crates can then trigger it automatically with their own feature flag depending on it. That's what I've been planning to do in ring, for example.

# Unstable/test-only feature to run wasm-bindgen tests in a browser
test-in-browser = []

Expand Down
3 changes: 3 additions & 0 deletions src/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// compatibility with implementations that rely on that (e.g. Rust
// implementations that construct a `&mut [u8]` slice from `dest` and
// `len`).
// XXX: Because we do this, memory sanitizer isn't able to detect when
// `__getrandom_custom` fails to fill `dest`, but we can't poison `dest`
// here either, for the same reason we have to fill it in the first place.
let dest = uninit_slice_fill_zero(dest);
let ret = unsafe { __getrandom_custom(dest.as_mut_ptr(), dest.len()) };
match NonZeroU32::new(ret) {
Expand Down
16 changes: 16 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@
//!
//! Pull Requests that add support for new targets to `getrandom` are always welcome.
//!
//! ## Memory Sanitizer (msan) support
//!
//! The `unstable-sanitize` feature adds Memory Sanitizer support. You must use
//! Rust Nightly, e.g.
//! ```sh
//! RUSTFLAGS="-Zsanitizer=memory" \
//! cargo +nightly test \
//! -Zbuild-std --target=x86_64-unknown-linux-gnu --features=unstable-sanitize
//! ```
//! It is assumed that libstd/libc have had their APis instrumented to support
//! sanitizers, so we only provide special support on Linux when using the
//! `getrandom` syscall.
//!
//! ## Unsupported targets
//!
//! By default, `getrandom` will not compile on unsupported targets, but certain
Expand Down Expand Up @@ -208,6 +221,7 @@
#![no_std]
#![warn(rust_2018_idioms, unused_lifetimes, missing_docs)]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![cfg_attr(feature = "unstable-sanitize", feature(cfg_sanitize))]

#[macro_use]
extern crate cfg_if;
Expand Down Expand Up @@ -300,9 +314,11 @@ cfg_if! {
mod use_file;
mod lazy;
mod linux_android;
mod util_syscall_linux;
#[path = "linux_android_with_fallback.rs"] mod imp;
} else if #[cfg(any(target_os = "android", target_os = "linux"))] {
mod util_libc;
mod util_syscall_linux;
#[path = "linux_android.rs"] mod imp;
} else if #[cfg(target_os = "solaris")] {
mod util_libc;
Expand Down
19 changes: 15 additions & 4 deletions src/linux_android.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Implementation for Linux / Android without `/dev/urandom` fallback
use crate::{util_libc, Error};
use crate::{util_libc, util_syscall_linux, Error};
use core::mem::MaybeUninit;

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
Expand All @@ -8,12 +8,23 @@ pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {

// Also used by linux_android_with_fallback to check if the syscall is available.
pub fn getrandom_syscall(buf: &mut [MaybeUninit<u8>]) -> libc::ssize_t {
unsafe {
util_syscall_linux::pre_write_range(buf.as_mut_ptr(), buf.len());
let res = unsafe {
libc::syscall(
libc::SYS_getrandom,
buf.as_mut_ptr().cast::<core::ffi::c_void>(),
buf.len(),
0,
) as libc::ssize_t
}
)
} as libc::ssize_t;
if let Ok(written) = usize::try_from(res) {
// XXX: LLVM has support to do this automatically if/when libc is
// compiled with it, but glibc that ships in typical Linux distros
// doesn't. Assume Android's Bionic is similar. `-Zsanitizer=memory`
// is not compatible with `+crt-static` according to rustc.
unsafe {
util_syscall_linux::post_write_range(buf.as_mut_ptr(), written);
}
};
res
}
104 changes: 104 additions & 0 deletions src/util_syscall_linux.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Support for raw system calls on Linux.
//
// # Sanitizers
//
// Currently only Memory Sanitizer is actively supported.
//
// TODO: Support address sanitizer, in particular in `pre_write_range`.
//
// ## Memory Sanitizer
//
// See https://github.com/llvm/llvm-project/commit/ac9ee01fcbfac745aaedca0393a8e1c8a33acd8d:
// LLVM uses:
// ```c
// COMMON_INTERCEPTOR_ENTER(ctx, getrandom, buf, buflen, flags);
// SSIZE_T n = REAL(getrandom)(buf, buflen, flags);
// if (n > 0) {
// COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, n);
// }
// ```
// and:
// ```c
// #define PRE_SYSCALL(name) \
// SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_syscall_pre_impl_##name
// #define PRE_WRITE(p, s) COMMON_SYSCALL_PRE_WRITE_RANGE(p, s)
// #define POST_WRITE(p, s) COMMON_SYSCALL_POST_WRITE_RANGE(p, s)
// PRE_SYSCALL(getrandom)(void *buf, uptr count, long flags) {
// if (buf) {
// PRE_WRITE(buf, count);
// }
// }
//
// POST_SYSCALL(getrandom)(long res, void *buf, uptr count, long flags) {
// if (res > 0 && buf) {
// POST_WRITE(buf, res);
// }
// }
// ```

use core::mem::MaybeUninit;

// MSAN defines:
//
// ```c
// #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
// if (msan_init_is_running) \
// return REAL(func)(__VA_ARGS__); \
// ENSURE_MSAN_INITED(); \
// MSanInterceptorContext msan_ctx = {IsInInterceptorScope()}; \
// ctx = (void *)&msan_ctx; \
// (void)ctx; \
// InterceptorScope interceptor_scope; \
// __msan_unpoison(__errno_location(), sizeof(int));
// ```
//
// * We assume that memory sanitizer will not use the this crate during the
// initialization of msan, so we don't have to worry about
// `msan_init_is_running`.
// * We assume that rustc/LLVM initializes MSAN before executing any Rust code,
// so we don't need to call `ENSURE_MSAN_INITED`.
// * Notice that `COMMON_INTERCEPTOR_WRITE_RANGE` doesn't use `ctx`, which
// means it is oblivious to `IsInInterceptorScope()`, so we don't have to
// call it. More generally, we don't have to worry about interceptor scopes
// because we are not an interceptor.
// * We don't read from `__errno_location()` so we don't need to unpoison it.
//
// Consequently, MSAN's `COMMON_INTERCEPTOR_ENTER` is a no-op.
//
// MSAN defines:
// ```c
// #define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
// do { \
// } while (false)
// ```
// So MSAN's PRE_SYSCALL hook is also a no-op.
//
// Consequently, we have nothing to do before invoking the syscall unless/until
// we support other sanitizers like ASAN.
#[allow(unused_variables)]
pub fn pre_write_range(_ptr: *mut MaybeUninit<u8>, _size: usize) {}

// MSNA defines:
// ```c
// #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
// __msan_unpoison(ptr, size)
// ```
// and:
// ```c
// #define COMMON_SYSCALL_POST_WRITE_RANGE(p, s) __msan_unpoison(p, s)
// ```
#[allow(unused_variables)]
pub unsafe fn post_write_range(ptr: *mut MaybeUninit<u8>, size: usize) {
#[cfg(feature = "unstable-sanitize")]
{
#[cfg(sanitize = "memory")]
{
use core::ffi::c_void;
extern "C" {
// void __msan_unpoison(const volatile void *a, size_t size);
fn __msan_unpoison(a: *mut c_void, size: usize);
}
__msan_unpoison(ptr.cast::<c_void>(), size)
}
}
}
95 changes: 78 additions & 17 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
use super::getrandom_impl;
use super::{getrandom_impl, getrandom_uninit_impl};
use core::mem::MaybeUninit;
#[cfg(not(feature = "custom"))]
use getrandom::Error;

#[cfg(all(target_arch = "wasm32", target_os = "unknown"))]
use wasm_bindgen_test::wasm_bindgen_test as test;

#[cfg(feature = "test-in-browser")]
wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

#[cfg(not(feature = "custom"))]
fn wrapped_getrandom(dest: &mut [u8]) -> Result<&mut [u8], Error> {
getrandom_impl(dest).map(|()| dest)
}

// Test that APIs are happy with zero-length requests
#[test]
fn test_zero() {
// Test that APIs are happy with zero-length requests
getrandom_impl(&mut [0u8; 0]).unwrap();
getrandom_impl(&mut []).unwrap();
}
#[test]
fn test_zero_uninit() {
getrandom_uninit_impl(&mut []).unwrap();
}

// Return the number of bits in which s1 and s2 differ
Expand All @@ -23,52 +35,101 @@ fn num_diff_bits(s1: &[u8], s2: &[u8]) -> usize {
}

// Tests the quality of calling getrandom on two large buffers
#[test]

#[cfg(not(feature = "custom"))]
fn test_diff() {
let mut v1 = [0u8; 1000];
getrandom_impl(&mut v1).unwrap();
fn test_diff_large<T: Copy>(initial: T, f: impl Fn(&mut [T]) -> Result<&mut [u8], Error>) {
let mut v1 = [initial; 1000];
let r1 = f(&mut v1).unwrap();

let mut v2 = [0u8; 1000];
getrandom_impl(&mut v2).unwrap();
let mut v2 = [initial; 1000];
let r2 = f(&mut v2).unwrap();

// Between 3.5 and 4.5 bits per byte should differ. Probability of failure:
// ~ 2^(-94) = 2 * CDF[BinomialDistribution[8000, 0.5], 3500]
let d = num_diff_bits(&v1, &v2);
let d = num_diff_bits(r1, r2);
assert!(d > 3500);
assert!(d < 4500);
}

// Tests the quality of calling getrandom repeatedly on small buffers
#[cfg(not(feature = "custom"))]
#[test]
fn test_large() {
test_diff_large(0u8, wrapped_getrandom);
}

#[cfg(not(feature = "custom"))]
fn test_small() {
#[test]
fn test_large_uninit() {
test_diff_large(MaybeUninit::uninit(), getrandom_uninit_impl);
}

// Tests the quality of calling getrandom repeatedly on small buffers

#[cfg(not(feature = "custom"))]
fn test_diff_small<T: Copy>(initial: T, f: impl Fn(&mut [T]) -> Result<&mut [u8], Error>) {
// For each buffer size, get at least 256 bytes and check that between
// 3 and 5 bits per byte differ. Probability of failure:
// ~ 2^(-91) = 64 * 2 * CDF[BinomialDistribution[8*256, 0.5], 3*256]
for size in 1..=64 {
let mut num_bytes = 0;
let mut diff_bits = 0;
while num_bytes < 256 {
let mut s1 = vec![0u8; size];
getrandom_impl(&mut s1).unwrap();
let mut s2 = vec![0u8; size];
getrandom_impl(&mut s2).unwrap();
let mut s1 = vec![initial; size];
let r1 = f(&mut s1).unwrap();
let mut s2 = vec![initial; size];
let r2 = f(&mut s2).unwrap();

num_bytes += size;
diff_bits += num_diff_bits(&s1, &s2);
diff_bits += num_diff_bits(r1, r2);
}
assert!(diff_bits > 3 * num_bytes);
assert!(diff_bits < 5 * num_bytes);
}
}

#[cfg(not(feature = "custom"))]
#[test]
fn test_small() {
test_diff_small(0u8, wrapped_getrandom);
}

#[cfg(not(feature = "custom"))]
#[test]
fn test_small_unnit() {
test_diff_small(MaybeUninit::uninit(), getrandom_uninit_impl);
}

#[test]
fn test_huge() {
let mut huge = [0u8; 100_000];
getrandom_impl(&mut huge).unwrap();
}

#[test]
fn test_huge_uninit() {
let mut huge = [MaybeUninit::uninit(); 100_000];
getrandom_uninit_impl(&mut huge).unwrap();
check_initialized(&huge);
}

#[allow(unused_variables)]
fn check_initialized(buf: &[MaybeUninit<u8>]) {
#[cfg(feature = "unstable-sanitize")]
{
#[cfg(sanitize = "memory")]
{
use core::ffi::c_void;
extern "C" {
// void __msan_check_mem_is_initialized(const volatile void *x, size_t size);
fn __msan_check_mem_is_initialized(x: *const c_void, size: usize);
}
unsafe {
__msan_check_mem_is_initialized(buf.as_ptr().cast::<c_void>(), buf.len());
}
}
}
}

// On WASM, the thread API always fails/panics
#[cfg(not(target_arch = "wasm32"))]
#[test]
Expand Down
3 changes: 2 additions & 1 deletion tests/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
feature = "custom",
not(feature = "js")
))]
#![cfg_attr(feature = "unstable-sanitize", feature(cfg_sanitize))]

use wasm_bindgen_test::wasm_bindgen_test as test;

Expand Down Expand Up @@ -34,7 +35,7 @@ fn super_insecure_rng(buf: &mut [u8]) -> Result<(), Error> {

register_custom_getrandom!(super_insecure_rng);

use getrandom::getrandom as getrandom_impl;
use getrandom::{getrandom as getrandom_impl, getrandom_uninit as getrandom_uninit_impl};
mod common;

#[test]
Expand Down
Loading
Loading