[PATCH 1/3] vhost: fix virtqueue array size for control queue

[PATCH 1/3] vhost: fix virtqueue array size for control queue

[Maxime Coquelin]

When max_queue_pairs is set to VHOST_MAX_QUEUE_PAIRS (128), VDUSE
calculates total_queues as max_queue_pairs * 2 + 1 = 257 to account
for the control queue. However, the virtqueue array was sized as
VHOST_MAX_QUEUE_PAIRS * 2, causing an out-of-bounds array access.

Fix by defining VHOST_MAX_VRING to explicitly account for the control
queue (VHOST_MAX_QUEUE_PAIRS * 2 + 1) and using it for the virtqueue
array size.

Fixes: f0a37cc6a1e2 ("vhost: add multiqueue support to VDUSE")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/vhost.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index e9e71c1707..ee61f7415e 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -261,8 +261,9 @@ struct vhost_async {
 };
 
 #define VHOST_RECONNECT_VERSION		0x0
-#define VHOST_MAX_VRING			0x100
 #define VHOST_MAX_QUEUE_PAIRS		0x80
+/* Max vring count: 2 per queue pair plus 1 control queue */
+#define VHOST_MAX_VRING			(VHOST_MAX_QUEUE_PAIRS * 2 + 1)
 
 struct __rte_cache_aligned vhost_reconnect_vring {
 	uint16_t last_avail_idx;
@@ -501,7 +502,7 @@ struct __rte_cache_aligned virtio_net {
 
 	int			extbuf;
 	int			linearbuf;
-	struct vhost_virtqueue	*virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];
+	struct vhost_virtqueue	*virtqueue[VHOST_MAX_VRING];
 
 	rte_rwlock_t	iotlb_pending_lock;
 	struct vhost_iotlb_entry *iotlb_pool;
-- 
2.52.0

Re: [PATCH 1/3] vhost: fix virtqueue array size for control queue

[David Marchand]

On Thu, 8 Jan 2026 at 14:50, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>
> When max_queue_pairs is set to VHOST_MAX_QUEUE_PAIRS (128), VDUSE
> calculates total_queues as max_queue_pairs * 2 + 1 = 257 to account
> for the control queue. However, the virtqueue array was sized as
> VHOST_MAX_QUEUE_PAIRS * 2, causing an out-of-bounds array access.
>
> Fix by defining VHOST_MAX_VRING to explicitly account for the control
> queue (VHOST_MAX_QUEUE_PAIRS * 2 + 1) and using it for the virtqueue
> array size.
>
> Fixes: f0a37cc6a1e2 ("vhost: add multiqueue support to VDUSE")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

[PATCH 2/3] vhost: fix descriptor chain bounds check in control queue

[Maxime Coquelin]

The virtio_net_ctrl_pop() function traverses descriptor chains from
guest-controlled memory without validating that the descriptor index
stays within bounds and without a counter to prevent infinite loops
from circular chains.

A malicious guest could craft descriptors with a next field pointing
out of bounds causing memory corruption, or create circular descriptor
chains causing an infinite loop and denial of service.

Add bounds checking and a loop counter to both descriptor chain
traversal loops, similar to the existing protection in virtio_net.c
fill_vec_buf_split().

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

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/virtio_net_ctrl.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/virtio_net_ctrl.c b/lib/vhost/virtio_net_ctrl.c
index 603a8db728..8149885384 100644
--- a/lib/vhost/virtio_net_ctrl.c
+++ b/lib/vhost/virtio_net_ctrl.c
@@ -28,7 +28,7 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 		struct virtio_net_ctrl_elem *ctrl_elem)
 	__rte_requires_shared_capability(&cvq->iotlb_lock)
 {
-	uint16_t avail_idx, desc_idx, n_descs = 0;
+	uint16_t avail_idx, desc_idx, n_descs = 0, nr_descs, cnt = 0;
 	uint64_t desc_len, desc_addr, desc_iova, data_len = 0;
 	uint8_t *ctrl_req;
 	struct vring_desc *descs;
@@ -59,12 +59,19 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 			goto err;
 		}
 
+		nr_descs = desc_len / sizeof(struct vring_desc);
 		desc_idx = 0;
 	} else {
 		descs = cvq->desc;
+		nr_descs = cvq->size;
 	}
 
 	while (1) {
+		if (unlikely(desc_idx >= nr_descs || ++cnt > nr_descs)) {
+			VHOST_CONFIG_LOG(dev->ifname, ERR, "Invalid ctrl descriptor chain");
+			goto err;
+		}
+
 		desc_len = descs[desc_idx].len;
 		desc_iova = descs[desc_idx].addr;
 
@@ -142,12 +149,23 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
 			goto free_err;
 		}
 
+		nr_descs = desc_len / sizeof(struct vring_desc);
 		desc_idx = 0;
 	} else {
 		descs = cvq->desc;
+		nr_descs = cvq->size;
 	}
 
-	while (!(descs[desc_idx].flags & VRING_DESC_F_WRITE)) {
+	cnt = 0;
+	while (1) {
+		if (unlikely(desc_idx >= nr_descs || ++cnt > nr_descs)) {
+			VHOST_CONFIG_LOG(dev->ifname, ERR, "Invalid ctrl descriptor chain");
+			goto free_err;
+		}
+
+		if (descs[desc_idx].flags & VRING_DESC_F_WRITE)
+			break;
+
 		desc_len = descs[desc_idx].len;
 		desc_iova = descs[desc_idx].addr;
 
-- 
2.52.0

Re: [PATCH 2/3] vhost: fix descriptor chain bounds check in control queue

[David Marchand]

On Thu, 8 Jan 2026 at 14:50, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>
> The virtio_net_ctrl_pop() function traverses descriptor chains from
> guest-controlled memory without validating that the descriptor index
> stays within bounds and without a counter to prevent infinite loops
> from circular chains.
>
> A malicious guest could craft descriptors with a next field pointing
> out of bounds causing memory corruption, or create circular descriptor
> chains causing an infinite loop and denial of service.
>
> Add bounds checking and a loop counter to both descriptor chain
> traversal loops, similar to the existing protection in virtio_net.c
> fill_vec_buf_split().

Too bad we have two separate implementations..

>
> Fixes: 474f4d7840ad ("vhost: add control virtqueue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/vhost/virtio_net_ctrl.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/lib/vhost/virtio_net_ctrl.c b/lib/vhost/virtio_net_ctrl.c
> index 603a8db728..8149885384 100644
> --- a/lib/vhost/virtio_net_ctrl.c
> +++ b/lib/vhost/virtio_net_ctrl.c
> @@ -28,7 +28,7 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
>                 struct virtio_net_ctrl_elem *ctrl_elem)
>         __rte_requires_shared_capability(&cvq->iotlb_lock)
>  {
> -       uint16_t avail_idx, desc_idx, n_descs = 0;
> +       uint16_t avail_idx, desc_idx, n_descs = 0, nr_descs, cnt = 0;
>         uint64_t desc_len, desc_addr, desc_iova, data_len = 0;
>         uint8_t *ctrl_req;
>         struct vring_desc *descs;
> @@ -59,12 +59,19 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
>                         goto err;
>                 }
>
> +               nr_descs = desc_len / sizeof(struct vring_desc);
>                 desc_idx = 0;
>         } else {
>                 descs = cvq->desc;
> +               nr_descs = cvq->size;
>         }
>
>         while (1) {
> +               if (unlikely(desc_idx >= nr_descs || ++cnt > nr_descs)) {

I don't like pre increment.
Can we use post increment and stay aligned with known to work
fill_vec_buf_split?


> +                       VHOST_CONFIG_LOG(dev->ifname, ERR, "Invalid ctrl descriptor chain");
> +                       goto err;
> +               }
> +
>                 desc_len = descs[desc_idx].len;
>                 desc_iova = descs[desc_idx].addr;
>
> @@ -142,12 +149,23 @@ virtio_net_ctrl_pop(struct virtio_net *dev, struct vhost_virtqueue *cvq,
>                         goto free_err;
>                 }
>
> +               nr_descs = desc_len / sizeof(struct vring_desc);
>                 desc_idx = 0;
>         } else {
>                 descs = cvq->desc;
> +               nr_descs = cvq->size;
>         }
>
> -       while (!(descs[desc_idx].flags & VRING_DESC_F_WRITE)) {
> +       cnt = 0;
> +       while (1) {
> +               if (unlikely(desc_idx >= nr_descs || ++cnt > nr_descs)) {

Idem.


> +                       VHOST_CONFIG_LOG(dev->ifname, ERR, "Invalid ctrl descriptor chain");
> +                       goto free_err;
> +               }
> +
> +               if (descs[desc_idx].flags & VRING_DESC_F_WRITE)
> +                       break;
> +
>                 desc_len = descs[desc_idx].len;
>                 desc_iova = descs[desc_idx].addr;
>

I wonder if we are missing some call to vhost_alloc_copy_ind_table() as well.


-- 
David Marchand

[PATCH 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler

[Maxime Coquelin]

The mmap() function returns MAP_FAILED on failure, not NULL.
The current check for !mmap_addr will never detect mmap failures.

When mmap fails but the error is not detected, an invalid address (-1)
is inserted into the IOTLB cache via vhost_user_iotlb_cache_insert().
Subsequent attempts to access this address will cause memory
corruption or crash.

Fix by checking for MAP_FAILED instead of NULL. Also add strerror to
the error message for easier debugging.

Fixes: f27d5206c598 ("vhost: add VDUSE callback for IOTLB miss")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/vduse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index 897dee9f1b..0b5d158fee 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -86,9 +86,10 @@ vduse_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm __rte_unuse
 
 	size = entry.last - entry.start + 1;
 	mmap_addr = mmap(0, size + entry.offset, entry.perm, MAP_SHARED, fd, 0);
-	if (!mmap_addr) {
+	if (mmap_addr == MAP_FAILED) {
 		VHOST_CONFIG_LOG(dev->ifname, ERR,
-				"Failed to mmap IOTLB entry for 0x%" PRIx64, iova);
+				"Failed to mmap IOTLB entry for 0x%" PRIx64 ": %s",
+				iova, strerror(errno));
 		ret = -1;
 		goto close_fd;
 	}
-- 
2.52.0

Re: [PATCH 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler

[David Marchand]

On Thu, 8 Jan 2026 at 14:50, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>
> The mmap() function returns MAP_FAILED on failure, not NULL.
> The current check for !mmap_addr will never detect mmap failures.
>
> When mmap fails but the error is not detected, an invalid address (-1)
> is inserted into the IOTLB cache via vhost_user_iotlb_cache_insert().
> Subsequent attempts to access this address will cause memory
> corruption or crash.
>
> Fix by checking for MAP_FAILED instead of NULL. Also add strerror to
> the error message for easier debugging.
>
> Fixes: f27d5206c598 ("vhost: add VDUSE callback for IOTLB miss")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Ugh.. I should have caught it when reviewing initial commit..
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand