linux-kernel - Re: [PATCH] x86/alternatives: Use C int3 selftest but disable KASAN

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] [day] [month] [year] [list]

Date:   Tue, 12 Nov 2019 13:10:35 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/alternatives: Use C int3 selftest but disable KASAN

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