Re: [PATCH v2 09/12] thermal/of: Introduce cooling device of_index

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] Redundant call to `thermal_cooling_device_init_complete()` in `devm_thermal_of_child_cooling_device_register()` causes list corruption and system hangs.
--

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

thermal/of: Introduce cooling device of_index

This commit augments thermal_of_cooling_device_register() to receive a
cooling device id. It stores this id in the cooling device structure
and updates the remaining drivers to use the new signature.

> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index d06487bb5e675..a9319ff81d1d3 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c

[ ... ]

> @@ -538,6 +539,7 @@ thermal_of_cooling_device_register(struct device_node *np,
>  		return cdev;
>  
>  	cdev->np = np;
> +	cdev->of_index = of_index;
>  	thermal_cooling_device_init_complete(cdev);
>  
>  	return cdev;

[ ... ]

> @@ -582,7 +584,7 @@ devm_thermal_of_child_cooling_device_register(struct device *dev,
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	tcd = thermal_of_cooling_device_register(np, type, devdata, ops);
> +	tcd = thermal_of_cooling_device_register(np, 0, type, devdata, ops);
>  	if (IS_ERR(tcd)) {
>  		devres_free(ptr);
>  		return tcd;

Could this regression cause list corruption by calling
thermal_cooling_device_init_complete() redundantly?

It looks like thermal_of_cooling_device_register() internally invokes
thermal_cooling_device_init_complete() just above. Immediately after,
devm_thermal_of_child_cooling_device_register() calls
thermal_cooling_device_init_complete() again on the same object:

drivers/thermal/thermal_of.c:devm_thermal_of_child_cooling_device_register() {
        ...
        tcd = thermal_of_cooling_device_register(np, 0, type, devdata, ops);
        if (IS_ERR(tcd)) {
                devres_free(ptr);
                return tcd;
        }

        *ptr = tcd;
        devres_add(dev, ptr);

        thermal_cooling_device_init_complete(tcd);
        ...
}

Does calling list_add() twice on the same node overwrite its pointers and
create a cycle where new->next = new, potentially causing an infinite loop
during subsequent traversals of thermal_cdev_list if debug list checking is
not enabled?

I noticed this problem is present in the code at this commit, but it is
fixed later in the series by commit 61302b0ce72b9 ("thermal/of: Pass the
of_index and add a function to register with an index") which removes the
duplicate call.

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