[PATCH] net/memif: fix multi-segment RX data corruption and truncation

[PATCH] net/memif: fix multi-segment RX data corruption and truncation

[Sriram Yagnaraman]

Fix dst_off being reset per-descriptor instead of per-packet in the RX
slow path. When processing chained descriptors (MEMIF_DESC_FLAG_NEXT),
goto next_slot2 reset dst_off to 0, overwriting the beginning of the
current mbuf with data from subsequent descriptors. Move dst_off
initialization before the next_slot2 label so it is only reset once
per packet.

Add boundary check in both RX paths before processing next segment.
If MEMIF_DESC_FLAG_NEXT is set but n_slots is 0, free the incomplete
mbuf chain and exit gracefully to prevent reading beyond available
descriptors.

Bugzilla ID: 1609
Fixes: aa17df860891 ("net/memif: add a Rx fast path")
Cc: stable@dpdk.org

Reported-by: Mike Bly <bly454@gmail.com>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
 drivers/net/memif/rte_eth_memif.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index 0002e24..cef1b0d 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -376,6 +376,13 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			n_slots--;
 
 			if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+				if (unlikely(n_slots == 0)) {
+					MIF_LOG(ERR, "Truncated packet: NEXT flag set but no more slots");
+					rte_pktmbuf_free(mbuf_head);
+					rte_pktmbuf_free_bulk(mbufs + rx_pkts,
+							MAX_PKT_BURST - rx_pkts);
+					goto no_free_bufs;
+				}
 				mbuf_tail = mbuf;
 				mbuf = rte_pktmbuf_alloc(mq->mempool);
 				if (unlikely(mbuf == NULL)) {
@@ -414,13 +421,13 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				goto no_free_bufs;
 			mbuf = mbuf_head;
 			mbuf->port = mq->in_port;
+			dst_off = 0;
 
 next_slot2:
 			s0 = cur_slot & mask;
 			d0 = &ring->desc[s0];
 
 			src_len = d0->length;
-			dst_off = 0;
 			src_off = 0;
 
 			do {
@@ -462,8 +469,14 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			cur_slot++;
 			n_slots--;
 
-			if (d0->flags & MEMIF_DESC_FLAG_NEXT)
+			if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+				if (unlikely(n_slots == 0)) {
+					MIF_LOG(ERR, "Truncated packet: NEXT flag set but no more slots");
+					rte_pktmbuf_free(mbuf_head);
+					goto no_free_bufs;
+				}
 				goto next_slot2;
+			}
 
 			mq->n_bytes += rte_pktmbuf_pkt_len(mbuf_head);
 			*bufs++ = mbuf_head;
-- 
2.43.7

Re: [PATCH] net/memif: fix multi-segment RX data corruption and truncation

[Stephen Hemminger]

On Mon, 16 Mar 2026 16:59:18 +0100
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:

> Fix dst_off being reset per-descriptor instead of per-packet in the RX
> slow path. When processing chained descriptors (MEMIF_DESC_FLAG_NEXT),
> goto next_slot2 reset dst_off to 0, overwriting the beginning of the
> current mbuf with data from subsequent descriptors. Move dst_off
> initialization before the next_slot2 label so it is only reset once
> per packet.
> 
> Add boundary check in both RX paths before processing next segment.
> If MEMIF_DESC_FLAG_NEXT is set but n_slots is 0, free the incomplete
> mbuf chain and exit gracefully to prevent reading beyond available
> descriptors.
> 
> Bugzilla ID: 1609
> Fixes: aa17df860891 ("net/memif: add a Rx fast path")
> Cc: stable@dpdk.org
> 
> Reported-by: Mike Bly <bly454@gmail.com>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>

It looked good to me, but AI review spotted an issue.


---

OK, so my error finding stands but with refined reasoning. Here's the corrected summary:

**Error: Double free of `mbuf_head` in the fast-path truncation error path.**

In the new truncation check added to the fast path:
```c
if (unlikely(n_slots == 0)) {
    MIF_LOG(ERR, "Truncated packet: ...");
    rte_pktmbuf_free(mbuf_head);
    rte_pktmbuf_free_bulk(mbufs + rx_pkts,
            MAX_PKT_BURST - rx_pkts);
    goto no_free_bufs;
}
```

`mbuf_head` is `mbufs[rx_pkts]` (assigned at line 359). `rte_pktmbuf_free(mbuf_head)` frees the head and its entire chain. Then `rte_pktmbuf_free_bulk(mbufs + rx_pkts, ...)` processes the same array starting at `mbufs[rx_pkts]`, which still holds the now-freed pointer. Since `rte_pktmbuf_free_bulk` walks `m->next` chains (confirmed in `lib/mbuf/rte_mbuf.c:574`), this frees `mbuf_head` a second time.

The existing error paths at lines 383-386 and 391-394 do NOT have this bug — they don't call `rte_pktmbuf_free(mbuf_head)` first; they let `rte_pktmbuf_free_bulk` handle the entire chain. The fix is to remove the separate `rte_pktmbuf_free(mbuf_head)` and just do:
```c
rte_pktmbuf_free_bulk(mbufs + rx_pkts,
        MAX_PKT_BURST - rx_pkts);
```

This works because `rte_pktmbuf_free_bulk` already walks the chain on `mbufs[rx_pkts]`, freeing `mbuf_head` and all segments chained onto it, plus the remaining unused mbufs.

No other issues found. The `dst_off` fix and the slow-path bounds check are both correct.

[PATCH v2] net/memif: fix multi-segment Rx corruption

[Sriram Yagnaraman]

Fix dst_off being reset per-descriptor instead of per-packet in the Rx
slow path. When processing chained descriptors (MEMIF_DESC_FLAG_NEXT),
goto next_slot2 reset dst_off to 0, overwriting the beginning of the
current mbuf with data from subsequent descriptors. Move dst_off
initialization before the next_slot2 label so it is only reset once
per packet.

Add boundary check in both Rx paths before processing next segment.
If MEMIF_DESC_FLAG_NEXT is set but n_slots is 0, free the incomplete
mbuf chain and exit gracefully to prevent reading beyond available
descriptors.

Bugzilla ID: 1609
Fixes: aa17df860891 ("net/memif: add a Rx fast path")
Cc: stable@dpdk.org

Reported-by: Mike Bly <bly454@gmail.com>
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
v2:
- Fix double-free in fast-path truncation: mbuf_head is mbufs[rx_pkts],
  so rte_pktmbuf_free_bulk already frees it and its chain. Remove the
  separate rte_pktmbuf_free(mbuf_head) call. (Stephen Hemminger)
- Remove MIF_LOG from truncation paths, increment mq->n_err and report
  via stats->ierrors instead.

 drivers/net/memif/rte_eth_memif.c | 19 +++++++++++++++++--
 drivers/net/memif/rte_eth_memif.h |  1 +
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index effcee3721..5d153c3a5a 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -378,6 +378,12 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			n_slots--;
 
 			if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+				if (unlikely(n_slots == 0)) {
+					mq->n_err++;
+					rte_pktmbuf_free_bulk(mbufs + rx_pkts,
+							MAX_PKT_BURST - rx_pkts);
+					goto no_free_bufs;
+				}
 				mbuf_tail = mbuf;
 				mbuf = rte_pktmbuf_alloc(mq->mempool);
 				if (unlikely(mbuf == NULL)) {
@@ -416,13 +422,13 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				goto no_free_bufs;
 			mbuf = mbuf_head;
 			mbuf->port = mq->in_port;
+			dst_off = 0;
 
 next_slot2:
 			s0 = cur_slot & mask;
 			d0 = &ring->desc[s0];
 
 			src_len = d0->length;
-			dst_off = 0;
 			src_off = 0;
 
 			do {
@@ -464,8 +470,14 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 			cur_slot++;
 			n_slots--;
 
-			if (d0->flags & MEMIF_DESC_FLAG_NEXT)
+			if (d0->flags & MEMIF_DESC_FLAG_NEXT) {
+				if (unlikely(n_slots == 0)) {
+					mq->n_err++;
+					rte_pktmbuf_free(mbuf_head);
+					goto no_free_bufs;
+				}
 				goto next_slot2;
+			}
 
 			mq->n_bytes += rte_pktmbuf_pkt_len(mbuf_head);
 			*bufs++ = mbuf_head;
@@ -1607,6 +1619,7 @@ memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats,
 		}
 		stats->ipackets += mq->n_pkts;
 		stats->ibytes += mq->n_bytes;
+		stats->ierrors += mq->n_err;
 	}
 
 	tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_c2s_rings :
@@ -1639,12 +1652,14 @@ memif_stats_reset(struct rte_eth_dev *dev)
 		    dev->data->rx_queues[i];
 		mq->n_pkts = 0;
 		mq->n_bytes = 0;
+		mq->n_err = 0;
 	}
 	for (i = 0; i < pmd->run.num_s2c_rings; i++) {
 		mq = (pmd->role == MEMIF_ROLE_CLIENT) ? dev->data->rx_queues[i] :
 		    dev->data->tx_queues[i];
 		mq->n_pkts = 0;
 		mq->n_bytes = 0;
+		mq->n_err = 0;
 	}
 
 	return 0;
diff --git a/drivers/net/memif/rte_eth_memif.h b/drivers/net/memif/rte_eth_memif.h
index d4e625ab51..9c7a3a93f0 100644
--- a/drivers/net/memif/rte_eth_memif.h
+++ b/drivers/net/memif/rte_eth_memif.h
@@ -69,6 +69,7 @@ struct memif_queue {
 	/* rx/tx info */
 	uint64_t n_pkts;			/**< number of rx/tx packets */
 	uint64_t n_bytes;			/**< number of rx/tx bytes */
+	uint64_t n_err;				/**< number of rx/tx errors */
 
 	struct rte_intr_handle *intr_handle;	/**< interrupt handle */
 
-- 
2.43.7

RE: [PATCH] net/memif: fix multi-segment RX data corruption and truncation

[Sriram Yagnaraman]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, 24 March 2026 00:33
> To: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> Cc: dev@dpdk.org; jgrajcia@cisco.com; stable@dpdk.org; bly454@gmail.com
> Subject: Re: [PATCH] net/memif: fix multi-segment RX data corruption and
> truncation
> 
> On Mon, 16 Mar 2026 16:59:18 +0100
> Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:
> 
> > Fix dst_off being reset per-descriptor instead of per-packet in the RX
> > slow path. When processing chained descriptors
> (MEMIF_DESC_FLAG_NEXT),
> > goto next_slot2 reset dst_off to 0, overwriting the beginning of the
> > current mbuf with data from subsequent descriptors. Move dst_off
> > initialization before the next_slot2 label so it is only reset once
> > per packet.
> >
> > Add boundary check in both RX paths before processing next segment.
> > If MEMIF_DESC_FLAG_NEXT is set but n_slots is 0, free the incomplete
> > mbuf chain and exit gracefully to prevent reading beyond available
> > descriptors.
> >
> > Bugzilla ID: 1609
> > Fixes: aa17df860891 ("net/memif: add a Rx fast path")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Mike Bly <bly454@gmail.com>
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> 
> It looked good to me, but AI review spotted an issue.
> 
> 
> ---
> 
> OK, so my error finding stands but with refined reasoning. Here's the
> corrected summary:
> 
> **Error: Double free of `mbuf_head` in the fast-path truncation error path.**
> 
> In the new truncation check added to the fast path:
> ```c
> if (unlikely(n_slots == 0)) {
>     MIF_LOG(ERR, "Truncated packet: ...");
>     rte_pktmbuf_free(mbuf_head);
>     rte_pktmbuf_free_bulk(mbufs + rx_pkts,
>             MAX_PKT_BURST - rx_pkts);
>     goto no_free_bufs;
> }
> ```
> 
> `mbuf_head` is `mbufs[rx_pkts]` (assigned at line 359).
> `rte_pktmbuf_free(mbuf_head)` frees the head and its entire chain. Then
> `rte_pktmbuf_free_bulk(mbufs + rx_pkts, ...)` processes the same array
> starting at `mbufs[rx_pkts]`, which still holds the now-freed pointer. Since
> `rte_pktmbuf_free_bulk` walks `m->next` chains (confirmed in
> `lib/mbuf/rte_mbuf.c:574`), this frees `mbuf_head` a second time.
> 
> The existing error paths at lines 383-386 and 391-394 do NOT have this bug —
> they don't call `rte_pktmbuf_free(mbuf_head)` first; they let
> `rte_pktmbuf_free_bulk` handle the entire chain. The fix is to remove the
> separate `rte_pktmbuf_free(mbuf_head)` and just do:
> ```c
> rte_pktmbuf_free_bulk(mbufs + rx_pkts,
>         MAX_PKT_BURST - rx_pkts);
> ```
> 
> This works because `rte_pktmbuf_free_bulk` already walks the chain on
> `mbufs[rx_pkts]`, freeing `mbuf_head` and all segments chained onto it, plus
> the remaining unused mbufs.
> 

Thanks Stephen, it is a good catch, I corrected this and posted a v2.

> No other issues found. The `dst_off` fix and the slow-path bounds check are
> both correct.

Re: [PATCH v2] net/memif: fix multi-segment Rx corruption

[Stephen Hemminger]

On Tue, 24 Mar 2026 17:14:36 +0100
Sriram Yagnaraman <sriram.yagnaraman@ericsson.com> wrote:

> Fix dst_off being reset per-descriptor instead of per-packet in the Rx
> slow path. When processing chained descriptors (MEMIF_DESC_FLAG_NEXT),
> goto next_slot2 reset dst_off to 0, overwriting the beginning of the
> current mbuf with data from subsequent descriptors. Move dst_off
> initialization before the next_slot2 label so it is only reset once
> per packet.
> 
> Add boundary check in both Rx paths before processing next segment.
> If MEMIF_DESC_FLAG_NEXT is set but n_slots is 0, free the incomplete
> mbuf chain and exit gracefully to prevent reading beyond available
> descriptors.
> 
> Bugzilla ID: 1609
> Fixes: aa17df860891 ("net/memif: add a Rx fast path")
> Cc: stable@dpdk.org
> 
> Reported-by: Mike Bly <bly454@gmail.com>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> ---

This version go clean review from AI.
Applied to next-net