Re: [PATCH v2 07/12] thermal/core: Put of_node field cooling device structure under Kconfig option

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] Double initialization of the cooling device leads to linked list corruption in devm_thermal_of_cooling_device_register().
--

commit ec613b33ae03267fedce00f5da67c6af4d2e4061
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

thermal/core: Put of_node field cooling device structure under Kconfig option

The device node pointer in the cooling device structure is only needed
by the thermal OF in order to bind a thermal zone with a cooling
device. Now only the OF based drivers are using the thermal OF
functions and do not call the function with a NULL device_node
parameter as before. We can put this field under the CONFIG_THERMAL_OF
condition and make it go away if the device tree is not used.

> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index ba2c8b4dda873..0a95cfeffa74e 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -125,7 +125,6 @@ struct thermal_cooling_device {
>  	const char *type;
>  	unsigned long max_state;
>  	struct device device;
> -	struct device_node *np;
>  	void *devdata;
>  	void *stats;
>  	const struct thermal_cooling_device_ops *ops;
> @@ -133,6 +132,9 @@ struct thermal_cooling_device {
>  	struct mutex lock; /* protect thermal_instances list */
>  	struct list_head thermal_instances;
>  	struct list_head node;
> +#ifdef CONFIG_THERMAL_OF
> +	struct device_node *np;
> +#endif

This isn't a bug in this patch, but I noticed a regression introduced by an
earlier patch in the series that affects thermal OF cooling device
registration.

Does devm_thermal_of_cooling_device_register() call
thermal_cooling_device_init_complete() twice on the same cooling device?

When devm_thermal_of_cooling_device_register() calls
thermal_of_cooling_device_register(), it successfully registers the device
and calls thermal_cooling_device_init_complete(cdev), which adds the node
to thermal_cdev_list via list_add().

After returning, devm_thermal_of_cooling_device_register() calls
thermal_cooling_device_init_complete(tcd) again with the same pointer.

Calling list_add() twice on the same node corrupts the linked list. If
CONFIG_DEBUG_LIST is disabled, the node's next pointer points to itself,
causing infinite loops during subsequent traversals of thermal_cdev_list.

I see this regression was fixed later in the series in commit 61302b0ce72b9
("thermal/of: Pass the of_index and add a function to register with an index").

>  #ifdef CONFIG_THERMAL_DEBUGFS
>  	struct thermal_debugfs *debugfs;
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422174305.2899095-1-daniel.lezcano@oss.qualcomm.com?part=7