[PATCH v2 0/2] thermal: hwmon: Fix thermal zone removal deadlock

[PATCH v2 0/2] thermal: hwmon: Fix thermal zone removal deadlock

[Rafael J. Wysocki]

This series is a replacement for

https://lore.kernel.org/linux-pm/23195575.EfDdHjke4D@rafael.j.wysocki/

with an additional patch to fix a UAF issue reintroduced by the
reverts at

https://lore.kernel.org/linux-pm/6319276.lOV4Wx5bFT@rafael.j.wysocki/

The first patch needs to go into 7.2-rc along with these revers,
while the second one is regarded as 7.3 material.

Thanks!

[PATCH v2 1/2] Revert "thermal/drivers/hwmon: Cleanup coding style a bit"

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

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 <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 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;
 }
-- 
2.51.0

[PATCH v2 2/2] thermal: hwmon: Remove hwmon class device along with its parent

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The current code creates one hwmon device per thermal zone type and that
device is registered under the first thermal zone of the given type.

That turns out to be problematic when the thermal zone holding the
hwmon device is removed.

For example, say that there are two ACPI thermal zones on a system

 /sys/devices/virtual/thermal/thermal_zone0/
 /sys/devices/virtual/thermal/thermal_zone1/

The current code registers a hwmon class device for thermal_zone0 only:

 /sys/devices/virtual/thermal/thermal_zone0/hwmon0/

because the type is "acpitz" for both of them, but it adds a sysfs
attribute that belongs to thermal_zone1 under it:

 /sys/devices/virtual/thermal/thermal_zone0/hwmon0/temp2_input

There is also

 /sys/devices/virtual/thermal/thermal_zone0/hwmon0/temp1_input

which belongs to thermal_zone0.

When thermal_zone0 is removed, say because the ACPI thermal driver is
unbound from the underlying platform device, thermal_remove_hwmon_sysfs()
skips the removal of hwmon0 because of the temp2_input attribute
belonging to thermal_zone1 which effectively prevents thermal_zone0
removal from making progress.

Address this by making thermal_remove_hwmon_sysfs() remove the entire
hwmon class device interface for the given thermal zone type when the
thermal zone device holding it is removed.

To prevent races with thermal_add_hwmon_sysfs() that may interfere
with this, carry out the entire addition and removal of hwmon sysfs
interfaces for thermal zones under thermal_hwmon_list_lock.

Also adjust the layout of the labels in thermal_add_hwmon_sysfs() to
the current kernel coding style to align with the new "unlock" label.

Link: https://lore.kernel.org/linux-pm/20260402021828.16556-1-liujia6264@gmail.com/
Fixes: f6b6b52ef7a5 ("thermal_hwmon: Pass the originating device down to hwmon_device_register_with_info")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rebase on top of the new [1/2]

---
 drivers/thermal/thermal_hwmon.c |   89 +++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 54 deletions(-)

--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -95,34 +95,12 @@ thermal_hwmon_lookup_by_type(const struc
 	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);
+		if (!strcmp(hwmon->type, type))
 			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;
 }
@@ -138,7 +116,9 @@ int thermal_add_hwmon_sysfs(struct therm
 	struct thermal_hwmon_device *hwmon;
 	struct thermal_hwmon_temp *temp;
 	int new_hwmon_device = 1;
-	int result;
+	int result = 0;
+
+	mutex_lock(&thermal_hwmon_list_lock);
 
 	hwmon = thermal_hwmon_lookup_by_type(tz);
 	if (hwmon) {
@@ -147,8 +127,10 @@ int thermal_add_hwmon_sysfs(struct therm
 	}
 
 	hwmon = kzalloc_obj(*hwmon);
-	if (!hwmon)
-		return -ENOMEM;
+	if (!hwmon) {
+		result = -ENOMEM;
+		goto unlock;
+	}
 
 	INIT_LIST_HEAD(&hwmon->tz_list);
 	strscpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
@@ -196,24 +178,24 @@ int thermal_add_hwmon_sysfs(struct therm
 		temp->temp_crit_present = true;
 	}
 
-	mutex_lock(&thermal_hwmon_list_lock);
 	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;
+	goto unlock;
 
- unregister_input:
+unregister_input:
 	device_remove_file(hwmon->device, &temp->temp_input.attr);
- free_temp_mem:
+free_temp_mem:
 	kfree(temp);
- unregister_name:
+unregister_name:
 	if (new_hwmon_device)
 		hwmon_device_unregister(hwmon->device);
- free_mem:
+free_mem:
 	if (new_hwmon_device)
 		kfree(hwmon);
+unlock:
+	mutex_unlock(&thermal_hwmon_list_lock);
 
 	return result;
 }
@@ -222,8 +204,11 @@ EXPORT_SYMBOL_GPL(thermal_add_hwmon_sysf
 
 void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 {
+	struct thermal_hwmon_temp *temp, *entry;
 	struct thermal_hwmon_device *hwmon;
-	struct thermal_hwmon_temp *temp;
+	bool unregister;
+
+	guard(mutex)(&thermal_hwmon_list_lock);
 
 	hwmon = thermal_hwmon_lookup_by_type(tz);
 	if (unlikely(!hwmon)) {
@@ -232,29 +217,25 @@ void thermal_remove_hwmon_sysfs(struct t
 		return;
 	}
 
-	temp = thermal_hwmon_lookup_temp(hwmon, tz);
-	if (unlikely(!temp)) {
-		/* Should never happen... */
-		dev_dbg(&tz->device, "temperature input lookup failed!\n");
-		return;
-	}
+	unregister = hwmon->device->parent == &tz->device;
 
-	device_remove_file(hwmon->device, &temp->temp_input.attr);
-	if (temp->temp_crit_present)
-		device_remove_file(hwmon->device, &temp->temp_crit.attr);
+	list_for_each_entry_safe_reverse(temp, entry, &hwmon->tz_list, hwmon_node) {
+		if (!unregister && temp->tz != tz)
+			continue;
 
-	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;
+		device_remove_file(hwmon->device, &temp->temp_input.attr);
+		if (temp->temp_crit_present)
+			device_remove_file(hwmon->device, &temp->temp_crit.attr);
+
+		list_del(&temp->hwmon_node);
+		kfree(temp);
 	}
-	list_del(&hwmon->node);
-	mutex_unlock(&thermal_hwmon_list_lock);
 
-	hwmon_device_unregister(hwmon->device);
-	kfree(hwmon);
+	if (unregister) {
+		list_del(&hwmon->node);
+		hwmon_device_unregister(hwmon->device);
+		kfree(hwmon);
+	}
 }
 EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
 

Re: [PATCH v2 2/2] thermal: hwmon: Remove hwmon class device along with its parent

[sashiko-bot]

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The current code creates one hwmon device per thermal zone type and that
> device is registered under the first thermal zone of the given type.
> 
> That turns out to be problematic when the thermal zone holding the
> hwmon device is removed.
> 
> For example, say that there are two ACPI thermal zones on a system
> 
>  /sys/devices/virtual/thermal/thermal_zone0/
>  /sys/devices/virtual/thermal/thermal_zone1/
> 
> The current code registers a hwmon class device for thermal_zone0 only:
> 
> [ ... ]
> the current kernel coding style to align with the new "unlock" label.
> 
> Link: https://lore.kernel.org/linux-pm/20260402021828.16556-1-liujia6264@gmail.com/
> Fixes: f6b6b52ef7a5 ("thermal_hwmon: Pass the originating device down to hwmon_device_register_with_info")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12947893.O9o76ZdvQC@rafael.j.wysocki?part=2

Re: [PATCH v2 1/2] Revert "thermal/drivers/hwmon: Cleanup coding style a bit"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Lack of synchronization during hwmon device lookup and creation results in Use-After-Free of `hwmon` and Time-of-Check to Time-of-Use (TOCTOU) list duplication.
- [High] The hwmon sysfs attribute callbacks do not use `hwmon_lock()` as required by the subsystem guidelines.
--

--- Patch [1]: [PATCH v2 1/2] Revert "thermal/drivers/hwmon: Cleanup coding style a bit" ---
commit 85ae4605921a40cd95c1000cfc56b0df8bf8c9f1
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: Revert "thermal/drivers/hwmon: Cleanup coding style a bit"

Reverts a commit that introduced a use-after-free in the error path of
thermal_add_hwmon_sysfs() by removing a valid check.

Link: https://lore.kernel.org/linux-hwmon/20260803183915.4ED7D1F000E9@smtp.kernel.org/

> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index 597c33c8a5550..ec73d03a1e60a 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;
>  }

[Severity: High]
This is a pre-existing issue, but is there a use-after-free and TOCTOU race
when looking up and adding a new hwmon device?

If one thread calls thermal_add_hwmon_sysfs() and looks up the hwmon device
via thermal_hwmon_lookup_by_type(), the list lock is dropped and an
unrefcounted pointer is returned:

drivers/thermal/thermal_hwmon.c:thermal_hwmon_lookup_by_type() {
    ...
		if (!strcmp(hwmon->type, type)) {
			mutex_unlock(&thermal_hwmon_list_lock);
			return hwmon;
		}
    ...
}

Concurrently, another thread could call thermal_remove_hwmon_sysfs() for the
last thermal zone sharing that hwmon device. It removes the last entry,
drops the list lock, unregisters the device, and frees the hwmon pointer
without synchronizing with active readers:

drivers/thermal/thermal_hwmon.c:thermal_remove_hwmon_sysfs() {
    ...
	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);
}

The first thread could then resume in thermal_add_hwmon_sysfs() and
dereference the freed pointer when incrementing the count:

drivers/thermal/thermal_hwmon.c:thermal_add_hwmon_sysfs() {
    ...
 register_sys_interface:
	temp = kzalloc_obj(*temp);
	if (!temp) {
		result = -ENOMEM;
		goto unregister_name;
	}

	temp->tz = tz;
	hwmon->count++;
    ...
}

Could this lead to memory corruption during concurrent device probing or
unbinding?

[Severity: High]
This is a pre-existing issue, but do the hwmon sysfs attribute callbacks
need to use hwmon_lock() and hwmon_unlock() wrappers?

The hwmon subsystem guidelines require drivers to implement locking for
attributes registered by other means than hwmon_device_register_with_info().

The temp_input_show() and temp_crit_show() callbacks read attributes directly
without the required lock:

drivers/thermal/thermal_hwmon.c:temp_input_show() {
    ...
	struct thermal_zone_device *tz = temp->tz;

	ret = thermal_zone_get_temp(tz, &temperature);
    ...
}

drivers/thermal/thermal_hwmon.c:temp_crit_show() {
    ...
	struct thermal_zone_device *tz = temp->tz;
	int temperature;
	int ret;

	guard(thermal_zone)(tz);

	ret = tz->ops.get_crit_temp(tz, &temperature);
    ...
}

Could this result in a race condition with interrupt handlers or driver
unregistration since it bypasses the core kernel locking rules for the hwmon
subsystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12947893.O9o76ZdvQC@rafael.j.wysocki?part=1

Re: [PATCH v2 1/2] Revert "thermal/drivers/hwmon: Cleanup coding style a bit"

[Lukasz Luba]

On 8/4/26 21:09, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> 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 <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   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;
>   }

LGTM, a tricky plumbing. I'm sorry for being late with review.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v2 2/2] thermal: hwmon: Remove hwmon class device along with its parent

[Lukasz Luba]

On 8/4/26 21:11, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The current code creates one hwmon device per thermal zone type and that
> device is registered under the first thermal zone of the given type.
> 
> That turns out to be problematic when the thermal zone holding the
> hwmon device is removed.
> 
> For example, say that there are two ACPI thermal zones on a system
> 
>   /sys/devices/virtual/thermal/thermal_zone0/
>   /sys/devices/virtual/thermal/thermal_zone1/
> 
> The current code registers a hwmon class device for thermal_zone0 only:
> 
>   /sys/devices/virtual/thermal/thermal_zone0/hwmon0/
> 
> because the type is "acpitz" for both of them, but it adds a sysfs
> attribute that belongs to thermal_zone1 under it:
> 
>   /sys/devices/virtual/thermal/thermal_zone0/hwmon0/temp2_input
> 
> There is also
> 
>   /sys/devices/virtual/thermal/thermal_zone0/hwmon0/temp1_input
> 
> which belongs to thermal_zone0.
> 
> When thermal_zone0 is removed, say because the ACPI thermal driver is
> unbound from the underlying platform device, thermal_remove_hwmon_sysfs()
> skips the removal of hwmon0 because of the temp2_input attribute
> belonging to thermal_zone1 which effectively prevents thermal_zone0
> removal from making progress.
> 
> Address this by making thermal_remove_hwmon_sysfs() remove the entire
> hwmon class device interface for the given thermal zone type when the
> thermal zone device holding it is removed.
> 
> To prevent races with thermal_add_hwmon_sysfs() that may interfere
> with this, carry out the entire addition and removal of hwmon sysfs
> interfaces for thermal zones under thermal_hwmon_list_lock.
> 
> Also adjust the layout of the labels in thermal_add_hwmon_sysfs() to
> the current kernel coding style to align with the new "unlock" label.
> 
> Link: https://lore.kernel.org/linux-pm/20260402021828.16556-1-liujia6264@gmail.com/
> Fixes: f6b6b52ef7a5 ("thermal_hwmon: Pass the originating device down to hwmon_device_register_with_info")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v1 -> v2:
>     * Rebase on top of the new [1/2]
> 
> ---
>   drivers/thermal/thermal_hwmon.c |   89 +++++++++++++++-------------------------
>   1 file changed, 35 insertions(+), 54 deletions(-)
> 
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -95,34 +95,12 @@ thermal_hwmon_lookup_by_type(const struc
>   	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);
> +		if (!strcmp(hwmon->type, type))
>   			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;
>   }
> @@ -138,7 +116,9 @@ int thermal_add_hwmon_sysfs(struct therm
>   	struct thermal_hwmon_device *hwmon;
>   	struct thermal_hwmon_temp *temp;
>   	int new_hwmon_device = 1;
> -	int result;
> +	int result = 0;
> +
> +	mutex_lock(&thermal_hwmon_list_lock);
>   
>   	hwmon = thermal_hwmon_lookup_by_type(tz);
>   	if (hwmon) {
> @@ -147,8 +127,10 @@ int thermal_add_hwmon_sysfs(struct therm
>   	}
>   
>   	hwmon = kzalloc_obj(*hwmon);
> -	if (!hwmon)
> -		return -ENOMEM;
> +	if (!hwmon) {
> +		result = -ENOMEM;
> +		goto unlock;
> +	}
>   
>   	INIT_LIST_HEAD(&hwmon->tz_list);
>   	strscpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
> @@ -196,24 +178,24 @@ int thermal_add_hwmon_sysfs(struct therm
>   		temp->temp_crit_present = true;
>   	}
>   
> -	mutex_lock(&thermal_hwmon_list_lock);
>   	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;
> +	goto unlock;
>   
> - unregister_input:
> +unregister_input:
>   	device_remove_file(hwmon->device, &temp->temp_input.attr);
> - free_temp_mem:
> +free_temp_mem:
>   	kfree(temp);
> - unregister_name:
> +unregister_name:
>   	if (new_hwmon_device)
>   		hwmon_device_unregister(hwmon->device);
> - free_mem:
> +free_mem:
>   	if (new_hwmon_device)
>   		kfree(hwmon);
> +unlock:
> +	mutex_unlock(&thermal_hwmon_list_lock);
>   
>   	return result;
>   }
> @@ -222,8 +204,11 @@ EXPORT_SYMBOL_GPL(thermal_add_hwmon_sysf
>   
>   void thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
>   {
> +	struct thermal_hwmon_temp *temp, *entry;
>   	struct thermal_hwmon_device *hwmon;
> -	struct thermal_hwmon_temp *temp;
> +	bool unregister;
> +
> +	guard(mutex)(&thermal_hwmon_list_lock);
>   
>   	hwmon = thermal_hwmon_lookup_by_type(tz);
>   	if (unlikely(!hwmon)) {
> @@ -232,29 +217,25 @@ void thermal_remove_hwmon_sysfs(struct t
>   		return;
>   	}
>   
> -	temp = thermal_hwmon_lookup_temp(hwmon, tz);
> -	if (unlikely(!temp)) {
> -		/* Should never happen... */
> -		dev_dbg(&tz->device, "temperature input lookup failed!\n");
> -		return;
> -	}
> +	unregister = hwmon->device->parent == &tz->device;
>   
> -	device_remove_file(hwmon->device, &temp->temp_input.attr);
> -	if (temp->temp_crit_present)
> -		device_remove_file(hwmon->device, &temp->temp_crit.attr);
> +	list_for_each_entry_safe_reverse(temp, entry, &hwmon->tz_list, hwmon_node) {
> +		if (!unregister && temp->tz != tz)
> +			continue;
>   
> -	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;
> +		device_remove_file(hwmon->device, &temp->temp_input.attr);
> +		if (temp->temp_crit_present)
> +			device_remove_file(hwmon->device, &temp->temp_crit.attr);
> +
> +		list_del(&temp->hwmon_node);
> +		kfree(temp);
>   	}
> -	list_del(&hwmon->node);
> -	mutex_unlock(&thermal_hwmon_list_lock);
>   
> -	hwmon_device_unregister(hwmon->device);
> -	kfree(hwmon);
> +	if (unregister) {
> +		list_del(&hwmon->node);
> +		hwmon_device_unregister(hwmon->device);
> +		kfree(hwmon);
> +	}
>   }
>   EXPORT_SYMBOL_GPL(thermal_remove_hwmon_sysfs);
>   
> 
> 
> 


Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v2 1/2] Revert "thermal/drivers/hwmon: Cleanup coding style a bit"

[Rafael J. Wysocki (Intel)]

On Wed, Aug 5, 2026 at 12:32 PM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 8/4/26 21:09, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > 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 <stable@vger.kernel.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >   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;
> >   }
>
> LGTM, a tricky plumbing. I'm sorry for being late with review.

No worries.

> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

And thanks!