linux-kernel - RE: [PATCH v3] provide rule for finding refcounters

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] [day] [month] [year] [list]

Date:   Tue, 29 Aug 2017 10:57:07 +0000
From:   "Reshetova, Elena" <elena.reshetova@...el.com>
To:     Julia Lawall <julia.lawall@...6.fr>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "cocci@...teme.lip6.fr" <cocci@...teme.lip6.fr>,
        "Gilles.Muller@...6.fr" <Gilles.Muller@...6.fr>,
        "nicolas.palix@...g.fr" <nicolas.palix@...g.fr>,
        "mmarek@...e.com" <mmarek@...e.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "ishkamiel@...il.com" <ishkamiel@...il.com>
Subject: RE: [PATCH v3] provide rule for finding refcounters


> On Tue, 29 Aug 2017, Reshetova, Elena wrote:
> 
> > Hi, I am very sorry for the delayed reply. Finally unrigging my inbox :(
> >
> > > A few more small issues:
> > >
> > > When you deleted the disjunction, you kept the surrounding parentheses.
> > > you can drop them (lines 83 and 85).
> > >
> > > I guess that the "del" regular expression is supposed to be matching
> > > delete.  But it also matches delayed, eg
> > >
> > > net/batman-adv/bridge_loop_avoidance.c:1495:8-27:
> > > atomic_dec_and_test variation before object free at line 1507.
> >
> > Actually the idea is to match them both :) "delete" because it is obvious,
> > "delay", exactly because "queue_delayed_work" (in addition to "queue_work") is a
> common way some
> > structure destruction might be scheduled. It might give false positives, since
> > the queued work might not be related to freeing the object, but at least
> > we don't miss such cases. The issue also that you do want to have "del" pattern
> > since I think some functions are of kind xyz_del() also and I want to catch them as
> well.
> > Of course del then might catch some other non-queue related "delay", but I
> haven't seen that many
> > to consider it a problem.
> >
> > >
> > > In the following result, the lines are at least quite far apart.  I don't
> > > know if there is some way to consider this to be a false positive:
> > >
> > > fs/btrfs/disk-io.c:708:14-33: atomic_dec_and_test
> > > variation before object free at line 775.
> >
> > I actually think this is a valid result. Yes, there are a lot of things happening
> > in between the dec_and_test and actual free on the buffer, but kernel structures
> > can be so complicated that it might legitimately (like in this case) take that long
> > for it to cleanup before the real free can be done.
> 
> OK, if you are happy with the results of the regexps, I think that the
> only remaining improvement was an extra pair of () where there was no
> longer a disjuction.  If you want to send it back, then I can ack it.

Sure, I will fix that extra pair of brackets and resend. Thank you very much!

Best Regards,
Elena.
> 
> julia
> 
> >
> > Best Regards,
> > Elena.
> >
> > >
> > > julia
> > >
> > > On Wed, 16 Aug 2017, Elena Reshetova wrote:
> > >
> > > > changes in v3:
> > > > Removed unnessesary rule 4 conditions pointed by Julia.
> > > >
> > > > changes in v2:
> > > > Following the suggestion from Julia the first rule is split into
> > > > 2. The output does not differ that much between these two versions,
> > > > but rule became more precise.
> > > >
> > > > Elena Reshetova (1):
> > > >   Coccinelle: add atomic_as_refcounter script
> > > >
> > > >  scripts/coccinelle/api/atomic_as_refcounter.cocci | 133
> > > ++++++++++++++++++++++
> > > >  1 file changed, 133 insertions(+)
> > > >  create mode 100644 scripts/coccinelle/api/atomic_as_refcounter.cocci
> > > >
> > > > --
> > > > 2.7.4
> > > >
> > > >
> >