2013-10-07 00:54:54

by Felipe Pena

Subject: [PATCH 1/1] clk/zynq: Fix possible memory leak

The zynq_clk_register_fclk function can leak memory (fclk_lock) when unable
to alloc memory for fclk_gate_lock

Signed-off-by: Felipe Pena <[email protected]>
---
drivers/clk/zynq/clkc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index cc40fe6..7ea4b5c 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -117,6 +117,7 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
goto err;
fclk_gate_lock = kmalloc(sizeof(*fclk_gate_lock), GFP_KERNEL);
if (!fclk_gate_lock)
+ kfree(fclk_lock);
goto err;
spin_lock_init(fclk_lock);
spin_lock_init(fclk_gate_lock);
--
1.7.10.4

2013-10-07 02:19:35

by Baruch Siach

Subject: Re: [PATCH 1/1] clk/zynq: Fix possible memory leak

Hi Felipe,

On Sun, Oct 06, 2013 at 09:55:17PM -0300, Felipe Pena wrote:
> The zynq_clk_register_fclk function can leak memory (fclk_lock) when unable
> to alloc memory for fclk_gate_lock
>
> Signed-off-by: Felipe Pena <[email protected]>
> ---
> drivers/clk/zynq/clkc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index cc40fe6..7ea4b5c 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -117,6 +117,7 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
> goto err;
> fclk_gate_lock = kmalloc(sizeof(*fclk_gate_lock), GFP_KERNEL);
> if (!fclk_gate_lock)
> + kfree(fclk_lock);
> goto err;

Missing braces.

> spin_lock_init(fclk_lock);
> spin_lock_init(fclk_gate_lock);

baruch

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -

2013-10-07 16:05:55

by Soren Brinkmann

Subject: Re: [PATCH 1/1] clk/zynq: Fix possible memory leak

On Sun, Oct 06, 2013 at 09:55:17PM -0300, Felipe Pena wrote:
> The zynq_clk_register_fclk function can leak memory (fclk_lock) when unable
> to alloc memory for fclk_gate_lock
>
> Signed-off-by: Felipe Pena <[email protected]>
> ---
> drivers/clk/zynq/clkc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index cc40fe6..7ea4b5c 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -117,6 +117,7 @@ static void __init zynq_clk_register_fclk(enum zynq_clk fclk,
> goto err;
> fclk_gate_lock = kmalloc(sizeof(*fclk_gate_lock), GFP_KERNEL);
> if (!fclk_gate_lock)
> + kfree(fclk_lock);
> goto err;

Looking at that function, the following calls to various clk_register_*
functions miss correct error handling as well. Therefore it might make
sense to consolidate this free in an error path at the bottom of the
function. That would make things easier when somebody wants to add more
error handling.

Other than that you're right and feel free to add my Acked-by, once you
have fixed the missing braces, Baruch pointed out.

Sören