Re: [PATCH v21 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] The error path in `qce_submit_cmd_desc()` attempts to free a non-reusable DMA descriptor using `dmaengine_desc_free()`, which will fail and leak the descriptor.
--

--- Patch [14]: [PATCH v21 14/14] crypto: qce - Communicate the base physical address to the dmaengine ---
commit abb736f89a48907c45dba4ccf37ffa56752bda22
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

This commit attaches metadata containing the scratchpad address and direction
to the BAM DMA descriptor. It also adds an error handling path to free the
descriptor if metadata attachment or 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 leak the DMA descriptor?

Looking at dmaengine_desc_free() in include/linux/dmaengine.h, it checks if
the descriptor is reusable and returns -EPERM if it is not:

static inline int dmaengine_desc_free(struct dma_async_tx_descriptor *desc)
{
        if (!dmaengine_desc_test_reuse(desc))
                return -EPERM;

        return desc->desc_free(desc);
}

Since the descriptor allocated earlier in qce_submit_cmd_desc() by
dmaengine_prep_slave_sg() is a standard descriptor and not marked as
reusable, it appears dmaengine_desc_free() will fail without actually
freeing the memory if either dmaengine_desc_attach_metadata() or
dma_submit_error() triggers the goto.

Is there a different method intended for cleaning up non-reusable
descriptors in this error path, or should the descriptor be allowed to
complete with a failure instead?

>  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/20260713-qcom-qce-cmd-descr-v21-0-bc2583e18475@oss.qualcomm.com?part=14