[PATCH 0/4] hwmon: Support guard() and scoped_guard for subsystem locks

[PATCH 0/4] hwmon: Support guard() and scoped_guard for subsystem locks

[Guenter Roeck]

Add support for guard() and scoped_guard() for the hwmon subsystem lock
to simplify its use, and convert existing drivers to use the new functions.

----------------------------------------------------------------
Guenter Roeck (4):
      hwmon: Support guard() and scoped_guard for subsystem locks
      hwmon: (lm90) Use guard() to acquire subsystem lock
      hwmon: (ina2xx) Use scoped_guard() to acquire the subsystem lock
      hwmon: (adt7411) Use scoped_guard() to acquire the subsystem lock

 Documentation/hwmon/hwmon-kernel-api.rst |  7 ++++---
 drivers/hwmon/adt7411.c                  | 10 +++++-----
 drivers/hwmon/ina2xx.c                   | 10 +++++-----
 drivers/hwmon/lm90.c                     |  9 ++-------
 include/linux/hwmon.h                    |  2 ++
 5 files changed, 18 insertions(+), 20 deletions(-)

[PATCH 1/4] hwmon: Support guard() and scoped_guard for subsystem locks

[Guenter Roeck]

Add support for guard() and scoped_guard() for the hwmon subsystem lock
to simplify its use.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/hwmon-kernel-api.rst | 7 ++++---
 include/linux/hwmon.h                    | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index 1d7f1397a827..9fcde32a140d 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -85,9 +85,10 @@ removal.
 When using ``[devm_]hwmon_device_register_with_info()`` to register the
 hardware monitoring device, accesses using the associated access functions
 are serialised by the hardware monitoring core. If a driver needs locking
-for other functions such as interrupt handlers or for attributes which are
-fully implemented in the driver, hwmon_lock() and hwmon_unlock() can be used
-to ensure that calls to those functions are serialized.
+for other functions such as interrupt handlers, attributes which are fully
+implemented in the driver, or debugfs functions, hwmon_lock() and hwmon_unlock()
+can be used to ensure that calls to those functions are serialized. Those
+functions also support guard() and scoped_guard() variants.
 
 Using devm_hwmon_device_register_with_info()
 --------------------------------------------
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 301a83afbd66..04959e044fd0 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -495,6 +495,8 @@ char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
 void hwmon_lock(struct device *dev);
 void hwmon_unlock(struct device *dev);
 
+DEFINE_GUARD(hwmon_lock, struct device *, hwmon_lock(_T), hwmon_unlock(_T))
+
 /**
  * hwmon_is_bad_char - Is the char invalid in a hwmon name
  * @ch: the char to be considered
-- 
2.45.2

Re: [PATCH 1/4] hwmon: Support guard() and scoped_guard for subsystem locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The header include/linux/hwmon.h uses the DEFINE_GUARD macro but fails to explicitly include <linux/cleanup.h>.
--

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

hwmon: Support guard() and scoped_guard for subsystem locks
    
This commit adds support for guard() and scoped_guard() macros to the
hwmon subsystem lock, aiming to simplify lock management.

> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index 301a83afbd663..04959e044fd0f 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
[ ... ]
> @@ -495,6 +495,8 @@ char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
>  void hwmon_lock(struct device *dev);
>  void hwmon_unlock(struct device *dev);
>  
> +DEFINE_GUARD(hwmon_lock, struct device *, hwmon_lock(_T), hwmon_unlock(_T))

Does this header need to include <linux/cleanup.h>?

The DEFINE_GUARD macro is provided by <linux/cleanup.h>. While many files
might implicitly pull it in through <linux/device.h> or <linux/mutex.h>,
could this introduce a build regression for any compilation units that include
<linux/hwmon.h> without an implicit include path to <linux/cleanup.h>?

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

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

[Guenter Roeck]

Use guard() instead of hwmon_lock() / hwmon_unlock() to acquire and release
the hardware monitoring subsystem lock.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 3c10a5066b53..9ca49abc1a93 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);
+	return lm90_update_alarms_locked(data, force);
 }
 
 static void lm90_alert_work(struct work_struct *__work)
-- 
2.45.2

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

[sashiko-bot]

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

[PATCH 3/4] hwmon: (ina2xx) Use scoped_guard() to acquire the subsystem lock

[Guenter Roeck]

Use scoped_guard() instead of hwmon_lock() / hwmon_unlock() to acquire
and release the hardware monitoring subsystem lock.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina2xx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 613ffb622b7c..3765c8542a75 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -875,11 +875,11 @@ static ssize_t shunt_resistor_store(struct device *dev,
 	if (status < 0)
 		return status;
 
-	hwmon_lock(dev);
-	status = ina2xx_set_shunt(data, val);
-	hwmon_unlock(dev);
-	if (status < 0)
-		return status;
+	scoped_guard(hwmon_lock, dev) {
+		status = ina2xx_set_shunt(data, val);
+		if (status < 0)
+			return status;
+	}
 	return count;
 }
 
-- 
2.45.2

[PATCH 4/4] hwmon: (adt7411) Use scoped_guard() to acquire the subsystem lock

[Guenter Roeck]

Use scoped_guard() instead of hwmon_lock() / hwmon_unlock() to acquire
and release the hardware monitoring subsystem lock.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/adt7411.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/adt7411.c b/drivers/hwmon/adt7411.c
index b9991a69e6c6..5bf3b3bd37da 100644
--- a/drivers/hwmon/adt7411.c
+++ b/drivers/hwmon/adt7411.c
@@ -169,11 +169,11 @@ static ssize_t adt7411_set_bit(struct device *dev,
 	if (ret || flag > 1)
 		return -EINVAL;
 
-	hwmon_lock(dev);
-	ret = adt7411_modify_bit(client, s_attr2->index, s_attr2->nr, flag);
-	/* force update */
-	data->next_update = jiffies;
-	hwmon_unlock(dev);
+	scoped_guard(hwmon_lock, dev) {
+		ret = adt7411_modify_bit(client, s_attr2->index, s_attr2->nr, flag);
+		/* force update */
+		data->next_update = jiffies;
+	}
 
 	return ret < 0 ? ret : count;
 }
-- 
2.45.2