[PATCH] hwmon: (hp-wmi-sensors) Serialize alarm flag read-and-clear with state->lock

[PATCH] hwmon: (hp-wmi-sensors) Serialize alarm flag read-and-clear with state->lock

[Guenter Roeck]

In hp_wmi_hwmon_read(), temperature and fan alarm flags (info->alarm)
are evaluated and cleared without acquiring state->lock, while only the
hwmon subsystem lock is held by the hwmon core. Meanwhile, the
asynchronous WMI event handler hp_wmi_notify() sets info->alarm under
state->lock. If hp_wmi_notify() sets info->alarm between the read and
the subsequent clear in hp_wmi_hwmon_read(), the newly reported fan
stall or critical temperature alarm is overwritten with false and
permanently lost.

The driver-local state->lock cannot be replaced with the hwmon
subsystem lock (hwmon_lock()):
- When no connected hwmon-supported sensors are present (!count), the
  driver operates in debugfs-only mode without registering a hwmon
  device, while debugfs attributes (fungible_show()) still require
  state->lock to serialize WMI queries and sensor state updates.
- Even when sensors are connected, add_event_handler() must subscribe
  to WMI events before make_chip_info() and
  devm_hwmon_device_register_with_info() because visibility of alarm
  and intrusion attributes depends on whether event registration
  succeeded. As a result, hp_wmi_notify() can run before the hwmon
  device exists during probe and after devm unregisters and frees the
  hwmon device during removal.

Acquire state->lock in hp_wmi_hwmon_read() around reading and clearing
info->alarm to prevent losing WMI alarm events.

Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
Cc: James Seo <james@equiv.tech>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/hp-wmi-sensors.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index 1953d672edaf..1a4955dbddd0 100644
--- a/drivers/hwmon/hp-wmi-sensors.c
+++ b/drivers/hwmon/hp-wmi-sensors.c
@@ -1480,8 +1480,10 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 
 	if ((type == hwmon_temp && attr == hwmon_temp_alarm) ||
 	    (type == hwmon_fan  && attr == hwmon_fan_alarm)) {
+		mutex_lock(&state->lock);
 		*out_val = info->alarm ? 1 : 0;
 		info->alarm = false;
+		mutex_unlock(&state->lock);
 
 		return 0;
 	}
-- 
2.45.2

Re: [PATCH] hwmon: (hp-wmi-sensors) Serialize alarm flag read-and-clear with state->lock

[sashiko-bot]

> In hp_wmi_hwmon_read(), temperature and fan alarm flags (info->alarm)
> are evaluated and cleared without acquiring state->lock, while only the
> hwmon subsystem lock is held by the hwmon core. Meanwhile, the
> asynchronous WMI event handler hp_wmi_notify() sets info->alarm under
> state->lock. If hp_wmi_notify() sets info->alarm between the read and
> the subsequent clear in hp_wmi_hwmon_read(), the newly reported fan
> stall or critical temperature alarm is overwritten with false and
> permanently lost.
> 
> The driver-local state->lock cannot be replaced with the hwmon
> subsystem lock (hwmon_lock()):
> - When no connected hwmon-supported sensors are present (!count), the
>   driver operates in debugfs-only mode without registering a hwmon
>   device, while debugfs attributes (fungible_show()) still require
>   state->lock to serialize WMI queries and sensor state updates.
> - Even when sensors are connected, add_event_handler() must subscribe
>   to WMI events before make_chip_info() and
>   devm_hwmon_device_register_with_info() because visibility of alarm
>   and intrusion attributes depends on whether event registration
>   succeeded. As a result, hp_wmi_notify() can run before the hwmon
>   device exists during probe and after devm unregisters and frees the
>   hwmon device during removal.
> 
> Acquire state->lock in hp_wmi_hwmon_read() around reading and clearing
> info->alarm to prevent losing WMI alarm events.
> 
> Fixes: 23902f98f8d4 ("hwmon: add HP WMI Sensors driver")
> Cc: James Seo <james@equiv.tech>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues.

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