From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933632AbcIVOhQ (ORCPT ); Thu, 22 Sep 2016 10:37:16 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:38333 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753256AbcIVOhN (ORCPT ); Thu, 22 Sep 2016 10:37:13 -0400 Date: Thu, 22 Sep 2016 16:34:47 +0200 (CEST) From: Thomas Gleixner To: Arvind Yadav 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 In-Reply-To: <1474554365-8928-1-git-send-email-arvind.yadav.cs@gmail.com> Message-ID: References: <1474554365-8928-1-git-send-email-arvind.yadav.cs@gmail.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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