Re: [PATCH v7] dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The QPIC NAND driver explicitly restricts the coherent DMA mask to 32 bits but fails to restrict the streaming DMA mask, allowing streaming mappings to return addresses >4GB which are then silently truncated to 32 bits, causing DMA memory corruption.
--

commit d15e0af034d65bcfce2ab8e3f8608f0913f11a7c
Author: Md Sadre Alam <md.alam@oss.qualcomm.com>

dmaengine: qcom: bam_dma: Fix command element mask field for BAM v1.6.0+

This commit updates the BAM DMA driver to correctly handle the command
element mask field for BAM v1.6.0 and later. For read commands, the mask is
now set to 0 to support 32-bit addressing, while write commands retain the
0xffffffff mask.

> diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h
> index 68fc0e643b1b9..d9d07a9ab3132 100644
> --- a/include/linux/dma/qcom_bam_dma.h
> +++ b/include/linux/dma/qcom_bam_dma.h
[ ... ]
> @@ -50,7 +57,11 @@ bam_prep_ce_le32(struct bam_cmd_element *bam_ce, u32 addr,
>  	bam_ce->cmd_and_addr =
>  		cpu_to_le32((addr & 0xffffff) | ((cmd & 0xff) << 24));
>  	bam_ce->data = data;
> -	bam_ce->mask = cpu_to_le32(0xffffffff);
> +	if (cmd == BAM_READ_COMMAND)
> +		bam_ce->mask = cpu_to_le32(0x0); /* 32-bit addressing */

[Severity: Critical]
This is a pre-existing issue, but does hardcoding the upper address bits
to zero here cement a silent truncation bug caused by the QPIC NAND driver?

In drivers/mtd/nand/qpic_common.c:qcom_nandc_alloc(), the driver sets the
coherent DMA mask to 32 bits, but does not restrict the streaming DMA mask:

	ret = dma_set_coherent_mask(nandc->dev, DMA_BIT_MASK(32));
	if (ret) {
		dev_err(nandc->dev, "failed to set DMA mask\n");
		return ret;
	}

On modern 64-bit systems, dma_map_single() can map streaming buffers to
physical addresses above 4GB. When qcom_prep_bam_dma_desc_cmd() prepares the
BAM descriptor:

	if (read)
		bam_prep_ce(&bam_ce_buffer[i],
			    offset, BAM_READ_COMMAND,
			    reg_buf_dma_addr(nandc, (__le32 *)vaddr + i));

The potentially >4GB physical address is passed as the 'data' argument to
bam_prep_ce(). However, 'data' is strongly typed as u32, silently dropping
the upper 32 bits in C:

static inline void
bam_prep_ce(struct bam_cmd_element *bam_ce, u32 addr,
	    enum bam_command_type cmd, u32 data)

With this patch explicitly hardcoding bam_ce->mask to 0 (upper 4 bits), the
BAM hardware will unconditionally execute the DMA write at the incorrect,
truncated 32-bit physical address. Could this lead to silent memory
corruption instead of raising a fault when a buffer resides above 4GB?

Should the QPIC NAND driver be updated to use dma_set_mask_and_coherent()
to properly restrict both masks and ensure the use of DMA bounce buffers?

> +	else
> +		bam_ce->mask = cpu_to_le32(0xffffffff); /* Write mask */
> +	bam_ce->reserved = 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615060908.1263171-1-varadarajan.narayanan@oss.qualcomm.com?part=1