Re: [PATCH v21 12/14] crypto: qce - Map crypto memory for DMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Devres teardown ordering leads to Use-After-Free of DMA channels.
--

--- Patch [12]: [PATCH v21 12/14] crypto: qce - Map crypto memory for DMA ---
commit b1c15310873afc3b915460ae4afcab5b35bb8ea3
Author: Bartosz Golaszewski <brgl@kernel.org>

crypto: qce - Map crypto memory for DMA

As the first step in converting the driver to using DMA for register
I/O, let's map the crypto memory range.

> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index aa4a0b1774908..4031b4516d651 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -254,6 +263,18 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  	qce->async_req_enqueue = qce_async_request_enqueue;
>  	qce->async_req_done = qce_async_request_done;
>  
> +	qce->dma_size = resource_size(res);
> +	qce->base_dma = dma_map_resource(dev, res->start, qce->dma_size,
> +					 DMA_BIDIRECTIONAL, 0);
> +	qce->base_phys = res->start;
> +	ret = dma_mapping_error(dev, qce->base_dma);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(qce->dev, qce_crypto_unmap_dma, qce);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
This is a pre-existing issue, but looking at qce_crypto_probe() earlier in
the function, could devres teardown ordering lead to a use-after-free of
the DMA channels?

Currently, workqueue cancellation is registered before the DMA channels are
requested:

drivers/crypto/qce/core.c:qce_crypto_probe() {
    ...
	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);
	if (ret)
		return ret;
    ...
}

Because devres actions execute in LIFO order during device unbind or late
probe failure, the devm-managed DMA channels will be freed before
qce_cancel_work() runs.

If done_work is scheduled but executes during or after the DMA channels are
freed, it will dequeue the next request and attempt to map and submit it
using the now-freed txchan and rxchan.

Should the workqueue cancellation be registered after the DMA channels are
requested to ensure any pending work is stopped before the channels are
released?

>  	return devm_qce_register_algs(qce);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-qce-cmd-descr-v21-0-bc2583e18475@oss.qualcomm.com?part=12