[PATCH] vhost: fix wrapping on control virtqueue rings

[PATCH] vhost: fix wrapping on control virtqueue rings

[David Marchand]

The idx field of a virtqueue available ring is increased by the driver
regardless of the ring size. It is for the device to mask this index
modulo the ring size (2.7.6 of the virtio 1.3 specification).
The same applies to the used ring.

Failing to mask triggers:
- crashes when popping message received on the cvq,
- system lockups (in the case of VDUSE) when the virtio-net driver waits
  infinitely,

Fixes: 474f4d7840ad ("vhost: add control virtqueue")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/virtio_net_ctrl.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/vhost/virtio_net_ctrl.c b/lib/vhost/virtio_net_ctrl.c
index 999e84db7c..63c0a06b4f 100644
--- a/lib/vhost/virtio_net_ctrl.c
+++ b/lib/vhost/virtio_net_ctrl.c
@@ -40,7 +40,7 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 		return 0;
 	}
 
-	desc_idx = cvq->avail->ring[cvq->last_avail_idx];
+	desc_idx = cvq->avail->ring[cvq->last_avail_idx & (cvq->size - 1)];
 	if (desc_idx >= cvq->size) {
 		VHOST_CONFIG_LOG(dev->ifname, ERR, "Out of range desc index, dropping");
 		goto err;
@@ -167,8 +167,6 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 	}
 
 	cvq->last_avail_idx++;
-	if (cvq->last_avail_idx >= cvq->size)
-		cvq->last_avail_idx -= cvq->size;
 	vhost_virtqueue_reconnect_log_split(cvq);
 
 	if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX))
@@ -180,8 +178,6 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 	free(ctrl_elem->ctrl_req);
 err:
 	cvq->last_avail_idx++;
-	if (cvq->last_avail_idx >= cvq->size)
-		cvq->last_avail_idx -= cvq->size;
 	vhost_virtqueue_reconnect_log_split(cvq);
 
 	if (dev->features & (1ULL << VIRTIO_RING_F_EVENT_IDX))
@@ -231,13 +227,11 @@ virtio_net_ctrl_push(struct virtio_net *dev, struct virtio_net_ctrl_elem *ctrl_e
 	struct vhost_virtqueue *cvq = dev->cvq;
 	struct vring_used_elem *used_elem;
 
-	used_elem = &cvq->used->ring[cvq->last_used_idx];
+	used_elem = &cvq->used->ring[cvq->last_used_idx & (cvq->size - 1)];
 	used_elem->id = ctrl_elem->head_idx;
 	used_elem->len = ctrl_elem->n_descs;
 
 	cvq->last_used_idx++;
-	if (cvq->last_used_idx >= cvq->size)
-		cvq->last_used_idx -= cvq->size;
 
 	rte_atomic_store_explicit((unsigned short __rte_atomic *)&cvq->used->idx,
 		cvq->last_used_idx, rte_memory_order_release);
-- 
2.48.1

Re: [PATCH] vhost: fix wrapping on control virtqueue rings

[Maxime Coquelin]

On 4/2/25 8:53 AM, David Marchand wrote:
> The idx field of a virtqueue available ring is increased by the driver
> regardless of the ring size. It is for the device to mask this index
> modulo the ring size (2.7.6 of the virtio 1.3 specification).
> The same applies to the used ring.
> 
> Failing to mask triggers:
> - crashes when popping message received on the cvq,
> - system lockups (in the case of VDUSE) when the virtio-net driver waits
>    infinitely,
> 
> Fixes: 474f4d7840ad ("vhost: add control virtqueue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   lib/vhost/virtio_net_ctrl.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 


Thanks for the fix!

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Maxime