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+55aFxxXYuuReoQYJxnGX4UTVLbZ-jDh3r1t0NpEN-tqFMxzw@mail.gmail.com>
Date:   Mon, 19 Mar 2018 19:06:19 -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:50 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> Add it to everything. If it's an invalid optimization, it shouldn't be on.

IOW, why isn't this just something like

  diff --git a/Makefile b/Makefile
  index d65e2e229017..01abedc2e79f 100644
  --- a/Makefile
  +++ b/Makefile
  @@ -826,6 +826,9 @@ KBUILD_CFLAGS += $(call cc-disable-warning, pointer-sign)
   # disable invalid "can't wrap" optimizations for signed / pointers
   KBUILD_CFLAGS        += $(call cc-option,-fno-strict-overflow)

  +# disable invalid optimization on clang
  +KBUILD_CFLAGS   += $(call cc-option,-fno-merge-all-constants)
  +
   # Make sure -fstack-check isn't enabled (like gentoo apparently did)
   KBUILD_CFLAGS  += $(call cc-option,-fno-stack-check,)

(whitespace-damaged, but you get the gist of it).

We disable some optimizations that are technically _valid_, because
they are too dangerous and a bad idea.

Disabling an optimization that isn't valid EVEN IN THEORY is an
absolute no-brainer, particularly if it has already shown itself to
cause problems.

We have other situations where we generate multiple static structures
and expect them to be unique. I'm not sure any of them would trigger
the clang rules, but the clang rules are obviously complete garbage
anyway, so who knows?

That optimization seems to teuly be pure and utter garbage. Clang can
even *see* the address comparison happening in that file.

Some clang person needs to be publicly shamed for enabling this kind
of garbage by default, particularly since they apparently _knew_ it
was invalid.

              Linus