Re: [PATCH V3 3/4] net: xilinx: axienet: Derive RX frame length from DMA residue

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pending SKBs and DMA mappings are completely leaked during device stop when using dmaengine.
- [High] TX ring head/tail desynchronization on DMA mapping failure leads to use-after-free and double-free in the TX callback.
--

--- Patch [3]: [PATCH V3 3/4] net: xilinx: axienet: Derive RX frame length from DMA residue ---
commit 0ead77c343fc9310122738aba9dee99007a10e45
Author: Srinivas Neeli <srinivas.neeli@amd.com>

net: xilinx: axienet: Derive RX frame length from DMA residue

The dmaengine RX path previously determined the received frame length by
reading APP word 4 of the DMA descriptor metadata. This patch updates the
code to use the standard dmaengine residue mechanism instead, allowing the
driver to work on designs without the AXI4-Stream status/control interface.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index fcf517069d161..67d1b8e91d683 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -53,7 +53,6 @@
>  #define TX_BD_NUM_MAX			4096
>  #define RX_BD_NUM_MAX			4096
>  #define DMA_NUM_APP_WORDS		5
> -#define LEN_APP				4
>  #define RX_BUF_NUM_DEFAULT		128
>  
>  /* Must be shorter than length of ethtool_drvinfo.driver field to fit */

[ ... ]

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_stop() {
    ...
		for (i = 0; i < TX_BD_NUM_MAX; i++)
			kfree(lp->tx_skb_ring[i]);
		kfree(lp->tx_skb_ring);
		for (i = 0; i < RX_BUF_NUM_DEFAULT; i++)
			kfree(lp->rx_skb_ring[i]);
		kfree(lp->rx_skb_ring);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but are pending SKBs and DMA
mappings leaked during device stop when using dmaengine?

The descriptor wrapper objects are freed here without first unmapping the
associated DMA addresses or freeing the encapsulated SKBs.

Since the driver loses all pointers to previously allocated SKBs and their DMA
mappings, could this exhaust IOMMU space and leak memory each time the interface
is brought down?


drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit_dmaengine() {
    ...
	lp->tx_ring_head++;
	sg_init_table(skbuf_dma->sgl, sg_len);
	ret = skb_to_sgvec(skb, skbuf_dma->sgl, 0, skb->len);
	if (ret < 0)
		goto xmit_error_drop_skb;

	ret = dma_map_sg(lp->dev, skbuf_dma->sgl, sg_len, DMA_TO_DEVICE);
	if (!ret)
		goto xmit_error_drop_skb;
    ...
}

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_dma_tx_cb() {
    ...
	skbuf_dma = axienet_get_tx_desc(lp, lp->tx_ring_tail++);
	len = skbuf_dma->skb->len;
	txq = skb_get_tx_queue(lp->ndev, skbuf_dma->skb);
    ...
	dev_consume_skb_any(skbuf_dma->skb);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this lead to a TX ring head and tail
desynchronization on DMA mapping failure?

In axienet_start_xmit_dmaengine(), the tx_ring_head is advanced before the
failure points, and the error path does not roll it back or submit a dummy
descriptor.

If skb_to_sgvec() or dma_map_sg() fails, and a subsequent packet successfully
completes, axienet_dma_tx_cb() will unconditionally increment tx_ring_tail.

Would it then incorrectly operate on the out-of-sync tail index, dereferencing
and freeing the SKB pointer of the previously failed mapping?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708100652.603074-1-srinivas.neeli@amd.com?part=3