Re: [PATCH 00/11] net/dpaa2: fixes and improvements

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 18 Feb 2026 17:04:42 +0100
Maxime Leroy <maxime@leroys.fr> wrote:

> Various fixes and improvements for the dpaa2 net driver and fslmc bus.
> 
> Patches 1-2, 4-5 fix resource leaks on port close and in error paths.
> 
> Patch 3 fixes a misleading Rx descriptor limit warning.
> 
> Patches 6-7 replace getenv-based configuration with proper devargs
> for taildrop and data stashing options. There are still 8 remaining getenv
> calls in the driver that should be converted to devargs.
> Note: the taildrop disable path has never been reachable until now
> and is untested. NXP maintainers should validate this feature.
> 
> Patch 8 fixes link status not updating after port stop/start when
> link state change interrupts are enabled.
> Patch 9 is a minor cleanup in the same area.
> 
> Patches 10-11 fix devargs propagation on DPNI hotplug.

I decided to ask AI "what remaining bugs are still found by review
after applying these patches"

# dpaa2: remaining issues after Leroy series (v26.03-rc1)

The Leroy 11-patch series covers the resource leak and softparser
cleanup bugs well.  The items below are what is left.

---

## 1. NULL deref and packet leak in dump_err_pkts()  (dpaa2_rxtx.c ~729-746)

Three related problems in the same function:

(a) `mbuf` is NULL-checked on line 729 (`if (mbuf)`) but the very next
    statement dereferences `mbuf->nb_segs` unconditionally — crashes
    when mbuf is NULL.

(b) In the multi-segment path the while-loop walks `mbuf` to NULL, then
    `rte_pktmbuf_free(mbuf)` frees NULL — no-op — so the packet is
    never freed.

(c) `sprintf(title, "Payload seg[%d]", i)` writes into a 32-byte stack
    buffer with no bounds check.

Suggested fix: save the head pointer before iterating, move the hexdump
and free inside the NULL guard, and switch to `snprintf`.

## 2. Unbounded SG chain walk in eth_sg_fd_to_mbuf()  (dpaa2_rxtx.c:334)

```c
while (!DPAA2_SG_IS_FINAL(sge)) {
    sge = &sgt[i++];
    ...
}
```

No upper bound on `i`.  If hardware or corrupt DMA data fails to set
the FINAL bit, this walks past the end of the SGT buffer.  Adding
`&& i < DPAA2_MAX_SGS` to the loop condition is the minimal fix.

## 3. MAC stats path can deref NULL DMA pointers  (dpaa2_ethdev.c ~2005-2024)

`dpaa2_dev_mac_setup_stats()` is void-returning and can fail silently
(malloc or IOVA mapping failure), setting both DMA pointers to NULL.
The caller in `dpaa2_dev_xstats_get()` does not check and proceeds to
pass zero IOVAs to firmware and dereference `cnt_values_dma_mem`.
Either make the setup function return an error code or add a NULL guard
before use.

## 4. sw_td label in dpaa2_dev_tx() may double-free  (dpaa2_rxtx.c ~1516-1523)

At the `sw_td:` label, `bufs` has already been advanced past the
prepared frames and `num_tx` counts packets already handed to HW.
The loop frees `num_tx` packets via `*bufs++`, which are the same
buffers HW will also release — potential double-free.  Needs someone
with the HW context to verify the intended semantics.

## 5. dpaa2_dev_loopback_rx() always returns 0  (dpaa2_rxtx.c:2144)

The function updates internal counters correctly but unconditionally
returns 0, so the framework never sees received traffic.  Should
probably return `num_rx`.

## 6. Burst mode info reports only the first matching offload  (dpaa2_ethdev.c ~464-506)

Both `dpaa2_dev_rx_burst_mode_get()` and the TX variant break out of
the offload loop after the first match.  When multiple offloads are
enabled only the first is shown in `mode->info`.  The loop should
concatenate all matching strings.