Skip to content

Commit adb19e7

Browse files
committed
Implement Memory Sanitizer unpoisoning more precisely.
Avoid masking bugs in backends that return `Ok` without writing the entire output buffer. For built-in backends, this makes MSAN useful for validating the correctness of the control flow of each backend. For custom backends in particular, this makes MSAN useful for validating that a custom backend actually filled the entire output buffer when it returned `Ok`. When MSAN is enabled, this is a breaking change for a custom backend that is implemented in a way that avoids built-in unpoisoning provided by Rust/libc/MSAN.. Since MSAN support is unstable in Rust anyway, I think this is acceptable breakage. As a side effect, this minimizes the number of configurations that reference the `__msan_unpoison` symbol unnecessarily.
1 parent c5b35fd commit adb19e7

File tree

6 files changed

+84
-12
lines changed

6 files changed

+84
-12
lines changed

src/backends.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//!
33
//! This module should provide `fill_inner` with the signature
44
//! `fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error>`.
5-
//! The function MUST fully initialize `dest` when `Ok(())` is returned.
5+
//! The function MUST fully initialize `dest` when `Ok(())` is returned;
6+
//! the function may need to use `sanitizer::unpoison` as well.
67
//! The function MUST NOT ever write uninitialized bytes into `dest`,
78
//! regardless of what value it returns.
89
@@ -12,9 +13,11 @@ cfg_if! {
1213
pub use custom::*;
1314
} else if #[cfg(getrandom_backend = "linux_getrandom")] {
1415
mod getrandom;
16+
mod sanitizer;
1517
pub use getrandom::*;
1618
} else if #[cfg(getrandom_backend = "linux_raw")] {
1719
mod linux_raw;
20+
mod sanitizer;
1821
pub use linux_raw::*;
1922
} else if #[cfg(getrandom_backend = "rdrand")] {
2023
mod rdrand;
@@ -43,6 +46,7 @@ cfg_if! {
4346
pub use unsupported::*;
4447
} else if #[cfg(all(target_os = "linux", target_env = ""))] {
4548
mod linux_raw;
49+
mod sanitizer;
4650
pub use linux_raw::*;
4751
} else if #[cfg(target_os = "espidf")] {
4852
mod esp_idf;
@@ -102,6 +106,7 @@ cfg_if! {
102106
))] {
103107
mod use_file;
104108
mod linux_android_with_fallback;
109+
mod sanitizer;
105110
pub use linux_android_with_fallback::*;
106111
} else if #[cfg(any(
107112
target_os = "android",
@@ -116,6 +121,8 @@ cfg_if! {
116121
all(target_os = "horizon", target_arch = "arm"),
117122
))] {
118123
mod getrandom;
124+
#[cfg(any(target_os = "android", target_os = "linux"))]
125+
mod sanitizer;
119126
pub use getrandom::*;
120127
} else if #[cfg(target_os = "solaris")] {
121128
mod solaris;

src/backends/getrandom.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ mod util_libc;
2626
#[inline]
2727
pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
2828
util_libc::sys_fill_exact(dest, |buf| unsafe {
29-
libc::getrandom(buf.as_mut_ptr().cast(), buf.len(), 0)
29+
let ret = libc::getrandom(buf.as_mut_ptr().cast(), buf.len(), 0);
30+
31+
#[cfg(any(target_os = "android", target_os = "linux"))]
32+
#[allow(unused_unsafe)] // TODO(MSRV 1.65): Remove this.
33+
unsafe {
34+
super::sanitizer::unpoison_linux_getrandom_result(buf, ret);
35+
}
36+
37+
ret
3038
})
3139
}

src/backends/linux_android_with_fallback.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//! Implementation for Linux / Android with `/dev/urandom` fallback
2-
use super::use_file;
2+
use super::{sanitizer, use_file};
33
use crate::Error;
44
use core::{
55
ffi::c_void,
@@ -95,7 +95,9 @@ pub fn fill_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
9595
// note: `transmute` is currently the only way to convert a pointer into a function reference
9696
let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) };
9797
util_libc::sys_fill_exact(dest, |buf| unsafe {
98-
getrandom_fn(buf.as_mut_ptr().cast(), buf.len(), 0)
98+
let ret = getrandom_fn(buf.as_mut_ptr().cast(), buf.len(), 0);
99+
sanitizer::unpoison_linux_getrandom_result(buf, ret);
100+
ret
99101
})
100102
}
101103
}

src/backends/linux_raw.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Implementation for Linux / Android using `asm!`-based syscalls.
2-
use crate::{Error, MaybeUninit};
3-
2+
use super::sanitizer;
43
pub use crate::util::{inner_u32, inner_u64};
4+
use crate::{Error, MaybeUninit};
55

66
#[cfg(not(any(target_os = "android", target_os = "linux")))]
77
compile_error!("`linux_raw` backend can be enabled only for Linux/Android targets!");
@@ -118,6 +118,7 @@ pub fn fill_inner(mut dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
118118

119119
loop {
120120
let ret = unsafe { getrandom_syscall(dest.as_mut_ptr().cast(), dest.len(), 0) };
121+
unsafe { sanitizer::unpoison_linux_getrandom_result(dest, ret) };
121122
match usize::try_from(ret) {
122123
Ok(0) => return Err(Error::UNEXPECTED),
123124
Ok(len) => {

src/backends/sanitizer.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
use core::mem::MaybeUninit;
2+
3+
/// Unpoisons `buf` if MSAN support is enabled.
4+
///
5+
/// Most backends do not need to unpoison their output. Rust language- and
6+
/// library- provided functionality unpoisons automatically. Similarly, libc
7+
/// either natively supports MSAN and/or MSAN hooks libc-provided functions
8+
/// to unpoison outputs on success. Only when all of these things are
9+
/// bypassed do we need to do it ourselves.
10+
///
11+
/// The call to unpoison should be done as close to the write as possible.
12+
/// For example, if the backend partially fills the output buffer in chunks,
13+
/// each chunk should be unpoisoned individually. This way, the correctness of
14+
/// the chunking logic can be validated (in part) using MSAN.
15+
pub unsafe fn unpoison(buf: &mut [MaybeUninit<u8>]) {
16+
cfg_if! {
17+
if #[cfg(getrandom_msan)] {
18+
extern "C" {
19+
fn __msan_unpoison(a: *mut core::ffi::c_void, size: usize);
20+
}
21+
let a = buf.as_mut_ptr().cast();
22+
let size = buf.len();
23+
#[allow(unused_unsafe)] // TODO(MSRV 1.65): Remove this.
24+
unsafe {
25+
__msan_unpoison(a, size);
26+
}
27+
} else {
28+
let _ = buf;
29+
}
30+
}
31+
}
32+
33+
/// Interprets the result of the `getrandom` syscall of Linux, unpoisoning any
34+
/// written part of `buf`.
35+
///
36+
/// `buf` must be the output buffer that was originally passed to the `getrandom`
37+
/// syscall.
38+
///
39+
/// `ret` must be the result returned by `getrandom`. If `ret` is negative or
40+
/// larger than the length of `buf` then nothing is done.
41+
///
42+
/// Memory Sanitizer only intercepts `getrandom` on this condition (from its
43+
/// source code):
44+
/// ```c
45+
/// #define SANITIZER_INTERCEPT_GETRANDOM \
46+
/// ((SI_LINUX && __GLIBC_PREREQ(2, 25)) || SI_FREEBSD || SI_SOLARIS)
47+
/// ```
48+
/// So, effectively, we have to assume that it is never intercepted on Linux.
49+
#[cfg(any(target_os = "android", target_os = "linux"))]
50+
pub unsafe fn unpoison_linux_getrandom_result(buf: &mut [MaybeUninit<u8>], ret: isize) {
51+
if let Ok(bytes_written) = usize::try_from(ret) {
52+
if let Some(written) = buf.get_mut(..bytes_written) {
53+
#[allow(unused_unsafe)] // TODO(MSRV 1.65): Remove this.
54+
unsafe {
55+
unpoison(written)
56+
}
57+
}
58+
}
59+
}

src/lib.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,7 @@ pub fn fill_uninit(dest: &mut [MaybeUninit<u8>]) -> Result<&mut [u8], Error> {
106106

107107
// SAFETY: `dest` has been fully initialized by `imp::fill_inner`
108108
// since it returned `Ok`.
109-
Ok(unsafe {
110-
#[cfg(getrandom_msan)]
111-
__msan_unpoison(dest.as_mut_ptr().cast(), dest.len());
112-
113-
util::slice_assume_init_mut(dest)
114-
})
109+
Ok(unsafe { util::slice_assume_init_mut(dest) })
115110
}
116111

117112
/// Get random `u32` from the system's preferred random number source.

0 commit comments

Comments
 (0)