Re: [PATCH v7 10/11] iio: adc: hx711: pass iio_chan_spec to hx711_reset_read

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The triggered buffer interrupt handler (`hx711_trigger`) erroneously processes the IIO soft timestamp channel as a physical data channel, causing an out-of-bounds array write and unintended hardware state mutations.
--

commit 44e12b9836d72a45314f8d5048344b3fe6398fb7
Author: Piyush Patle <piyushpatle228@gmail.com>

iio: adc: hx711: pass iio_chan_spec to hx711_reset_read

Change hx711_reset_read() to accept the channel descriptor directly and
split out the channel-selection work into a small helper.

> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index b4cdd1dc98518..a0e73aa475de0 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c

[ ... ]

> @@ -390,8 +405,8 @@ static irqreturn_t hx711_trigger(int irq, void *p)
>  	memset(&hx711_data->buffer, 0, sizeof(hx711_data->buffer));
>  
>  	iio_for_each_active_channel(indio_dev, i) {
> -		hx711_data->buffer.channel[j] = hx711_reset_read(hx711_data,
> -					indio_dev->channels[i].channel);
> +		hx711_data->buffer.channel[j] =
> +			hx711_reset_read(hx711_data, &indio_dev->channels[i]);

When the IIO soft timestamp channel is enabled, does this loop also process
it as a physical data channel?

Since iio_for_each_active_channel() iterates over all set bits in the
active_scan_mask, it includes the timestamp channel. For the timestamp
channel, the .channel value is typically -1.

Does passing the timestamp channel to hx711_reset_read() cause it to fall
back to the channel B configuration, resulting in an unintended hardware
state change and a slow physical read?

Additionally, if all physical channels and the timestamp are active, does
writing to hx711_data->buffer.channel[j] cause an out-of-bounds array write?
The buffer.channel array only has room for the physical channels:

struct {
        u32 channel[2];
        aligned_s64 timestamp;
} buffer;

While the out-of-bounds write happens to land in the adjacent timestamp
struct member, this will trigger undefined behavior and can cause kernel
panics when compiled with CONFIG_UBSAN_BOUNDS or CONFIG_FORTIFY_SOURCE.

Could this be fixed by explicitly skipping the timestamp channel in the
loop, such as checking if indio_dev->channels[i].type == IIO_TIMESTAMP?

>  		j++;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510194947.31997-1-piyushpatle228@gmail.com?part=10