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: [email protected]
Suggested-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael Bommarito <[email protected]>
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/[email protected]/
v2:
https://lore.kernel.org/all/[email protected]/
v3:
https://lore.kernel.org/all/[email protected]/
---
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
