[PATCH] crypto: qce - use the mapped scatterlist count for ahash requests

[PATCH] crypto: qce - use the mapped scatterlist count for ahash requests

[Jinming Zhao]

qce_ahash_async_req_handle() maps the source scatterlist with
dma_map_sg(), which returns the number of entries after the DMA mapper
has coalesced them.  The driver keeps that value in ret but immediately
overwrites it with the result of the second dma_map_sg() call for the
result buffer, so qce_dma_prep_sgs() and the later unmaps use
rctx->src_nents, the count from sg_nents_for_len() before mapping.

When the DMA mapper merges entries, the descriptor is prepared with more
entries than were mapped, so the DMA engine may read sg_dma_address and
sg_dma_len from entries that were never populated.  This can lead to
IOMMU faults or memory corruption.  Store the mapped count in
rctx->src_nents so the descriptor and both unmap paths use the count
returned by dma_map_sg().

This issue was found in the Sashiko review of the "crypto: qce - fix CCM
AAD buffer leak and AEAD error-path bugs" series.

Link: https://sashiko.dev/#/patchset/20261016-fix-qce-ccm-adata-leak-v1-0-f54a82af2de0@uniontech.com
Fixes: fea40451530fe ("crypto: qce - dma_map_sg can handle chained SG")
Cc: stable@vger.kernel.org
Signed-off-by: Jinming Zhao <zhaojinming@uniontech.com>
---
qce_ahash_async_req_handle() maps the source scatterlist with
dma_map_sg(), which returns the number of entries after the DMA mapper
has coalesced them.  The driver keeps that value in ret but then
overwrites it with the result of the second dma_map_sg() call for the
result buffer, so qce_dma_prep_sgs() and the later unmaps use the
rctx->src_nents value computed before mapping.

When the DMA mapper merges entries, the descriptor is prepared with more
entries than were mapped, so the DMA engine may read sg_dma_address and
sg_dma_len from entries that were never populated, which can lead to
IOMMU faults or memory corruption.  Store the mapped count in
rctx->src_nents so the descriptor and both unmap paths use the count
returned by dma_map_sg().

This issue was found in the Sashiko review of the "crypto: qce - fix CCM
AAD buffer leak and AEAD error-path bugs" series:
https://sashiko.dev/#/patchset/20261016-fix-qce-ccm-adata-leak-v1-0-f54a82af2de0%40uniontech.com
---
 drivers/crypto/qce/sha.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index b67d749db05f..c53d0e53784e 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -97,6 +97,12 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req)
 	if (!ret)
 		return -EIO;
 
+	/*
+	 * dma_map_sg() can coalesce entries, so use the number of mapped
+	 * entries for the descriptor and for the later unmap.
+	 */
+	rctx->src_nents = ret;
+
 	sg_init_one(&rctx->result_sg, qce->dma.result_buf, QCE_RESULT_BUF_SZ);
 
 	ret = dma_map_sg(qce->dev, &rctx->result_sg, 1, DMA_FROM_DEVICE);

---
base-commit: 7e1392a42ea288d70a266f6553fe06f57f8647ea
change-id: 20261113-qce-ahash-dma-count-41e47c59b17a

Best regards,
--  
Jinming Zhao <zhaojinming@uniontech.com>

Re: [PATCH] crypto: qce - use the mapped scatterlist count for ahash requests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] crypto: qce - wrong nents passed to dma_unmap_sg() in ahash
  paths
--

commit a88269ea6101fd18834fc68bd1c31da34ddad7a9
Author: Jinming Zhao <zhaojinming@uniontech.com>

crypto: qce - use the mapped scatterlist count for ahash requests

This commit stores the entry count returned by dma_map_sg() in
rctx->src_nents so that qce_dma_prep_sgs() and the subsequent dma_unmap_sg()
calls use the mapped scatterlist count.

Link: https://sashiko.dev/#/patchset/20261016-fix-qce-ccm-adata-leak-v1-0-f54a82af2de0@uniontech.com

> diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
> index b67d749db05f2..c53d0e53784e0 100644
> --- a/drivers/crypto/qce/sha.c
> +++ b/drivers/crypto/qce/sha.c
> @@ -97,6 +97,12 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req)
>  	if (!ret)
>  		return -EIO;
>  
> +	/*
> +	 * dma_map_sg() can coalesce entries, so use the number of mapped
> +	 * entries for the descriptor and for the later unmap.
> +	 */
> +	rctx->src_nents = ret;

[Severity: High]
Does overwriting rctx->src_nents pass the wrong entry count to
dma_unmap_sg()?

Both qce_ahash_done() and the error_unmap_src path pass rctx->src_nents to
dma_unmap_sg(), which the DMA API requires to be the original nents passed
to dma_map_sg() rather than the coalesced count it returned.  When
dma_map_sg() merges entries, unmapping with the smaller count triggers a
DMA-API debug warning and leaves trailing scatterlist segments unsynchronized
or unmapped.

Like qce_skcipher_async_req_handle(), keeping the return value of
dma_map_sg() in a local variable for qce_dma_prep_sgs() would preserve
rctx->src_nents for dma_unmap_sg().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-qce-ahash-dma-count-v1-1-0423b54d75c8@uniontech.com?part=1