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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.DEB.2.20.1708291121480.2398@hadrien>
Date:   Tue, 29 Aug 2017 11:23:01 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...6.fr>
To:     "Reshetova, Elena" <elena.reshetova@...el.com>
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.

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
> > >
> > >
>