thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering

Hello Daniel Lezcano,

The patch 6fd1b186d900: "thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering" from Mar 14, 2021, leads to the following static checker warning:

drivers/thermal/cpuidle_cooling.c:218 __cpuidle_cooling_register()
warn: 'name' was already freed.

drivers/thermal/cpuidle_cooling.c
200
201 dev = get_cpu_device(cpumask_first(drv->cpumask));
202
203 name = kasprintf(GFP_KERNEL, "idle-%s", dev_name(dev));
204 if (!name) {
205 ret = -ENOMEM;
206 goto out_unregister;
207 }
208
209 cdev = thermal_of_cooling_device_register(np, name, idle_cdev,
210 &cpuidle_cooling_ops);

   211          kfree(name);
                ^^^^^^^^^^^^

212
213 if (IS_ERR(cdev)) {
214 ret = PTR_ERR(cdev);
215 goto out_unregister;
216 }
217
218 pr_debug("%s: Idle injection set with idle duration=%u, latency=%u\n",

   219                   name, idle_duration_us, latency_us);
                         ^^^^

Used after free

   220  
   221          return 0;
   222  
   223  out_unregister:
   224          idle_inject_unregister(ii_dev);
   225  out_kfree:
   226          kfree(idle_cdev);
   227  out:
   228          return ret;
   229  }

regards,
dan carpenter

Indeed. I'll fix it.

Thanks for the report

-- Daniel

<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

When the function successfully finishes it logs an information about the registration of the cooling device and use its name to build the message. Unfortunately it was freed right before:

drivers/thermal/cpuidle_cooling.c:218 __cpuidle_cooling_register()
warn: 'name' was already freed.

Fix this by freeing after the message happened.

---
 drivers/thermal/cpuidle_cooling.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index f32976163bad..4f41102e8b16 100644
@@ -208,18 +208,20 @@ static int __cpuidle_cooling_register(struct device_node *np,
 
 	cdev = thermal_of_cooling_device_register(np, name, idle_cdev,
 						  &cpuidle_cooling_ops);
-	kfree(name);
-
 	if (IS_ERR(cdev)) {
 		ret = PTR_ERR(cdev);
-		goto out_unregister;
+		goto out_kfree_name;
 	}
 
 	pr_debug("%s: Idle injection set with idle duration=%u, latency=%u\n",
 		 name, idle_duration_us, latency_us);
 
+	kfree(name);
+
 	return 0;
 
+out_kfree_name:
+	kfree(name);
 out_unregister:
 	idle_inject_unregister(ii_dev);
 out_kfree:

2.25.1

Why not merge this with the Fixes patch itself since it isn't there in Linus's tree yet ?

Or is your branch strictly immutable ?

viresh

Hi Viresh;

The changes follow the path:

testing -> linux-next -> next

The branch next is never rebased. The patch above reached it. This is notified by the thermal-bot [1].

-- Daniel

[1]
https://lore.kernel.org/linux-pm/[email protected]/T/#ma257519efc70ee60faca47dbd458b05de5449bf8

<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Ahh, I see.

Here you go :)

viresh