Page MenuHomePhabricator

[Support,Windows] Tolerate failure of CryptGenRandom
ClosedPublic

Authored by simon_tatham on Apr 6 2020, 7:55 AM.

Details

Summary

In Unix/Process.inc, we seed a random number generator from
/dev/urandom if possible, but if not, we're happy to fall back to
ordinary pseudorandom strategies, like the current time and PID.

The corresponding function on Windows calls CryptGenRandom, but it
doesn't have a fallback if that strategy fails. But CryptGenRandom
can fail, if a cryptography provider isn't properly initialized, or
occasionally (by our observation) simply intermittently.

If it's reasonable on Unix to implement traditional pseudorandom-number
seeding as a fallback, then it's surely reasonable to do the same on
Windows. So this patch adds a last-ditch use of ordinary rand(), using
much the same strategy as the Unix fallback code.

Diff Detail

Event Timeline

simon_tatham created this revision.Apr 6 2020, 7:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2020, 7:55 AM
hans accepted this revision.Apr 6 2020, 9:58 AM

Seems reasonable to me.

llvm/lib/Support/Windows/Process.inc
455

Why do you need the static_cast<void>?

This revision is now accepted and ready to land.Apr 6 2020, 9:58 AM
simon_tatham marked an inline comment as done.Apr 7 2020, 1:12 AM
simon_tatham added inline comments.
llvm/lib/Support/Windows/Process.inc
455

I must admit that I copied the idiom from the corresponding function in unix/Process.inc without really thinking about what the cast was doing there.

But now I look, it was added on purpose in rL261270, apparently in the expectation that at some point it would be useful for suppress a warning.

This revision was automatically updated to reflect the committed changes.