Re: [PATCH net-next 2/7] net: xilinx: axienet: Introduce axienet_config for MAC-specific ops

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] In `axienet_start_xmit_dmaengine()`, if DMA mapping or scatterlist preparation fails, the software TX ring becomes permanently desynchronized from the hardware completions, leading to a double-free of SKBs and memory corruption.
--

--- Patch [2]: [PATCH net-next 2/7] net: xilinx: axienet: Introduce axienet_config for MAC-specific ops ---
commit 1bd6db20edd05b230c296fde4c925b468a67473b
Author: Suraj Gupta <suraj.gupta2@amd.com>
Subject: net: xilinx: axienet: Introduce axienet_config for MAC-specific ops

Introduce an axienet_config structure to abstract MAC-type-specific
operations. This prepares the driver for supporting additional MAC types
beyond the existing 1G Ethernet.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index fcf517069d161..b3deba1d96f7e 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
[ ... ]
> @@ -939,19 +976,21 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
>  		goto xmit_error_drop_skb;

[Severity: Critical]
This is a pre-existing issue, but doesn't this error path cause permanent
desynchronization of the TX ring and a double-free?

In axienet_start_xmit_dmaengine(), just before this context:

	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;

If skb_to_sgvec() or dma_map_sg() fails, the function jumps to the error path,
drops the SKB, and returns. However, lp->tx_ring_head remains incremented while
no descriptor is actually submitted to the DMA engine.

When a subsequent packet is successfully submitted and completes, the DMA engine
invokes axienet_dma_tx_cb(), which unconditionally fetches the descriptor at
lp->tx_ring_tail++:

	skbuf_dma = axienet_get_tx_desc(lp, lp->tx_ring_tail++);
	...
	dma_unmap_sg(lp->dev, skbuf_dma->sgl, skbuf_dma->sg_len, DMA_TO_DEVICE);
	dev_consume_skb_any(skbuf_dma->skb);

This will fetch the skipped descriptor slot instead of the one that completed.
Since the skipped slot's skb pointer was never updated, it will still point to
the previously freed SKB, leading to a double-free and memory corruption.

Could we fix this by decrementing tx_ring_head on the error path before
returning?

>  	/* Fill up app fields for checksum */
> -	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -		if (lp->features & XAE_FEATURE_FULL_TX_CSUM) {
> -			/* Tx Full Checksum Offload Enabled */
> -			app_metadata[0] |= 2;
> +	if (lp->axienet_config->dma_tx_csum) {
> +		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +			if (lp->features & XAE_FEATURE_FULL_TX_CSUM) {
> +				/* Tx Full Checksum Offload Enabled */
> +				app_metadata[0] |= 2;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723123838.125145-1-suraj.gupta2@amd.com?part=2