linux-kernel - Re: [PATCH] coccinelle: if(ret)return ret; return ret; semantic patch

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: <Pine.LNX.4.64.1106111741250.13465@ask.diku.dk>
Date:	Sat, 11 Jun 2011 17:43:34 +0200 (CEST)
From:	Julia Lawall <julia@...u.dk>
To:	Greg Dietsche <gregory.dietsche@....edu>
Cc:	Gilles.Muller@...6.fr, npalix.work@...il.com, cocci@...u.dk,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coccinelle: if(ret)return ret; return ret; semantic
 patch

On Sat, 11 Jun 2011, Greg Dietsche wrote:

> On 06/05/2011 11:55 PM, Julia Lawall wrote:
> > Thanks.  I tried this too, but I wasn't sure about the results.  The
> > question is why stop here.  For example, there are IS_ERR calls that one
> > could consider as well.  Or ret<  0.  Or why not just:
> >
> > @@
> > expression ret;
> > @@
> >
> > - if (...) return ret;
> >    return ret;
> >
> >    
> I've been thinking a bit more about this. Is there a community preference
> towards patches that are highly reliable v.s. ones that find things that might
> not be a problem? I'm leaning towards updating my patch to do the above and
> then later coming back when I have more time to fix it up to find only things
> that are really problems.

I tend to write the patch that returns the kinds of thing that I know how 
to fix :).  That is, if I see a lot of reports that I don't know what to 
do with, I write the semantic patch to avoid showing them to me.

When you put your patch in the kernel, you can put some comments at the 
beginning of it that say your confidence in the results and give some 
hints about possible false positives.

Overall, I think you should just use your judgement :)

thanks,
julia

> Greg
> > Although there might be function calls that one doesn't want to touch, so:
> >
> > @@
> > identifier f != IS_ERR;
> > expression ret;
> > statement S;
> > @@
> >
> > (
> > if (<+...f(...)...+>) S
> > |
> > - if (...) return ret;
> >    return ret;
> > )
> >
> > julia
> >
> >
> > On Sun, 5 Jun 2011, Greg Dietsche wrote:
> >
> >    
> > > This semantic patch finds code matching this pattern:
> > >  if(ret)
> > >  	return ret;
> > >  return ret;
> > >
> > > I will be submitting patches shortly against the mainline to cleanup all
> > > code matching this pattern.
> > >
> > > Signed-off-by: Greg Dietsche<Gregory.Dietsche@....edu>
> > > ---
> > >   scripts/coccinelle/misc/doublereturn.cocci |   20 ++++++++++++++++++++
> > >   1 files changed, 20 insertions(+), 0 deletions(-)
> > >   create mode 100644 scripts/coccinelle/misc/doublereturn.cocci
> > >
> > > diff --git a/scripts/coccinelle/misc/doublereturn.cocci
> > > b/scripts/coccinelle/misc/doublereturn.cocci
> > > new file mode 100644
> > > index 0000000..656a118
> > > --- /dev/null
> > > +++ b/scripts/coccinelle/misc/doublereturn.cocci
> >> @@ -0,0 +1,20 @@
> > > +/// Remove unecessary if/return in code that follows this pattern:
> > > +///	if(retval)
> > > +///		return retval;
> > > +///	return retval;
> > > +//
> > > +// Confidence: High
> > > +// Copyright: (C) 2011 Greg Dietsche GPLv2.
> > > +// URL: http://www.gregd.org
> > > +// Comments:
> > > +// Options: -no_includes
> > > +
> > > +virtual patch
> > > +
> > > +@@
> > > +identifier retval;
> > > +@@
> > > +-if (retval)
> > > +-	return retval;
> > > +-return retval;
> > > ++return retval;
> > > -- 
> > > 1.7.2.5
> > >
> > >
> > >      
> >    
> 
--
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/