[PATCH] vhost: fix possible wrap in SVQ descriptor ring

Hawkins Jiawei posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230420074102.2317-1-yin31149@gmail.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++-
hw/virtio/vhost-shadow-virtqueue.h | 3 +++
2 files changed, 11 insertions(+), 1 deletion(-)
[PATCH] vhost: fix possible wrap in SVQ descriptor ring
Posted by Hawkins Jiawei 1 year ago
QEMU invokes vhost_svq_add() when adding a guest's element into SVQ.
In vhost_svq_add(), it uses vhost_svq_available_slots() to check
whether QEMU can add the element into the SVQ. If there is
enough space, then QEMU combines some out descriptors and
some in descriptors into one descriptor chain, and add it into
svq->vring.desc by vhost_svq_vring_write_descs().

Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx`
in vhost_svq_available_slots() return the number of occupied elements,
or the number of descriptor chains, instead of the number of occupied
descriptors, which may cause wrapping in SVQ descriptor ring.

Here is an example. In vhost_handle_guest_kick(), QEMU forwards
as many available buffers to device by virtqueue_pop() and
vhost_svq_add_element(). virtqueue_pop() return a guest's element,
and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to
add this element into SVQ. If QEMU invokes virtqueue_pop() and
vhost_svq_add_element() `svq->vring.num` times, vhost_svq_available_slots()
thinks QEMU just ran out of slots and everything should work fine.
But in fact, virtqueue_pop() return `svq-vring.num` elements or
descriptor chains, more than `svq->vring.num` descriptors, due to
guest memory fragmentation, and this cause wrapping in SVQ descriptor ring.

Therefore, this patch adds `num_free` field in VhostShadowVirtqueue
structure, updates this field in vhost_svq_add() and
vhost_svq_get_buf(), to record the number of free descriptors.
Then we can avoid wrap in SVQ descriptor ring by refactoring
vhost_svq_available_slots().

Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding")
Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++-
 hw/virtio/vhost-shadow-virtqueue.h | 3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 8361e70d1b..e1c6952b10 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp)
  */
 static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq)
 {
-    return svq->vring.num - (svq->shadow_avail_idx - svq->shadow_used_idx);
+    return svq->num_free;
 }
 
 /**
@@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg,
         return -EINVAL;
     }
 
+    /* Update the size of SVQ vring free descriptors */
+    svq->num_free -= ndescs;
+
     svq->desc_state[qemu_head].elem = elem;
     svq->desc_state[qemu_head].ndescs = ndescs;
     vhost_svq_kick(svq);
@@ -450,6 +453,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
     svq->desc_next[last_used_chain] = svq->free_head;
     svq->free_head = used_elem.id;
 
+    /* Update the size of SVQ vring free descriptors */
+    svq->num_free += num;
+
     *len = used_elem.len;
     return g_steal_pointer(&svq->desc_state[used_elem.id].elem);
 }
@@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,
     svq->iova_tree = iova_tree;
 
     svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq));
+    svq->num_free = svq->vring.num;
     driver_size = vhost_svq_driver_area_size(svq);
     device_size = vhost_svq_device_area_size(svq);
     svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), driver_size);
diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
index 926a4897b1..6efe051a70 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue {
 
     /* Next head to consume from the device */
     uint16_t last_used_idx;
+
+    /* Size of SVQ vring free descriptors */
+    uint16_t num_free;
 } VhostShadowVirtqueue;
 
 bool vhost_svq_valid_features(uint64_t features, Error **errp);
-- 
2.25.1