[PATCH] drivers: clocksource: fix memory leak in davinci_timer_register

Qinrun Dai posted 1 patch 3 years ago
There is a newer version of this series
drivers/clocksource/timer-davinci.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
[PATCH] drivers: clocksource: fix memory leak in davinci_timer_register
Posted by Qinrun Dai 3 years ago
Smatch reports:
drivers/clocksource/timer-davinci.c:332 davinci_timer_register()
warn: 'base' from ioremap() not released on lines: 274.

Fix this by defining a unified function exit
to iounmap 'base' and return corresponding value.

Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
Signed-off-by: Qinrun Dai <flno@hust.edu.cn>
---
 drivers/clocksource/timer-davinci.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
index 9996c0542520..a6dd1da9e6d1 100644
--- a/drivers/clocksource/timer-davinci.c
+++ b/drivers/clocksource/timer-davinci.c
@@ -270,8 +270,10 @@ int __init davinci_timer_register(struct clk *clk,
 	tick_rate = clk_get_rate(clk);
 
 	clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
-	if (!clockevent)
-		return -ENOMEM;
+	if (!clockevent) {
+		rv = -ENOMEM;
+		goto iounmap_base;
+	}
 
 	clockevent->dev.name = "tim12";
 	clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
@@ -296,7 +298,7 @@ int __init davinci_timer_register(struct clk *clk,
 			 "clockevent/tim12", clockevent);
 	if (rv) {
 		pr_err("Unable to request the clockevent interrupt\n");
-		return rv;
+		goto iounmap_base;
 	}
 
 	davinci_clocksource.dev.rating = 300;
@@ -323,13 +325,17 @@ int __init davinci_timer_register(struct clk *clk,
 	rv = clocksource_register_hz(&davinci_clocksource.dev, tick_rate);
 	if (rv) {
 		pr_err("Unable to register clocksource\n");
-		return rv;
+		goto iounmap_base;
 	}
 
 	sched_clock_register(davinci_timer_read_sched_clock,
 			     DAVINCI_TIMER_CLKSRC_BITS, tick_rate);
 
 	return 0;
+
+iounmap_base:
+	iounmap(base);
+	return rv;
 }
 
 static int __init of_davinci_timer_register(struct device_node *np)
-- 
2.37.2
Re: [PATCH] drivers: clocksource: fix memory leak in davinci_timer_register
Posted by 戴钦润 3 years ago
ping?

> -----Original Messages-----
> From: "Qinrun Dai" <flno@hust.edu.cn>
> Sent Time: 2023-03-22 23:19:45 (Wednesday)
> To: "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Thomas Gleixner" <tglx@linutronix.de>, "Bartosz Golaszewski" <bgolaszewski@baylibre.com>
> Cc: hust-os-kernel-patches@googlegroups.com, "Qinrun Dai" <flno@hust.edu.cn>, linux-kernel@vger.kernel.org
> Subject: [PATCH] drivers: clocksource: fix memory leak in davinci_timer_register
> 
> Smatch reports:
> drivers/clocksource/timer-davinci.c:332 davinci_timer_register()
> warn: 'base' from ioremap() not released on lines: 274.
> 
> Fix this by defining a unified function exit
> to iounmap 'base' and return corresponding value.
> 
> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> Signed-off-by: Qinrun Dai <flno@hust.edu.cn>
> ---
>  drivers/clocksource/timer-davinci.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> index 9996c0542520..a6dd1da9e6d1 100644
> --- a/drivers/clocksource/timer-davinci.c
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -270,8 +270,10 @@ int __init davinci_timer_register(struct clk *clk,
>  	tick_rate = clk_get_rate(clk);
>  
>  	clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
> -	if (!clockevent)
> -		return -ENOMEM;
> +	if (!clockevent) {
> +		rv = -ENOMEM;
> +		goto iounmap_base;
> +	}
>  
>  	clockevent->dev.name = "tim12";
>  	clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
> @@ -296,7 +298,7 @@ int __init davinci_timer_register(struct clk *clk,
>  			 "clockevent/tim12", clockevent);
>  	if (rv) {
>  		pr_err("Unable to request the clockevent interrupt\n");
> -		return rv;
> +		goto iounmap_base;
>  	}
>  
>  	davinci_clocksource.dev.rating = 300;
> @@ -323,13 +325,17 @@ int __init davinci_timer_register(struct clk *clk,
>  	rv = clocksource_register_hz(&davinci_clocksource.dev, tick_rate);
>  	if (rv) {
>  		pr_err("Unable to register clocksource\n");
> -		return rv;
> +		goto iounmap_base;
>  	}
>  
>  	sched_clock_register(davinci_timer_read_sched_clock,
>  			     DAVINCI_TIMER_CLKSRC_BITS, tick_rate);
>  
>  	return 0;
> +
> +iounmap_base:
> +	iounmap(base);
> +	return rv;
>  }
>  
>  static int __init of_davinci_timer_register(struct device_node *np)
> -- 
> 2.37.2
Re: [PATCH] drivers: clocksource: fix memory leak in davinci_timer_register
Posted by Daniel Lezcano 3 years ago
Hi,

thanks for the fix.

However, the changes are incomplete, you should create the rollback 
labels to undo all the allocated resources.

On 10/04/2023 14:04, 戴钦润 wrote:
> 
> ping?



>> -----Original Messages-----
>> From: "Qinrun Dai" <flno@hust.edu.cn>
>> Sent Time: 2023-03-22 23:19:45 (Wednesday)
>> To: "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Thomas Gleixner" <tglx@linutronix.de>, "Bartosz Golaszewski" <bgolaszewski@baylibre.com>
>> Cc: hust-os-kernel-patches@googlegroups.com, "Qinrun Dai" <flno@hust.edu.cn>, linux-kernel@vger.kernel.org
>> Subject: [PATCH] drivers: clocksource: fix memory leak in davinci_timer_register
>>
>> Smatch reports:
>> drivers/clocksource/timer-davinci.c:332 davinci_timer_register()
>> warn: 'base' from ioremap() not released on lines: 274.
>>
>> Fix this by defining a unified function exit
>> to iounmap 'base' and return corresponding value.
>>
>> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
>> Signed-off-by: Qinrun Dai <flno@hust.edu.cn>
>> ---
>>   drivers/clocksource/timer-davinci.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
>> index 9996c0542520..a6dd1da9e6d1 100644
>> --- a/drivers/clocksource/timer-davinci.c
>> +++ b/drivers/clocksource/timer-davinci.c
>> @@ -270,8 +270,10 @@ int __init davinci_timer_register(struct clk *clk,
>>   	tick_rate = clk_get_rate(clk);
>>   
>>   	clockevent = kzalloc(sizeof(*clockevent), GFP_KERNEL);
>> -	if (!clockevent)
>> -		return -ENOMEM;
>> +	if (!clockevent) {
>> +		rv = -ENOMEM;
>> +		goto iounmap_base;
>> +	}
>>   
>>   	clockevent->dev.name = "tim12";
>>   	clockevent->dev.features = CLOCK_EVT_FEAT_ONESHOT;
>> @@ -296,7 +298,7 @@ int __init davinci_timer_register(struct clk *clk,
>>   			 "clockevent/tim12", clockevent);
>>   	if (rv) {
>>   		pr_err("Unable to request the clockevent interrupt\n");
>> -		return rv;
>> +		goto iounmap_base;
>>   	}
>>   
>>   	davinci_clocksource.dev.rating = 300;
>> @@ -323,13 +325,17 @@ int __init davinci_timer_register(struct clk *clk,
>>   	rv = clocksource_register_hz(&davinci_clocksource.dev, tick_rate);
>>   	if (rv) {
>>   		pr_err("Unable to register clocksource\n");
>> -		return rv;
>> +		goto iounmap_base;
>>   	}
>>   
>>   	sched_clock_register(davinci_timer_read_sched_clock,
>>   			     DAVINCI_TIMER_CLKSRC_BITS, tick_rate);
>>   
>>   	return 0;
>> +
>> +iounmap_base:
>> +	iounmap(base);
>> +	return rv;
>>   }
>>   
>>   static int __init of_davinci_timer_register(struct device_node *np)
>> -- 
>> 2.37.2

-- 
<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