2019-10-21 20:27:15

by Navid Emamdoost

Subject: [PATCH] clocksource/drivers/davinci: Fix memory leak in davinci_timer_register

In the impelementation of davinci_timer_register() the allocated memory
for clockevent should be released if request_irq() fails.

Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
Signed-off-by: Navid Emamdoost <[email protected]>
---
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.17.1

2019-10-22 09:13:51

by Markus Elfring

Subject: Re: [PATCH] clocksource/drivers/davinci: Fix memory leak in davinci_timer_register

> In the impelementation of davinci_timer_register() the allocated memory
> for clockevent should be released if request_irq() fails.

* Please avoid the copying of typos from previous change descriptions.

* Under which circumstances will an “imperative mood” matter for you here?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=7d194c2100ad2a6dded545887d02754948ca5241#n151


> +++ 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;
> }

* Should a complete source code analysis point out that a similar fix
will be needed also in the if branch after a failed call of
the function “clocksource_register_hz”?

* Can any more exception handling become relevant because of previous
resource allocations in this function implementation?

Regards,
Markus

2019-10-22 09:34:06

by Bartosz Golaszewski

Subject: Re: [PATCH] clocksource/drivers/davinci: Fix memory leak in davinci_timer_register

pon., 21 paź 2019 o 22:26 Navid Emamdoost <[email protected]>
napisał(a):
>
> In the impelementation of davinci_timer_register() the allocated memory
> for clockevent should be released if request_irq() fails.
>
> Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
> Signed-off-by: Navid Emamdoost <[email protected]>
> ---
> 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.17.1
>

Any failure in this driver means the system is fried. I explicitly
didn't bother freeing any resources. Nack.

Bart