From: Thomas Gleixner <tglx@linutronix.de>
To: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: daniel.lezcano@linaro.org, vincent.guittot@linaro.org,
linux-kernel@vger.kernel.org
Subject: Re: [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path
Date: Thu, 22 Sep 2016 16:34:47 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.20.1609221624460.5599@nanos> (raw)
In-Reply-To: <1474554365-8928-1-git-send-email-arvind.yadav.cs@gmail.com>
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
prev parent reply other threads:[~2016-09-22 14:37 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-22 14:26 [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path Arvind Yadav
2016-09-22 14:34 ` Thomas Gleixner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.20.1609221624460.5599@nanos \
--to=tglx@linutronix.de \
--cc=arvind.yadav.cs@gmail.com \
--cc=daniel.lezcano@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox