Re: [PATCH v21 07/14] crypto: qce - Cancel work on device detach

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Devres ordering introduces Use-After-Free and Mutex Use-After-Destroy vulnerabilities because the workqueue is canceled after its required resources (mutex and DMA channels) are freed.
- [High] Terminating DMA and cancelling the workqueue during unbind without explicitly completing pending crypto requests causes callers to hang indefinitely.
--

--- Patch [7]: [PATCH v21 07/14] crypto: qce - Cancel work on device detach ---
commit 4cbda8145d4c325fc75ff4841164bf36bd06014c
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

crypto: qce - Cancel work on device detach

This commit introduces a devres action to clean up the workqueue on
device detach. It moves the initialization earlier to ensure outstanding
DMA transfers are terminated before the work is canceled.

Closes: https://sashiko.dev/#/patchset/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc%40oss.qualcomm.com?part=7

> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index ac74f69914d61..b52a26ffff5ee 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
[ ... ]
> @@ -226,6 +233,11 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	INIT_WORK(&qce->done_work, qce_req_done_work);
> +	ret = devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work);

[Severity: High]
Could this devres ordering introduce a use-after-free and mutex
use-after-destroy during driver unbind?

Because devres cleanup executes in LIFO order, registering qce_cancel_work()
here before mutex and DMA initialization means it will execute last during
cleanup. 

When the driver unbinds, qce_dma_release() and mutex destruction will occur
before cancel_work_sync() runs. If the worker thread qce_req_done_work() is
scheduled concurrently, could it attempt to lock the destroyed mutex and use
the already freed DMA channels and result buffers?

> +	if (ret)
> +		return ret;
> +
>  	ret = devm_qce_dma_request(qce->dev, &qce->dma);
[ ... ]
> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 68cafd4741ad3..7ec9d72fd690f 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c
> @@ -13,6 +13,8 @@ static void qce_dma_release(void *data)
>  {
>  	struct qce_dma_data *dma = data;
>  
> +	dmaengine_terminate_sync(dma->txchan);
> +	dmaengine_terminate_sync(dma->rxchan);

[Severity: High]
Will terminating the DMA transfers here cause callers to hang indefinitely?

By aborting the active DMA transfer during unbind, the normal completion
callback won't fire, so qce->done_work is not scheduled. The driver then
cancels the workqueue via qce_cancel_work(), but it does not appear to
explicitly complete the active request (qce->req) or drain qce->queue.

Will callers waiting on these requests block forever when the device is
unbound?

>  	dma_release_channel(dma->txchan);
>  	dma_release_channel(dma->rxchan);
>  	kfree(dma->result_buf);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-qce-cmd-descr-v21-0-bc2583e18475@oss.qualcomm.com?part=7