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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect devres registration order allows DMA callbacks to schedule work after it has been cancelled, causing a use-after-destroy and use-after-free on driver unbind.
--

commit 0da71e23ef53eb17f76a32aa2d25f7d24ce0dd3a
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

crypto: qce - Cancel work on device detach

This commit adds a devres action to cancel done_work on device removal or
probe error, ensuring the workqueue is properly cleaned up.

Fixes: eb7986e5e14d ("crypto: qce - convert tasklet to workqueue")
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 b966f3365b7de..e82fc862c74b2 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
[ ... ]
> @@ -240,6 +247,11 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	INIT_WORK(&qce->done_work, qce_req_done_work);
> +
> +	ret = devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work);
> +	if (ret)
> +		return ret;
> +

Since devres cleanup actions execute in reverse order of their registration,
won't qce_cancel_work() be executed before the DMA channels are terminated?

If an active hardware DMA transfer completes immediately after
cancel_work_sync() returns, the DMA completion interrupt could fire. Could
this invoke the DMA callback and schedule_work(&qce->done_work), re-queuing
the work that was just cancelled?

Because the devres teardown will then proceed to destroy the mutex, release
the DMA channels, and free the qce structure, won't this result in a
use-after-free when qce_req_done_work() eventually executes?

Would it be safer to register the cancel action before requesting the DMA
channels via devm_qce_dma_request(), ensuring that the source of
asynchronous events is disabled before the background work is cancelled?

>  	crypto_init_queue(&qce->queue, QCE_QUEUE_LENGTH);
>  
>  	qce->async_req_enqueue = qce_async_request_enqueue;

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