Re: [PATCH v3 5/5] iio: light: stk3310: support the Sensortek STK36C61

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Possible Use-After-Free of IIO event interface (ev_int) during probe failure due to asymmetric IRQ lifecycle.
--

commit b4e1397431b4f12cb9a540f222673e8512090f40
Author: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
Subject: iio: light: stk3310: support the Sensortek STK36C61

The Sensortek STK36C61 is a 3-in-1 ambient light / proximity / RGB
colour sensor. This patch adds its chip ID to the known-ID list and
device table entries, exposing the colour channels alongside ALS and
proximity.

> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c
> index f5782a1e19bbc..0a16e326e9390 100644
> --- a/drivers/iio/light/stk3310.c
> +++ b/drivers/iio/light/stk3310.c
[ ... ]
> @@ -866,6 +866,7 @@ static const struct i2c_device_id stk3310_i2c_id[] = {
>  	{ .name = "stk3310", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
>  	{ .name = "stk3311", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
>  	{ .name = "stk3335", .driver_data = (kernel_ulong_t)&stk3310_chip_info },
> +	{ .name = "stk36c61", .driver_data = (kernel_ulong_t)&stk36c61_chip_info },

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the
pre-existing IRQ setup in stk3310_probe() (drivers/iio/light/stk3310.c,
around line 766) to trigger a use-after-free if probe fails?

Looking at stk3310_probe(), the IRQ handler is registered before the IIO
device:

	if (client->irq > 0) {
		ret = devm_request_threaded_irq(&client->dev, client->irq,
						stk3310_irq_handler,
						stk3310_irq_event_handler,
						IRQF_TRIGGER_FALLING |
						IRQF_ONESHOT,
						"stk3310_event", indio_dev);
		if (ret)
			goto err_standby;
	}

	ret = iio_device_register(indio_dev);
	if (ret < 0) {
		dev_err(&client->dev, "device_register failed\n");
		goto err_standby;
	}

If iio_device_register() fails late in its execution, it cleans up its
partial state and frees the ev_int structure via kfree(). Because the IRQ
is managed by devres, it remains active until stk3310_probe() returns and
the devres unwinding phase disables it.

If a hardware interrupt fires during this window, stk3310_irq_event_handler()
(around line 714) will execute:

	event = IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1,
				     IIO_EV_TYPE_THRESH,
				     (dir ? IIO_EV_DIR_FALLING :
					    IIO_EV_DIR_RISING));
	iio_push_event(indio_dev, event, data->timestamp);

Can iio_push_event() dereference the freed ev_int structure here since there
is no synchronization with the iio_device_unregister_eventset() cleanup?

>  	{ }
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901005114.203062-1-jorijnvdgraaf@catcrafts.net?part=5