Re: [PATCH v6 09/10] dmaengine: dw-edma: Use burst array instead of linked list

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] In non-linked-list (non_ll) mode, the burst array index is incorrectly calculated using `chan->ll_max` instead of `bursts_max`. This causes a heap buffer overflow and leaves the actual DMA descriptor uninitialized.
--

--- Patch [9]: [PATCH v6 09/10] dmaengine: dw-edma: Use burst array instead of linked list ---
commit 755f4cc5fb98a43f3634cf5e1dc7f15a35af8c49
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Use burst array instead of linked list

Creating a DMA descriptor requires at least three kzalloc() calls because
each burst is allocated as a linked-list node. This commit allocates a
burst array when creating each chunk to simplify the code and eliminate
one kzalloc() call.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index f52d9fd18e573..202862a828b4d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -166,23 +103,17 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
>  static void dw_edma_core_start(struct dw_edma_chunk *chunk, bool first)
>  {
>  	struct dw_edma_chan *chan = chunk->chan;
> -	struct dw_edma_burst *child;
>  	u32 i = 0;
> -	int j;
>  
>  	if (chan->non_ll) {
> -		child = list_first_entry_or_null(&chunk->burst->list,
> -						 struct dw_edma_burst, list);
> -		if (child)
> -			chan->dw->core->non_ll_start(chunk->chan, child);
> +		if (chunk->nburst == 1)
> +			chan->dw->core->non_ll_start(chunk->chan, &chunk->burst[0]);

[Severity: Critical]
If chunk->burst[0] is bypassed during allocation in dw_edma_device_transfer()
when i > 0, does this code submit an uninitialized descriptor to the
hardware?

[ ... ]
> @@ -530,15 +456,15 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer,
>  		if (xfer->type == EDMA_XFER_SCATTER_GATHER && !sg)
>  			break;
>  
> -		if (chunk->bursts_alloc == bursts_max) {
> -			chunk = dw_edma_alloc_chunk(desc);
> +		if (!(i % bursts_max)) {
> +			u32 n = min(cnt - i, bursts_max);
> +
> +			chunk = dw_edma_alloc_chunk(desc, n);
>  			if (unlikely(!chunk))
>  				goto err_alloc;
>  		}
>  
> -		burst = dw_edma_alloc_burst(chunk);
> -		if (unlikely(!burst))
> -			goto err_alloc;
> +		burst = chunk->burst + (i % chan->ll_max);

[Severity: Critical]
Does this code overflow chunk->burst[] when chan->non_ll is true?

If chan->non_ll is true, bursts_max is 1, so dw_edma_alloc_chunk() allocates
a chunk with a 1-element burst array for each i. However, burst is indexed
using chan->ll_max.

For example, when i = 1:
- i % bursts_max is 0, so a new chunk is allocated with nburst = 1.
- If chan->ll_max > 1, i % chan->ll_max is 1.
- burst is set to &chunk->burst[1], which is out of bounds.

Consequently, chunk->burst[0] remains uninitialized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-edma_ll-v6-0-1471d278b73a@nxp.com?part=9