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

[Stephen Hemminger <stephen@networkplumber.org>]

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.