linux-kernel - Re: [patch 119/197] scripts/coccinelle/free: Delete NULL test before freeing functions

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: <alpine.DEB.2.02.1408091028520.2016@localhost6.localdomain6>
Date:	Sat, 9 Aug 2014 10:37:51 +0200 (CEST)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	SF Markus Elfring <elfring@...rs.sourceforge.net>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Coccinelle <cocci@...teme.lip6.fr>, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org, Fabian Frederick <fabf@...net.be>,
	Joe Perches <joe@...ches.com>
Subject: Re: [patch 119/197] scripts/coccinelle/free: Delete NULL test before
 freeing functions

On Sat, 9 Aug 2014, SF Markus Elfring wrote:

> > +@r depends on context || report || org @
> > +expression E;
> > +position p;
> > +@@
> > +
> > +* if (E)
> > +*	\(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|usb_free_urb\)(E);
> 
> How do you think about to add the construct "@p" also behind the function name
> "usb_free_urb"?

Yes, that would be needed, thanks.  Also, it was recently suggested to add 
of_node_put to the list of things that are checked for.

> Is my previous update suggestion for such an issue worth for another look, too?

I still don't think this should be done for any random function that 
performs a null test on its argument.  The corrections involved here are 
not as trivial as they would seem.  Often it is not the case that the null 
test at the call site should be just deleted, instead the code should be 
reorganized.  Making a rule that treats 5000 functions that are 
automatically selected will just encourage people to do sloppy things.

(Personally, I don't like the whole null test removal idea.  It mixes the 
notion of something that is necessary and has failed and something that is 
optional.  In the case of something that is necessary and has failed, the 
null value is usually statically apparent, and the code can be reorganized 
so that a null value doesn't go where it is not wanted.  In the case of 
something that is simply optional, the presence of the null test gives 
the reader of the code some information.  Removing this information in 
rarely executed code seems unfortunate.)

julia

> 
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
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/