[PATCH v2] virtio: validate split queue head count before popping

Jia Jia posted 1 patch 1 day, 16 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260710073517.13472-1-physicalmtea@gmail.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/virtio.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
[PATCH v2] virtio: validate split queue head count before popping
Posted by Jia Jia 1 day, 16 hours ago
virtqueue_split_pop() reads the next split avail ring entry after
checking that the ring is configured. Call virtqueue_num_heads() before
consuming that entry, so an avail index distance larger than the queue
size is rejected.

virtqueue_num_heads() also reports an empty queue and keeps the read
barrier needed before the avail ring entry is read. Drop the now unused
virtio_queue_empty_rcu() helper.

This prevents an invalid split queue state from being expanded into
repeated device command processing.

Link: https://gitlab.com/qemu-project/qemu/-/issues/3930
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
v2:
- Drop virtio_queue_empty_rcu() from the split pop path.
- Keep only the existing !vq->vring.avail early exit before
  virtqueue_num_heads().
- Remove the now unused virtio_queue_empty_rcu() helper.

Tested with qemu-system-x86_64 11.0.50, built from origin/master
f893c46c3931 plus this patch, configured with:
  --target-list=x86_64-softmmu --enable-kvm --disable-tcg

The original virtio-iommu live-vring qtest reproducer used for the
report completed successfully on the patched build without host OOM,
confirming that this reproducer is fixed by the patch.

 hw/virtio/virtio.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f4d86a3655..68ec3f0751 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -716,26 +716,6 @@ static inline bool is_desc_avail(uint16_t flags, bool wrap_counter)
     return (avail != used) && (avail == wrap_counter);
 }
 
-/* Fetch avail_idx from VQ memory only when we really need to know if
- * guest has added some buffers.
- * Called within rcu_read_lock().  */
-static int virtio_queue_empty_rcu(VirtQueue *vq)
-{
-    if (virtio_device_disabled(vq->vdev)) {
-        return 1;
-    }
-
-    if (unlikely(!vq->vring.avail)) {
-        return 1;
-    }
-
-    if (vq->shadow_avail_idx != vq->last_avail_idx) {
-        return 0;
-    }
-
-    return vring_avail_idx(vq) == vq->last_avail_idx;
-}
-
 static int virtio_queue_split_empty(VirtQueue *vq)
 {
     bool empty;
@@ -1748,12 +1728,14 @@ static void *virtqueue_split_pop(VirtQueue *vq, size_t sz)
     address_space_cache_init_empty(&indirect_desc_cache);
 
     RCU_READ_LOCK_GUARD();
-    if (virtio_queue_empty_rcu(vq)) {
+    if (unlikely(!vq->vring.avail)) {
+        goto done;
+    }
+
+    rc = virtqueue_num_heads(vq, vq->last_avail_idx);
+    if (rc <= 0) {
         goto done;
     }
-    /* Needed after virtio_queue_empty(), see comment in
-     * virtqueue_num_heads(). */
-    smp_rmb();
 
     /* When we start there are none of either input nor output. */
     out_num = in_num = elem_entries = 0;
-- 
2.34.1