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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The I2C read transaction uses overlapping transmit and receive buffers, causing a DMA mapping violation and potential cacheline corruption.
- [High] Missing NULL check for `chip_info` during driver probe when bound via sysfs.
--

--- Patch [1]: [PATCH v7 1/7] iio: dac: ad5696: properly check i2c_transfer() return value ---
commit 7918e0fc4a2bf3dec679b7e0bd608081aff01ef3
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.

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..6f726e6301a6e 100644
> --- a/drivers/iio/dac/ad5696-i2c.c
> +++ b/drivers/iio/dac/ad5696-i2c.c
> @@ -40,8 +40,8 @@ static int ad5686_i2c_read(struct ad5686_state *st, u8 addr)
>  				      0x00);
>  
>  	ret = i2c_transfer(i2c->adapter, msg, 2);
> -	if (ret < 0)
> -		return ret;
> +	if (ret != 2)
> +		return ret < 0 ? ret : -EIO;

[Severity: High]
This is a pre-existing issue, but does this I2C read transaction use
overlapping transmit and receive buffers?

Looking at the msg array initialization earlier in ad5686_i2c_read():

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

The transmit buffer and the receive buffer are mapped to the same
st->data[0] union, meaning they overlap and share the same cacheline.

When the I2C controller uses non-cache-coherent DMA, mapping overlapping
buffers for both DMA_TO_DEVICE and DMA_FROM_DEVICE within the same
cacheline violates the DMA API.

Could the cache invalidation for the receive buffer clobber the dirty
cacheline containing the transmit buffer's data, leading to corrupted data
being sent to the DAC or corrupted readback?

>  	return be16_to_cpu(st->data[0].d16);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing NULL
check for chip_info during driver probe when bound via sysfs?

In drivers/iio/dac/ad5696-i2c.c:ad5686_i2c_probe():

    return ad5686_probe(&i2c->dev, i2c_get_match_data(i2c), ...);

If a user binds the driver using driver_override via sysfs with a device
name not present in the driver's id_table or of_match_table,
i2c_get_match_data() will return NULL.

This NULL pointer is then passed to ad5686_probe() in
drivers/iio/dac/ad5686.c:

    st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;

Will this unconditionally dereference chip_info and cause a kernel panic
during device probe?

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