* [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return
@ 2019-11-09 15:58 Colin King
2019-11-10 15:59 ` Bartosz Golaszewski
0 siblings, 1 reply; 4+ messages in thread
From: Colin King @ 2019-11-09 15:58 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Bartosz Golaszewski, linux-kernel
Cc: kernel-janitors
From: Colin Ian King <colin.king@canonical.com>
In the case where request_irq fails, the return path does not kfree
clockevent and hence we have a memory leak. Fix this by kfree'ing
clockevent before returning.
Addresses-Coverity: ("Resource leak")
Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/clocksource/timer-davinci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
index 62745c962049..910d4d2f0d64 100644
--- a/drivers/clocksource/timer-davinci.c
+++ b/drivers/clocksource/timer-davinci.c
@@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
"clockevent/tim12", clockevent);
if (rv) {
pr_err("Unable to request the clockevent interrupt");
+ kfree(clockevent);
return rv;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return
2019-11-09 15:58 [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return Colin King
@ 2019-11-10 15:59 ` Bartosz Golaszewski
2019-11-11 23:37 ` Thomas Gleixner
0 siblings, 1 reply; 4+ messages in thread
From: Bartosz Golaszewski @ 2019-11-10 15:59 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Colin King, Thomas Gleixner, LKML, kernel-janitors
sob., 9 lis 2019 o 16:58 Colin King <colin.king@canonical.com> napisał(a):
>
> From: Colin Ian King <colin.king@canonical.com>
>
> In the case where request_irq fails, the return path does not kfree
> clockevent and hence we have a memory leak. Fix this by kfree'ing
> clockevent before returning.
>
> Addresses-Coverity: ("Resource leak")
> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/clocksource/timer-davinci.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> index 62745c962049..910d4d2f0d64 100644
> --- a/drivers/clocksource/timer-davinci.c
> +++ b/drivers/clocksource/timer-davinci.c
> @@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
> "clockevent/tim12", clockevent);
> if (rv) {
> pr_err("Unable to request the clockevent interrupt");
> + kfree(clockevent);
> return rv;
> }
>
> --
> 2.20.1
>
Hi Daniel,
this is what I think the third time someone tries to "fix" this
driver's "memory leaks". I'm not sure what the general approach in
clocksource is but it doesn't make sense to free resources on
non-recoverable errors, does it? Should I add a comment about it or
you'll just take those "fixes" to stop further such submissions?
Best regards,
Bartosz Golaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return
2019-11-10 15:59 ` Bartosz Golaszewski
@ 2019-11-11 23:37 ` Thomas Gleixner
2019-11-12 8:34 ` Bartosz Golaszewski
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2019-11-11 23:37 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: Daniel Lezcano, Colin King, LKML, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1912 bytes --]
Bartosz,
On Sun, 10 Nov 2019, Bartosz Golaszewski wrote:
> sob., 9 lis 2019 o 16:58 Colin King <colin.king@canonical.com> napisał(a):
> >
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > In the case where request_irq fails, the return path does not kfree
> > clockevent and hence we have a memory leak. Fix this by kfree'ing
s/we have/creates/ or whatever verb you prefer.
> > clockevent before returning.
> >
> > Addresses-Coverity: ("Resource leak")
> > Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> > drivers/clocksource/timer-davinci.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> > index 62745c962049..910d4d2f0d64 100644
> > --- a/drivers/clocksource/timer-davinci.c
> > +++ b/drivers/clocksource/timer-davinci.c
> > @@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
> > "clockevent/tim12", clockevent);
> > if (rv) {
> > pr_err("Unable to request the clockevent interrupt");
> > + kfree(clockevent);
> > return rv;
> > }
> >
> > --
> > 2.20.1
> >
>
> Hi Daniel,
>
> this is what I think the third time someone tries to "fix" this
> driver's "memory leaks". I'm not sure what the general approach in
> clocksource is but it doesn't make sense to free resources on
> non-recoverable errors, does it? Should I add a comment about it or
> you'll just take those "fixes" to stop further such submissions?
There are two ways to deal with that:
1) If the error is really unrecoverable, panic right there. No point
to continue.
2) If there is even a minimal chance to survive, free the memory and
return.
Adding a comment is just a useless non-option.
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return
2019-11-11 23:37 ` Thomas Gleixner
@ 2019-11-12 8:34 ` Bartosz Golaszewski
0 siblings, 0 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2019-11-12 8:34 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Daniel Lezcano, Colin King, LKML, kernel-janitors
wt., 12 lis 2019 o 00:37 Thomas Gleixner <tglx@linutronix.de> napisał(a):
>
> Bartosz,
>
> On Sun, 10 Nov 2019, Bartosz Golaszewski wrote:
> > sob., 9 lis 2019 o 16:58 Colin King <colin.king@canonical.com> napisał(a):
> > >
> > > From: Colin Ian King <colin.king@canonical.com>
> > >
> > > In the case where request_irq fails, the return path does not kfree
> > > clockevent and hence we have a memory leak. Fix this by kfree'ing
>
> s/we have/creates/ or whatever verb you prefer.
>
> > > clockevent before returning.
> > >
> > > Addresses-Coverity: ("Resource leak")
> > > Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> > > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > > ---
> > > drivers/clocksource/timer-davinci.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/clocksource/timer-davinci.c b/drivers/clocksource/timer-davinci.c
> > > index 62745c962049..910d4d2f0d64 100644
> > > --- a/drivers/clocksource/timer-davinci.c
> > > +++ b/drivers/clocksource/timer-davinci.c
> > > @@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
> > > "clockevent/tim12", clockevent);
> > > if (rv) {
> > > pr_err("Unable to request the clockevent interrupt");
> > > + kfree(clockevent);
> > > return rv;
> > > }
> > >
> > > --
> > > 2.20.1
> > >
> >
> > Hi Daniel,
> >
> > this is what I think the third time someone tries to "fix" this
> > driver's "memory leaks". I'm not sure what the general approach in
> > clocksource is but it doesn't make sense to free resources on
> > non-recoverable errors, does it? Should I add a comment about it or
> > you'll just take those "fixes" to stop further such submissions?
>
> There are two ways to deal with that:
>
> 1) If the error is really unrecoverable, panic right there. No point
> to continue.
Fair enough.
Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> 2) If there is even a minimal chance to survive, free the memory and
> return.
>
> Adding a comment is just a useless non-option.
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-12 8:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-09 15:58 [PATCH] clocksource/drivers/davinci: fix memory leak on clockevent on error return Colin King
2019-11-10 15:59 ` Bartosz Golaszewski
2019-11-11 23:37 ` Thomas Gleixner
2019-11-12 8:34 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).