From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752211AbbCYGLw (ORCPT ); Wed, 25 Mar 2015 02:11:52 -0400 Received: from mail-ob0-f177.google.com ([209.85.214.177]:34066 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750738AbbCYGLv (ORCPT ); Wed, 25 Mar 2015 02:11:51 -0400 Message-ID: <551251A4.1030501@lwfinger.net> Date: Wed, 25 Mar 2015 01:11:48 -0500 From: Larry Finger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Nicholas Krause , rmk+kernel@arm.linux.org.uk CC: dbaryshkov@gmail.com, linux-pcmcia@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] pcmcia:Fix memory leaks in the function, sa11xx_drv_pcmcia_probe References: <1427257453-22159-1-git-send-email-xerofoify@gmail.com> In-Reply-To: <1427257453-22159-1-git-send-email-xerofoify@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/24/2015 11:24 PM, Nicholas Krause wrote: > Fixes memory leaks in the function,sa11xx_drv_pcmcia_probe for > when either clk_get returns a error value or when we cannot allocate > memory with the pointer sinfo to memory required for this function > to continue and return successfully. Further more this was caught by > running coccinelle on the lastet kernel tree. > > Signed-off-by: Nicholas Krause > --- > drivers/pcmcia/sa11xx_base.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/pcmcia/sa11xx_base.c b/drivers/pcmcia/sa11xx_base.c > index cf6de2c..a95ce73 100644 > --- a/drivers/pcmcia/sa11xx_base.c > +++ b/drivers/pcmcia/sa11xx_base.c > @@ -223,14 +223,19 @@ int sa11xx_drv_pcmcia_probe(struct device *dev, struct pcmcia_low_level *ops, > struct clk *clk; > > clk = clk_get(dev, NULL); > - if (IS_ERR(clk)) > + if (IS_ERR(clk)) { > + clk_put(clk); > return PTR_ERR(clk); > + } If the clk_get() failed, then there is no need to use a clk_put(). In addition, clk contains an error code, not a valid pointer. This change would crash the system every time clk_get() failed. > > sa11xx_drv_pcmcia_ops(ops); > > sinfo = kzalloc(SKT_DEV_INFO_SIZE(nr), GFP_KERNEL); > - if (!sinfo) > + if (!sinfo) { > + clk_put(clk); > + kfree(sinfo); > return -ENOMEM; If the memory allocation failed, there is nothing to free. That kfree() call is not needed; however, the clk_put() is needed here. NACK. Larry > + } > > sinfo->nskt = nr; > sinfo->clk = clk; >