Re: [PATCH 2/4] hwmon: (lm90) Use guard() to acquire subsystem lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Devm registration ordering causes a Use-After-Free of `data->hwmon_dev` if delayed works run during driver removal.
- [High] Modifying `data->config` in `lm90_alert()` without holding `hwmon_lock` creates a data race with sysfs operations.
--

commit 7118ba7da109d5ac53da9e4739a437d57be7a2f6
Author: Guenter Roeck <linux@roeck-us.net>
Subject: hwmon: (lm90) Use guard() to acquire subsystem lock

This commit converts the lm90 driver to use the scope-based guard() macro
for acquiring and releasing the hardware monitoring subsystem lock, replacing
the explicit hwmon_lock() and hwmon_unlock() calls.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 3c10a5066b53d..9ca49abc1a93a 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -1222,13 +1222,8 @@ static int lm90_update_alarms_locked(struct lm90_data *data, bool force)
>  
>  static int lm90_update_alarms(struct lm90_data *data, bool force)
>  {
> -	int err;
> -
> -	hwmon_lock(data->hwmon_dev);
> -	err = lm90_update_alarms_locked(data, force);
> -	hwmon_unlock(data->hwmon_dev);
> -
> -	return err;
> +	guard(hwmon_lock)(data->hwmon_dev);

Can this cause a use-after-free of data->hwmon_dev if the delayed works
run during driver removal?

This is a pre-existing issue, but looking at lm90_probe(), the devm action
to cancel the alert_work and report_work (lm90_restore_conf) is registered
in lm90_init_client() before devm_hwmon_device_register_with_info() is called.

Because devm executes cleanup actions in reverse order during module
unbind or probe failure, the hwmon device is unregistered and freed first.

If lm90_alert_work() or lm90_report_alarms() runs in the window between
the hwmon device being freed and the delayed works being cancelled,
lm90_update_alarms() will dereference the freed data->hwmon_dev here.

Is there also a data race when modifying data->config in lm90_alert() without
holding the subsystem lock?

This is another pre-existing issue. lm90_alert() executes in the smbus alert
context and calls lm90_update_confreg() to disable the hardware alert line,
without acquiring hwmon_lock.

Concurrently, sysfs write operations (such as lm90_write_convrate) hold
the hwmon_lock, temporarily modify data->config, and then restore it.

If an alert interrupt occurs concurrently with a sysfs write, the sysfs
path will overwrite the alert handler's modifications to data->config
and the hardware register. 

This unintentionally re-enables the hardware alert line while the alarm is 
still active, causing an interrupt storm.

> +	return lm90_update_alarms_locked(data, force);
>  }
>  
>  static void lm90_alert_work(struct work_struct *__work)

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