Re: [PATCH v2 1/2] hwmon: (lm90) Stop work before releasing hwmon device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in `lm90_stop_work()` allows canceled workers to be re-armed via concurrent sysfs accesses, leading to a Use-After-Free.
--

commit e9b97015cb5267aa1cbf5c4dd89535d551b6b503
Author: Guenter Roeck <linux@roeck-us.net>
hwmon: (lm90) Stop work before releasing hwmon device

This commit fixes a use-after-free issue by canceling the workers after
registering the hwmon device and before registering the interrupt handler.
It introduces a shutdown flag to prevent workers from being re-armed during
device teardown.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 3c10a5066b53d..08a568f7b56f9 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
[ ... ]
> @@ -2584,15 +2588,21 @@ static void lm90_restore_conf(void *_data)
>  	struct lm90_data *data = _data;
>  	struct i2c_client *client = data->client;
>  
> -	cancel_delayed_work_sync(&data->alert_work);
> -	cancel_work_sync(&data->report_work);
> -
>  	/* Restore initial configuration */
>  	if (data->flags & LM90_HAVE_CONVRATE)
>  		lm90_write_convrate(data, data->convrate_orig);
>  	lm90_write_reg(client, LM90_REG_CONFIG1, data->config_orig);
>  }
>  
> +static void lm90_stop_work(void *_data)
> +{
> +	struct lm90_data *data = _data;
> +
> +	data->shutdown = true;

Can this race with a concurrent sysfs read, allowing the workers to be
re-armed after they are canceled?

Since devres unwinds in reverse order, lm90_stop_work() executes before the
hwmon device is unregistered. This leaves the sysfs attributes fully
accessible.

If a sysfs read happens concurrently, the hwmon core will acquire the hwmon
lock, and the driver could call lm90_update_alarms_locked(). Because
lm90_stop_work() sets data->shutdown without acquiring hwmon_lock(), the
concurrent sysfs read might read data->shutdown as false before it is updated.

If the sysfs thread blocks on I2C reads, lm90_stop_work() could set
data->shutdown to true and call cancel_delayed_work_sync(). Since the work is
not yet queued, the cancellation returns immediately.

When the sysfs thread finishes its I2C transfers, it could call
schedule_work() or schedule_delayed_work(), successfully re-arming the workers
after they were supposedly canceled.

As the devres unbind continues, it unregisters the hwmon device and frees the
data structure. When the newly scheduled work executes, it would dereference
the freed memory, causing a use-after-free.

Should lm90_stop_work() hold hwmon_lock(data->hwmon_dev) while setting
data->shutdown to synchronize with sysfs reads, releasing it before calling
the sync cancellation functions to avoid deadlocks?

> +	cancel_delayed_work_sync(&data->alert_work);
> +	cancel_work_sync(&data->report_work);
> +}

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