virtio_blk_handle_scsi() only validates the input/output descriptor
counts and then unconditionally treats the second-to-last input
descriptor as a struct virtio_scsi_inhdr.
If that descriptor is shorter than struct virtio_scsi_inhdr, the host
still performs a 4-byte virtio_stl_p() store while writing scsi->errors.
This is reproducible as a host-side heap-buffer-overflow under ASAN:
==4022698==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x504000023570 at pc 0x5e4be9c09800 bp 0x7ffebf4d7510 sp 0x7ffebf4d7500
WRITE of size 4 at 0x504000023570 thread T0
#0 0x5e4be9c097ff in stl_he_p include/qemu/bswap.h:284
#1 0x5e4be9c09c4d in stl_le_p include/qemu/bswap.h:331
#2 0x5e4be9c0a48b in virtio_stl_p include/hw/virtio/virtio-access.h:38
#3 0x5e4be9c0c201 in virtio_blk_handle_scsi ../hw/block/virtio-blk.c:207
#4 0x5e4be9c1578b in virtio_blk_handle_request ../hw/block/virtio-blk.c:926
#5 0x5e4be9c160e3 in virtio_blk_handle_vq ../hw/block/virtio-blk.c:1025
#6 0x5e4be9c16529 in virtio_blk_handle_output ../hw/block/virtio-blk.c:1058
#7 0x5e4bea713ad9 in virtio_queue_notify_vq ../hw/virtio/virtio.c:2507
#8 0x5e4bea724bfc in virtio_queue_host_notifier_read ../hw/virtio/virtio.c:3981
The same run shows the short descriptor being mapped through the
bounce-buffer path:
allocated by thread T0 here:
#0 0x736faf8b4a57 in __interceptor_calloc
#1 0x736faf1a5c50 in g_malloc0
#2 0x5e4bea925458 in address_space_map ../system/physmem.c:3746
#3 0x5e4bea6f7633 in dma_memory_map include/system/dma.h:212
#4 0x5e4bea70610a in virtqueue_map_desc ../hw/virtio/virtio.c:1637
#5 0x5e4bea70824e in virtqueue_split_pop ../hw/virtio/virtio.c:1817
#6 0x5e4bea70c9a8 in virtqueue_pop ../hw/virtio/virtio.c:2039
#7 0x5e4be9c0be03 in virtio_blk_get_request ../hw/block/virtio-blk.c:172
Reject requests whose second-to-last input descriptor is too short to
hold struct virtio_scsi_inhdr.
Cc: qemu-stable@nongnu.org
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
hw/block/virtio-blk.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9cb9f1fb2b..418e0dd9c6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -197,6 +197,12 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
goto fail;
}
+ if (elem->in_sg[elem->in_num - 2].iov_len <
+ sizeof(struct virtio_scsi_inhdr)) {
+ status = VIRTIO_BLK_S_IOERR;
+ goto fail;
+ }
+
/*
* The scsi inhdr is placed in the second-to-last input segment, just
* before the regular inhdr.
--
2.34.1