Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path
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
Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path
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