Logic inversion in drivers/char/random.c

From: Daniel Franke
Date: Thu Mar 31 2016 - 17:12:09 EST

Quoting http://lxr.free-electrons.com/source/drivers/char/random.c#L999 :

/* For /dev/random's pool, always leave two wakeups' worth */
int rsvd_bytes = r->limit ? 0 : random_read_wakeup_bits / 4;

The apparent intent of these lines is to ensure that transfers from
the input pool to the blocking pool (triggered by reading from
/dev/random) leave at least 128 bits left in the input pool afterward,
so that this remaining entropy is available for urandom's
once-a-minute reseed. However, the test is backward. r->limit is 1 for
the blocking pool and 0 for the non-blocking pool, so rsvd_bytes is 0
when transferring to the blocking pool and 16 when transferring to the
the non-blocking pool, rather than the other way around. As a result,
if some process is constantly hammering on /dev/random, /dev/urandom
may be starved of entropy and never get a chance to reseed.

This bug does not impact the *initial* seeding of the non-blocking
pool, because the first 128 bits of entropy collected after each boot
are mixed directly into the non-blocking pool, bypassing the input
pool (see lines 804 and 924). Therefore, I don't think this is a
serious security issue. However, if you regard it as a security goal
that /dev/urandom should be able to recover after an adversary somehow
obtains a single moment-in-time snapshot of the entropy pool, then
this bug thwarts that goal. Personally, I think caring about this
entails a very silly threat model, but at least some RNGs, such as
Yarrow, are explicitly designed to support it.