From c89039b724cc4d5e63f295d45b754e0de0faa0ad Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 31 Jul 2026 15:00:36 +0200 Subject: [PATCH 1/3] Revert "thermal: hwmon: Use extra_groups for adding temperature attributes" Revert commit cfb5dc0f60fb ("thermal: hwmon: Use extra_groups for adding temperature attributes") because it is depended on by another one that turned out to be problematic. Signed-off-by: Rafael J. Wysocki Link: https://patch.msgid.link/1992232.tdWV9SEqCh@rafael.j.wysocki --- drivers/hwmon/hwmon.c | 6 +- drivers/thermal/thermal_hwmon.c | 122 ++++++++++++++++++++------------ include/linux/hwmon.h | 3 +- 3 files changed, 80 insertions(+), 51 deletions(-) diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index 55a9a3ddd4aa..29dc90a2c3fe 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -1083,7 +1083,6 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_info); * @dev: the parent device * @name: hwmon name attribute * @drvdata: driver data to attach to created device - * @extra_groups: pointer to list of additional non-standard attribute groups * * The use of this function is restricted. It is provided for legacy reasons * and must only be called from the thermal subsystem. @@ -1095,13 +1094,12 @@ EXPORT_SYMBOL_GPL(hwmon_device_register_with_info); */ struct device * hwmon_device_register_for_thermal(struct device *dev, const char *name, - void *drvdata, - const struct attribute_group **extra_groups) + void *drvdata) { if (!name || !dev) return ERR_PTR(-EINVAL); - return __hwmon_device_register(dev, name, drvdata, NULL, extra_groups); + return __hwmon_device_register(dev, name, drvdata, NULL, NULL); } EXPORT_SYMBOL_NS_GPL(hwmon_device_register_for_thermal, "HWMON_THERMAL"); diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index 386dfb9f559e..223ae1571655 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -25,13 +25,25 @@ */ #define THERMAL_HWMON_NAME_LENGTH (THERMAL_NAME_LENGTH + 11) +struct thermal_hwmon_attr { + struct device_attribute attr; +}; + +/* one temperature input for each thermal zone */ +struct thermal_hwmon_temp { + struct thermal_zone_device *tz; + struct thermal_hwmon_attr temp_input; /* hwmon sys attr */ + struct thermal_hwmon_attr temp_crit; /* hwmon sys attr */ + bool temp_crit_present; +}; + /* hwmon sys I/F */ /* thermal zone devices with the same type share one hwmon device */ struct thermal_hwmon_device { char name[THERMAL_HWMON_NAME_LENGTH]; struct device *device; struct list_head node; - struct thermal_zone_device *tz; + struct thermal_hwmon_temp tz_temp; }; static LIST_HEAD(thermal_hwmon_list); @@ -39,14 +51,19 @@ static LIST_HEAD(thermal_hwmon_list); static DEFINE_MUTEX(thermal_hwmon_list_lock); static ssize_t -temp1_input_show(struct device *dev, struct device_attribute *attr, char *buf) +temp_input_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev); - struct thermal_zone_device *tz = hwmon->tz; int temperature; int ret; + struct thermal_hwmon_attr *hwmon_attr + = container_of(attr, struct thermal_hwmon_attr, attr); + struct thermal_hwmon_temp *temp + = container_of(hwmon_attr, struct thermal_hwmon_temp, + temp_input); + struct thermal_zone_device *tz = temp->tz; ret = thermal_zone_get_temp(tz, &temperature); + if (ret) return ret; @@ -54,10 +71,14 @@ temp1_input_show(struct device *dev, struct device_attribute *attr, char *buf) } static ssize_t -temp1_crit_show(struct device *dev, struct device_attribute *attr, char *buf) +temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct thermal_hwmon_device *hwmon = dev_get_drvdata(dev); - struct thermal_zone_device *tz = hwmon->tz; + struct thermal_hwmon_attr *hwmon_attr + = container_of(attr, struct thermal_hwmon_attr, attr); + struct thermal_hwmon_temp *temp + = container_of(hwmon_attr, struct thermal_hwmon_temp, + temp_crit); + struct thermal_zone_device *tz = temp->tz; int temperature; int ret; @@ -70,49 +91,22 @@ temp1_crit_show(struct device *dev, struct device_attribute *attr, char *buf) return sysfs_emit(buf, "%d\n", temperature); } -static DEVICE_ATTR_RO(temp1_input); -static DEVICE_ATTR_RO(temp1_crit); - -static struct attribute *thermal_hwmon_attrs[] = { - &dev_attr_temp1_input.attr, - &dev_attr_temp1_crit.attr, - NULL, -}; - -static umode_t thermal_hwmon_attr_is_visible(struct kobject *kobj, - struct attribute *a, int n) +static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz) { - if (a == &dev_attr_temp1_input.attr) - return a->mode; - - if (a == &dev_attr_temp1_crit.attr) { - struct thermal_hwmon_device *hwmon = dev_get_drvdata(kobj_to_dev(kobj)); - struct thermal_zone_device *tz = hwmon->tz; - int dummy; - - if (tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &dummy)) - return a->mode; - } - - return 0; + int temp; + return tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &temp); } -static const struct attribute_group thermal_hwmon_group = { - .attrs = thermal_hwmon_attrs, - .is_visible = thermal_hwmon_attr_is_visible, -}; - -__ATTRIBUTE_GROUPS(thermal_hwmon); - int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) { struct thermal_hwmon_device *hwmon; + struct thermal_hwmon_temp *temp; + int result; hwmon = kzalloc_obj(*hwmon); if (!hwmon) return -ENOMEM; - hwmon->tz = tz; /* * Append the thermal zone ID preceded by an underline character to the * type to disambiguate the sensors command output. @@ -120,13 +114,35 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) scnprintf(hwmon->name, THERMAL_HWMON_NAME_LENGTH, "%s_%d", tz->type, tz->id); strreplace(hwmon->name, '-', '_'); hwmon->device = hwmon_device_register_for_thermal(&tz->device, - hwmon->name, hwmon, - thermal_hwmon_groups); + hwmon->name, hwmon); if (IS_ERR(hwmon->device)) { - int result = PTR_ERR(hwmon->device); + result = PTR_ERR(hwmon->device); + goto free_mem; + } - kfree(hwmon); - return result; + temp = &hwmon->tz_temp; + + temp->tz = tz; + + temp->temp_input.attr.attr.name = "temp1_input"; + temp->temp_input.attr.attr.mode = 0444; + temp->temp_input.attr.show = temp_input_show; + sysfs_attr_init(&temp->temp_input.attr.attr); + result = device_create_file(hwmon->device, &temp->temp_input.attr); + if (result) + goto unregister_name; + + if (thermal_zone_crit_temp_valid(tz)) { + temp->temp_crit.attr.attr.name = "temp1_crit"; + temp->temp_crit.attr.attr.mode = 0444; + temp->temp_crit.attr.show = temp_crit_show; + sysfs_attr_init(&temp->temp_crit.attr.attr); + result = device_create_file(hwmon->device, + &temp->temp_crit.attr); + if (result) + goto unregister_input; + + temp->temp_crit_present = true; } /* The list is needed for hwmon lookup during removal. */ @@ -135,6 +151,15 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) mutex_unlock(&thermal_hwmon_list_lock); return 0; + + unregister_input: + device_remove_file(hwmon->device, &temp->temp_input.attr); + unregister_name: + hwmon_device_unregister(hwmon->device); + free_mem: + kfree(hwmon); + + return result; } EXPORT_SYMBOL_GPL(thermal_add_hwmon_sysfs); @@ -144,7 +169,7 @@ thermal_hwmon_lookup(const struct thermal_zone_device *tz) struct thermal_hwmon_device *hwmon; list_for_each_entry(hwmon, &thermal_hwmon_list, node) { - if (hwmon->tz == tz) + if (hwmon->tz_temp.tz == tz) return hwmon; } return NULL; @@ -153,6 +178,7 @@ thermal_hwmon_lookup(const struct thermal_zone_device *tz) void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz) { struct thermal_hwmon_device *hwmon; + struct thermal_hwmon_temp *temp; scoped_guard(mutex, &thermal_hwmon_list_lock) { hwmon = thermal_hwmon_lookup(tz); @@ -162,6 +188,12 @@ void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz) list_del(&hwmon->node); } + temp = &hwmon->tz_temp; + + device_remove_file(hwmon->device, &temp->temp_input.attr); + if (temp->temp_crit_present) + device_remove_file(hwmon->device, &temp->temp_crit.attr); + hwmon_device_unregister(hwmon->device); kfree(hwmon); } diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h index 77a6f2bffcba..dd713e193d0c 100644 --- a/include/linux/hwmon.h +++ b/include/linux/hwmon.h @@ -480,8 +480,7 @@ hwmon_device_register_with_info(struct device *dev, const struct attribute_group **extra_groups); struct device * hwmon_device_register_for_thermal(struct device *dev, const char *name, - void *drvdata, - const struct attribute_group **extra_groups); + void *drvdata); struct device * devm_hwmon_device_register_with_info(struct device *dev, const char *name, void *drvdata, From f93d951ce0d02b5dca01c0c72add411fb17849bb Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 31 Jul 2026 15:01:15 +0200 Subject: [PATCH 2/3] Revert "thermal: hwmon: Register a hwmon device for each thermal zone" Revert commit d6323469bcfb ("thermal: hwmon: Register a hwmon device for each thermal zone") that changed the names of hwmon class devices associated with thermal zones and their sysfs layout which made user space unhappy. Closes: https://lore.kernel.org/linux-pm/cafd8af9-c6e9-4bf2-b496-23e796fbc9a6@linux.dev/ Closes: https://lore.kernel.org/linux-hwmon/ab8b093b-46e6-4738-afcf-4b97c9ad5af9@googlemail.com/ Cc: stable@vger.kernel.org Signed-off-by: Rafael J. Wysocki Link: https://patch.msgid.link/2301040.irdbgypaU6@rafael.j.wysocki --- drivers/thermal/thermal_hwmon.c | 151 ++++++++++++++++++++++---------- 1 file changed, 104 insertions(+), 47 deletions(-) diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index 223ae1571655..597c33c8a555 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -19,33 +19,30 @@ #include "thermal_hwmon.h" #include "thermal_core.h" -/* - * Needs to be large enough to hold a thermal zone type string followed by an - * underline character and a 32-bit integer in decimal representation. - */ -#define THERMAL_HWMON_NAME_LENGTH (THERMAL_NAME_LENGTH + 11) +/* hwmon sys I/F */ +/* thermal zone devices with the same type share one hwmon device */ +struct thermal_hwmon_device { + char type[THERMAL_NAME_LENGTH]; + struct device *device; + int count; + struct list_head tz_list; + struct list_head node; +}; struct thermal_hwmon_attr { struct device_attribute attr; + char name[16]; }; /* one temperature input for each thermal zone */ struct thermal_hwmon_temp { + struct list_head hwmon_node; struct thermal_zone_device *tz; struct thermal_hwmon_attr temp_input; /* hwmon sys attr */ struct thermal_hwmon_attr temp_crit; /* hwmon sys attr */ bool temp_crit_present; }; -/* hwmon sys I/F */ -/* thermal zone devices with the same type share one hwmon device */ -struct thermal_hwmon_device { - char name[THERMAL_HWMON_NAME_LENGTH]; - struct device *device; - struct list_head node; - struct thermal_hwmon_temp tz_temp; -}; - static LIST_HEAD(thermal_hwmon_list); static DEFINE_MUTEX(thermal_hwmon_list_lock); @@ -91,6 +88,45 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) return sysfs_emit(buf, "%d\n", temperature); } + +static struct thermal_hwmon_device * +thermal_hwmon_lookup_by_type(const struct thermal_zone_device *tz) +{ + struct thermal_hwmon_device *hwmon; + char type[THERMAL_NAME_LENGTH]; + + mutex_lock(&thermal_hwmon_list_lock); + list_for_each_entry(hwmon, &thermal_hwmon_list, node) { + strscpy(type, tz->type); + strreplace(type, '-', '_'); + if (!strcmp(hwmon->type, type)) { + mutex_unlock(&thermal_hwmon_list_lock); + return hwmon; + } + } + mutex_unlock(&thermal_hwmon_list_lock); + + return NULL; +} + +/* Find the temperature input matching a given thermal zone */ +static struct thermal_hwmon_temp * +thermal_hwmon_lookup_temp(const struct thermal_hwmon_device *hwmon, + const struct thermal_zone_device *tz) +{ + struct thermal_hwmon_temp *temp; + + mutex_lock(&thermal_hwmon_list_lock); + list_for_each_entry(temp, &hwmon->tz_list, hwmon_node) + if (temp->tz == tz) { + mutex_unlock(&thermal_hwmon_list_lock); + return temp; + } + mutex_unlock(&thermal_hwmon_list_lock); + + return NULL; +} + static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz) { int temp; @@ -101,39 +137,54 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) { struct thermal_hwmon_device *hwmon; struct thermal_hwmon_temp *temp; + int new_hwmon_device = 1; int result; + hwmon = thermal_hwmon_lookup_by_type(tz); + if (hwmon) { + new_hwmon_device = 0; + goto register_sys_interface; + } + hwmon = kzalloc_obj(*hwmon); if (!hwmon) return -ENOMEM; - /* - * Append the thermal zone ID preceded by an underline character to the - * type to disambiguate the sensors command output. - */ - scnprintf(hwmon->name, THERMAL_HWMON_NAME_LENGTH, "%s_%d", tz->type, tz->id); - strreplace(hwmon->name, '-', '_'); + INIT_LIST_HEAD(&hwmon->tz_list); + strscpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH); + strreplace(hwmon->type, '-', '_'); hwmon->device = hwmon_device_register_for_thermal(&tz->device, - hwmon->name, hwmon); + hwmon->type, hwmon); if (IS_ERR(hwmon->device)) { result = PTR_ERR(hwmon->device); goto free_mem; } - temp = &hwmon->tz_temp; + register_sys_interface: + temp = kzalloc_obj(*temp); + if (!temp) { + result = -ENOMEM; + goto unregister_name; + } temp->tz = tz; + hwmon->count++; - temp->temp_input.attr.attr.name = "temp1_input"; + snprintf(temp->temp_input.name, sizeof(temp->temp_input.name), + "temp%d_input", hwmon->count); + temp->temp_input.attr.attr.name = temp->temp_input.name; temp->temp_input.attr.attr.mode = 0444; temp->temp_input.attr.show = temp_input_show; sysfs_attr_init(&temp->temp_input.attr.attr); result = device_create_file(hwmon->device, &temp->temp_input.attr); if (result) - goto unregister_name; + goto free_temp_mem; if (thermal_zone_crit_temp_valid(tz)) { - temp->temp_crit.attr.attr.name = "temp1_crit"; + snprintf(temp->temp_crit.name, + sizeof(temp->temp_crit.name), + "temp%d_crit", hwmon->count); + temp->temp_crit.attr.attr.name = temp->temp_crit.name; temp->temp_crit.attr.attr.mode = 0444; temp->temp_crit.attr.show = temp_crit_show; sysfs_attr_init(&temp->temp_crit.attr.attr); @@ -145,17 +196,21 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) temp->temp_crit_present = true; } - /* The list is needed for hwmon lookup during removal. */ mutex_lock(&thermal_hwmon_list_lock); - list_add_tail(&hwmon->node, &thermal_hwmon_list); + if (new_hwmon_device) + list_add_tail(&hwmon->node, &thermal_hwmon_list); + list_add_tail(&temp->hwmon_node, &hwmon->tz_list); mutex_unlock(&thermal_hwmon_list_lock); return 0; unregister_input: device_remove_file(hwmon->device, &temp->temp_input.attr); + free_temp_mem: + kfree(temp); unregister_name: - hwmon_device_unregister(hwmon->device); + if (new_hwmon_device) + hwmon_device_unregister(hwmon->device); free_mem: kfree(hwmon); @@ -163,37 +218,39 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) } EXPORT_SYMBOL_GPL(thermal_add_hwmon_sysfs); -static struct thermal_hwmon_device * -thermal_hwmon_lookup(const struct thermal_zone_device *tz) -{ - struct thermal_hwmon_device *hwmon; - - list_for_each_entry(hwmon, &thermal_hwmon_list, node) { - if (hwmon->tz_temp.tz == tz) - return hwmon; - } - return NULL; -} - void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz) { struct thermal_hwmon_device *hwmon; struct thermal_hwmon_temp *temp; - scoped_guard(mutex, &thermal_hwmon_list_lock) { - hwmon = thermal_hwmon_lookup(tz); - if (!hwmon) - return; - - list_del(&hwmon->node); + hwmon = thermal_hwmon_lookup_by_type(tz); + if (unlikely(!hwmon)) { + /* Should never happen... */ + dev_dbg(&tz->device, "hwmon device lookup failed!\n"); + return; } - temp = &hwmon->tz_temp; + temp = thermal_hwmon_lookup_temp(hwmon, tz); + if (unlikely(!temp)) { + /* Should never happen... */ + dev_dbg(&tz->device, "temperature input lookup failed!\n"); + return; + } device_remove_file(hwmon->device, &temp->temp_input.attr); if (temp->temp_crit_present) device_remove_file(hwmon->device, &temp->temp_crit.attr); + mutex_lock(&thermal_hwmon_list_lock); + list_del(&temp->hwmon_node); + kfree(temp); + if (!list_empty(&hwmon->tz_list)) { + mutex_unlock(&thermal_hwmon_list_lock); + return; + } + list_del(&hwmon->node); + mutex_unlock(&thermal_hwmon_list_lock); + hwmon_device_unregister(hwmon->device); kfree(hwmon); } From ff8da20b6f47c48d46e47f93f7a59e2d56ee9107 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 4 Aug 2026 22:09:10 +0200 Subject: [PATCH 3/3] Revert "thermal/drivers/hwmon: Cleanup coding style a bit" Revert commit 030a48b0f6ce ("thermal/drivers/hwmon: Cleanup coding style a bit") that introduced a use-after-free into the error path of thermal_add_hwmon_sysfs() by removing a valid check from it. Link: https://lore.kernel.org/linux-hwmon/20260803183915.4ED7D1F000E9@smtp.kernel.org/ Cc: All applicable Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba Link: https://patch.msgid.link/5123895.31r3eYUQgx@rafael.j.wysocki --- drivers/thermal/thermal_hwmon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index 597c33c8a555..ec73d03a1e60 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -212,7 +212,8 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz) if (new_hwmon_device) hwmon_device_unregister(hwmon->device); free_mem: - kfree(hwmon); + if (new_hwmon_device) + kfree(hwmon); return result; }