[PATCH] x86/alternatives: Use C int3 selftest but disable KASAN

[PATCH] x86/alternatives: Use C int3 selftest but disable KASAN

[Kees Cook]

Instead of using inline asm for the int3 selftest (which confuses the
Clang's ThinLTO pass), 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.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/kernel/alternative.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 9d3a971ea364..efb5ef8a7885 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -625,23 +625,10 @@ extern struct paravirt_patch_site __start_parainstructions[],
  *
  * See entry_{32,64}.S for more details.
  */
-
-/*
- * We define the int3_magic() function in assembly to control the calling
- * convention such that we can 'call' it from assembly.
- */
-
-extern void int3_magic(unsigned int *ptr); /* defined in asm */
-
-asm (
-"	.pushsection	.init.text, \"ax\", @progbits\n"
-"	.type		int3_magic, @function\n"
-"int3_magic:\n"
-"	movl	$1, (%" _ASM_ARG1 ")\n"
-"	ret\n"
-"	.size		int3_magic, .-int3_magic\n"
-"	.popsection\n"
-);
+static void __init __no_sanitize_address notrace int3_magic(unsigned int *ptr)
+{
+	*ptr = 1;
+}
 
 extern __initdata unsigned long int3_selftest_ip; /* defined in asm below */
 
-- 
2.17.1


-- 
Kees Cook

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

[Peter Zijlstra]

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 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.

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

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

[Kees Cook]

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