Re: [PATCH v23 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 2 potential issue(s) to consider:
- [Medium] Unintended hardware misconfiguration due to partially initialized `dma_slave_config`.
- [Medium] Potential memory leak in qce_submit_cmd_desc() error path because dmaengine_desc_free() refuses to free non-reusable descriptors.
--

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

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

This patch configures the BAM DMA engine with the base physical address
to be used as a scratchpad for dummy writes related to BAM pipe locking.
It passes the peripheral configuration structure to the DMA driver via
dmaengine_slave_config().

> diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
> index 88d253d9147cf..09699d96575b1 100644
> --- a/drivers/crypto/qce/dma.c
> +++ b/drivers/crypto/qce/dma.c
> @@ -116,8 +117,16 @@ static void qce_dma_terminate(void *data)
>  
>  int devm_qce_dma_request(struct qce_device *qce)
>  {
> +	struct bam_config peripheral_cfg = {
> +		.lock_scratchpad_addr = (u32)qce->base_phys + REG_VERSION,
> +	};
> +	struct dma_slave_config cfg = {
> +		.direction = DMA_MEM_TO_DEV,
> +		.peripheral_config = &peripheral_cfg,
> +	};

[Severity: Medium]
Does this partially initialized dma_slave_config cause an unintended
hardware misconfiguration?

By only setting the direction and peripheral_config, the designated
initializer silently zeroes out src_maxburst and dst_maxburst.

When dmaengine_slave_config() is called, the BAM DMA driver copies this
configuration. Later, if the engine is not remotely controlled, it will
write this zeroed maxburst value to the hardware threshold register:

drivers/dma/qcom/bam_dma.c:bam_apply_new_config() {
    ...
    if (dir == DMA_DEV_TO_MEM)
        maxburst = bchan->slave.src_maxburst;
    else
        maxburst = bchan->slave.dst_maxburst;

    writel_relaxed(maxburst,
               bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
    ...
}

This alters the hardware configuration from its default (4) and might
break DMA interrupt generation or cause an interrupt storm on systems
where the BAM is natively configured.

[ ... ]

Also, there is a second issue I'd like to ask about regarding
qce_submit_cmd_desc():

[Severity: Medium]
Can this error path leak the DMA descriptor?

In qce_submit_cmd_desc(), the code calls dmaengine_prep_slave_sg() with
attrs = DMA_PREP_CMD but lacks the DMA_CTRL_REUSE flag. If
dma_submit_error() detects an error, it branches to err_free_desc:

drivers/crypto/qce/dma.c:qce_submit_cmd_desc() {
    ...
err_free_desc:
    dmaengine_desc_free(dma_desc);
    ...
}

Because the descriptor was not prepared with DMA_CTRL_REUSE,
dmaengine_desc_free() will immediately return -EPERM without freeing the
descriptor.

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