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

[Daniel Lezcano]

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@xxxxxxxxxxx>
> > Sent Time: 2023-03-22 23:19:45 (Wednesday)
> > To: "Daniel Lezcano" <daniel.lezcano@xxxxxxxxxx>, "Thomas Gleixner" <tglx@xxxxxxxxxxxxx>, "Bartosz Golaszewski" <bgolaszewski@xxxxxxxxxxxx>
> > Cc: hust-os-kernel-patches@xxxxxxxxxxxxxxxx, "Qinrun Dai" <flno@xxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxx>
> > ---
> >   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