linux-kernel - Re: [PATCH 04/14] dmaengine: dma350: Check dma_cookie

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57528c01-2a1c-4d70-b70c-ed4b64bd93fc@arm.com>
Date: Fri, 29 Aug 2025 11:42:14 +0100
From: Robin Murphy <robin.murphy@....com>
To: Jisheng Zhang <jszhang@...nel.org>, Vinod Koul <vkoul@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/14] dmaengine: dma350: Check dma_cookie_status() ret
 code and txstate

On 2025-08-23 4:39 pm, Jisheng Zhang wrote:
> If dma_cookie_status() returns DMA_COMPLETE, we can return immediately.
> 
>  From another side, the txstate is an optional parameter used to get a
> struct with auxilary transfer status information. When not provided
> the call to device_tx_status() should return the status of the dma
> cookie. Return the status of dma cookie when the txstate optional
> parameter is not provided.

Again, the current code was definitely intentional - I think this was 
down to the hardware error case, where for reasons I now can't remember 
I still had to nominally complete the aborted descriptor from the IRQ 
handler to avoid causing some worse problem, and hence we don't return 
early without cross-checking dch->status here, because returning 
DMA_COMPLETE when the descriptor hasn't done its job makes dmatest unhappy.

I did spend a *lot* of time exercising all the error cases, and trying 
to get a sensible result across all of the different reporting APIs was 
fiddly to say the least - there's a huge lack of consistency between 
drivers in this regard, and this was just my attempt to be the 
least-worst one :)

Thanks,
Robin.

> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
> ---
>   drivers/dma/arm-dma350.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/arm-dma350.c b/drivers/dma/arm-dma350.c
> index 96350d15ed85..17af9bb2a18f 100644
> --- a/drivers/dma/arm-dma350.c
> +++ b/drivers/dma/arm-dma350.c
> @@ -377,6 +377,8 @@ static enum dma_status d350_tx_status(struct dma_chan *chan, dma_cookie_t cookie
>   	u32 residue = 0;
>   
>   	status = dma_cookie_status(chan, cookie, state);
> +	if (status == DMA_COMPLETE || !state)
> +		return status;
>   
>   	spin_lock_irqsave(&dch->vc.lock, flags);
>   	if (cookie == dch->cookie) {