Re: [PATCH v18 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] This problem wasn't introduced by this patch, but in the error path of `devm_qce_register_algs()`, the driver fails to properly unregister already registered algorithms and repeatedly unregisters the failed one.
--

commit 56e64a8a181facb8444e80e7d550153e25a41208
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 a0e2eadc3afd5..d7b7a3dda4649 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
[ ... ]
> @@ -255,6 +264,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;
> +
>  	return devm_qce_register_algs(qce);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but in the error path of
devm_qce_register_algs(), the driver fails to properly unregister already
registered algorithms and repeatedly unregisters the failed one.

Looking at devm_qce_register_algs() in drivers/crypto/qce/core.c:

	for (i = 0; i < ARRAY_SIZE(qce_ops); i++) {
		ops = qce_ops[i];
		ret = ops->register_algs(qce);
		if (ret) {
			for (j = i - 1; j >= 0; j--)
				ops->unregister_algs(qce);
			return ret;
		}
	}

The rollback loop iterates backwards with j, but calls ops->unregister_algs()
where ops still points to the currently failed algorithm qce_ops[i].

Should it be calling qce_ops[j]->unregister_algs(qce) instead?

As a result, previously successfully registered algorithms are leaked, and the
driver repeatedly unregisters an algorithm that either failed to register or
was never successfully registered.

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