Re: [PATCH v20 14/14] crypto: qce - Communicate the base physical address to the dmaengine

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Ineffective descriptor cleanup on error paths leads to a memory leak because `dmaengine_desc_free()` is called on a non-reusable descriptor.
--

commit 7c11f5b2471b4e0caed5825bea7adc1ce06d76de
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

crypto: qce - Communicate the base physical address to the dmaengine

This commit attaches metadata containing the base physical address to the
DMA descriptor for the BAM DMA engine. It also adds error handling paths
to free the descriptor and unmap the scatterlist if the metadata attachment
or the DMA submission fails.

> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 1b43c56503334..6410f8dc5bcf5 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c

[ ... ]

> @@ -60,15 +65,21 @@ int qce_submit_cmd_desc(struct qce_device *qce)
>  		goto err_unmap_sg;
>  	}
>  
> +	ret = dmaengine_desc_attach_metadata(dma_desc, &meta, sizeof(meta));
> +	if (ret)
> +		goto err_free_desc;
> +
>  	qce_desc->dma_desc = dma_desc;
>  	cookie = dmaengine_submit(qce_desc->dma_desc);
>  
>  	ret = dma_submit_error(cookie);
>  	if (ret)
> -		goto err_unmap_sg;
> +		goto err_free_desc;
>  
>  	return 0;
>  
> +err_free_desc:
> +	dmaengine_desc_free(dma_desc);

[Severity: Medium]
Does this error path successfully free the descriptor?

Earlier in qce_submit_cmd_desc(), the descriptor attributes are defined
without the reuse flag:

    unsigned long attrs = DMA_PREP_CMD;
    ...
    dma_desc = dmaengine_prep_slave_sg(chan, bam_txn->wr_sgl, mapped,
                                       DMA_MEM_TO_DEV, attrs);

Since the descriptor is not prepared with the DMA_CTRL_REUSE flag, 
dmaengine_desc_free() will return an error instead of freeing the memory:

include/linux/dmaengine.h:dmaengine_desc_free() {
	/* this is supported for reusable desc, so check that */
	if (!dmaengine_desc_test_reuse(desc))
		return -EPERM;

	return desc->desc_free(desc);
}

Could this cause a memory leak of the DMA descriptor if 
dmaengine_desc_attach_metadata() fails or dma_submit_error() returns an error?

>  err_unmap_sg:
>  	dma_unmap_sg(qce->dev, bam_txn->wr_sgl, bam_txn->wr_sgl_cnt, DMA_TO_DEVICE);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-qcom-qce-cmd-descr-v20-0-56f67da84c05@oss.qualcomm.com?part=14