[PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access

Emre Cecanpunar posted 5 patches 1 week, 5 days ago
[PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access
Posted by Emre Cecanpunar 1 week, 5 days ago
hp_wmi_hwmon_priv.mode and .pwm are written by hp_wmi_hwmon_write()
in sysfs context and read by hp_wmi_hwmon_keep_alive_handler() running
in a workqueue. There is no synchronisation between them, so a
concurrent sysfs write and keep-alive expiry can observe an
inconsistent combination of mode and pwm (e.g. mode=MANUAL with an
stale pwm value from the previous mode).

Add a mutex to hp_wmi_hwmon_priv that protects mode and pwm. Acquire
it in hp_wmi_hwmon_write() for the full write operation (field update
plus hp_wmi_apply_fan_settings() call), and in
hp_wmi_hwmon_keep_alive_handler() before calling
hp_wmi_apply_fan_settings().

In hp_wmi_hwmon_read(), only the pwm_enable attribute reads priv->mode
and requires the lock. Use scoped_guard() to hold the mutex only for
the mode read, avoiding holding it across unrelated WMI hardware calls.

Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index af57cb0dbf9e..0947aaeabff3 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -434,6 +434,7 @@ enum pwm_modes {
 };
 
 struct hp_wmi_hwmon_priv {
+	struct mutex lock;	/* protects mode, pwm */
 	u8 min_rpm;
 	u8 max_rpm;
 	int gpu_delta;
@@ -2403,6 +2404,7 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct hp_wmi_hwmon_priv *priv;
 	int rpm, ret;
+	u8 mode;
 
 	priv = dev_get_drvdata(dev);
 	switch (type) {
@@ -2426,11 +2428,13 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			*val = rpm_to_pwm(rpm / 100, priv);
 			return 0;
 		}
-		switch (priv->mode) {
+		scoped_guard(mutex, &priv->lock)
+			mode = priv->mode;
+		switch (mode) {
 		case PWM_MODE_MAX:
 		case PWM_MODE_MANUAL:
 		case PWM_MODE_AUTO:
-			*val = priv->mode;
+			*val = mode;
 			return 0;
 		default:
 			/* shouldn't happen */
@@ -2448,6 +2452,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 	int rpm;
 
 	priv = dev_get_drvdata(dev);
+	guard(mutex)(&priv->lock);
 	switch (type) {
 	case hwmon_pwm:
 		if (attr == hwmon_pwm_input) {
@@ -2516,6 +2521,8 @@ static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
 
 	dwork = to_delayed_work(work);
 	priv = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
+
+	guard(mutex)(&priv->lock);
 	/*
 	 * Re-apply the current hwmon context settings.
 	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
@@ -2573,6 +2580,8 @@ static int hp_wmi_hwmon_init(void)
 	if (!priv)
 		return -ENOMEM;
 
+	mutex_init(&priv->lock);
+
 	ret = hp_wmi_setup_fan_settings(priv);
 	if (ret)
 		return ret;
-- 
2.53.0