netdev - Re: [PATCH bpf] bpf: fix crash due to inode i

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: <CA+55aFyjU4nLn4jykr1aYH7X7u-FruZjOLZ+OJnH_1RbfEimAQ@mail.gmail.com>
Date:   Mon, 19 Mar 2018 18:50:57 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        psodagud@...eaurora.org, fengc@...gle.com,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf] bpf: fix crash due to inode i_op mismatch with clang/llvm

On Mon, Mar 19, 2018 at 6:17 PM, Daniel Borkmann <daniel@...earbox.net> wrote:
>
> Reason for this miscompilation is that clang has the more aggressive
> -fmerge-all-constants enabled by default. In fact, clang source code
> has an 'insightful' comment about it in its source code under
> lib/AST/ExprConstant.cpp:
>
>   // Pointers with different bases cannot represent the same object.
>   // (Note that clang defaults to -fmerge-all-constants, which can
>   // lead to inconsistent results for comparisons involving the address
>   // of a constant; this generally doesn't matter in practice.)
>
> gcc on the other hand does not enable -fmerge-all-constants by default
> and *explicitly* states in it's option description that using this
> flag results in non-conforming behavior, quote from man gcc:
>
>   Languages like C or C++ require each variable, including multiple
>   instances of the same variable in recursive calls, to have distinct
>   locations, so using this option results in non-conforming behavior.
>
> Given there are users with clang/LLVM out there today that triggered
> this, fix this mess by explicitly adding -fno-merge-all-constants to
> inode.o as CFLAGS via Kbuild system.

Oh, please do *NOT* add it to just that one file.

Add it to everything. If it's an invalid optimization, it shouldn't be on.

And even if it happens to trigger in only that one file, then
disabling it globally is just the safe thing to do.

What is the code generation difference if you just enable it globally?
I would certainly _hope_ that it's not noticeable, but if it's
noticeable that would certainly imply that it's very dangerous
somewhere else too!

                Linus