arc4random: Avoid KMSAN false positives from pre-seeding results

[markj]

Summary

If code calls arc4random(), and our RNG is not yet seeded and
random_bypass_before_seeding is true, we'll compute a key using the
SHA256 hash of some hopefully hard-to-predict data, including the
contents of an uninitialized stack buffer (which is also the output
buffer).

When KMSAN is enabled, this use of uninitialized state propagtes through
to the arc4random() output, resulting in false positives. To fix this,
lie to KMSAN and explicitly mark the buffer as initialized.

[cem]

It's not exactly a false positive, although poisoning the output as uninitialized is sort of unhelpful. Are we confident the compiler isn't eliminating access to it (due to UB) outside of KMSAN? In the abstract, I think we would prefer to eliminate bypass_before_seeding.

I'd probably add some summarized form of the commit message as a comment adjacent to the kmsan_mark() invocation.

[markj]

In D31510#710348, @cem wrote:

It's not exactly a false positive, although poisoning the output as uninitialized is sort of unhelpful.

Yeah, everything is working as expected here, I've just been tending to use "false positive" to mean, "any KMSAN report that doesn't correspond to a code bug."

Are we confident the compiler isn't eliminating access to it (due to UB) outside of KMSAN? In the abstract, I think we would prefer to eliminate bypass_before_seeding.

What exactly is the UB here? I'm not sure how the compiler could legally eliminate SHA256_Update(&ctx, key, sizeof(key)).

I'd probably add some summarized form of the commit message as a comment adjacent to the kmsan_mark() invocation.

Will do, thanks.

[cem]

In D31510#710608, @markj wrote:

What exactly is the UB here? I'm not sure how the compiler could legally eliminate SHA256_Update(&ctx, key, sizeof(key)).

Any use of uninitialized variables is UB, I think?

[markj]

In D31510#710613, @cem wrote:

In D31510#710608, @markj wrote:

What exactly is the UB here? I'm not sure how the compiler could legally eliminate SHA256_Update(&ctx, key, sizeof(key)).

Any use of uninitialized variables is UB, I think?

I believe so, I just don't see how the compiler can infer that key is being accessed here.

[cem]

LTO builds can see across CUs. I don’t know of any particular pass that would eliminate this, though.