On 21/7/26 16:26, Philippe Mathieu-Daudé wrote:
On 21/7/26 15:44, Stefan Hajnoczi wrote:
Virtqueue handler functions in device emulation code often look
something like this:
while (!virtio_queue_empty(vq)) {
...pop and process virtqueue element...
}
virtio-blk, virtio-scsi, virtio-crypto, and vhost-shadow-virtqueue use
this pattern.
The device may break (i.e. hit an error that requires device reset)
during the loop. virtio_queue_empty() returns 1 for broken split vrings
but not for broken packed vrings, leading to an infinite loop.
Adjust the packed vring behavior to match split vrings and avoid
infinite loops.
Fixes: CVE-2026-16457
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3968
Reported-by: Anatol Belski <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
---
hw/virtio/virtio.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f4d86a36553..3795e3c8c68 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -763,6 +763,10 @@ static int
virtio_queue_packed_empty_rcu(VirtQueue *vq)
struct VRingPackedDesc desc;
VRingMemoryRegionCaches *cache;
+ if (virtio_device_disabled(vq->vdev)) {
+ return 1;
+ }
Correct, so:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
But why not run this check in virtio_queue_empty() entry?
To be clear:
-- >8 --
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f4d86a36553..019c9c9e293 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -786,6 +786,9 @@ static int virtio_queue_packed_empty(VirtQueue *vq)
int virtio_queue_empty(VirtQueue *vq)
{
+ if (virtio_device_disabled(vq->vdev)) {
+ return 1;
+ }
if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
return virtio_queue_packed_empty(vq);
} else {
---
Having:
static inline bool virtio_device_disabled(VirtIODevice *vdev)
{
return unlikely(vdev->disabled || vdev->broken);
}
that would avoid the RCU_READ_LOCK access.
