Re: [PATCH v2 2/2] i2c: qcom-geni: Add support for I2C High-Speed mode

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Double free and double unmap of the `rx_buf` bounce buffer during multi-message GPI transfers.
--

commit 86b736c2ca3097acfcf4a35074f23d04a9c9f622
Author: Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>

i2c: qcom-geni: Add support for I2C High-Speed mode

Implement I2C High-Speed (HS) mode for the Qualcomm GENI I2C controller,
detecting requests based on 3.4 MHz frequency and configuring timing
parameters accordingly. The patch uses HS-specific opcodes and updates
the GPI DMA configuration with a CONFIG1 TRE to pass timing parameters.

> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
[ ... ]
> @@ -883,14 +940,25 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
[ ... ]
>  		if (msgs[i].flags & I2C_M_RD) {
> -			ret =  geni_i2c_gpi(gi2c, msgs, &config,
> -					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
> +			ret =  geni_i2c_gpi(gi2c, msgs, &config, &rx_addr, &rx_buf,
> +					    gi2c->is_hs_mode ? I2C_HS_READ : I2C_READ, gi2c->rx_c);

[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for a
multi-message transfer to double-free the rx_buf bounce buffer here?

The rx_buf pointer is initialized to NULL outside the loop at the start
of geni_i2c_gpi_xfer().

If a sequence consists of a read message followed by a write message,
iteration 0 (read) will populate rx_buf via geni_i2c_gpi() and then unmap
and free it at the end of the loop using geni_i2c_gpi_unmap().

In iteration 1 (write), this read block is skipped, meaning rx_buf retains
the freed pointer from the previous iteration.

At the end of iteration 1, geni_i2c_gpi_unmap() is called again:

		if (!gi2c->is_tx_multi_desc_xfer)
			geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);

Since rx_buf still contains the pointer from iteration 0, will this result
in a double dma_unmap_single() and double kfree() for the same address?

>  			if (ret)
>  				goto err;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-i2c-hs-v2-0-b0901791e244@oss.qualcomm.com?part=2