[PATCH v4] hwrng: virtio: clamp device-reported used.len at copy_data()

[PATCH v4] hwrng: virtio: clamp device-reported used.len at copy_data()

[Michael Bommarito]

copy_data() trusts the device-reported used.len stored in vi->data_avail
and memcpy()s that many bytes out of the inline vi->data buffer without
bounding it against sizeof(vi->data) (SMP_CACHE_BYTES, typically 32 or
64).  A malicious or buggy virtio-rng backend can report a used.len past
the buffer and steer the memcpy() into adjacent slab memory;
hwrng_fillfn() then mixes those bytes into the guest RNG and guest root
can read them back via /dev/hwrng.  No guest userspace action is required
to first trigger the read.

Clamp data_avail to sizeof(vi->data) at point of use and bail if the
running index has already reached the clamped bound.  Same class as
commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport
layer").

Fixes: f7f510ec1957 ("virtio: An entropy device, as suggested by hpa.")
Cc: stable@vger.kernel.org
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
KASAN on a v7.1-rc4 guest whose backend reports used.len = 0x10000:

  BUG: KASAN: slab-out-of-bounds in virtio_read+0x394/0x5d0
  Read of size 64 at addr ffff88800ae0ba20 by task hwrng/52
   __asan_memcpy+0x23/0x60
   virtio_read+0x394/0x5d0
   hwrng_fillfn+0xb2/0x470
  located 0 bytes to the right of the 544-byte kmalloc-1k region.

With the clamp the same harness boots clean: copy_data() returns 0 for
the bogus report and the driver reissues the request.

Confidential-compute angle: a malicious hypervisor plus compromised guest
root could use /dev/hwrng as a guest-kernel heap leak channel, though
SEV-SNP/TDX guests usually disable virtio-rng.  The memory-safety fix is
worth carrying regardless.

Changes in v4:
- Drop array_index_nospec() on vi->data_idx (and linux/nospec.h) per
  Herbert Xu and Michael S. Tsirkin: data_idx is driver-maintained and
  already bounded by the check above, with no demonstrated speculation
  gadget.  Clamp unchanged; KASAN repro re-run (stock splats, patched
  clean).

Changes in v3: repost of v2 after the thread went quiet, rebased onto
v7.1-rc4.

Changes in v2 (Michael S. Tsirkin): move the check into copy_data() next
to the memcpy(); clamp to sizeof(vi->data) instead of forcing len = 0 so
an occasionally-over-reporting device does not start returning
zero-length reads.

v1: https://lore.kernel.org/all/20260418000020.1847122-1-michael.bommarito@gmail.com/
v2: https://lore.kernel.org/all/20260418150613.3522589-1-michael.bommarito@gmail.com/
v3: https://lore.kernel.org/all/20260531142251.2792061-1-michael.bommarito@gmail.com/
---
 drivers/char/hw_random/virtio-rng.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 0ce02d7e5048e..7413d24a67a9d 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -69,7 +69,22 @@ static void request_entropy(struct virtrng_info *vi)
 static unsigned int copy_data(struct virtrng_info *vi, void *buf,
 			      unsigned int size)
 {
-	size = min_t(unsigned int, size, vi->data_avail);
+	unsigned int avail;
+
+	/*
+	 * vi->data_avail was set from the device-reported used.len and
+	 * vi->data_idx was advanced by previous copy_data() calls.  A
+	 * malicious or buggy virtio-rng backend can drive data_avail past
+	 * sizeof(vi->data), so clamp it at point of use before the memcpy()
+	 * below can be steered into adjacent slab memory.
+	 */
+	avail = min_t(unsigned int, vi->data_avail, sizeof(vi->data));
+	if (vi->data_idx >= avail) {
+		vi->data_avail = 0;
+		request_entropy(vi);
+		return 0;
+	}
+	size = min_t(unsigned int, size, avail - vi->data_idx);
 	memcpy(buf, vi->data + vi->data_idx, size);
 	vi->data_idx += size;
 	vi->data_avail -= size;

base-commit: a1f173eb51db0dc78536334729ef832c62d6c65a
-- 
2.53.0