Re: [PATCH net-next v5 05/13] net: lan966x: export FDMA helpers for reuse

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing dma_wmb() when appending new DMA descriptors to an active ring.
- [High] Lack of endianness conversions for hardware DMA descriptor fields, incompatible with PCIe on Big-Endian hosts.
- [High] Missing bounds validation on hardware-provided length before calling skb_put(), risking kernel panic.
--

commit 71ed27dbfea60bc58684878c1afb6584c28b04fb
Author: Daniel Machon <daniel.machon@microchip.com>

net: lan966x: export FDMA helpers for reuse

This patch makes shared FDMA helpers non-static to allow their reuse by the
PCIe FDMA implementation.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index 6c5761e886d47..25e673bdf0841 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c

[ ... ]

> @@ -471,7 +471,7 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
>  	return NULL;
>  }
>  
> -static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
> +int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but since this helper is being
exported for PCIe reuse, does the platform RX path need bounds validation on
the hardware-provided length?

In lan966x_fdma_rx_get_frame() (called by this NAPI poll function), the
packet length is extracted directly from the hardware descriptor and passed
unconditionally to skb_put():

	skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));

If the hardware returns a length exceeding the allocated SKB data region
minus the XDP headroom, skb_put() will extend the tail beyond skb->end,
resulting in an immediate skb_over_panic(). Should the length be validated
against the available headroom and at least IFH_LEN_BYTES before calling
skb_pull()?

[Severity: High]
This is a pre-existing issue, but since the FDMA implementation is being
exposed for PCIe, could the lack of endianness conversions cause issues on
Big-Endian hosts?

PCIe endpoints use strictly little-endian DMA descriptors. However, the driver
defines descriptor fields (like status and dataptr in struct fdma_db) as
native u64 and accesses them using native bitwise macros. 

If this code runs on a Big-Endian architecture with the PCIe device, will the
CPU read and write the status word in Big-Endian, causing the Little-Endian
hardware to interpret it incorrectly and fail?

[Severity: High]
This is a pre-existing issue, but is there a missing dma_wmb() when appending
new DMA descriptors to an active ring?

When lan966x_fdma_napi_poll() allocates new pages, it calls fdma_dcb_add()
which initializes the new descriptor's contents and then updates the previous
descriptor's nextptr to link it.

There doesn't appear to be a dma_wmb() executed between writing the new
descriptor's status/data pointers and updating the previous descriptor's
nextptr. Without this barrier, could the CPU reorder the memory writes,
allowing the hardware DMA engine to follow the updated nextptr and read the
new descriptor before its data and status fields are fully written to memory?

>  	struct lan966x *lan966x = container_of(napi, struct lan966x, napi);
>  	struct lan966x_rx *rx = &lan966x->rx;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-lan966x-pci-fdma-v5-0-ca56197ae05b@microchip.com?part=5