linux-kernel - Re: KCSAN: data-race in __alloc_file / __alloc

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: <CAHk-=wg9yCKVdenys6FfcpAc9KSxVMOTT2DddVcj-Ofu1MYNhQ@mail.gmail.com>
Date:   Tue, 12 Nov 2019 13:13:35 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Marco Elver <elver@...gle.com>, Eric Dumazet <edumazet@...gle.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        syzbot <syzbot+3ef049d50587836c0606@...kaller.appspotmail.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Andrea Parri <parri.andrea@...il.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        LKMM Maintainers -- Akira Yokosawa <akiyks@...il.com>
Subject: Re: KCSAN: data-race in __alloc_file / __alloc_file

On Tue, Nov 12, 2019 at 12:58 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> Honestly, my preferred model would have been to just add a comment,
> and have the reporting tool know to then just ignore it. So something
> like
>
> +               // Benign data-race on min_flt
>                 tsk->min_flt++;
>                 perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS_MIN, 1, regs, address);
>
> for the case that Eric mentioned - the tool would trigger on
> "data-race", and the rest of the comment could/should be for humans.
> Without making the code uglier, but giving the potential for a nice
> leghibl.e explanation instead of a completely illegible "let's
> randomly use WRITE_ONCE() here" or something like that.

Hmm. Looking at the practicality of this, it actually doesn't look
*too* horrible.

I note that at least clang already has a "--blacklist" ability. I
didn't find a list of complete syntax for that, and it looks like it
might be just "whole functions" or "whole source files", but maybe the
clang people would be willing to add "file and line ranges" to the
blacklists?

Then you could generate the blacklist with that trivial grep before
you start the build, and -fsanitize=thread would automatically simply
not look at those lines.

For a simple first case, maybe the rule could be that the comment has
to be on the line. A bit less legible for humans, but it could be

-               tsk->min_flt++;
+               // Benign race min_flt - statistics only
+               tsk->min_flt++; // data-race

instead.

Wouldn't that be a much better annotation than having to add code?

               Linus