[PATCH 1/3] hwmon: (acpi_power_meter) Fix missing locking in sysfs show callbacks and resume

[Guenter Roeck <linux@roeck-us.net>]

During system resume, acpi_power_meter_resume() calls free_capabilities()
and read_capabilities() without acquiring resource->lock. Because ACPI
notifications run on the non-freezable kacpi_notify_wq workqueue, a
concurrent METER_NOTIFY_CONFIG notification during resume can race with
acpi_power_meter_resume() and trigger a double-free or use-after-free on
the model_number, serial_number, and oem_info strings as well as
concurrently overwrite resource->caps. When CONFIG_SUSPEND_FREEZER is
disabled, concurrent sysfs readers can also dereference the freed
capability strings mid-update.

In addition, none of the extra sysfs attribute show callbacks acquire
res->lock:
- power1_average_min_show() and power1_average_max_show() read the 64-bit
  res->trip[] entries twice without locking while set_trip() modifies
  them under res->lock (including temporarily storing a candidate value
  before evaluating _PTP and rolling it back on failure). On 32-bit
  architectures this exposes torn 64-bit reads, and on all architectures
  it exposes uncommitted trip values and a TOCTOU race between the < 0
  check and multiplication (emitting "-1000" instead of "unknown" if
  set_trip() rolls back to -1).
- power1_model_number_show(), power1_oem_info_show(),
  power1_serial_number_show(), power1_cap_hyst_show(),
  power1_accuracy_show(), and power1_is_battery_show() read capability
  strings and 64-bit capability fields without synchronization against
  resume updates.

Acquire resource->lock using guard(mutex) in acpi_power_meter_resume()
and in all extra sysfs show callbacks. Also return -ENODATA in the
capability string show callbacks if a string pointer is NULL after a
failed capability re-read.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: de584afa5e18 ("hwmon driver for ACPI 4.0 power meters")
Fixes: 16746ce8adfe ("hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/acpi_power_meter.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 3891962f03db..38d6a1e56ffc 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -584,6 +584,8 @@ static ssize_t power1_average_min_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->trip[POWER_METER_TRIP_AVERAGE_MIN_IDX] < 0)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -596,6 +598,8 @@ static ssize_t power1_average_max_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->trip[POWER_METER_TRIP_AVERAGE_MAX_IDX] < 0)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -608,6 +612,8 @@ static ssize_t power1_cap_hyst_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	if (res->caps.hysteresis == UNKNOWN_HYSTERESIS)
 		return sysfs_emit(buf, "unknown\n");
 
@@ -619,8 +625,11 @@ static ssize_t power1_accuracy_show(struct device *dev,
 				    char *buf)
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
-	unsigned int acc = res->caps.accuracy;
+	unsigned int acc;
 
+	guard(mutex)(&res->lock);
+
+	acc = res->caps.accuracy;
 	return sysfs_emit(buf, "%u.%u%%\n", acc / 1000, acc % 1000);
 }
 
@@ -630,6 +639,8 @@ static ssize_t power1_is_battery_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
 	return sysfs_emit(buf, "%u\n",
 			  res->caps.flags & POWER_METER_IS_BATTERY ? 1 : 0);
 }
@@ -640,6 +651,11 @@ static ssize_t power1_model_number_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->model_number)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->model_number);
 }
 
@@ -649,6 +665,11 @@ static ssize_t power1_oem_info_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->oem_info)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->oem_info);
 }
 
@@ -658,6 +679,11 @@ static ssize_t power1_serial_number_show(struct device *dev,
 {
 	struct acpi_power_meter_resource *res = dev_get_drvdata(dev);
 
+	guard(mutex)(&res->lock);
+
+	if (!res->serial_number)
+		return -ENODATA;
+
 	return sysfs_emit(buf, "%s\n", res->serial_number);
 }
 
@@ -984,6 +1010,8 @@ static int acpi_power_meter_resume(struct device *dev)
 {
 	struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
 
+	guard(mutex)(&resource->lock);
+
 	free_capabilities(resource);
 	read_capabilities(resource);
 
-- 
2.45.2