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

1

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
2

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
3

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.

4

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

5

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

6

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.

7

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

8

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.

9

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++)

10

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

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.