Re: [PATCH v18 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:
- [Critical] Use-After-Free and use-after-destroy due to incorrect devres teardown order and failure to terminate DMA
--

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

crypto: qce - Cancel work on device detach

This commit attempts to clean up a previously uncancelled work struct on
device detach by setting up a devres action. It moves the initialization 
earlier in the probe routine, with the stated goal of avoiding work 
cancellation before outstanding DMA transfers are terminated.

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..f671946cf7351 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c

[ ... ]

> @@ -227,6 +234,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);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_qce_dma_request(qce->dev, &qce->dma);

[Severity: Critical]
Does this devres teardown order cause a use-after-free and use-after-destroy
on device detach?

Because devres actions are executed in LIFO order, registering
qce_cancel_work() before devm_qce_dma_request() and devm_mutex_init() means
the work will be cancelled after the DMA resources are freed and the mutex is
destroyed.

The commit message mentions moving the initialization earlier to allow
outstanding DMA transfers to terminate. However, devm_qce_dma_request()
registers qce_dma_release() which frees dma->result_buf, but it does not
explicitly terminate the DMA hardware (e.g., via dmaengine_terminate_sync()).

If a DMA transfer is active during teardown, won't the BAM DMA hardware
continue running and write into the freed dma->result_buf?

When the hardware eventually finishes, the DMA completion callback could
access the freed result buffer and schedule done_work. Since
cancel_work_sync() might have already run, this could result in
qce_req_done_work() executing with freed resources and a destroyed mutex.

Could we explicitly stop the DMA hardware first, then cancel the workqueue,
and finally free the memory and locks?

>  	if (ret)
>  		return ret;

[ ... ]

> @@ -239,7 +251,6 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	INIT_WORK(&qce->done_work, qce_req_done_work);
>  	crypto_init_queue(&qce->queue, QCE_QUEUE_LENGTH);
>  
>  	qce->async_req_enqueue = qce_async_request_enqueue;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-qcom-qce-cmd-descr-v18-0-99103926bafc@oss.qualcomm.com?part=7