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

Date:   Tue, 29 Aug 2017 08:54:25 +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

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. 

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