Re: [PATCH 2/3] net: xilinx: axienet: use dmaengine_prep_slave_sg_context()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] net: xilinx: axienet: unused dma_dev left in
  axienet_start_xmit_dmaengine()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] net: axienet: use-after-free in axienet_stop() [1]
--

commit b4bb968eb4c8d380631e7d9eb4acd99d61288c16
Author: Frank Li <Frank.Li@nxp.com>

net: xilinx: axienet: use dmaengine_prep_slave_sg_context()

This commit replaces the direct call to dma_dev->device_prep_slave_sg() in
axienet_start_xmit_dmaengine() with dmaengine_prep_slave_sg_context().

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 8a50cad1c104f..fff3530d9359d 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1006,9 +1006,10 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
>  		app_metadata[0] |= 2; /* Tx Full Checksum Offload Enabled */
>  	}
>  
> -	dma_tx_desc = dma_dev->device_prep_slave_sg(lp->tx_chan, skbuf_dma->sgl,
> -			sg_len, DMA_MEM_TO_DEV,
> -			DMA_PREP_INTERRUPT, (void *)app_metadata);
> +	dma_tx_desc = dmaengine_prep_slave_sg_context(lp->tx_chan, skbuf_dma->sgl,
> +						      sg_len, DMA_MEM_TO_DEV,
> +						      DMA_PREP_INTERRUPT,
> +						      (void *)app_metadata);
>  	if (!dma_tx_desc)
>  		goto xmit_error_unmap_sg;

[Severity: Low]
Does replacing dma_dev->device_prep_slave_sg() leave dma_dev unused in
axienet_start_xmit_dmaengine()?

Earlier in axienet_start_xmit_dmaengine(), dma_dev is still declared and
assigned from lp->tx_chan->device:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_start_xmit_dmaengine() {
	...
	struct dma_device *dma_dev;
	...
	dma_dev = lp->tx_chan->device;
	...
}

With its only reader removed, this triggers a -Wunused-but-set-variable
compiler warning and keeps a direct reference to struct dma_device and
lp->tx_chan->device.

[Severity: Critical]
This problem wasn't introduced by this patch, but can axienet_stop() cause
a use-after-free on lp->tx_skb_ring [1] while
axienet_start_xmit_dmaengine() is running?

When lp->use_dmaengine is enabled, axienet_stop() frees lp->tx_skb_ring and
its entries without quiescing the transmit path:

drivers/net/ethernet/xilinx/xilinx_axienet_main.c:axienet_stop() {
	...
	if (!lp->use_dmaengine) {
		WRITE_ONCE(lp->stopping, true);
		flush_work(&lp->dma_err_task);

		napi_disable(&lp->napi_tx);
		napi_disable(&lp->napi_rx);
	}
	...
		for (i = 0; i < TX_BD_NUM_MAX; i++) {
			skbuf_dma = lp->tx_skb_ring[i];
			...
			kfree(skbuf_dma);
		}
		kfree(lp->tx_skb_ring);
	...
}

CPU0                              CPU1
axienet_stop()
                                  axienet_start_xmit_dmaengine()
  kfree(skbuf_dma)
  kfree(lp->tx_skb_ring)
                                    axienet_get_tx_desc()
                                      <-- use-after-free

A concurrent transmit on CPU1 can fetch and use a descriptor from
lp->tx_skb_ring after CPU0 has freed it, corrupting memory or crashing the
kernel.

[1] https://sashiko.dev/#/bug/linux-6b22a4e9-355d-4ab5-829f-c56ef38ce667

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-rio_sg-v1-0-bc54b4bd8c95@nxp.com?part=2