Re: [PATCH v4 4/4] iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] iio: light: ltr501: wrong channel lookup in
  ltr501_trigger_handler()
- [High] iio: light: ltr501: out-of-bounds write to als_gain_tbl in
  ltr501_probe()
--

commit 5a33f9170cb34d957e4ef6fd2754b95efed27186
Author: Svyatoslav Ryhel <clamor95@gmail.com>

iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

This commit adds support for the Lite-On LTR-659PS-01 proximity-only sensor
by introducing a no_als_support flag to skip ALS register accesses and
updating ltr501_trigger_handler() to check channel types when building the
data-ready mask.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index bf3cc32a55c3c..394e08c78005e 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
[ ... ]
> @@ -1251,16 +1286,21 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)
>  		aligned_s64 ts;
>  	} scan = { };
>  	__le16 als_buf[2];
> +	unsigned int chan_idx;
>  	u8 mask = 0;
>  	int j = 0;
>  	int ret, psdata;
>  
>  	/* figure out which data needs to be ready */
> -	if (test_bit(0, indio_dev->active_scan_mask) ||
> -	    test_bit(1, indio_dev->active_scan_mask))
> -		mask |= LTR501_STATUS_ALS_RDY;
> -	if (test_bit(2, indio_dev->active_scan_mask))
> -		mask |= LTR501_STATUS_PS_RDY;
> +	iio_for_each_active_channel(indio_dev, chan_idx) {
> +		const struct iio_chan_spec *chan = &indio_dev->channels[chan_idx];
> +
> +		if (chan->type == IIO_INTENSITY)
> +			mask |= LTR501_STATUS_ALS_RDY;
> +
> +		if (chan->type == IIO_PROXIMITY)
> +			mask |= LTR501_STATUS_PS_RDY;
> +	}

[Severity: High]
Does indexing indio_dev->channels[] with chan_idx look up the wrong channel
on chips that include LTR501_LIGHT_CHANNEL()?

iio_for_each_active_channel() iterates over set bit positions in
active_scan_mask, which are scan_index values rather than array indices into
indio_dev->channels[].  In ltr501_channels[] and ltr301_channels[],
channels[0] is LTR501_LIGHT_CHANNEL() with scan_index = -1, shifting all
buffered channels by one position:

drivers/iio/light/ltr501.c:ltr501_channels[] {
	LTR501_LIGHT_CHANNEL(),
	LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH,
				 ltr501_als_event_spec,
				 ARRAY_SIZE(ltr501_als_event_spec)),
	LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR, NULL, 0),
	{
		.type = IIO_PROXIMITY,
    ...
		.scan_index = 2,
    ...
}

On LTR-501 and LTR-559, enabling proximity (scan_index 2) inspects
channels[2] (IIO_INTENSITY) instead of channels[3] (IIO_PROXIMITY), so
LTR501_STATUS_PS_RDY is never set in mask, LTR501_PS_DATA is never read, and
buffered proximity capture always pushes zeroed data.  Likewise, enabling
only scan_index 0 inspects channels[0] (IIO_LIGHT), leaving mask as 0 and
pushing zeroed ALS samples without reading the sensor.

[ ... ]
> @@ -1580,6 +1624,7 @@ static const struct i2c_device_id ltr501_id[] = {
>  	{ .name = "ltr329", .driver_data = (kernel_ulong_t)&ltr329_chip_info_tbl },
>  	{ .name = "ltr501", .driver_data = (kernel_ulong_t)&ltr501_chip_info_tbl },
>  	{ .name = "ltr559", .driver_data = (kernel_ulong_t)&ltr559_chip_info_tbl },
> +	{ .name = "ltr659", .driver_data = (kernel_ulong_t)&ltr659_chip_info_tbl },
>  	{ }
>  };

[Severity: High]
In ltr501_probe(), introduced earlier in this series by commit "iio: light:
ltr501: Switch to read_avail() op from manual attribute registration", does
the gain table initialization overflow data->als_gain_tbl[]?

drivers/iio/light/ltr501.c:ltr501_probe() {
    ...
	for (int i = 0; i < data->chip_info->als_gain_tbl_size; i++) {
		if (data->chip_info->als_gain[i].scale == LTR501_RESERVED_GAIN)
			continue;

		data->als_gain_tbl[j++].scale = data->chip_info->als_gain[i].scale;
		data->als_gain_tbl[j++].uscale = data->chip_info->als_gain[i].uscale;
	}
	data->als_gain_tbl_len = j;
    ...
}

Because j is post-incremented on both assignments, each iteration advances j
by two elements instead of one, leaving half of each entry zeroed.  When
probing LTR-303, LTR-329, or LTR-559, which have 6 non-reserved entries in
ltr559_als_gain_tbl, j reaches 12 and writes past the 8-element
data->als_gain_tbl[] array at the end of struct ltr501_data, corrupting up
to 32 bytes of adjacent kernel heap memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115083741.62140-1-clamor95@gmail.com?part=4