Re: [PATCH v23 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:

New issues:
- [High] Incorrect devres registration order causes Use-After-Free and Use-After-Destroy vulnerabilities during driver teardown.

Pre-existing issues:
- [High] The driver acquires a sleeping lock (`mutex`) in a crypto enqueue path that can be called from atomic contexts.
--

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

crypto: qce - Cancel work on device detach

The patch adds a devres action to cancel the done_work workqueue on device
detach or probe error. It moves the work initialization earlier and ensures
DMA transfers are terminated during DMA release.

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);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
Will registering the devres action for work cancellation here cause a
use-after-free and use-after-destroy during driver teardown?

Because devres actions are executed in reverse order (LIFO), registering
the work cancellation before DMA and mutex initialization means the work
will be cancelled last during unbind or probe failure.

The teardown sequence would be:
1. mutex_destroy(&qce->lock)
2. qce_dma_release() (terminates DMA, releases channels, frees dma->result_buf)
3. qce_cancel_work()

If qce_req_done_work() is already scheduled or executing concurrently,
it will attempt to acquire the destroyed qce->lock, then dequeue
pending requests and submit new operations using the freed DMA channels
and result_buf.

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

[Severity: High]
This is a pre-existing issue, but could qce_async_request_enqueue() trigger
a kernel warning by acquiring a sleeping lock in an atomic context?

The crypto API allows asynchronous algorithms to have their enqueue functions
invoked from atomic contexts, such as a softirq. However,
qce_async_request_enqueue() calls qce_handle_queue(), which attempts to
acquire a mutex:

drivers/crypto/qce/core.c:qce_handle_queue() {
	...
	scoped_guard(mutex, &qce->lock) {
		...
	}
	...
}

Can this result in a "BUG: sleeping function called from invalid context"
when processing network traffic like IPsec in softirq?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-qcom-qce-cmd-descr-v23-0-1be02deffda9@oss.qualcomm.com?part=7