Should core::mem::MaybeUninit::assume_init integrate with MSan?

In running the various sanitizers available on Rustls, I ran into an issue where MSan triggers false positives in assembly-initialized memory.

Should assume_init() call __msan_unpoison when the sanitizer is running?

See [Proof of concept]: MSan branch. by sayrer · Pull Request #587 · ctz/rustls · GitHub for the experience that lead to this question.

1 Like

assume_init is only allowed when the MaybeUninit has been initialized before calling it. It does not freeze uninitialized bytes. Calling __msan_unpoison would hide UB from calling assume_uninit without first initializing the MaybeUninit. You should probably call __msan_unpoison yourself in this case as you know that the assembly has initialized the MaybeUninit.

3 Likes

As we discussed this in the other bug, your point was brought up. Perhaps the core library could provide a method that calls __msan_unpoison when MSan is running via -Zsanitizer=memory, and otherwise does nothing. That way, this issue isn't constantly rediscovered.

Here's what Ring ended up doing: Add support for Memory Sanitizer (msan). by briansmith · Pull Request #1228 · briansmith/ring · GitHub

How is this handled in C? Just disable ASM when using msan?

Yeah, typically provide fallback C implementations of ASM functions. That helps support more target architectures, is good for testing the ASM, and can be used with MSan.

In that case, should we expect Rust crates to do the same?

Well, perhaps. There is still a missing cfg value here (crates should be able to detect they are running under msan without doing their own flag). As a consumer of ring, I want to test its asm routines while running under msan. It's other crates I'm worried about, but I don't want to test code I don't ship, either.

Sanitizers can't see into assembly, so they'll give false positives with variables that assume the assembly did stuff.

The only tool I know of that can look into assembly for initialization is valgrind.

(Note that this isn't a rust limitation, the same thing is true for uninit in C/C++)

Yes, notice how ring added assume_init_asm / assume_init_not_asm in the link above...