Re: [PATCH v17 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] Incorrect devres registration order causes the DMA memory region to be unmapped before the workqueue is canceled and DMA channels are released, leading to an IOMMU fault or cross-device memory corruption during device unbind.
--

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

crypto: qce - Map crypto memory for DMA

This commit maps the crypto memory range using dma_map_resource and adds a
devres action to unmap it during cleanup. This is intended as the 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 90f44db660617..92e551f4570c0 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c

[ ... ]

> @@ -256,6 +265,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);

Because devres executes cleanup actions in LIFO order, will this cause
the DMA memory region to be unmapped before the workqueue is canceled and
DMA channels are released?

Looking earlier in qce_crypto_probe(), the driver requests DMA channels
with devm_qce_dma_request() and registers the workqueue cancellation
with devm_add_action_or_reset(dev, qce_cancel_work, &qce->done_work)
before getting here.

During device unbind or a late probe failure, the unwinding sequence would
execute qce_crypto_unmap_dma() first.

If a cryptographic request is still in flight during unbind, could the
DMA controller attempt to access the unmapped IOVA before the asynchronous
work is canceled, potentially resulting in an IOMMU fault or cross-device
memory corruption?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom-qce-cmd-descr-v17-0-53a595414b79@oss.qualcomm.com?part=12