[PATCH] random: continually use hwgenerator randomness

[PATCH] random: continually use hwgenerator randomness

[Dominik Brodowski]

The rngd kernel thread may sleep indefinitely if the entropy count is
kept above random_write_wakeup_bits by other entropy sources. To make
best use of multiple sources of randomness, mix entropy from hardware
RNGs into the pool at least once within CRNG_RESEED_INTERVAL.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/char/random.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 68613f0b6887..3e133d9583a0 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2205,13 +2205,15 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 			return;
 	}
 
-	/* Suspend writing if we're above the trickle threshold.
+	/* Throttle writing if we're above the trickle threshold.
 	 * We'll be woken up again once below random_write_wakeup_thresh,
-	 * or when the calling thread is about to terminate.
+	 * when the calling thread is about to terminate, or once
+	 * CRNG_RESEED_INTERVAL has lapsed.
 	 */
-	wait_event_interruptible(random_write_wait,
+	wait_event_interruptible_timeout(random_write_wait,
 			!system_wq || kthread_should_stop() ||
-			POOL_ENTROPY_BITS() <= random_write_wakeup_bits);
+			POOL_ENTROPY_BITS() <= random_write_wakeup_bits,
+			CRNG_RESEED_INTERVAL);
 	mix_pool_bytes(buffer, count);
 	credit_entropy_bits(entropy);
 }
-- 
2.34.1

Re: [PATCH] random: continually use hwgenerator randomness

[Jason A. Donenfeld]

Hi Dominik,

On Tue, Jan 25, 2022 at 9:19 PM Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> The rngd kernel thread may sleep indefinitely if the entropy count is
> kept above random_write_wakeup_bits by other entropy sources. To make
> best use of multiple sources of randomness, mix entropy from hardware
> RNGs into the pool at least once within CRNG_RESEED_INTERVAL.

Thanks for this patch. It exposes the result of an interesting set of
changes that have occurred over time.

When does POOL_ENTROPY_BITS() decrease? There are two direct mutators:

- Calls to account(> 0, ..)
- RNDCLEARPOOL/RNDZAPENTCNT (though this fails to do the
wake_up_interruptible/kill_fasync dance that account() does. bug?)

The latter is a userspace decision. The former has one callsite:
extract_entropy(.., > 0, ..).

The underscore-less extract_entropy function in turn has one caller:
crng_reseed(.., true). This in turn gets called from three places:

- From credit_entropy_bits, once, when the entropy pool initializes
for the first time.
- From extract_crng after 5 minutes or after crng_global_init_time is set.
- From RNDRESEEDCRNG (which is also the only non-init mutator of
crng_global_init_time).

The first is an init time thing, so not really relevant. The middle is
already at our 5 minute interval. And the last is a userspace
decision.

Therefore, with the code as it currently exists, if there are no calls
to the ioctls, that wait will unblock a minimum of every 5 minutes
times random_write_wakeup_bits / 256 rounded up, and assuming a low no
new entropy from interrupts or elsewhere (unlikely), so in the best
case, that seems like 20 minutes? And if you want to make it happen
more than every 20 minutes, you must call call RNDCLEARPOOL followed
by a RNDRESEEDCRNG (due to the maybe-bug I mentioned earlier), or call
RNDRESEEDCRNG a bunch in a loop from userspace.

That certainly seems pretty weird and not nearly as automatic as it
ought to be. Even though we don't _need_ more hw-backed entropy once
the RNG is up, I don't see it as a bad thing to periodically stir some
in. In fact, it's probably a reasonable thing to do.

I mentioned at the beginning that this is an interesting thing
resulting from changes over time. Specifically, the reason we're in
this situation is because /dev/random no longer debits from the
entropy pool (aside from the 5 minute interval calls to
extract_entropy). And thank goodness for that. But some of the
assumptions made in that era of random.c might be a little out of date
now.

So on first analysis of this, I think your patch is correct and I'll
apply it. However, I'd like to wait a little bit to give others a
chance to chime in, particularly those who maintain the actual
hw-random driver (which is separate from random.c) and might have a
better picture of potential hardware constraints or something along
those lines.

Regards,
Jason