Re: [PATCH v17 09/14] crypto: qce - Remove unused ignore_buf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] qce_dma_release() frees the DMA channels and result buffer without explicitly terminating in-flight background DMA operations.
- [Critical] The surrounding error rollback loop in devm_qce_register_algs() uses the wrong loop variable, leaking registered algorithms and causing subsequent UAF.
--

commit 86b6706b62fa03437f8e5e0bdb45e4e4d945b1d3
Author: Bartosz Golaszewski <brgl@kernel.org>

crypto: qce - Remove unused ignore_buf

This commit removes the ignore_buf field from struct qce_dma_data and the
assignment in devm_qce_dma_request() because it was unused. It also moves the
QCE_IGNORE_BUF_SZ macro definition into dma.c since it is only used locally.

> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 68cafd4741ad3..08bf3e8ec1243 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c
> @@ -9,6 +9,8 @@
>  
>  #include "dma.h"
>  
> +#define QCE_IGNORE_BUF_SZ		(2 * QCE_BAM_BURST_SIZE)
> +
>  static void qce_dma_release(void *data)
>  {
>  	struct qce_dma_data *dma = data;

While reviewing this code, I noticed a pre-existing regression in
qce_dma_release().

When the driver is unbound, devres cleanup calls qce_dma_release(), which
drops the DMA channel references via dma_release_channel() and then frees
dma->result_buf.

Since dma_release_channel() does not stop active DMA transfers, could the BAM
DMA engine continue to write data to the newly freed result_buf if a crypto
request is in-flight?

Would this lead to a use-after-free and memory corruption?

Should the driver explicitly call dmaengine_terminate_sync() before releasing
the channels?

[ ... ]

I also noticed another pre-existing regression in devm_qce_register_algs()
regarding the error rollback loop.

When ops->register_algs(qce) fails at index i, the rollback loop correctly
counts backward with j = i - 1, but it appears to fail to update the ops
pointer to qce_ops[j].

Does this result in repeatedly calling unregister_algs() for the failed
qce_ops[i] instead of the successfully registered algorithms?

If probe fails and devres frees the device memory, is it possible these
leaked algorithms pointing to freed memory cause a use-after-free when
accessed by the crypto subsystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom-qce-cmd-descr-v17-0-53a595414b79@oss.qualcomm.com?part=9