Re: [PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] dw_edma_device_pause and dw_edma_device_resume modify channel state and start transfers without holding the virtual channel lock.
--

--- Patch [3]: [PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates ---
commit 9fc733b7a8d11a99c619ff0dfa84d208446f0ab9
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Serialize abort state updates
    
This commit fixes a race condition in dw_edma_abort_interrupt() where dropping
the virtual channel lock before updating the channel request and status could
cause queued descriptors to be stranded. It keeps the descriptor completion
and state transition within the same critical section.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 44ef5fbe3fd4a..272b034057469 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -740,9 +740,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  		list_del(&vd->node);
>  		vchan_cookie_complete(vd);
>  	}
> -	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  	chan->request = EDMA_REQ_NONE;
>  	chan->status = EDMA_ST_IDLE;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);

[Severity: High]
This is a pre-existing issue, but since this patch fixes locking around
state updates, are dw_edma_device_pause() and dw_edma_device_resume() also
missing necessary locks?

Looking at dw_edma_device_pause(), it modifies chan->request without holding
chan->vc.lock. Similarly, dw_edma_device_resume() modifies chan->status and
invokes dw_edma_start_transfer() locklessly:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_resume() {
        ...
        } else {
                chan->status = EDMA_ST_BUSY;
                dw_edma_start_transfer(chan);
        }

        return err;
}

Since dw_edma_start_transfer() accesses protected lists and calls
vchan_next_desc() which expects the lock to be held
(lockdep_assert_held(&vc->lock)), could concurrent access from interrupt
handlers or issue_pending() lead to list corruption or invalid channel
states?

>  }

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