[PATCH] hw_random: don't wait on add_early_randomness()

[PATCH] hw_random: don't wait on add_early_randomness()

[Laurent Vivier]

add_early_randomness() is called by hwrng_register() when the
hardware is added. If this hardware and its module are present
at boot, and if there is no data available the boot hangs until
data are available and can't be interrupted.

To avoid that, call rng_get_data() in non-blocking mode (wait=0)
from add_early_randomness().

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 drivers/char/hw_random/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 9044d31ab1a1..8d53b8ef545c 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -67,7 +67,7 @@ static void add_early_randomness(struct hwrng *rng)
 	size_t size = min_t(size_t, 16, rng_buffer_size());
 
 	mutex_lock(&reading_mutex);
-	bytes_read = rng_get_data(rng, rng_buffer, size, 1);
+	bytes_read = rng_get_data(rng, rng_buffer, size, 0);
 	mutex_unlock(&reading_mutex);
 	if (bytes_read > 0)
 		add_device_randomness(rng_buffer, bytes_read);
-- 
2.21.0

Re: [PATCH] hw_random: don't wait on add_early_randomness()

[Herbert Xu]

On Tue, Sep 17, 2019 at 11:54:50AM +0200, Laurent Vivier wrote:
> add_early_randomness() is called by hwrng_register() when the
> hardware is added. If this hardware and its module are present
> at boot, and if there is no data available the boot hangs until
> data are available and can't be interrupted.
> 
> To avoid that, call rng_get_data() in non-blocking mode (wait=0)
> from add_early_randomness().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  drivers/char/hw_random/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please provide more context in your patch description such as which
driver actually causes a hang here.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] hw_random: don't wait on add_early_randomness()

[Laurent Vivier]

On 17/09/2019 14:40, Herbert Xu wrote:
> On Tue, Sep 17, 2019 at 11:54:50AM +0200, Laurent Vivier wrote:
>> add_early_randomness() is called by hwrng_register() when the
>> hardware is added. If this hardware and its module are present
>> at boot, and if there is no data available the boot hangs until
>> data are available and can't be interrupted.
>>
>> To avoid that, call rng_get_data() in non-blocking mode (wait=0)
>> from add_early_randomness().
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  drivers/char/hw_random/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Please provide more context in your patch description such as which
> driver actually causes a hang here.

I can add in the next version:

"For instance, in the case of virtio-rng, in some cases the host can be
not able to provide enough entropy for all the guests.

We can have two easy ways to reproduce the problem but they rely on
misconfiguration of the hypervisor or the egd daemon:

- if virtio-rng device is configured to connect to the egd daemon of the
host but when the virtio-rng driver asks for data the daemon is not
connected,

- if virtio-rng device is configured to connect to the egd daemon of the
host but the egd daemon doesn't provide data.

The guest kernel will hang at boot until the virtio-rng driver provides
enough data."

More context:

I've proposed to add a watchdog at the hypervisor level to release the
read after a timeout but this changes the behavior of the blocking read
to become non-blocking after a while, and making the call to
rng_get_data() in add_early_randomness() non-blocking seems to me a
better approach: I'm not sure it is really needed to have a blocking
call at this time in the boot sequence.

Any advice is welcome.

Thanks,
Laurent

Re: [PATCH] hw_random: don't wait on add_early_randomness()

[Herbert Xu]

On Tue, Sep 17, 2019 at 03:02:26PM +0200, Laurent Vivier wrote:
> On 17/09/2019 14:40, Herbert Xu wrote:
> > On Tue, Sep 17, 2019 at 11:54:50AM +0200, Laurent Vivier wrote:
> >> add_early_randomness() is called by hwrng_register() when the
> >> hardware is added. If this hardware and its module are present
> >> at boot, and if there is no data available the boot hangs until
> >> data are available and can't be interrupted.
> >>
> >> To avoid that, call rng_get_data() in non-blocking mode (wait=0)
> >> from add_early_randomness().
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  drivers/char/hw_random/core.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Please provide more context in your patch description such as which
> > driver actually causes a hang here.
> 
> I can add in the next version:
> 
> "For instance, in the case of virtio-rng, in some cases the host can be
> not able to provide enough entropy for all the guests.
> 
> We can have two easy ways to reproduce the problem but they rely on
> misconfiguration of the hypervisor or the egd daemon:
> 
> - if virtio-rng device is configured to connect to the egd daemon of the
> host but when the virtio-rng driver asks for data the daemon is not
> connected,
> 
> - if virtio-rng device is configured to connect to the egd daemon of the
> host but the egd daemon doesn't provide data.
> 
> The guest kernel will hang at boot until the virtio-rng driver provides
> enough data."

Patch applied with this addition.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] hw_random: don't wait on add_early_randomness()

[Theodore Y. Ts'o]

On Tue, Sep 17, 2019 at 11:54:50AM +0200, Laurent Vivier wrote:
> add_early_randomness() is called by hwrng_register() when the
> hardware is added. If this hardware and its module are present
> at boot, and if there is no data available the boot hangs until
> data are available and can't be interrupted.
> 
> To avoid that, call rng_get_data() in non-blocking mode (wait=0)
> from add_early_randomness().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Looks good, you can add:

Reviewed-by: Theodore Ts'o <tytso@mit.edu>