Re: [PATCH 1/7] dmaengine: dw-edma: Fix HDMA channel status register access

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Hardware lifecycle mismanagement allows memory corruption via continuing DMA transfers after `dmaengine_terminate_all`.
--

--- Patch [1]: [PATCH 1/7] dmaengine: dw-edma: Fix HDMA channel status register access ---
commit 3731db5499eb78e6e698a87ead1446781041195a
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Fix HDMA channel status register access
    
This commit fixes the argument order for GET_CH_32() in
dw_hdma_v0_core_ch_status(). The previous incorrect order of passing the
channel ID and direction caused the status callback to read from the wrong
HDMA channel status register.

> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 632abb8b481cf..2beec876b1842 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -79,7 +79,7 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
>  	u32 tmp;
>  
>  	tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> -			GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> +			GET_CH_32(dw, chan->dir, chan->id, ch_stat));

[Severity: High]
This is a pre-existing issue, but I noticed a potential hardware lifecycle
mismanagement in dw_edma_device_terminate_all() which relies on this status
check.

When dmaengine_terminate_all() is called:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_terminate_all() {
    ...
	} else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
    ...
	} else if (chan->request > EDMA_REQ_PAUSE) {
		err = -EPERM;
	} else {
		chan->request = EDMA_REQ_STOP;
	}

	return err;
}

The function returns success after merely setting chan->request = EDMA_REQ_STOP,
without issuing hardware commands to abort the active transfer.

Since the hardware continues reading or writing memory in the background, if a
DMA client unmaps or frees the memory based on this function's successful
return, could the DMA hardware access unmapped or reallocated memory, leading
to memory corruption? 

For cyclic transfers, does this cause the hardware to loop indefinitely?

>  
>  	if (tmp == 1)
>  		return DMA_IN_PROGRESS;

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