Re: [PATCH 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] Re-ordering work cancellation before hwmon device unregistration introduces a Use-After-Free vulnerability due to a race with sysfs accesses.
--

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

hwmon: (lm90) Stop work before releasing hwmon device
    
This commit addresses a use-after-free issue by moving the cancellation
of alert_work and report_work to a separate devm action. It registers
this new action after the hwmon device is registered, ensuring the works
are cancelled before the hwmon device is freed.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 3c10a5066b53d..50b30d7192256 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c

[ ... ]

> @@ -2902,6 +2907,10 @@ static int lm90_probe(struct i2c_client *client)
>  
>  	data->hwmon_dev = hwmon_dev;
>  
> +	err = devm_add_action_or_reset(&client->dev, lm90_stop_work, data);
> +	if (err)
> +		return err;
> +

Does this reordering introduce a regression where the workqueue can be
re-armed after it is cancelled?

Because this devm action is registered after
devm_hwmon_device_register_with_info(), lm90_stop_work() will be called first
during teardown, cancelling alert_work and report_work.

However, at this point the hwmon device is still registered and its sysfs
attributes remain exposed.

If user-space reads a sysfs attribute in this window, it triggers the
hwmon core to invoke lm90_temp_read(), which calls lm90_update_device()
and then lm90_update_alarms_locked().

If alerts are disabled on the chip, lm90_update_alarms_locked() will call
schedule_work(&data->report_work) or schedule_delayed_work(&data->alert_work).

Could this dynamically re-arm the workqueue after lm90_stop_work() has
already run?

If the work is re-armed, the teardown process will then unregister the
hwmon device and free the data structure. The asynchronously executing work
would then dereference the freed data pointer, resulting in a use-after-free.

Would it be possible to introduce synchronization state, such as checking a
flag in the data structure before scheduling works, or decoupling the
teardown order?

>  	if (client->irq) {
>  		dev_dbg(dev, "IRQ: %d\n", client->irq);

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