Re: [PATCH v8 1/8] iio: dac: ad5696: properly check i2c_transfer() return value

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The I2C message array in `ad5686_i2c_read()` uses overlapping memory buffers for the TX and RX messages in a single I2C transaction, violating DMA API rules.
--

--- Patch [1]: [PATCH v8 1/8] iio: dac: ad5696: properly check i2c_transfer() return value ---
commit 07e6524ea7c055d37c8056da74c0ca49030fd8cc
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>

iio: dac: ad5696: properly check i2c_transfer() return value

Verify that the expected number of i2c messages were transferred when
ad5686_i2c_read() is called. This issue exists since the support for I2C
devices was first introduced.

Closes: https://lore.kernel.org/all/20260705114746.1485F1F000E9@smtp.kernel.org/

> diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
> index 279309329b64b..551b200bfe140 100644
> --- a/drivers/iio/dac/ad5696-i2c.c
> +++ b/drivers/iio/dac/ad5696-i2c.c

[ ... ]

> @@ -39,9 +40,11 @@ static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
>  				      AD5686_ADDR(addr) |
>  				      0x00);
>  
> -	ret = i2c_transfer(i2c->adapter, msg, 2);

[Severity: High]
This is a pre-existing issue, but does passing this msg array to
i2c_transfer() violate the DMA API rules?

Looking at the setup of the msg array earlier in ad5686_i2c_read():

	struct i2c_msg msg[2] = {
		{
			.len = 3,
			.buf = &st->data[0].d8[1],
		},
		{
			.flags = i2c->flags | I2C_M_RD,
			.len = 2,
			.buf = (char *)&st->data[0].d16,
		},
	};

Both the TX and RX buffers are using overlapping offsets from the same
union member st->data[0].

Since they have conflicting directions (DMA_TO_DEVICE and DMA_FROM_DEVICE),
could an I2C adapter with DMA map both simultaneously and cause memory
corruption via cache maintenance collisions, or trigger SWIOTLB bounce
buffer issues?

It appears the driver provisions st->data as an array to prevent this
(as is done in the SPI driver), but the I2C driver reused data[0] for
both messages here.

> +	ret = i2c_transfer(i2c->adapter, msg, ARRAY_SIZE(msg));
>  	if (ret < 0)
>  		return ret;
> +	if (ret != ARRAY_SIZE(msg))
> +		return -EIO;
>  
>  	return be16_to_cpu(st->data[0].d16);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-ad5686-new-features-v8-0-ebb0051af5e5@analog.com?part=1