Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1488939

Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path

From Thomas Gleixner <tglx@linutronix.de>
Newsgroups linux.kernel
Subject Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path
Date 2016-09-22 16:40 +0200
Message-ID <skdkl-7zs-11@gated-at.bofh.it> (permalink)
References <skdaG-7wd-27@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, 22 Sep 2016, Arvind Yadav wrote:

Darn. Take your time and think about stuff instead of sending a new patch
out 5 seconds after you got a review comment.

> Subject : [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path

1) This lacks a [PATCH v2] prefix

2) clocksrouce is not a valid subsytem

> Free memory and memory mapping, if mxc_timer_init_dt is not successful.

3) There is no memory mapping to free. Simply because that code maps a
   peripheral and not memory.

Stop acting like a shell script or your mail address ends up in one which
sends your mails immediately to /dev/null.

> diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c
> index f595460..19f6860 100644
> --- a/drivers/clocksource/timer-imx-gpt.c
> +++ b/drivers/clocksource/timer-imx-gpt.c
> @@ -489,12 +489,17 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
>  		return -ENOMEM;
>  
>  	imxtm->base = of_iomap(np, 0);
> -	if (!imxtm->base)
> +	if (!imxtm->base) {
> +		kfree(imxtm);
>  		return -ENXIO;
> +	}
>  
>  	imxtm->irq = irq_of_parse_and_map(np, 0);
> -	if (imxtm->irq <= 0)
> +	if (imxtm->irq <= 0) {
> +		iounmap(imxtm->base);
> +		kfree(imxtm);
>  		return -EINVAL;
> +	}
>  
>  	imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
>  
> @@ -506,8 +511,11 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
>  	imxtm->type = type;
>  
>  	ret = _mxc_timer_init(imxtm);
> -	if (ret)
> +	if (ret) {
> +		iounmap(imxtm->base);
> +		kfree(imxtm);

Ever heard about the concept of goto? Just adding blindly iounmap and kfree
at every place plugs the leak, but is far away from proper kernel code.

Thanks,

	tglx

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path Arvind Yadav <arvind.yadav.cs@gmail.com> - 2016-09-22 16:30 +0200
  Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error  path Thomas Gleixner <tglx@linutronix.de> - 2016-09-22 16:40 +0200

csiph-web