* [PATCH v2 1/7] crypto: qce - unmap the correct scatterlist in the CCM error path
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 2/7] crypto: qce - fix memory leak of the CCM AAD buffer ZhaoJinming
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming,
Eric Biggers, Bartosz Golaszewski, stable
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")
Reported-by: Eric Biggers <ebiggers@kernel.org>
Link: https://lore.kernel.org/linux-crypto/20260723183438.GB93534@quark/
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/7] crypto: qce - fix memory leak of the CCM AAD buffer
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 1/7] crypto: qce - unmap the correct scatterlist in the CCM error path ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 7:06 ` sashiko-bot
2026-11-13 6:54 ` [PATCH v2 3/7] crypto: qce - check dma_map_sg() result for the source scatterlist ZhaoJinming
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming,
Eric Biggers, Bartosz Golaszewski, stable
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")
Reported-by: Eric Biggers <ebiggers@kernel.org>
Link: https://lore.kernel.org/linux-crypto/20260723183438.GB93534@quark/
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/7] crypto: qce - fix memory leak of the CCM AAD buffer
2026-11-13 6:54 ` [PATCH v2 2/7] crypto: qce - fix memory leak of the CCM AAD buffer ZhaoJinming
@ 2026-11-13 7:06 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-13 7:06 UTC (permalink / raw)
To: ZhaoJinming, Bartosz Golaszewski, Herbert Xu, David S. Miller,
Thara Gopinath
Cc: linux-crypto, Eric Biggers, Bartosz Golaszewski
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] crypto: qce: single-word name in Signed-off-by trailer
--
commit 8b5fc4cf5bc1b9c0817753d0bd63aca7091e3300
Author: ZhaoJinming <zhaojinming@uniontech.com>
> crypto: qce - fix memory leak of the CCM AAD buffer
>
> 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")
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Link: https://lore.kernel.org/linux-crypto/20260723183438.GB93534@quark/
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
[Severity: High]
Should the Author and Signed-off-by lines separate the given name and
family name with a space (e.g., "Zhao Jinming" or "Jinming Zhao") instead
of using the single word "ZhaoJinming", as required by the Developer's
Certificate of Origin in Documentation/process/submitting-patches.rst?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-fix-qce-ccm-adata-leak-v2-0-914501377a62@uniontech.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/7] crypto: qce - check dma_map_sg() result for the source scatterlist
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 1/7] crypto: qce - unmap the correct scatterlist in the CCM error path ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 2/7] crypto: qce - fix memory leak of the CCM AAD buffer ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 4/7] crypto: qce - do not free an uninitialized dst_tbl on the error path ZhaoJinming
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming,
Eric Biggers, Bartosz Golaszewski, stable
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")
Reported-by: Eric Biggers <ebiggers@kernel.org>
Link: https://lore.kernel.org/linux-crypto/20260723183438.GB93534@quark/
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/7] crypto: qce - do not free an uninitialized dst_tbl on the error path
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
` (2 preceding siblings ...)
2026-11-13 6:54 ` [PATCH v2 3/7] crypto: qce - check dma_map_sg() result for the source scatterlist ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 5/7] crypto: qce - terminate the DMA channels if descriptor preparation fails ZhaoJinming
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming, stable
qce_aead_async_req_handle() jumps to error_free when mapping or starting
a request fails. For a CCM request without associated data that is
decrypted in place, qce_aead_ccm_prepare_buf() does not allocate
rctx->dst_tbl, so the else branch of error_free releases an uninitialized
sg_table:
} else {
sg_free_table(&rctx->dst_tbl);
}
The request context comes from aead_request_alloc(), which uses
kmalloc(), so this can pass a garbage sgl pointer to sg_free_table() and
crash the kernel.
qce_aead_done() already skips this case with
!(IS_DECRYPT(rctx->flags) && !diff_dst). Apply the same check to the
error path so the uninitialized table is not freed.
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 15ce778f019a..dd369328c8a6 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -493,7 +493,7 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
if (diff_dst)
sg_free_table(&rctx->dst_tbl);
kfree(rctx->adata);
- } else {
+ } else if (!(IS_CCM(rctx->flags) && IS_DECRYPT(rctx->flags) && !diff_dst)) {
sg_free_table(&rctx->dst_tbl);
}
return ret;
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/7] crypto: qce - terminate the DMA channels if descriptor preparation fails
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
` (3 preceding siblings ...)
2026-11-13 6:54 ` [PATCH v2 4/7] crypto: qce - do not free an uninitialized dst_tbl on the error path ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 6/7] crypto: qce - reject a negative scatterlist count in CCM AAD preparation ZhaoJinming
2026-11-13 6:54 ` [PATCH v2 7/7] crypto: qce - release dst_tbl when the source scatterlist is invalid ZhaoJinming
6 siblings, 0 replies; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming, stable
qce_dma_prep_sgs() submits the source descriptor to the RX channel before
preparing the destination descriptor on the TX channel. If the TX
preparation fails, the RX descriptor stays on the channel's pending queue
and qce_dma_prep_sgs() returns an error.
qce_aead_async_req_handle() then jumps to error_unmap_src, which unmaps
the scatterlists without terminating the channels. A later
qce_dma_issue_pending() from another request will run the stale RX
descriptor against memory that has already been unmapped and possibly
reused.
Jump to error_terminate on this failure as well, so
qce_dma_terminate_all() drops the partially submitted descriptor before
the scatterlists are unmapped. The error_unmap_src label is no longer
needed; error_terminate falls through to the same source unmap.
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 | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index dd369328c8a6..00c5924c057e 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -470,7 +470,7 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
ret = qce_dma_prep_sgs(&qce->dma, rctx->src_sg, src_nents, rctx->dst_sg, dst_nents,
qce_aead_done, async_req);
if (ret)
- goto error_unmap_src;
+ goto error_terminate;
qce_dma_issue_pending(&qce->dma);
@@ -482,7 +482,6 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
error_terminate:
qce_dma_terminate_all(&qce->dma);
-error_unmap_src:
if (diff_dst)
dma_unmap_sg(qce->dev, rctx->src_sg, rctx->src_nents, dir_src);
error_unmap_dst:
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/7] crypto: qce - reject a negative scatterlist count in CCM AAD preparation
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
` (4 preceding siblings ...)
2026-11-13 6:54 ` [PATCH v2 5/7] crypto: qce - terminate the DMA channels if descriptor preparation fails ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
2026-11-13 7:08 ` sashiko-bot
2026-11-13 6:54 ` [PATCH v2 7/7] crypto: qce - release dst_tbl when the source scatterlist is invalid ZhaoJinming
6 siblings, 1 reply; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming, stable
qce_aead_ccm_prepare_buf_assoclen() computes rctx->src_nents from
sg_nents_for_len() without checking for an error. If the source
scatterlist is shorter than the requested length the helper returns a
negative errno, which is added to the extra AAD/result entries and then
passed to sg_alloc_table(). That argument is unsigned int, so a negative
value turns into a huge entry count and sg_alloc_table() keeps allocating
memory until the system runs out.
Check the return value and bail out, releasing the already allocated AAD
buffer through err_free_adata.
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 | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 00c5924c057e..f75e3827d8bc 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -186,6 +186,7 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
unsigned int adata_header_len, cryptlen, totallen;
gfp_t gfp;
bool diff_dst;
+ int nents;
int ret;
if (IS_DECRYPT(rctx->flags))
@@ -231,10 +232,16 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
diff_dst = (req->src != req->dst) ? true : false;
+ nents = sg_nents_for_len(req->src, totallen);
+ if (nents < 0) {
+ ret = nents;
+ goto err_free_adata;
+ }
+
if (diff_dst)
- rctx->src_nents = sg_nents_for_len(req->src, totallen) + 1;
+ rctx->src_nents = nents + 1;
else
- rctx->src_nents = sg_nents_for_len(req->src, totallen) + 2;
+ rctx->src_nents = nents + 2;
gfp = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ? GFP_KERNEL : GFP_ATOMIC;
ret = sg_alloc_table(&rctx->src_tbl, rctx->src_nents, gfp);
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 6/7] crypto: qce - reject a negative scatterlist count in CCM AAD preparation
2026-11-13 6:54 ` [PATCH v2 6/7] crypto: qce - reject a negative scatterlist count in CCM AAD preparation ZhaoJinming
@ 2026-11-13 7:08 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-13 7:08 UTC (permalink / raw)
To: ZhaoJinming, Bartosz Golaszewski, Herbert Xu, David S. Miller,
Thara Gopinath
Cc: linux-crypto
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] crypto: qce: late sg_nents_for_len() check after
scatterwalk_ffwd()
--
commit 68e95033f32d45aeb2e2a9a17114843648589d9a
Author: ZhaoJinming <zhaojinming@uniontech.com>
crypto: qce - reject a negative scatterlist count in CCM AAD preparation
This commit checks the return value of sg_nents_for_len(req->src, totallen)
in qce_aead_ccm_prepare_buf_assoclen() before passing the entry count to
sg_alloc_table().
> diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
> index 00c5924c057e3..f75e3827d8bcb 100644
> --- a/drivers/crypto/qce/aead.c
> +++ b/drivers/crypto/qce/aead.c
> @@ -186,6 +186,7 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
> unsigned int adata_header_len, cryptlen, totallen;
> gfp_t gfp;
> bool diff_dst;
> + int nents;
> int ret;
>
> if (IS_DECRYPT(rctx->flags))
> @@ -231,10 +232,16 @@ qce_aead_ccm_prepare_buf_assoclen(struct aead_request *req)
>
> diff_dst = (req->src != req->dst) ? true : false;
>
> + nents = sg_nents_for_len(req->src, totallen);
> + if (nents < 0) {
> + ret = nents;
> + goto err_free_adata;
> + }
> +
> if (diff_dst)
> - rctx->src_nents = sg_nents_for_len(req->src, totallen) + 1;
> + rctx->src_nents = nents + 1;
> else
> - rctx->src_nents = sg_nents_for_len(req->src, totallen) + 2;
> + rctx->src_nents = nents + 2;
>
> gfp = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ? GFP_KERNEL : GFP_ATOMIC;
> ret = sg_alloc_table(&rctx->src_tbl, rctx->src_nents, gfp);
[Severity: High]
Is this sg_nents_for_len(req->src, totallen) check placed too late in
qce_aead_ccm_prepare_buf_assoclen()?
Earlier in the function, req->src is already walked and copied before
reaching this check:
drivers/crypto/qce/aead.c:qce_aead_ccm_prepare_buf_assoclen() {
...
totallen = cryptlen + req->assoclen;
/* Get the msg */
msg_sg = scatterwalk_ffwd(__sg, req->src, req->assoclen);
...
/* 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) {
...
}
If req->src is shorter than req->assoclen, scatterwalk_ffwd() walks past the
end of the scatterlist and triggers a NULL pointer dereference before the new
check is reached, and sg_copy_to_buffer() also receives an unchecked negative
return value from sg_nents_for_len(req->src, assoclen) as its unsigned int
nents argument. Checking sg_nents_for_len(req->src, totallen) immediately
after computing totallen would validate req->src before scatterwalk_ffwd(),
the rctx->adata allocation, and sg_copy_to_buffer() run.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-fix-qce-ccm-adata-leak-v2-0-914501377a62@uniontech.com?part=6
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 7/7] crypto: qce - release dst_tbl when the source scatterlist is invalid
2026-11-13 6:54 [PATCH v2 0/7] crypto: qce - fix CCM AAD buffer leak and AEAD error-path bugs ZhaoJinming
` (5 preceding siblings ...)
2026-11-13 6:54 ` [PATCH v2 6/7] crypto: qce - reject a negative scatterlist count in CCM AAD preparation ZhaoJinming
@ 2026-11-13 6:54 ` ZhaoJinming
6 siblings, 0 replies; 10+ messages in thread
From: ZhaoJinming @ 2026-11-13 6:54 UTC (permalink / raw)
To: Bartosz Golaszewski, Herbert Xu, David S. Miller, Thara Gopinath
Cc: linux-crypto, linux-arm-msm, linux-kernel, ZhaoJinming, stable
qce_aead_prepare_buf() calls qce_aead_prepare_dst_buf(), which allocates
rctx->dst_tbl, and then validates the source scatterlist with
sg_nents_for_len(). If the source is shorter than the requested length,
the function returns -EINVAL without freeing rctx->dst_tbl, and
qce_aead_async_req_handle() propagates the error with a plain return
instead of going through its error_free cleanup.
Free rctx->dst_tbl on this error path.
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 | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index f75e3827d8bc..78c168086c5b 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -322,6 +322,7 @@ static int qce_aead_prepare_buf(struct aead_request *req)
rctx->src_nents = sg_nents_for_len(req->src, totallen);
if (rctx->src_nents < 0) {
dev_err(qce->dev, "Invalid numbers of src SG.\n");
+ sg_free_table(&rctx->dst_tbl);
return -EINVAL;
}
rctx->src_sg = req->src;
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread