Re: [PATCH 2/2] hwmon: (lm90) Add lock protection to lm90_alert

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Use-After-Free due to incorrect devm teardown sequence
--

commit 8b53a979a14111ccbfe05e43369fd3214a55c211
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (lm90) Add lock protection to lm90_alert

This commit adds hwmon_lock protection to lm90_alert to prevent concurrent
sysfs write operations from overwriting the alert handler's modifications.
This solves an interrupt storm problem when an alert interrupt occurs
concurrently with a sysfs write.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 50b30d7192256..6658c743659b8 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -2939,6 +2939,7 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
>  		 */
>  		struct lm90_data *data = i2c_get_clientdata(client);
>  
> +		hwmon_lock(data->hwmon_dev);
>  		if ((data->flags & LM90_HAVE_BROKEN_ALERT) &&
>  		    (data->current_alarms & data->alert_alarms)) {
>  			if (!(data->config & 0x80)) {
> @@ -2948,6 +2949,7 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
>  			schedule_delayed_work(&data->alert_work,
>  				max_t(int, HZ, msecs_to_jiffies(data->update_interval)));
>  		}

This isn't a regression introduced by this patch, but it exists in the
driver's device teardown path. Can this workqueue re-arming lead to a
use-after-free of the data structure during device unbind?

In lm90_probe(), devm_hwmon_device_register_with_info() is called before
adding lm90_stop_work():

drivers/hwmon/lm90.c:lm90_probe() {
    ...
	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
							 data, &data->chip,
							 NULL);
    ...
	err = devm_add_action_or_reset(&client->dev, lm90_stop_work, data);
    ...
}

Because devres executes actions in reverse order of addition,
lm90_stop_work() runs first during device unbind or probe failure. This
synchronously cancels both alert_work and report_work:

drivers/hwmon/lm90.c:lm90_stop_work() {
    ...
	cancel_delayed_work_sync(&data->alert_work);
	cancel_work_sync(&data->report_work);
}

However, at this point the hwmon device is still registered, meaning its
sysfs attributes are fully accessible to userspace.

If a concurrent sysfs read occurs to a file like temp1_input, it will
invoke lm90_temp_read(), which calls lm90_update_device() and subsequently
lm90_update_alarms_locked().

If alarm conditions are still met, this path explicitly re-arms the
workqueues by calling schedule_delayed_work(&data->alert_work, ...) or
schedule_work(&data->report_work).

After the sysfs read finishes, devres proceeds to unregister the hwmon
device and free the lm90_data structure. The incorrectly re-armed work
will eventually execute in the background using the freed data pointer.

> +		hwmon_unlock(data->hwmon_dev);
>  	} else {
>  		dev_dbg(&client->dev, "Everything OK\n");
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514220214.1380831-1-linux@roeck-us.net?part=2