Re: [PATCH V6 3/4] net: xilinx: axienet: Derive RX frame length from residue in dmaengine path

Hi,

On 8/19/2026 11:17 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The DMA engine TX submission path fails to decrement `tx_ring_head` on error, corrupting the ring state and causing use-after-free and double-free on subsequent TX completions.
> - [High] The `axienet_stop()` function leaks pending `sk_buff` structures and their DMA mappings when using the DMA engine.
> --
>
> --- Patch [3]: [PATCH V6 3/4] net: xilinx: axienet: Derive RX frame length from residue in dmaengine path ---
> commit 86c32a60f9e05880f70cba98d261769dec79ad80
> Author: Srinivas Neeli <[email protected]>
>
> net: xilinx: axienet: Derive RX frame length from residue in dmaengine path
>
> The dmaengine RX path derived the received frame length from the descriptor
> APP metadata. That only works when the optional AXI4-Stream status/control
> interface is present, because the hardware populates the APP fields solely
> when that interface is enabled. On designs without it the length read back
> is invalid.
>
>> 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
> [ ... ]
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing the DMA ring
> management, I noticed an issue in axienet_start_xmit_dmaengine() where the
> ring state can become corrupted on error.
>
> If an error occurs during mapping, tx_ring_head remains incremented while the
> SKB is freed:
>
> 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;
> ...
> xmit_error_drop_skb:
> 	dev_kfree_skb_any(skb);
> 	return NETDEV_TX_OK;
> }
>
> When the next valid packet is successfully submitted and completes, will
> axienet_dma_tx_cb() fetch the descriptor using lp->tx_ring_tail++ and read
> the freed 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);
> }
>
> Does this out-of-sync ring state lead to a use-after-free and double-free
> of the SKB?
>
> [Severity: High]
> This is a pre-existing issue, but I also noticed a potential memory and DMA
> mapping leak in axienet_stop() when the interface is brought down.
>
> The teardown loops free the wrapper structures, but do they fail to unmap
> and free the associated SKBs?
>
> 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);
> ...
> }
>
> The SKBs and DMA mappings were allocated in axienet_rx_submit_desc():
>
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_rx_submit_desc() {
> ...
> 	addr = dma_map_single(lp->dev, skb->data, lp->max_frm_size, DMA_FROM_DEVICE);
> 	...
> 	skbuf_dma->skb = skb;
> 	skbuf_dma->dma_address = sg_dma_address(skbuf_dma->sgl);
> ...
> }
>
> Can this cause a permanent leak of SKBs and mappings each time the
> interface is stopped?


These issues are pre-existing and are not introduced by this patch. I 
plan to address them in a separate series.

Thanks
Srinivas Neeli