Re: [PATCH] hwrng: virtio: reject invalid used.len from the device

[Michael S. Tsirkin]

On Fri, Apr 17, 2026 at 08:00:20PM -0400, Michael Bommarito wrote:
> random_recv_done() stored the device-reported used.len directly into
> vi->data_avail without validating it against the posted buffer size
> sizeof(vi->data) (SMP_CACHE_BYTES bytes, typically 32 or 64).  A
> malicious or buggy virtio-rng backend could set used.len beyond
> vi->data so that the subsequent copy_data() in virtio_read() issues
> memcpy() from vi->data + vi->data_idx past the end of the inline
> array, reading adjacent kmalloc-1k slab bytes into the hwrng core's
> buffer and from there into /dev/hwrng consumers and the kernel
> entropy pool.
> 
> Exploitable most clearly in threat models that do not trust the
> hypervisor (confidential-compute guests on SEV-SNP or TDX; vhost-user
> split backends).

Exploitable? I don't get it. How is reading this data into hwrng
a problem?


> KASAN confirms the OOB on a guest booted under a QEMU 9.0 whose
> virtio-rng backend has been patched to report used.len = 0x10000:
> 
>   BUG: KASAN: slab-out-of-bounds in virtio_read+0x394/0x5d0
>   Read of size 64 at addr ffff8880089c2220 by task hwrng/52
>   Call Trace:
>    __asan_memcpy
>    virtio_read+0x394/0x5d0
>    hwrng_fillfn+0xb2/0x470
>    kthread
>   Allocated by task 1:
>    probe_common+0xa5/0x660
>    virtio_dev_probe+0x549/0xbc0
>   The buggy address belongs to the object at ffff8880089c2000
>    which belongs to the cache kmalloc-1k of size 1024
>   The buggy address is located 0 bytes to the right of
>    allocated 544-byte region [ffff8880089c2000, ffff8880089c2220)
> 
> hwrng_fillfn is a kernel thread that runs as soon as the device is
> probed; no guest userspace interaction is needed.
> 
> Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport layer"),
> which hardened usb9pfs_rx_complete against unchecked device-reported
> length in the USB 9p transport.
> 
> With the added len-vs-sizeof(vi->data) clamp in place the same
> harness boots cleanly: the driver logs "bogus used.len" once and
> subsequent reads wait for a honest response.
> 
> Fixes: f7f510ec1957 ("virtio: An entropy device, as suggested by hpa.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-7
> ---
>  drivers/char/hw_random/virtio-rng.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
> index 0ce02d7e5048..6cff480787ca 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -47,6 +47,18 @@ static void random_recv_done(struct virtqueue *vq)
>  	if (!virtqueue_get_buf(vi->vq, &len))
>  		return;
>  
> +	/*
> +	 * The device sets used.len; a malicious or buggy backend can
> +	 * report more bytes than we posted.  Clamp before it reaches
> +	 * copy_data() which indexes vi->data[].
> +	 */
> +	if (len > sizeof(vi->data)) {
> +		dev_err(&vq->vdev->dev,
> +			"bogus used.len %u > buffer size %zu\n",
> +			len, sizeof(vi->data));
> +		len = 0;
> +	}
> +
>  	smp_store_release(&vi->data_avail, len);
>  	complete(&vi->have_data);
>  }
> -- 
> 2.53.0