Re: [PATCH V3 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Modifying `chan->free_seg_list` without holding `chan->lock` causes linked list corruption.
--

--- Patch [4]: [PATCH V3 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA ---
commit 7c1e9fcfa16f54b84c1fd0328dc790510cde97be
Author: Suraj Gupta <suraj.gupta2@amd.com>

dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA
    
This commit extends metadata handling for AXI DMA and MCDMA to expose
status and sideband information. The metadata pointer now starts at the
descriptor status word instead of the APP fields, and probe logic is
updated to attach metadata_ops when an AXI Stream interface is present.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 9b8b6f1acd4ea..03c45369086b8 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c

[ ... ]

> @@ -2623,6 +2653,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>  		segment->hw.control |= XILINX_MCDMA_BD_EOP;
>  	}
>  
> +	if (chan->xdev->has_axistream_connected)
> +		desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
> +
>  	return &desc->async_tx;
>  
>  error:
>  	xilinx_dma_free_tx_descriptor(chan, desc);

[Severity: High]
This isn't a bug introduced by this patch, but does this error path corrupt
the channel's free segment list?

When xilinx_mcdma_prep_slave_sg() encounters an error, it calls
xilinx_dma_free_tx_descriptor() without holding chan->lock. This eventually
cascades to xilinx_mcdma_free_tx_segment() and xilinx_dma_free_tx_segment(),
which add the segment back to chan->free_seg_list:

xilinx_mcdma_free_tx_segment()
    list_add_tail(&segment->node, &chan->free_seg_list);

If another CPU core concurrently calls xilinx_aximcdma_alloc_tx_segment(),
which acquires chan->lock to remove items from this same list, could this
unprotected modification lead to linked list corruption?

This unprotected error path also appears to be present in
xilinx_dma_tx_submit() when chan->cyclic is true.

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