Re: [PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing locking in `dw_edma_device_pause()` and `dw_edma_device_resume()` leads to data races, list corruption, double free, and channel deadlock.
--

--- Patch [4]: [PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing ---
commit 59c4b510d7df19eb3243d231516e5c29dab50d72
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Complete descriptors before pausing

This patch modifies dw_edma_start_transfer() usage on resume to set the
channel state to idle if no transfer can be started. It also adds logic
to ignore done interrupts when the channel is paused and ensures
descriptors are completed during a pause request.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 272b034057469..b06b299661c02 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -327,7 +327,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		err = -EPERM;
>  	} else {
>  		chan->status = EDMA_ST_BUSY;
> -		dw_edma_start_transfer(chan);
> +		if (!dw_edma_start_transfer(chan))
> +			chan->status = EDMA_ST_IDLE;

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_resume() execute this
path without holding chan->vc.lock?

The dw_edma_start_transfer() function manipulates virt-dma lists, frees
chunk metadata, and invokes vchan_next_desc() which expects the lock to
be held:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_start_transfer()
    vd = vchan_next_desc(&chan->vc);

A concurrent interrupt in dw_edma_done_interrupt() also takes the lock
and can call dw_edma_start_transfer(). Could this race lead to linked
list corruption, double frees, or lockdep panics?

Additionally, does dw_edma_device_pause() similarly modify chan->request
locklessly?

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_pause()
    else
        chan->request = EDMA_REQ_PAUSE;

Could this cause data races with dw_edma_done_interrupt() when
dmaengine_pause() and dmaengine_resume() are called while transfers are
ongoing?

>  	}
>  
>  	return err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=4