Skip to contentSkip to conversation

arm: mach-zynq/timer.c: fix memory leakage

PATCH In review Cong Ding 2 participants 2 messages Jan 14, 2013

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm/Zynq architecture
lore
Share & tools
lore

Cong Ding

author
#0 lore raw

PATCH 1/1 arm: mach-zynq/timer.c: fix memory leakage

the variable ttccs allocated isn't freed when error occurs, so we call kfree before return.

Signed-off-by Cong Ding <[email protected]>
---
 arch/arm/mach-zynq/timer.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
arch/arm/mach-zynq/timer.c +16 -9
index f9fbc9c..df04761 100644
@@ -203,15 +203,15 @@ static void __init zynq_ttc_setup_clocksource(struct device_node *np,
 
 	err = of_property_read_u32(np, "reg", &reg);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	clk = of_clk_get_by_name(np, "cpu_1x");
 	if (WARN_ON(IS_ERR(clk)))
-		return;
+		goto out;
 
 	err = clk_prepare_enable(clk);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	ttccs->xttc.base_addr = base + reg * 4;
 

@@ -229,7 +229,10 @@ static void __init zynq_ttc_setup_clocksource(struct device_node *np,
 
 	err = clocksource_register_hz(&ttccs->cs, clk_get_rate(clk) / PRESCALE);
 	if (WARN_ON(err))
-		return;
+		goto out;
+	return;
+out:
+	kfree(ttccs);
 }
 
 static void __init zynq_ttc_setup_clockevent(struct device_node *np,

@@ -245,21 +248,21 @@ static void __init zynq_ttc_setup_clockevent(struct device_node *np,
 
 	err = of_property_read_u32(np, "reg", &reg);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	ttcce->xttc.base_addr = base + reg * 4;
 
 	ttcce->clk = of_clk_get_by_name(np, "cpu_1x");
 	if (WARN_ON(IS_ERR(ttcce->clk)))
-		return;
+		goto out;
 
 	err = clk_prepare_enable(ttcce->clk);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	irq = irq_of_parse_and_map(np, 0);
 	if (WARN_ON(!irq))
-		return;
+		goto out;
 
 	ttcce->ce.name = np->name;
 	ttcce->ce.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;

@@ -277,11 +280,15 @@ static void __init zynq_ttc_setup_clockevent(struct device_node *np,
 	err = request_irq(irq, xttcps_clock_event_interrupt, IRQF_TIMER,
 			  np->name, ttcce);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	clockevents_config_and_register(&ttcce->ce,
 					clk_get_rate(ttcce->clk) / PRESCALE,
 					1, 0xfffe);
+	return;
+
+out:
+	kfree(ttcce);
 }
 
 static const __initconst struct of_device_id zynq_ttc_match[] = {
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 32 lines · Cong Ding

the variable ttccs allocated isn't freed when error occurs, so we call kfree before return.

Signed-off-by Cong Ding <[email protected]>
---
 arch/arm/mach-zynq/timer.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
arch/arm/mach-zynq/timer.c +3 -3
index f9fbc9c..df04761 100644
@@ -203,15 +203,15 @@ static void __init zynq_ttc_setup_clocksource(struct device_node *np,
 
 	err = of_property_read_u32(np, "reg", &reg);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	clk = of_clk_get_by_name(np, "cpu_1x");
 	if (WARN_ON(IS_ERR(clk)))
-		return;
+		goto out;
 
 	err = clk_prepare_enable(clk);
 	if (WARN_ON(err))
-		return;
+		goto out;
 
 	ttccs->xttc.base_addr = base + reg * 4;

arch/arm/mach-zynq/timer.c static void __init zynq_ttc_setup_clocksource(struct device_node *np, · line 236

Quoted 9 lines
arch/arm/mach-zynq/timer.c +4 -1
@@ -229,7 +229,10 @@ static void __init zynq_ttc_setup_clocksource(struct device_node *np,
 
 	err = clocksource_register_hz(&ttccs->cs, clk_get_rate(clk) / PRESCALE);
 	if (WARN_ON(err))
-		return;
+		goto out;
+	return;
+out:
+	kfree(ttccs);
 }

Okay...hmm. If initialization of the timer fails, you'll have bigger issues then just a small memory leak. Especially since right now the TTC is the only supported timer on zynq.

But okay, to be forward looking we should probably properly handle these error cases.

You've only handled the memory leak here, but there are other potential leaks that you should be handling (the clk handle, clock event interrupt, ioremap()'d region, etc). Could you take care of those while your at it?

Josh