[bug report] thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering

[bug report] thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering

[Dan Carpenter]

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

Re: [bug report] thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering

[Daniel Lezcano]

On 18/03/2021 11:20, Dan Carpenter wrote:
> 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.

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

[PATCH] thermal/drivers/cpuidle_cooling: Fix use after error

[Daniel Lezcano]

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.

Fixes: 6fd1b186d900 ("thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/cpuidle_cooling.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/cpuidle_cooling.c b/drivers/thermal/cpuidle_cooling.c
index f32976163bad..4f41102e8b16 100644
--- a/drivers/thermal/cpuidle_cooling.c
+++ b/drivers/thermal/cpuidle_cooling.c
@@ -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

Re: [PATCH] thermal/drivers/cpuidle_cooling: Fix use after error

[Viresh Kumar]

On 19-03-21, 21:25, Daniel Lezcano wrote:
> 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.
> 
> Fixes: 6fd1b186d900 ("thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering")

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

Re: [PATCH] thermal/drivers/cpuidle_cooling: Fix use after error

[Daniel Lezcano]

On 22/03/2021 04:29, Viresh Kumar wrote:
> On 19-03-21, 21:25, Daniel Lezcano wrote:
>> 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.
>>
>> Fixes: 6fd1b186d900 ("thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering")
> 
> 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 ?

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/20210314111333.16551-3-daniel.lezcano@linaro.org/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

Re: [PATCH] thermal/drivers/cpuidle_cooling: Fix use after error

[Viresh Kumar]

On 22-03-21, 09:08, Daniel Lezcano wrote:
> On 22/03/2021 04:29, Viresh Kumar wrote:
> > On 19-03-21, 21:25, Daniel Lezcano wrote:
> >> 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.
> >>
> >> Fixes: 6fd1b186d900 ("thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering")
> > 
> > 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 ?
> 
> 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].

Ahh, I see.

Here you go :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[thermal: thermal/next] thermal/drivers/cpuidle_cooling: Fix use after error

[thermal-bot for Daniel Lezcano]

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     6cc7b38c0ca3187abd07af849ec179b42337bcf6
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//6cc7b38c0ca3187abd07af849ec179b42337bcf6
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Fri, 19 Mar 2021 21:25:22 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Thu, 15 Apr 2021 13:21:26 +02:00

thermal/drivers/cpuidle_cooling: Fix use after error

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.

Fixes: 6fd1b186d900 ("thermal/drivers/cpuidle_cooling: Use device name instead of auto-numbering")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Link: https://lore.kernel.org/r/20210319202522.891061-1-daniel.lezcano@linaro.org
---
 drivers/thermal/cpuidle_cooling.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/cpuidle_cooling.c b/drivers/thermal/cpuidle_cooling.c
index f329761..4f41102 100644
--- a/drivers/thermal/cpuidle_cooling.c
+++ b/drivers/thermal/cpuidle_cooling.c
@@ -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: