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

[Sriram Yagnaraman]

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Tuesday, 24 March 2026 00:33
> To: Sriram Yagnaraman <sriram.yagnaraman at ericsson.com>
> Cc: dev at dpdk.org; jgrajcia at cisco.com; stable at dpdk.org; bly454 at 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 at 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 at dpdk.org
> >
> > Reported-by: Mike Bly <bly454 at gmail.com>
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman at 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.