Skip to contentSkip to conversation

crypto: ccp - unmap descriptor before completing cookie

PATCH In review Wolfram Sang 2 participants 3 messages Oct 10 – Oct 12

Subsystems (3)
Crypto API dmaengine AMD cryptographic coprocessor (CCP) driver
lore
Share & tools
lore

Wolfram Sang

author
#0 lore raw

PATCH 1/1 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/[email protected] open here
Signed-off-by Wolfram Sang <[email protected]>

---

Build tested only.

 drivers/crypto/ccp/ccp-dmaengine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/crypto/ccp/ccp-dmaengine.c +1 -1
index b386a7063818..02c6c5fc7db1 100644
@@ -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);
 		}
 
 		desc = __ccp_next_dma_desc(chan, desc);
Signature · 1 line
2.53.0
AI review
#1 lore raw
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 --

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 <[email protected]>

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/[email protected] open here

drivers/crypto/ccp/ccp-dmaengine.c static struct ccp_dma_desc *ccp_handle_active_desc(struct ccp_dma_chan *chan, · line 228

Quoted 11 lines
drivers/crypto/ccp/ccp-dmaengine.c +1 -1
index b386a7063818b..02c6c5fc7db1f 100644
@@ -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.

Quoted 3 lines
drivers/crypto/ccp/ccp-dmaengine.c +0 -0
 		}
 
 		desc = __ccp_next_dma_desc(chan, desc);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1