Re: [PATCH v18 11/14] crypto: qce - Use existing devres APIs in devm_qce_dma_request()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The asynchronous workqueue `done_work` is canceled after the DMA resources (`txchan`, `rxchan`, `result_buf`) are released during driver unbind, leading to a Use-After-Free (UAF).
--

commit 9b35f36899d792c9a860c0ba48fbc33cb494e4e0
Author: Bartosz Golaszewski <brgl@kernel.org>

crypto: qce - Use existing devres APIs in devm_qce_dma_request()

This patch switches devm_qce_dma_request() to use devm_kmalloc() and
devm_dma_request_chan(). This removes the need for custom resource release
handling and shrinks the function.

> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index c29b0abe94453..3db46fc0c419a 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c
> @@ -12,47 +12,26 @@
[ ... ]
>  int devm_qce_dma_request(struct qce_device *qce)
>  {
>  	struct qce_dma_data *dma = &qce->dma;
>  	struct device *dev = qce->dev;
>  
> -	dma->txchan = dma_request_chan(dev, "tx");
> +	dma->result_buf = devm_kmalloc(dev, QCE_RESULT_BUF_SZ + QCE_IGNORE_BUF_SZ, GFP_KERNEL);
> +	if (!dma->result_buf)
> +		return -ENOMEM;
> +
> +	dma->txchan = devm_dma_request_chan(dev, "tx");
>  	if (IS_ERR(dma->txchan))
>  		return dev_err_probe(dev, PTR_ERR(dma->txchan),
>  				     "Failed to get TX DMA channel\n");
>  
> -	dma->rxchan = dma_request_chan(dev, "rx");
> -	if (IS_ERR(dma->rxchan)) {
[ ... ]
> +	dma->rxchan = devm_dma_request_chan(dev, "rx");
> +	if (IS_ERR(dma->rxchan))
> +		return dev_err_probe(dev, PTR_ERR(dma->rxchan),
> +				     "Failed to get RX DMA channel\n");
>  
> -error_nomem:
> -	dma_release_channel(dma->rxchan);
> -error_rx:
> -	dma_release_channel(dma->txchan);
> -	return ret;
> +	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but could this devm transition expose a
use-after-free during driver unbind?

In qce_crypto_probe(), a devres action is added to cancel the done_work task
before devm_qce_dma_request() is called. Because the devres framework executes
cleanup actions in reverse order of registration (LIFO), during unbind or probe
failure, devres will first free these managed DMA resources (result_buf,
txchan, and rxchan) before calling cancel_work_sync() for done_work.

If done_work is pending or executing when the device is unbound via sysfs,
qce_req_done_work() can call qce_handle_queue(), which dequeues backlog
requests and submits them using the already freed result_buf and released DMA
channels.

Would it be safer to register the devres action for qce_cancel_work after
the DMA resources are allocated to ensure safe cleanup ordering?

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