Re: [PATCH v23 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:
- [High] Devres LIFO teardown ordering causes DMA channel Use-After-Free and premature IOMMU unmapping.
--

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

crypto: qce - Map crypto memory for DMA

This commit maps the crypto memory range for DMA as a first step in
converting the driver to use DMA for register I/O.

> 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);

[Severity: High]
Does this devres registration order create a teardown race condition?

Because devres actions execute in reverse (LIFO) order, registering
qce_crypto_unmap_dma here means it will execute first during driver unbind.

Earlier in qce_crypto_probe(), we register the devres to cancel done_work:

	INIT_WORK(&qce->done_work, qce_req_done_work);
	ret = devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work);

And we request the DMA channels:

	ret = devm_qce_dma_request(qce);

If the driver is unbound while crypto requests are active, the unmapping of
base_dma happens first. Then the DMA channels are released.

Finally, the done_work is cancelled.

If qce->done_work is pending and executes after the channels are released but
before cancel_work_sync(), will it access the freed DMA channels and trigger
a use-after-free, or cause BAM DMA faults because the IOMMU mapping was
already removed?

> +	if (ret)
> +		return ret;
> +
>  	return devm_qce_register_algs(qce);
>  }

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