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

Stephen Hemminger
2026-02-18T17:21:17Z

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.