Re: [PATCH 1/2] dmaengine: qcom-gpi: Add I2C High-Speed mode configuration support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Heap Buffer Overflow and DMA Address Corruption in I2C High-Speed mode due to insufficient MAX_TRE
--

--- Patch [1]: [PATCH 1/2] dmaengine: qcom-gpi: Add I2C High-Speed mode configuration support ---
commit 0c2b77de6fa90756115b4394bd0db3e62bed9d2a
Author: Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>

dmaengine: qcom-gpi: Add I2C High-Speed mode configuration support

Add support in the Qualcomm GPI DMA engine to configure I2C High-Speed (HS)
mode transfers. This introduces support for the CONFIG1 Transfer Ring Element
(TRE) to convey HS-specific timing parameters.

> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
[ ... ]
> @@ -1631,7 +1637,7 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
>  	struct gpi_tre *tre;
>  	unsigned int i;
>  
> -	/* first create config tre if applicable */
> +	/* first create config0 tre if applicable */
>  	if (i2c->set_config) {
>  		tre = &desc->tre[tre_idx];
>  		tre_idx++;
[ ... ]
> +	/* Create CONFIG1 TRE if requested */
> +	if (i2c->set_config1) {
> +		tre = &desc->tre[tre_idx];
> +		tre_idx++;
[ ... ]
>  	/* create the GO tre for Tx */
> -	if (i2c->op == I2C_WRITE) {
> +	if (i2c->op == I2C_WRITE || i2c->op == I2C_HS_WRITE) {
[ ... ]
>  		tre = &desc->tre[tre_idx];
>  		tre_idx++;
[ ... ]
> -	if (i2c->op == I2C_READ || i2c->multi_msg == false) {
> +	if (i2c->op == I2C_READ || i2c->op == I2C_HS_READ || i2c->multi_msg == false) {
>  		/* create the DMA TRE */
>  		tre = &desc->tre[tre_idx];
>  		tre_idx++;
>
>  		address = sg_dma_address(sgl);
>  		tre->dword[0] = lower_32_bits(address);

[Severity: High]
When I2C High-Speed mode is used with both set_config and set_config1,
can this cause an out-of-bounds write to the tre array?

Looking at struct gpi_desc, the tre array is statically sized by MAX_TRE,
which is defined as 3:

#define MAX_TRE 3

struct gpi_desc {
        ...
        struct gpi_tre tre[MAX_TRE];
        u32 num_tre;
};

If we generate CONFIG0, CONFIG1, GO, and DMA TREs, tre_idx will reach 3
here. Writing to desc->tre[3] would overflow the array and overwrite the
adjacent num_tre field.

Later, gpi_prep_slave_sg() sets gpi_desc->num_tre = 4, which would
overwrite the first 4 bytes of the newly written DMA TRE, potentially
causing an IOMMU fault or bus error when the hardware attempts the transfer.

Should MAX_TRE be increased to accommodate the new CONFIG1 TRE?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-i2c-hs-v1-0-8dd94e5c6113@oss.qualcomm.com?part=1