Re: [PATCH v4 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing lock protection in `dw_edma_device_resume` leading to virtual channel list corruption.
--

--- Patch [8]: [PATCH v4 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA ---
commit e9a2a1385b7b45deaf7871886aa181ac959a00b4
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA

This commit consolidates the start functionality by introducing a common
dw_edma_core_start() for both eDMA and HDMA. It removes the separate
.start() callbacks in the eDMA and HDMA cores.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2652ad8e7a8f6..f52d9fd18e573 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -163,9 +163,37 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
>  	dw_edma_free_desc(vd2dw_edma_desc(vdesc));
>  }
>  
[ ... ]
> +
>  static int dw_edma_start_transfer(struct dw_edma_chan *chan)
>  {
> -	struct dw_edma *dw = chan->dw;
>  	struct dw_edma_chunk *child;
>  	struct dw_edma_desc *desc;
>  	struct virt_dma_desc *vd;

[Severity: High]
This isn't a bug introduced by this patch, but does dw_edma_start_transfer()
have a structural data race when called from dw_edma_device_resume()?

When a client calls dmaengine_resume(), dw_edma_device_resume() executes
dw_edma_start_transfer() without acquiring chan->vc.lock.

In dw_edma_start_transfer(), vchan_next_desc() reads the desc_issued list.

If a client concurrently calls dmaengine_issue_pending(), the driver will
acquire chan->vc.lock and modify the desc_issued list using
vchan_issue_pending().

Can this lead to linked list corruption or a use-after-free when one
thread mutates the list while another reads it unprotected?

> @@ -183,7 +211,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
>  	if (!child)
>  		return 0;
>  
> -	dw_edma_core_start(dw, child, !desc->xfer_sz);
> +	dw_edma_core_start(child, !desc->xfer_sz);
>  	desc->xfer_sz += child->xfer_sz;
>  	dw_edma_free_burst(child);
>  	list_del(&child->list);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-edma_ll-v4-0-cc128f0afb61@nxp.com?part=8