Implement Memory Sanitizer unpoisoning more precisely. #678

briansmith · 2025-06-03T02:49:55Z

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.

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.

briansmith · 2025-06-03T20:52:09Z

Rebased on top of #682.

newpavlov

Looks good! We can merge it after #682 is merged.

It may be worth to add a note about unpoisioning to the "custom backend" section of README.

josephlr · 2025-06-04T23:39:06Z

src/backends/sanitizer.rs

+/// For example, if the backend partially fills the output buffer in chunks,
+/// each chunk should be unpoisoned individually. This way, the correctness of
+/// the chunking logic can be validated (in part) using MSAN.
+pub unsafe fn unpoison(buf: &mut [MaybeUninit<u8>]) {


Should this be private? Do we have any existing places where we call this helper?

It could be private but it is intended to be useful for any future backends that need to do unpoisoning themselves.

Some history for this: Originally I wrote the unpoison function and called it only from the linux_raw backend because I expected that libc or MSAN would do the unpoisoning for libc::getrandom. But, tests failed without the unpoisoning. Then I factored out the logic I had added to linux_raw into a reusable function and put that function into sanitizer.rs. Now that one function is the only one that calls unpoison. I considered making unpoison non-public accordingly, but I wanted to emphasize that it is intended to be useful independent of its current single caller, if/when we need it.

Makes sense, I think these are fine to have as-is then.

josephlr

I just had a few questions about this, sorry if they were already asked in an earlier discussion.

josephlr · 2025-06-04T23:42:05Z

src/backends/getrandom.rs

+        #[cfg(any(target_os = "android", target_os = "linux"))]
+        #[allow(unused_unsafe)] // TODO(MSRV 1.65): Remove this.
+        unsafe {
+            super::sanitizer::unpoison_linux_getrandom_result(buf, ret);


Is unpoisoning needed if we are calling the libc version of getrandom? The comment in sanitizer.rs seems to indicate that the libc calls should handle unpoisioning.

See

getrandom/src/backends/sanitizer.rs

Lines 42 to 48 in 5f7a910

/// Memory Sanitizer only intercepts `getrandom` on this condition (from its

/// source code):

/// ```c

/// #define SANITIZER_INTERCEPT_GETRANDOM \

/// ((SI_LINUX && __GLIBC_PREREQ(2, 25)) || SI_FREEBSD || SI_SOLARIS)

/// ```

/// So, effectively, we have to assume that it is never intercepted on Linux.

. Basically the way MSAN is configured by Rust, to support older glibc, ensures that the interception is disabled for glibc. And, MSAN won't do interception for non-glibc.

It is possible that the libc has interception built-in, but we have no way of knowing. I think common open source implementations like musl libc don't, except maybe LLVM libc? If we have a way to detect that LLVM libc will be used, we could make an exception for it.

Note: Maybe Android bionic has built-in support for unpoisoning, in which case we could avoid unpoisoning on Android. IDK for sure.

I was confused mostly because libc::getrandom will only exist on GLibc 2.25 or later, so I figured we would either have:

An older glibc, so we fall back to /dev/urandom or fail depending on the backend

A glibc >= 2.25, so libc::getrandom exists and gets properly handled by msan/libc

But if there's an extra level of config between Rust libc and msan, it totally makes sense why this wouldn't work.

@josephlr Do you know what the Android bionic situation is? IDK if MSAN even works for Android. Would love to minimize the use of this as much as possible.

I have personally not ever used bionic with MSAN. When fuzzing or running tests against C libraries, we usually just run them on Linux, as most of the validation is not for OS specific stuff. But that doesn't mean it's not possible, I've asked some folks at work and we'll see if they get back to me.

I couldnt find any mention about MSan in Android's memory debugging guide: https://developer.android.com/ndk/guides/memory-debug and all the other tools I found were also of an ASan like flavor.

I would be fine assuming that:

Android Bionic (as part of the NDK) doesn't support MSan

if this support is added, it will correctly handle the libc-based functions which write buffers (libc::read, libc::getrandom, etc...) unconditionally, as newer versions of the libc will always have libc::getrandom

I might be wrong, but it seems fine to remove this for android.

What about MUSL targets?

josephlr · 2025-06-04T23:43:30Z

src/backends/linux_android_with_fallback.rs

        util_libc::sys_fill_exact(dest, |buf| unsafe {
-            getrandom_fn(buf.as_mut_ptr().cast(), buf.len(), 0)
+            let ret = getrandom_fn(buf.as_mut_ptr().cast(), buf.len(), 0);
+            sanitizer::unpoison_linux_getrandom_result(buf, ret);


Same here, this ends up calling libc::getrandom which I think should unpoision the buffer.

## [0.3.4] - 2025-10-14 ### Major change to `wasm_js` backend Now, when the `wasm_js` feature is enabled, the `wasm_js` backend will be used by default. Users of `wasm32-unknown-unknown` targeting JavaScript environments like the Web and Node.js will no longer need to specify: ``` --cfg getrandom_backend="wasm_js" ``` in `RUSTFLAGS` for the crate to compile. They can now simple enable a feature. Note: this should not affect non-JS users of the `wasm32-unknown-unknown` target. Using `--cfg getrandom_backend` will still override the source of randomness _even if_ the `wasm_js` feature is enabled. This includes `--cfg getrandom_backend=custom` and `--cfg getrandom_backend=unsupported`. For more information, see the discussions in [#671], [#675], and [#730]. ### Added - `unsupported` opt-in backend [#667] - `windows_legacy` opt-in backend [#724] ### Changed - Implement Memory Sanitizer unpoisoning more precisely [#678] - Relax MSRV for the `linux_raw` opt-in backend on ARM targets [#688] - Use `getrandom` syscall on all RISC-V Linux targets [#699] - Replaced `wasi` dependency with `wasip2` [#721] - Enable `wasm_js` backend by default if the `wasm_js` feature is enabled [#730] ### Removed - Unstable `rustc-dep-of-std` crate feature [#694] [#667]: #667 [#671]: #671 [#675]: #675 [#678]: #678 [#688]: #688 [#694]: #694 [#699]: #699 [#721]: #721 [#724]: #724 [#730]: #730 --------- Signed-off-by: Joe Richey <joerichey@google.com>

briansmith mentioned this pull request Jun 3, 2025

getrandom_msan flag masks bugs in underlying implementations #677

Closed

briansmith force-pushed the b/msan-2 branch from 0858b97 to adb19e7 Compare June 3, 2025 03:01

briansmith added 2 commits June 3, 2025 13:32

CI: Test all backends using Memory Sanitizer on Linux AArch64/x86_64.

6799497

briansmith force-pushed the b/msan-2 branch from adb19e7 to 6367233 Compare June 3, 2025 20:51

newpavlov approved these changes Jun 4, 2025

View reviewed changes

newpavlov merged commit 79ee57a into rust-random:master Jun 4, 2025
74 checks passed

josephlr reviewed Jun 4, 2025

View reviewed changes

josephlr mentioned this pull request Oct 14, 2025

Release Version v0.3.4 #735

Merged

andrewdavidmackenzie mentioned this pull request Oct 15, 2025

Bump getrandom from 0.3.3 to 0.3.4 andrewdavidmackenzie/pigg#1126

Open

	/// Memory Sanitizer only intercepts `getrandom` on this condition (from its
	/// source code):
	/// ```c
	/// #define SANITIZER_INTERCEPT_GETRANDOM \
	/// ((SI_LINUX && __GLIBC_PREREQ(2, 25)) \|\| SI_FREEBSD \|\| SI_SOLARIS)
	/// ```
	/// So, effectively, we have to assume that it is never intercepted on Linux.

Implement Memory Sanitizer unpoisoning more precisely. #678

Implement Memory Sanitizer unpoisoning more precisely. #678

Uh oh!

Conversation

briansmith commented Jun 3, 2025

Uh oh!

briansmith commented Jun 3, 2025

Uh oh!

newpavlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephlr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briansmith Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephlr Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephlr Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

briansmith Jun 4, 2025 •

edited

Loading

josephlr Jun 5, 2025 •

edited

Loading

josephlr Jun 5, 2025 •

edited

Loading