Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <CAKwvOdkjB2LGXGn-9chPDtGyVmw9JF1FZ1ZBjSKEtK7BqbwF2g@mail.gmail.com>
References: <20180402095033.nfzcrmxvpm46dhbl@gmail.com> <CA+55aFzV8L-uv9706ryJ2mP9VGBuzqFVPu1cgYY+sZAtDAroqg@mail.gmail.com>
 <20180403085904.GY4082@hirez.programming.kicks-ass.net> <20180403095118.rpf7tj577dppvx7d@gmail.com>
 <20180403180658.GE87376@google.com> <CAKwvOdkaBNTyNjackJcWYkdfwyR2wjAruMFqN8LuYqC8V0ii_A@mail.gmail.com>
 <20180404093823.GC25996@kroah.com> <CAKwvOdkjB2LGXGn-9chPDtGyVmw9JF1FZ1ZBjSKEtK7BqbwF2g@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 4 Apr 2018 10:13:12 -0700
Message-ID: <CA+55aFyZA5t-+OyzSDDPGjDoaKsh=Erz8zuoEdDo7hOCBU6ZfQ@mail.gmail.com>
Subject: Re: [GIT PULL] x86/build changes for v4.17
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Greg KH <gregkh@linuxfoundation.org>,
        Matthias Kaehlcke <mka@chromium.org>,
        Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        James Y Knight <jyknight@google.com>,
        Chandler Carruth <chandlerc@google.com>,
        Stephen Hines <srhines@google.com>,
        Kees Cook <keescook@google.com>, groeck@chromium.org,
        Greg Hackmann <ghackmann@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 4, 2018 at 9:49 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> It's definitely something curious that I'll need to sit down and investigate
> more.  If there are other known instances, it would be good to let me know.

Note that we explicitly use "-fno-delete-null-pointer-checks" because
we do *not* want NULL pointer check removal even in the case of a bug.

See commit a3ca86aea507 ("Add '-fno-delete-null-pointer-checks' to gcc
CFLAGS") for the reason: we had buggy code that accessed a pointer
before the NULL pointer check, but the bug was "benign" as long as the
compiler didn't actually remove the check.

And note how the buggy code *looked* safe. It was doing the right
check, it was just that the check was hidden and disabled by another
bug.

Removing the NULL pointer check turned a benign bug into a trivially
exploitable one by just mapping user space data at NULL (which avoided
the kernel oops, and then made the kernel use the user value!).

Now, admittedly we have a ton of other security features in place to
avoid these kinds of issues - SMAP helps on the hardware side, and not
allowing users to mmap() NULL in the first place helps with most
distributions, but the point remains: the kernel generally really
doesn't want optimizations that are perhaps allowed by the standard,
but that result in code generation that doesn't match the source code.

The NULL pointer removal is one such thing: don't remove checks in the
kernel based on "standard says". It's ok to do optimizations that are
based on "hardware does the exact same thing", but not on "the
standard says this is undefined so we can remove it".

Other examples of where the kernel doesn't want the compiler to do
"the standard says this is undefined so I can take shortcuts" include:

 -fno-strict-aliasing: the standard is just wrong and full of shit,
and the misguided type-based aliasing can cause serious problems for
the kernel (but also other code)

 -fno-strict-overflow: again, this is a stupid optimization that
purely depends on the compiler generating faster code by generating
incorrect code.

The one I'm actually upset about is when a compiler goes even
*further* and does things that are NOT EVEN allowed by the paper
standard, much less by real code.

The fact that clang by default enables "-fmerge-all-constants"
behavior is just inexcusable. That's not just "let's do invalid
optimizations based on undefined behavior". That's an actual "let's do
known invalid optimizations that are explicitly disallowed even by the
standard".

             Linus