public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Sami Tolvanen <samitolvanen@google.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/alternatives: Use C int3 selftest but disable KASAN
Date: Tue, 12 Nov 2019 13:10:35 -0800	[thread overview]
Message-ID: <201911121304.7C1C2D79E@keescook> (raw)
In-Reply-To: <20191112075746.GW4131@hirez.programming.kicks-ass.net>

On Tue, Nov 12, 2019 at 08:57:46AM +0100, Peter Zijlstra wrote:
> On Mon, Nov 11, 2019 at 01:51:16PM -0800, Kees Cook wrote:
> > Instead of using inline asm for the int3 selftest (which confuses the
> > Clang's ThinLTO pass), 
> 
> What is that and why do we care?

This was breaking my build when using Clang and the LLVM linker with Link
Time Optimization (LTO) enabled, which is a prerequisite for enabling
Clang's Control Flow Integrity (CFI) feature that seeks to protect
indirect function calls from intentional (or accidental) manipulation.
Adding CFI to kernel builds is an ongoing project to further defend the
kernel from attacks[1], which many system builders are interested in
deploying.

> > this restores the C function but disables KASAN
> > (and tracing for good measure) to keep the things simple and avoid
> > unexpected side-effects. This attempts to keep the fix from commit
> > ecc606103837 ("x86/alternatives: Fix int3_emulate_call() selftest stack
> > corruption") without using inline asm.
> 
> See, I don't much like that. The selftest basically does a naked CALL
> and hard relies on the callee saving everything if required, which is
> very much against the C calling convention.
> 
> Sure, by disabling KASAN and all the other crap the compiler probably
> does the right thing by accident, but it is still a C ABI violation.

Okay, fair enough. I thought the patch seemed like a reasonable middle
ground, but I'll revisit it.

> We use ASM all over the kernel, why is this one a problem?

There seem to be a lot of weird visibility differences between GCC and
Clang with respect to asm. This is just declared differently from the
other many cases.

I'll see if I can find a better work-around.

-Kees

[1] https://android-developers.googleblog.com/2018/10/control-flow-integrity-in-android-kernel.html

-- 
Kees Cook

      reply	other threads:[~2019-11-12 21:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-11 21:51 [PATCH] x86/alternatives: Use C int3 selftest but disable KASAN Kees Cook
2019-11-12  7:57 ` Peter Zijlstra
2019-11-12 21:10   ` Kees Cook [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201911121304.7C1C2D79E@keescook \
    --to=keescook@chromium.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox