Re: [PATCH] hwrng: core - fix task hung in hwrng_fillfn

From: Herbert Xu
Date: Fri Dec 01 2023 - 04:57:46 EST


Edward Adam Davis <eadavis@xxxxxx> wrote:
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 420f155d251f..7323ddc958ce 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -225,17 +225,18 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> goto out;
> }
>
> - if (mutex_lock_interruptible(&reading_mutex)) {
> - err = -ERESTARTSYS;
> - goto out_put;
> - }
> if (!data_avail) {
> + if (mutex_lock_interruptible(&reading_mutex)) {
> + err = -ERESTARTSYS;
> + goto out_put;
> + }
> bytes_read = rng_get_data(rng, rng_buffer,
> rng_buffer_size(),
> !(filp->f_flags & O_NONBLOCK));
> + mutex_unlock(&reading_mutex);
> if (bytes_read < 0) {
> err = bytes_read;
> - goto out_unlock_reading;
> + goto out_put;
> }
> data_avail = bytes_read;
> }

Does this change anything at all? Please explain why it was holding
this lock for 143 seconds in the first place. If it's doing it in
rng_get_data, then your change has zero effect.

> @@ -501,7 +499,10 @@ static int hwrng_fillfn(void *unused)
> rng = get_current_rng();
> if (IS_ERR(rng) || !rng)
> break;
> - mutex_lock(&reading_mutex);
> + if (mutex_lock_interruptible(&reading_mutex)) {
> + put_rng(rng);
> + return -ERESTARTSYS;
> + }

No this is just the symptom. The real problem is why is the driver
spending 143 seconds in rng_get_data?

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