On Sat, 2 Dec 2023 at 08:05, Herbert Xu <[email protected]> wrote:
>
> There is a dead-lock in the hwrng device read path. This triggers
> when the user reads from /dev/hwrng into memory also mmap-ed from
> /dev/hwrng. The resulting page fault triggers a recursive read
> which then dead-locks.
>
> Fix this by using a stack buffer when calling copy_to_user.
>
> Reported-by: Edward Adam Davis <[email protected]>
> Reported-by: [email protected]
> Fixes: 9996508b3353 ("hwrng: core - Replace u32 in driver API with byte array")
> Cc: <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 420f155d251f..a3bbdd6e60fc 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -23,10 +23,13 @@
> #include <linux/sched.h>
> #include <linux/sched/signal.h>
> #include <linux/slab.h>
> +#include <linux/string.h>
> #include <linux/uaccess.h>
>
> #define RNG_MODULE_NAME "hw_random"
>
> +#define RNG_BUFFER_SIZE (SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES)
> +
> static struct hwrng *current_rng;
> /* the current rng has been explicitly chosen by user via sysfs */
> static int cur_rng_set_by_user;
> @@ -58,7 +61,7 @@ static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
>
> static size_t rng_buffer_size(void)
> {
> - return SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES;
> + return RNG_BUFFER_SIZE;
> }
>
> static void add_early_randomness(struct hwrng *rng)
> @@ -209,6 +212,7 @@ static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
> static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> size_t size, loff_t *offp)
> {
> + u8 buffer[RNG_BUFFER_SIZE];
> ssize_t ret = 0;
> int err = 0;
> int bytes_read, len;
> @@ -236,34 +240,37 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> if (bytes_read < 0) {
> err = bytes_read;
> goto out_unlock_reading;
> - }
> - data_avail = bytes_read;
> - }
> -
> - if (!data_avail) {
> - if (filp->f_flags & O_NONBLOCK) {
> + } else if (bytes_read == 0 &&
> + (filp->f_flags & O_NONBLOCK)) {
> err = -EAGAIN;
> goto out_unlock_reading;
> }
> - } else {
> - len = data_avail;
> +
> + data_avail = bytes_read;
> + }
> +
> + len = data_avail;
> + if (len) {
> if (len > size)
> len = size;
>
> data_avail -= len;
>
> - if (copy_to_user(buf + ret, rng_buffer + data_avail,
> - len)) {
> + memcpy(buffer, rng_buffer + data_avail, len);
> + }
> + mutex_unlock(&reading_mutex);
> + put_rng(rng);
> +
> + if (len) {
> + if (copy_to_user(buf + ret, buffer, len)) {
> err = -EFAULT;
> - goto out_unlock_reading;
> + goto out;
> }
>
> size -= len;
> ret += len;
> }
>
> - mutex_unlock(&reading_mutex);
> - put_rng(rng);
>
> if (need_resched())
> schedule_timeout_interruptible(1);
> @@ -274,6 +281,7 @@ static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> }
> }
> out:
> + memzero_explicit(buffer, sizeof(buffer));
> return ret ? : err;
>
> out_unlock_reading:
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
Reviewed-by: PrasannaKumar Muralidharan <[email protected]>
Regards,
PrasannaKumar