linux-kernel - Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120913091959.GK19396@mwanda>
Date:	Thu, 13 Sep 2012 12:19:59 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	David Laight <David.Laight@...LAB.COM>
Cc:	Peter Senna Tschudin <peter.senna@...il.com>,
	Hansjoerg Lipp <hjlipp@....de>,
	kernel-janitors@...r.kernel.org, Tilman Schmidt <tilman@...p.cc>,
	Karsten Keil <isdn@...ux-pingi.de>,
	gigaset307x-common@...ts.sourceforge.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 9/9] drivers/isdn/gigaset/common.c: Remove useless kfree

On Thu, Sep 13, 2012 at 09:31:45AM +0100, David Laight wrote:
> > Remove useless kfree() and clean up code related to the removal.
> ...
> > diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> > index aa41485..30a6b17 100644
> > --- a/drivers/isdn/gigaset/common.c
> > +++ b/drivers/isdn/gigaset/common.c
> > @@ -1123,7 +1123,6 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
> >  	return drv;
> > 
> >  error:
> > -	kfree(drv->cs);
> >  	kfree(drv);
> >  	return NULL;
> >  }
> > 
> 
> Seems to me that (assuming kfree(NULL) is ok) the kfree()
> is best left in - just in case some other error path is
> added after drv->cs is assigned.
> Better safe than a memory leak.

No.  Delete vestigial code.  There are all kinds of no-ops we could
add to the unwind bits of code if we wanted to so why is "drv->cs"
better than anything else.

First of all, no one is going to change the ISDN code to add an
allocation, but if they did then they have to make sure they don't
leak memory.  That's how it always works.  They can't just assume
that there is going to be a forgotten kfree() hanging off to the
side which is going to take care of it.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/