Re: [PATCH net-next v2 09/13] net: lan966x: add PCIe FDMA support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Use-After-Free of `skb` in the RX path due to `skb_vlan_untag` freeing the buffer.
- [High] Prematurely exiting the NAPI poll loop on errors causes permanent RX queue stalls.
- [High] Unvalidated hardware-provided `data_len` in the RX path leads to out-of-bounds reads and kernel panics.
- [High] Unchecked access to potentially NULL `lan966x->ports[src_port]` in the RX path.
- [Medium] Missing DMA memory barriers (`dma_wmb()` / `dma_rmb()`) leading to potential data corruption on TX and RX.
--

Note: The format of this review has been altered to use a free-form summary
rather than inline quoting due to recitation restrictions.

commit 1dd886c56293690d145dbdf9f5bbd880c45747cf
Author: Daniel Machon <daniel.machon@microchip.com>
Subject: net: lan966x: add PCIe FDMA support

This commit adds PCIe FDMA support for lan966x, utilizing contiguous DMA
buffers mapped through the endpoint's ATU with memcpy-based frame transfers
instead of per-page DMA mappings.

I have a few questions about potential regressions introduced by this patch.

When lan966x_fdma_pci_rx_get_frame() calls lan966x_hw_offload() with the skb
passed by value, and lan966x_hw_offload() calls skb_vlan_untag(), could this
result in a use-after-free if skb_share_check() frees the original skb? The
caller continues to use the old pointer to update statistics and offload
marks.

In lan966x_fdma_pci_napi_poll(), if an individual frame has an error or
napi_alloc_skb() fails, the code jumps to the allocate_new label outside the
while loop. Does this prematurely exit the polling loop and leave remaining
completed frames unprocessed, potentially causing a permanent RX stall since
hardware interrupts won't re-trigger?

Is it safe to unconditionally trust the data_len extracted from the hardware
descriptor in lan966x_fdma_pci_rx_get_frame()? If a corrupted descriptor
returns a length larger than the mapped db_size, could this cause an
out-of-bounds read during the memcpy?

Furthermore, if the length from the descriptor is smaller than IFH_LEN_BYTES
or ETH_FCS_LEN, could the subsequent skb_pull() and skb_trim() operations
underflow and cause panics when the networking stack processes a wildly
oversized packet?

In lan966x_fdma_pci_rx_get_frame(), the code assigns the skb dev directly
from lan966x->ports[src_port]->dev. Since ports can fail to probe or be
intentionally disabled (which is explicitly checked for in functions like
lan966x_fdma_wakeup_netdev()), could a malformed IFH with an uninitialized
port index cause a NULL pointer dereference here?

The PCIe FDMA datapath uses contiguous coherent memory for descriptors and
frames, but operations like lan966x_fdma_pci_xmit() and
lan966x_fdma_pci_napi_poll() seem to lack dma_wmb() and dma_rmb() barriers.
Could weakly-ordered CPUs reorder the payload copies and descriptor status
updates, leading to the hardware transmitting uninitialized memory or the
CPU reading incomplete frames?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428-lan966x-pci-fdma-v2-0-d3ec66e06202@microchip.com?part=9