On Fri, Apr 17, 2026 at 08:18:06PM -0400, Michael Bommarito wrote: > "Actionable" is probably the better word there, sorry.
Actionable meaning what? > If it were > otherwise, I wouldn't have filed publicly > > If you end up ACKing the correctness change, I can send v2 with better log > > Thanks, > Michael Bommarito > > On Fri, Apr 17, 2026 at 8:13 PM Michael S. Tsirkin <[email protected]> wrote: > > > > 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: [email protected] > > > Signed-off-by: Michael Bommarito <[email protected]> > > > 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; > > > + } Maybe clamp at sizeof(vi->data) then? 0 might break buggy devices that were working earlier. Or just clamp where it's used, for clarity. And maybe we need the array_index dance, given you are worried about malicious. > > > + > > > smp_store_release(&vi->data_avail, len); > > > complete(&vi->have_data); > > > } > > > -- > > > 2.53.0 > >
