Re: [PATCH] crypto: ccp - unmap descriptor before completing cookie

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] crypto: ccp: silent error swallowing due to cleared dma cookie
--

commit c8a251448dee088f8a410c932932372165225507
Author: Wolfram Sang <wsa+renesas@sang-engineering.com>

crypto: ccp - unmap descriptor before completing cookie

To ensure that buffers are flushed, make sure that the descriptor gets
unmapped before completing the cookie.

Link: https://lore.kernel.org/r/20261009093745.539924-1-vkoul@kernel.org

> diff --git a/drivers/crypto/ccp/ccp-dmaengine.c b/drivers/crypto/ccp/ccp-dmaengine.c
> index b386a7063818b..02c6c5fc7db1f 100644
> --- a/drivers/crypto/ccp/ccp-dmaengine.c
> +++ b/drivers/crypto/ccp/ccp-dmaengine.c
> @@ -224,8 +224,8 @@ static struct ccp_dma_desc *ccp_handle_active_desc(struct ccp_dma_chan *chan,
>  				"%s - tx %d complete, status=%u\n", __func__,
>  				desc->tx_desc.cookie, desc->status);
>  
> -			dma_cookie_complete(tx_desc);
>  			dma_descriptor_unmap(tx_desc);
> +			dma_cookie_complete(tx_desc);

[Severity: High]
Does this clear the cookie to 0, preventing later error checking in
ccp_tx_status()?

When ccp_tx_status() attempts to find the completed descriptor to check for
errors, it skips the descriptor because its cookie was zeroed by
dma_cookie_complete():

drivers/crypto/ccp/ccp-dmaengine.c:ccp_tx_status() {
    ...
		list_for_each_entry(desc, &chan->complete, entry) {
			if (desc->tx_desc.cookie != cookie)
				continue;

			ret = desc->status;
			break;
		}
    ...
}

Since the cookie no longer matches, ccp_tx_status() drops the DMA_ERROR state
and erroneously returns DMA_COMPLETE.

Additionally, a few lines later in ccp_handle_active_desc(), the callback
invocation passes NULL for the result:

drivers/crypto/ccp/ccp-dmaengine.c:ccp_handle_active_desc() {
    ...
		if (tx_desc) {
			dmaengine_desc_get_callback_invoke(tx_desc, NULL);
    ...
}

Does passing NULL here cause dmaengine to report DMA_TRANS_NOERROR to the
client's callback even when the status is DMA_ERROR? This sequence appears
to completely swallow DMA and cryptographic hardware failures.

>  		}
>  
>  		desc = __ccp_next_dma_desc(chan, desc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010125001.19300-3-wsa+renesas@sang-engineering.com?part=1