Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of IIO event interface due to mixing devm_request_threaded_irq with manual iio_device_unregister
- [High] Kernel stack memory leak and logic failure on big-endian systems due to reading 2 bytes into an uninitialized 4-byte integer
--

--- Patch [3]: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support ---
commit 8ae95bcf91bc6043da3fe9505d6fab5f356128f7
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
LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
have.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7d045be78c6df..379e57ac5f5b2 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c

[ ... ]

> @@ -1531,6 +1556,12 @@ static int ltr501_probe(struct i2c_client *client)
>  		return ret;
>  
>  	if (client->irq > 0) {
> +		if (!ltr501_has_irq_support(data->chip_info)) {
> +			dev_err(&client->dev, "chip does not support irq\n");
> +			ret = -EINVAL;
> +			goto powerdown_on_error;
> +		}
> +
>  		ret = devm_request_threaded_irq(&client->dev, client->irq,

[Severity: High]
This is a pre-existing issue, but does using devres for the IRQ while
manually unregistering the IIO device cause a use-after-free?

In ltr501_remove(), iio_device_unregister() is called manually, which
frees the event interface. However, because the IRQ is allocated with
devm_request_threaded_irq(), it remains active until after
ltr501_remove() returns.

If an interrupt fires during this window, ltr501_interrupt_handler()
could call iio_push_event(), which accesses the freed event interface.


[Severity: High]
This is also a pre-existing issue, but is there a risk of leaking kernel
stack memory on big-endian systems in ltr501_trigger_handler()?

In ltr501_trigger_handler(), regmap_bulk_read() reads 2 bytes into an
uninitialized 4-byte integer:

ltr501_trigger_handler() {
    ...
    int psdata;
    ...
    ret = regmap_bulk_read(data->regmap, LTR501_PS_DATA, &psdata, 2);
    ...
}

On big-endian architectures, these 2 bytes overwrite the most
significant bytes of psdata, leaving the least significant bytes
uninitialized.

The driver then applies a mask:

    scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;

Could this extract uninitialized kernel stack data and push it to the
IIO ring buffer, exposing it to userspace?

>  						NULL, ltr501_interrupt_handler,
>  						IRQF_TRIGGER_FALLING |

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-liteon-ltr329-v2-0-d18af55edab5@geanix.com?part=3