Re: [PATCH v4 6/7] iio: light: ltr501: Add ltr329 driver support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized kernel stack memory leak and functional corruption on big-endian systems due to regmap_bulk_read/write type mismatch.
--

--- Patch [6]: [PATCH v4 6/7] iio: light: ltr501: Add ltr329 driver support ---
commit 1db9cef73562002d6e7e4eeeed1f2b197809ecbf
Author: Esben Haabendal <esben@geanix.com>

iio: light: ltr501: Add ltr329 driver support

This adds support for the LTR-329ALS-01 chip, which is similar to the
LTR-303ALS-01, except it does not have interrupt support.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index b36bd10a02624..cd5ff00ef5e26 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c

[ ... ]

> @@ -1257,6 +1271,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr301_channels,
>  		.no_channels = ARRAY_SIZE(ltr301_channels),
>  	},
> +	[ltr329] = {
> +		.partid = 0x0A,
> +		.als_gain = ltr559_als_gain_tbl,
> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> +		.als_mode_active = BIT(0),
> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> +		.als_gain_shift = 2,
> +		.no_irq_support = true,
> +		.info_no_irq = &ltr301_info_no_irq,
> +		.channels = ltr301_channels,
> +		.no_channels = ARRAY_SIZE(ltr301_channels),
> +	},
>  };

[Severity: High]
This is a pre-existing issue, but will this new ltr329 chip also be affected
by the big-endian stack leak in the driver's bulk read and write operations?

Looking at ltr501_trigger_handler() (and similarly in ltr501_read_thresh()
and ltr501_write_thresh()), the code uses regmap_bulk_read and 
regmap_bulk_write to read or write 2 bytes into a 4-byte integer variable:

drivers/iio/light/ltr501.c:ltr501_trigger_handler() {
    ...
    int psdata;
    ...
    ret = regmap_bulk_read(data->regmap, LTR501_PS_DATA, &psdata, 2);
    ...
    scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;
    ...
}

On big-endian systems, wouldn't reading 2 bytes into the address of psdata
only populate the upper half of the 32-bit integer, leaving the lower half
uninitialized? 

Since the lower half contains uninitialized kernel stack memory,
doesn't masking and storing this in the IIO scan buffer leak stack memory to
userspace?

Similarly, wouldn't ltr501_write_thresh() corrupt the threshold configuration
by writing 2 bytes from the lowest address of val (which are 0x00 on
big-endian)?

Could we fix this driver-wide issue by using 16-bit variables (like __le16)
for these 2-byte regmap_bulk_read and regmap_bulk_write operations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-liteon-ltr329-v4-0-8bd3dfd15280@geanix.com?part=6