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

["Neeli, Srinivas" <srneeli@amd.com>]

Hi,

On 7/9/2026 3:41 PM, sashiko-bot@kernel.org wrote:
> 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?
Both issues identified (SKB/DMA cleanup in axienet_stop and TX ring 
head/tail desync) are valid pre-existing concerns in the dmaengine path, 
not introduced by this patch.

   I will address these in a separate patch series.

   Thanks,
   Neeli Srinivas