public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] vhost: VDUSE-related fixes
@ 2026-01-14 15:34 Maxime Coquelin
  2026-01-14 15:34 ` [PATCH v2 1/3] vhost: fix virtqueue array size for control queue Maxime Coquelin
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Maxime Coquelin @ 2026-01-14 15:34 UTC (permalink / raw)
  To: dev, chenbox, david.marchand; +Cc: Maxime Coquelin

This series contains 3 fixes for issues spotted by Claude Code.

The first one is to avoid out-of-bound accesses in virtqueues array
in the case we have the maximum supported queue pairs and control queue.

Second one is a security issue that could result in theory in a denial
of service, but a CVE was not created because the control queue support
cannot currently be negotiated with the Kernel VDUSE driver.

Last patch is fixing mmap error handling in the VDUSE IOTLB miss handler.

Maxime Coquelin (3):
  vhost: fix virtqueue array size for control queue
  vhost: fix descriptor chain bounds check in control queue
  vhost: fix mmap error check in VDUSE IOTLB miss handler

Changes in v2:
==============
- use post-increment for readability and consistency.

 lib/vhost/vduse.c           |  5 +++--
 lib/vhost/vhost.h           |  5 +++--
 lib/vhost/virtio_net_ctrl.c | 22 ++++++++++++++++++++--
 3 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] vhost: fix virtqueue array size for control queue
  2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
@ 2026-01-14 15:34 ` Maxime Coquelin
  2026-01-16  2:06   ` Chenbo Xia
  2026-01-14 15:34 ` [PATCH v2 2/3] vhost: fix descriptor chain bounds check in " Maxime Coquelin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Maxime Coquelin @ 2026-01-14 15:34 UTC (permalink / raw)
  To: dev, chenbox, david.marchand; +Cc: Maxime Coquelin, stable

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>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] vhost: fix descriptor chain bounds check in control queue
  2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
  2026-01-14 15:34 ` [PATCH v2 1/3] vhost: fix virtqueue array size for control queue Maxime Coquelin
@ 2026-01-14 15:34 ` Maxime Coquelin
  2026-01-15  9:11   ` David Marchand
  2026-01-16  3:10   ` Chenbo Xia
  2026-01-14 15:34 ` [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler Maxime Coquelin
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 10+ messages in thread
From: Maxime Coquelin @ 2026-01-14 15:34 UTC (permalink / raw)
  To: dev, chenbox, david.marchand; +Cc: Maxime Coquelin, stable

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..b059fe1f02 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler
  2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
  2026-01-14 15:34 ` [PATCH v2 1/3] vhost: fix virtqueue array size for control queue Maxime Coquelin
  2026-01-14 15:34 ` [PATCH v2 2/3] vhost: fix descriptor chain bounds check in " Maxime Coquelin
@ 2026-01-14 15:34 ` Maxime Coquelin
  2026-01-16  3:11   ` Chenbo Xia
  2026-01-14 17:53 ` [PATCH v2 0/3] vhost: VDUSE-related fixes Stephen Hemminger
  2026-01-26 19:37 ` Maxime Coquelin
  4 siblings, 1 reply; 10+ messages in thread
From: Maxime Coquelin @ 2026-01-14 15:34 UTC (permalink / raw)
  To: dev, chenbox, david.marchand; +Cc: Maxime Coquelin, stable

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>
Reviewed-by: David Marchand <david.marchand@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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/3] vhost: VDUSE-related fixes
  2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
                   ` (2 preceding siblings ...)
  2026-01-14 15:34 ` [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler Maxime Coquelin
@ 2026-01-14 17:53 ` Stephen Hemminger
  2026-01-26 19:37 ` Maxime Coquelin
  4 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2026-01-14 17:53 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, chenbox, david.marchand

On Wed, 14 Jan 2026 16:34:42 +0100
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:

> This series contains 3 fixes for issues spotted by Claude Code.
> 
> The first one is to avoid out-of-bound accesses in virtqueues array
> in the case we have the maximum supported queue pairs and control queue.
> 
> Second one is a security issue that could result in theory in a denial
> of service, but a CVE was not created because the control queue support
> cannot currently be negotiated with the Kernel VDUSE driver.
> 
> Last patch is fixing mmap error handling in the VDUSE IOTLB miss handler.
> 
> Maxime Coquelin (3):
>   vhost: fix virtqueue array size for control queue
>   vhost: fix descriptor chain bounds check in control queue
>   vhost: fix mmap error check in VDUSE IOTLB miss handler
> 
> Changes in v2:
> ==============
> - use post-increment for readability and consistency.
> 
>  lib/vhost/vduse.c           |  5 +++--
>  lib/vhost/vhost.h           |  5 +++--
>  lib/vhost/virtio_net_ctrl.c | 22 ++++++++++++++++++++--
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 

AI review summary of V2.

## DPDK Patch Review: [PATCH v2 0/3] vhost: VDUSE-related fixes

**Series Author:** Maxime Coquelin <maxime.coquelin@redhat.com>

---

### Patch 1/3: vhost: fix virtqueue array size for control queue

**Subject Line Analysis:**
- ✅ Length: 50 characters (≤60)
- ✅ Format: `vhost: fix virtqueue array size for control queue` - correct `component: description` format
- ✅ Lowercase after colon
- ✅ Imperative mood ("fix")
- ✅ No trailing period

**Commit Body Analysis:**
- ✅ Lines wrap at 75 characters
- ✅ Does not start with "It"
- ✅ Good problem description explaining the off-by-one issue
- ✅ Explains the fix clearly

**Tags:**
- ✅ `Fixes:` tag present with 12-char SHA: `f0a37cc6a1e2`
- ✅ `Cc: stable@dpdk.org` present (appropriate for bug fix)
- ✅ `Signed-off-by:` present with real name and email
- ✅ `Reviewed-by:` present
- ✅ Tag order is correct (Fixes, Cc, blank line, Signed-off-by, Reviewed-by)

**Code Review:**
```c
-#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)
```
- ✅ Good explanatory comment added
- ✅ The fix is correct: `0x80 * 2 + 1 = 257` vs old `0x100 = 256`
- ✅ Proper use of parentheses in macro definition

```c
-	struct vhost_virtqueue	*virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];
+	struct vhost_virtqueue	*virtqueue[VHOST_MAX_VRING];
```
- ✅ Uses the new macro consistently

**Verdict: PASS** ✅

---

### Patch 2/3: vhost: fix descriptor chain bounds check in control queue

**Subject Line Analysis:**
- ✅ Length: 56 characters (≤60)
- ✅ Format: correct `component: description`
- ✅ Lowercase after colon
- ✅ Imperative mood
- ✅ No trailing period

**Commit Body Analysis:**
- ✅ Lines wrap at 75 characters
- ✅ Does not start with "It"
- ✅ Excellent security-focused description explaining the vulnerability
- ✅ Describes both attack vectors (OOB access and infinite loop)
- ✅ References similar existing protection pattern

**Tags:**
- ✅ `Fixes:` tag present with 12-char SHA: `474f4d7840ad`
- ✅ `Cc: stable@dpdk.org` present
- ✅ `Signed-off-by:` present
- ✅ Tag order correct

**Code Review:**
```c
-	uint16_t avail_idx, desc_idx, n_descs = 0;
+	uint16_t avail_idx, desc_idx, n_descs = 0, nr_descs, cnt = 0;
```
- ✅ Adds necessary tracking variables

```c
+		if (unlikely(desc_idx >= nr_descs || cnt++ >= nr_descs)) {
+			VHOST_CONFIG_LOG(dev->ifname, ERR, "Invalid ctrl descriptor chain");
+			goto err;
+		}
```
- ✅ Correct bounds check pattern
- ✅ Uses `unlikely()` for performance optimization on error path
- ✅ Proper error logging with `VHOST_CONFIG_LOG`
- ✅ Post-increment `cnt++` (per v2 changelog - good for readability)

**Minor Observation (Info):**
- The variable naming uses both `n_descs` and `nr_descs` in the same function. While `n_descs` tracks the number of descriptors processed and `nr_descs` is the maximum allowed, this could be slightly confusing. However, this follows existing DPDK patterns and the code is clear enough.

**Verdict: PASS** ✅

---

### Patch 3/3: vhost: fix mmap error check in VDUSE IOTLB miss handler

**Subject Line Analysis:**
- ✅ Length: 54 characters (≤60)
- ✅ Format: correct `component: description`
- ✅ Lowercase after colon
- ✅ Imperative mood
- ✅ No trailing period

**Commit Body Analysis:**
- ✅ Lines wrap at 75 characters
- ✅ Does not start with "It"
- ✅ Excellent explanation of the bug and consequences
- ✅ Documents potential impact (memory corruption/crash)

**Tags:**
- ✅ `Fixes:` tag present with 12-char SHA: `f27d5206c598`
- ✅ `Cc: stable@dpdk.org` present
- ✅ `Signed-off-by:` present
- ✅ `Reviewed-by:` present
- ✅ Tag order correct

**Code Review:**
```c
-	if (!mmap_addr) {
+	if (mmap_addr == MAP_FAILED) {
```
- ✅ **Critical bug fix**: `mmap()` returns `MAP_FAILED` (typically `(void *)-1`) on error, not `NULL`. The old check would never detect mmap failures.

```c
-			"Failed to mmap IOTLB entry for 0x%" PRIx64, iova);
+			"Failed to mmap IOTLB entry for 0x%" PRIx64 ": %s",
+			iova, strerror(errno));
```
- ✅ Uses proper `PRIx64` format specifier (not `%llx`)
- ✅ Added `strerror(errno)` for better debugging

**Potential Issue (Info):**
- The code uses `strerror(errno)`. While this is fine for error logging in most contexts, `strerror()` is not thread-safe in all implementations. However, DPDK typically uses glibc which has a thread-safe `strerror()` implementation, and this is only called on error paths. This is consistent with other DPDK error handling.

**Verdict: PASS** ✅

---

## Summary

| Patch | Status | Issues |
|-------|--------|--------|
| 1/3: fix virtqueue array size | ✅ PASS | None |
| 2/3: fix descriptor chain bounds check | ✅ PASS | None |  
| 3/3: fix mmap error check | ✅ PASS | None |

### Overall Assessment: **Series Ready for Merge** ✅

This is a high-quality patch series fixing real bugs. All three patches:

1. Follow DPDK commit message guidelines
2. Have proper Fixes tags referencing the commits that introduced the bugs
3. Include `Cc: stable@dpdk.org` for backporting
4. Have appropriate Signed-off-by and Reviewed-by tags
5. Contain correct and well-commented code changes

The fixes address genuine issues: an off-by-one array sizing bug, a security vulnerability allowing DoS via malicious guests, and a critical mmap error handling bug that would silently insert invalid addresses into the IOTLB cache.

**Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>**

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] vhost: fix descriptor chain bounds check in control queue
  2026-01-14 15:34 ` [PATCH v2 2/3] vhost: fix descriptor chain bounds check in " Maxime Coquelin
@ 2026-01-15  9:11   ` David Marchand
  2026-01-16  3:10   ` Chenbo Xia
  1 sibling, 0 replies; 10+ messages in thread
From: David Marchand @ 2026-01-15  9:11 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev, chenbox, stable

On Wed, 14 Jan 2026 at 16:35, 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().
>
> Fixes: 474f4d7840ad ("vhost: add control virtqueue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks for the update.
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] vhost: fix virtqueue array size for control queue
  2026-01-14 15:34 ` [PATCH v2 1/3] vhost: fix virtqueue array size for control queue Maxime Coquelin
@ 2026-01-16  2:06   ` Chenbo Xia
  0 siblings, 0 replies; 10+ messages in thread
From: Chenbo Xia @ 2026-01-16  2:06 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev@dpdk.org, david.marchand@redhat.com, stable@dpdk.org



> On Jan 14, 2026, at 23:34, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> 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>
> ---
> 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
> 

Reviewed-by: Chenbo Xia <chenbox@nvidia.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] vhost: fix descriptor chain bounds check in control queue
  2026-01-14 15:34 ` [PATCH v2 2/3] vhost: fix descriptor chain bounds check in " Maxime Coquelin
  2026-01-15  9:11   ` David Marchand
@ 2026-01-16  3:10   ` Chenbo Xia
  1 sibling, 0 replies; 10+ messages in thread
From: Chenbo Xia @ 2026-01-16  3:10 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev@dpdk.org, david.marchand@redhat.com, stable@dpdk.org



> On Jan 14, 2026, at 23:34, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> 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..b059fe1f02 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
> 

Reviewed-by: Chenbo Xia <chenbox@nvidia.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler
  2026-01-14 15:34 ` [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler Maxime Coquelin
@ 2026-01-16  3:11   ` Chenbo Xia
  0 siblings, 0 replies; 10+ messages in thread
From: Chenbo Xia @ 2026-01-16  3:11 UTC (permalink / raw)
  To: Maxime Coquelin; +Cc: dev@dpdk.org, david.marchand@redhat.com, stable@dpdk.org



> On Jan 14, 2026, at 23:34, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> 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>
> Reviewed-by: David Marchand <david.marchand@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
> 

Reviewed-by: Chenbo Xia <chenbox@nvidia.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/3] vhost: VDUSE-related fixes
  2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
                   ` (3 preceding siblings ...)
  2026-01-14 17:53 ` [PATCH v2 0/3] vhost: VDUSE-related fixes Stephen Hemminger
@ 2026-01-26 19:37 ` Maxime Coquelin
  4 siblings, 0 replies; 10+ messages in thread
From: Maxime Coquelin @ 2026-01-26 19:37 UTC (permalink / raw)
  To: dev, chenbox, david.marchand

On Wed, Jan 14, 2026 at 4:34 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This series contains 3 fixes for issues spotted by Claude Code.
>
> The first one is to avoid out-of-bound accesses in virtqueues array
> in the case we have the maximum supported queue pairs and control queue.
>
> Second one is a security issue that could result in theory in a denial
> of service, but a CVE was not created because the control queue support
> cannot currently be negotiated with the Kernel VDUSE driver.
>
> Last patch is fixing mmap error handling in the VDUSE IOTLB miss handler.
>
> Maxime Coquelin (3):
>   vhost: fix virtqueue array size for control queue
>   vhost: fix descriptor chain bounds check in control queue
>   vhost: fix mmap error check in VDUSE IOTLB miss handler
>
> Changes in v2:
> ==============
> - use post-increment for readability and consistency.
>
>  lib/vhost/vduse.c           |  5 +++--
>  lib/vhost/vhost.h           |  5 +++--
>  lib/vhost/virtio_net_ctrl.c | 22 ++++++++++++++++++++--
>  3 files changed, 26 insertions(+), 6 deletions(-)
>
> --
> 2.52.0
>

Applied to next-virtio/for-next-net.

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-01-26 19:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14 15:34 [PATCH v2 0/3] vhost: VDUSE-related fixes Maxime Coquelin
2026-01-14 15:34 ` [PATCH v2 1/3] vhost: fix virtqueue array size for control queue Maxime Coquelin
2026-01-16  2:06   ` Chenbo Xia
2026-01-14 15:34 ` [PATCH v2 2/3] vhost: fix descriptor chain bounds check in " Maxime Coquelin
2026-01-15  9:11   ` David Marchand
2026-01-16  3:10   ` Chenbo Xia
2026-01-14 15:34 ` [PATCH v2 3/3] vhost: fix mmap error check in VDUSE IOTLB miss handler Maxime Coquelin
2026-01-16  3:11   ` Chenbo Xia
2026-01-14 17:53 ` [PATCH v2 0/3] vhost: VDUSE-related fixes Stephen Hemminger
2026-01-26 19:37 ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox