hw_random: use add_hwgenerator_randomness() for early entropy

Rather than calling add_device_randomness(), the add_early_randomness()
function should use add_hwgenerator_randomness(), so that the early
entropy can be potentially credited, which allows for the RNG to
initialize earlier without having to wait for the kthread to come up.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/hw_random/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Sat, Nov 5, 2022 at 9:44 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Rather than calling add_device_randomness(), the add_early_randomness()
> function should use add_hwgenerator_randomness(), so that the early
> entropy can be potentially credited, which allows for the RNG to
> initialize earlier without having to wait for the kthread to come up.
>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  drivers/char/hw_random/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index cc002b0c2f0c..8c0819ce2781 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -69,8 +69,10 @@ static void add_early_randomness(struct hwrng *rng)
>         mutex_lock(&reading_mutex);
>         bytes_read = rng_get_data(rng, rng_fillbuf, 32, 0);
>         mutex_unlock(&reading_mutex);
> -       if (bytes_read > 0)
> -               add_device_randomness(rng_fillbuf, bytes_read);
> +       if (bytes_read > 0) {
> +               size_t entropy = bytes_read * 8 * rng->quality / 1024;
> +               add_hwgenerator_randomness(rng_fillbuf, bytes_read, entropy);
> +       }

This will cause problems, because add_hwgenerator_randomness() will
sleep. I'll look into a more robust change.

Jason

Hi Dominik,

On Sun, Nov 06, 2022 at 08:02:56AM +0100, Dominik Brodowski wrote:
> Am Sun, Nov 06, 2022 at 02:50:42AM +0100 schrieb Jason A. Donenfeld:
> > Rather than calling add_device_randomness(), the add_early_randomness()
> > function should use add_hwgenerator_randomness(), so that the early
> > entropy can be potentially credited, which allows for the RNG to
> > initialize earlier without having to wait for the kthread to come up.
> 
> We're already at device_initcall() level here, so that shouldn't be much of
> an additional delay.

Either the delay is not relevant, in which case we should entirely
remove `add_early_randomness()`, or the delay is relevant, in which case
hw_random should use the right function, so that it gets credited as it
should. (Semantically this is the right thing to do, too, so that
random.c can actually know what the deal is with the data it's getting;
knowing, "oh this comes from hw_random" could be useful.)

> > Since we don't want to sleep from add_early_randomness(), we also
> > refactor the API a bit so that hw_random/core.c can do the sleep, this
> > time using the correct function, hwrng_msleep, rather than having
> > random.c awkwardly do it.
> 
> Isn't this something you were quite hesistant about just recently[*]?

It is, yes, thanks. The concern is that now it means random.c can't ask
for more hw_random data by canceling that sleep. I was thinking that it
would be nice to cancel the sleep during system resume, vm fork, and
other similar events, so that we can get some fresh bits during the
times it matters most. But there's a bit of a problem of doing it in a
basic way like that: we might get the bits, but that doesn't mean we'll
reseed right away at that advanced stage in uptime. So we'd actually
need something more complicated like, "unblock from sleep now and reseed
after mixing data". When writing this patch last night, I was thinking
this is a more complicated thing that could benefit from a more
complicated API. But actually, maybe there's a way I can make it work
for the existing thing. So you're right: let's hold off on this v2. And
I'll send a v3 that just adds a boolean parameter of whether or not to
sleep. And then I'll also try tackling the unblock&reseed thing too at
the same time.

Jason

Hi Jason,

Am Sun, Nov 06, 2022 at 03:50:51PM +0100 schrieb Jason A. Donenfeld:
> On Sun, Nov 06, 2022 at 08:02:56AM +0100, Dominik Brodowski wrote:
> > Am Sun, Nov 06, 2022 at 02:50:42AM +0100 schrieb Jason A. Donenfeld:
> > > Rather than calling add_device_randomness(), the add_early_randomness()
> > > function should use add_hwgenerator_randomness(), so that the early
> > > entropy can be potentially credited, which allows for the RNG to
> > > initialize earlier without having to wait for the kthread to come up.
> > 
> > We're already at device_initcall() level here, so that shouldn't be much of
> > an additional delay.
> 
> Either the delay is not relevant, in which case we should entirely
> remove `add_early_randomness()`,

There's another subtlety going on here: add_device_randomness() is called
for *all* hw_random devices upon their registration, while the hwrng thread
currently only works with the hw_random device with the best quality
available.

Thanks,
	Dominik