From: Jann Horn jannh@google.com
commit 6c8e11e08a5b74bb8a5cdd5cbc1e5143df0fba72 upstream.
At the moment, urandom_read() (used for /dev/urandom) resets crng_init_cnt to zero when it is called at crng_init<2. This is inconsistent: We do it for /dev/urandom reads, but not for the equivalent getrandom(GRND_INSECURE).
(And worse, as Jason pointed out, we're only doing this as long as maxwarn>0.)
crng_init_cnt is only read in crng_fast_load(); it is relevant at crng_init==0 for determining when to switch to crng_init==1 (and where in the RNG state array to write).
As far as I understand:
- crng_init==0 means "we have nothing, we might just be returning the same exact numbers on every boot on every machine, we don't even have non-cryptographic randomness; we should shove every bit of entropy we can get into the RNG immediately" - crng_init==1 means "well we have something, it might not be cryptographic, but at least we're not gonna return the same data every time or whatever, it's probably good enough for TCP and ASLR and stuff; we now have time to build up actual cryptographic entropy in the input pool" - crng_init==2 means "this is supposed to be cryptographically secure now, but we'll keep adding more entropy just to be sure".
The current code means that if someone is pulling data from /dev/urandom fast enough at crng_init==0, we'll keep resetting crng_init_cnt, and we'll never make forward progress to crng_init==1. It seems to be intended to prevent an attacker from bruteforcing the contents of small individual RNG inputs on the way from crng_init==0 to crng_init==1, but that's misguided; crng_init==1 isn't supposed to provide proper cryptographic security anyway, RNG users who care about getting secure RNG output have to wait until crng_init==2.
This code was inconsistent, and it probably made things worse - just get rid of it.
Signed-off-by: Jann Horn jannh@google.com Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/char/random.c | 4 ---- 1 file changed, 4 deletions(-)
--- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1821,7 +1821,6 @@ urandom_read_nowarn(struct file *file, c static ssize_t urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) { - unsigned long flags; static int maxwarn = 10;
if (!crng_ready() && maxwarn > 0) { @@ -1829,9 +1828,6 @@ urandom_read(struct file *file, char __u if (__ratelimit(&urandom_warning)) pr_notice("%s: uninitialized urandom read (%zd bytes read)\n", current->comm, nbytes); - spin_lock_irqsave(&primary_crng.lock, flags); - crng_init_cnt = 0; - spin_unlock_irqrestore(&primary_crng.lock, flags); }
return urandom_read_nowarn(file, buf, nbytes, ppos);