Re: [PATCH v20 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] Incorrect devres registration order causes Use-After-Free and Use-After-Destroy during device unbind.
- [High] Canceling the workqueue without completing pending requests causes a system hang.
--

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

crypto: qce - Cancel work on device detach

The workqueue is setup in probe() but never cancelled on error or in
remove(). Set up a devres action to clean it up early, ensuring work is
not cancelled before outstanding DMA transfers terminate.

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
> @@ -186,6 +186,13 @@ static int qce_check_version(struct qce_device *qce)
>  	return 0;
>  }
>  
> +static void qce_cancel_work(void *data)
> +{
> +	struct work_struct *work = data;
> +
> +	cancel_work_sync(work);

[Severity: High]
If the work is cancelled here, does the active crypto request get leaked?

If a DMA transfer completes and schedules the work, but the device unbinds
before the work executes, cancel_work_sync() will discard the pending work
item. Since qce_cancel_work() does not implement a fallback to propagate an
error completion (like -ESHUTDOWN) to the active crypto request,
crypto_request_complete() is never called.

Will upstream callers of the crypto API waiting on the request hang
indefinitely, potentially causing a system lockup?

> +}
> +
>  static int qce_crypto_probe(struct platform_device *pdev)
>  {

[ ... ]

> @@ -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: High]
Will this devres registration order cause a use-after-free and
use-after-destroy during device unbind?

Devres executes actions in LIFO order. Because qce_cancel_work is registered
here before devm_qce_dma_request() and devm_mutex_init() (which happens later
in qce_crypto_probe), the teardown sequence will be:

1. Destroy qce->lock
2. Terminate and free DMA channels
3. Wait for the workqueue via cancel_work_sync()

If a DMA callback schedules qce->done_work just before teardown,
qce_req_done_work() could execute while cancel_work_sync() is waiting.
Does this allow the work item to attempt to lock the already destroyed
qce->lock and access the freed DMA channels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-qcom-qce-cmd-descr-v20-0-56f67da84c05@oss.qualcomm.com?part=7