[PATCH 0/3] crypto: qce - fix CCM AAD buffer leak and the AEAD error paths

[PATCH 0/3] crypto: qce - fix CCM AAD buffer leak and the AEAD error paths

[ZhaoJinming]

This series fixes a memory leak in the QCE AEAD driver together with two
bugs in the error handling around it.

qce_aead_ccm_prepare_buf_assoclen() allocates rctx->adata for every CCM
request that carries associated data, but the buffer is never freed.

While preparing that fix, two problems in the error path of
qce_aead_async_req_handle() turned out to matter:

  - It unmaps req->src although rctx->src_sg is the scatterlist that was
    passed to dma_map_sg().  For CCM requests with associated data,
    rctx->src_sg carries the AAD buffer, so its mapping is leaked, and
    freeing rctx->adata there would free memory that is still DMA-mapped.

  - A dma_map_sg() failure for the source scatterlist is not detected,
    because the result is checked for a negative value while
    dma_map_sg() returns 0 on failure.  The error path then unmaps a
    scatterlist that was never mapped.

Patch 1 unmaps the correct scatterlist, patch 2 adds the missing frees
and patch 3 fixes the dma_map_sg() check.

All three were reported as pre-existing in the Sashiko review of the QCE
BAM locking series.  They touch the same function that was recently fixed
by commit 7f2345f47dd1 ("crypto: qce - fix CCM AAD buffer
underallocation").

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
ZhaoJinming (3):
      crypto: qce - unmap the correct scatterlist in the CCM error path
      crypto: qce - fix memory leak of the CCM AAD buffer
      crypto: qce - check dma_map_sg() result for the source scatterlist

 drivers/crypto/qce/aead.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
---
base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053
change-id: 20261015-fix-qce-ccm-adata-leak-1ef34f172ba1

Best regards,
--  
ZhaoJinming <zhaojinming@uniontech.com>

[PATCH 1/3] crypto: qce - unmap the correct scatterlist in the CCM error path

[ZhaoJinming]

qce_aead_async_req_handle() maps the source with
dma_map_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src), but its
error path unmaps req->src instead:

	error_unmap_src:
		if (diff_dst)
			dma_unmap_sg(qce->dev, req->src, rctx->src_nents, dir_src);

For CCM requests with associated data and separate source and
destination buffers, rctx->src_sg is a scatterlist freshly built by
qce_aead_ccm_prepare_buf_assoclen() that carries the AAD, message and
result buffers, so it is not req->src.  The mapping created for
rctx->src_sg is therefore leaked, and req->src is unmapped with a nents
count that does not belong to it.

Unmap rctx->src_sg, matching the dma_map_sg() call, as qce_aead_done()
already does on the completion path.  This also makes it safe to free
rctx->adata on this path, which the next patch does.

Fixes: 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/crypto/qce/aead.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index a9642c67f476..f8e6054ff15d 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -478,7 +478,7 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
 	qce_dma_terminate_all(&qce->dma);
 error_unmap_src:
 	if (diff_dst)
-		dma_unmap_sg(qce->dev, req->src, rctx->src_nents, dir_src);
+		dma_unmap_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src);
 error_unmap_dst:
 	dma_unmap_sg(qce->dev, rctx->dst_sg, rctx->dst_nents, dir_dst);
 error_free:

-- 
2.50.1

[PATCH 2/3] crypto: qce - fix memory leak of the CCM AAD buffer

[ZhaoJinming]

qce_aead_ccm_prepare_buf_assoclen() allocates rctx->adata with kzalloc()
for every CCM request that carries associated data, but the buffer is
never freed: qce_aead_done() releases only the scatterlist tables, and
none of the error paths release it either.  Every such request therefore
leaks a small allocation.

Free rctx->adata in the completion handler and on all error paths: the
sg_copy_to_buffer() and sg_alloc_table() failures in
qce_aead_ccm_prepare_buf_assoclen() (through the new err_free_adata
label) and the error cleanup of qce_aead_async_req_handle().

The buffer is referenced by a scatterlist that is DMA-mapped, so it is
freed only after dma_unmap_sg() has run.  The frees are guarded the same
way as the surrounding sg_free_table() calls, so no path frees the
buffer twice or frees a stale pointer.

Fixes: 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/crypto/qce/aead.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index f8e6054ff15d..5bf59cb9ae9a 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -53,6 +53,7 @@ static void qce_aead_done(void *data)
 			sg_free_table(&rctx->src_tbl);
 			if (diff_dst)
 				sg_free_table(&rctx->dst_tbl);
+			kfree(rctx->adata);
 		} else {
 			if (!(IS_DECRYPT(rctx->flags) && !diff_dst))
 				sg_free_table(&rctx->dst_tbl);
@@ -220,8 +221,10 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
 	/* Copy the associated data */
 	if (sg_copy_to_buffer(req->src, sg_nents_for_len(req->src, assoclen),
 			      rctx->adata + adata_header_len,
-			      assoclen) != assoclen)
-		return -EINVAL;
+			      assoclen) != assoclen) {
+		ret = -EINVAL;
+		goto err_free_adata;
+	}
 
 	/* Pad associated data to block size */
 	rctx->assoclen = ALIGN(assoclen + adata_header_len, 16);
@@ -236,7 +239,7 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
 	gfp = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ? GFP_KERNEL : GFP_ATOMIC;
 	ret = sg_alloc_table(&rctx->src_tbl, rctx->src_nents, gfp);
 	if (ret)
-		return ret;
+		goto err_free_adata;
 
 	/* Associated Data */
 	sg_init_one(&rctx->adata_sg, rctx->adata, rctx->assoclen);
@@ -288,6 +291,9 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
 	return 0;
 err_free:
 	sg_free_table(&rctx->src_tbl);
+err_free_adata:
+	kfree(rctx->adata);
+	rctx->adata = NULL;
 	return ret;
 }
 
@@ -486,6 +492,7 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
 		sg_free_table(&rctx->src_tbl);
 		if (diff_dst)
 			sg_free_table(&rctx->dst_tbl);
+		kfree(rctx->adata);
 	} else {
 		sg_free_table(&rctx->dst_tbl);
 	}

-- 
2.50.1

[PATCH 3/3] crypto: qce - check dma_map_sg() result for the source scatterlist

[ZhaoJinming]

qce_aead_async_req_handle() maps the source scatterlist with
dma_map_sg(), which returns 0 on failure, but the result is checked
with:

	if (src_nents < 0) {
		ret = src_nents;
		goto error_unmap_dst;
	}

so a mapping failure is not detected.  The code then calls
qce_dma_prep_sgs() with zero source entries, which fails, and the error
path calls dma_unmap_sg() on a scatterlist that was never mapped.

Check for !src_nents instead and return -EIO, matching the check that is
already used for the destination scatterlist.  The error_unmap_dst path
skips the source unmap and only releases the destination mapping, which
is the one that was successfully created.

Fixes: 9363efb4181c ("crypto: qce - Add support for AEAD algorithms")
Cc: stable@vger.kernel.org
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/crypto/qce/aead.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 5bf59cb9ae9a..15ce778f019a 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -456,8 +456,8 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
 
 	if (diff_dst) {
 		src_nents = dma_map_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src);
-		if (src_nents < 0) {
-			ret = src_nents;
+		if (!src_nents) {
+			ret = -EIO;
 			goto error_unmap_dst;
 		}
 	} else {

-- 
2.50.1

Re: [PATCH 0/3] crypto: qce - fix CCM AAD buffer leak and the AEAD error paths

[Bartosz Golaszewski]

On Fri, 16 Oct 2026 04:09:42 +0200, ZhaoJinming
<zhaojinming@uniontech.com> said:
> This series fixes a memory leak in the QCE AEAD driver together with two
> bugs in the error handling around it.
>
> qce_aead_ccm_prepare_buf_assoclen() allocates rctx->adata for every CCM
> request that carries associated data, but the buffer is never freed.
>
> While preparing that fix, two problems in the error path of
> qce_aead_async_req_handle() turned out to matter:
>
>   - It unmaps req->src although rctx->src_sg is the scatterlist that was
>     passed to dma_map_sg().  For CCM requests with associated data,
>     rctx->src_sg carries the AAD buffer, so its mapping is leaked, and
>     freeing rctx->adata there would free memory that is still DMA-mapped.
>
>   - A dma_map_sg() failure for the source scatterlist is not detected,
>     because the result is checked for a negative value while
>     dma_map_sg() returns 0 on failure.  The error path then unmaps a
>     scatterlist that was never mapped.
>
> Patch 1 unmaps the correct scatterlist, patch 2 adds the missing frees
> and patch 3 fixes the dma_map_sg() check.
>
> All three were reported as pre-existing in the Sashiko review of the QCE
> BAM locking series.  They touch the same function that was recently fixed
> by commit 7f2345f47dd1 ("crypto: qce - fix CCM AAD buffer
> underallocation").
>
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
> ---

All three look correct to me, thanks!

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH 0/3] crypto: qce - fix CCM AAD buffer leak and the AEAD error paths

[Eric Biggers]

On Fri, Oct 16, 2026 at 10:09:42AM +0800, ZhaoJinming wrote:
> This series fixes a memory leak in the QCE AEAD driver together with two
> bugs in the error handling around it.
> 
> qce_aead_ccm_prepare_buf_assoclen() allocates rctx->adata for every CCM
> request that carries associated data, but the buffer is never freed.
> 
> While preparing that fix, two problems in the error path of
> qce_aead_async_req_handle() turned out to matter:
> 
>   - It unmaps req->src although rctx->src_sg is the scatterlist that was
>     passed to dma_map_sg().  For CCM requests with associated data,
>     rctx->src_sg carries the AAD buffer, so its mapping is leaked, and
>     freeing rctx->adata there would free memory that is still DMA-mapped.
> 
>   - A dma_map_sg() failure for the source scatterlist is not detected,
>     because the result is checked for a negative value while
>     dma_map_sg() returns 0 on failure.  The error path then unmaps a
>     scatterlist that was never mapped.
> 
> Patch 1 unmaps the correct scatterlist, patch 2 adds the missing frees
> and patch 3 fixes the dma_map_sg() check.
> 
> All three were reported as pre-existing in the Sashiko review of the QCE
> BAM locking series.  They touch the same function that was recently fixed
> by commit 7f2345f47dd1 ("crypto: qce - fix CCM AAD buffer
> underallocation").

Note that I reported these bugs (and others still unfixed) 3 months ago:
https://lore.kernel.org/linux-crypto/20260723183438.GB93534@quark/

- Eric