Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code

[Lendacky, Thomas]

On 4/8/19 2:08 PM, Borislav Petkov wrote:
> On Mon, Apr 08, 2019 at 06:41:30PM +0000, Gary R Hook wrote:
>> Again, not arguing. I completely understand. However, to be fair, this 
>> isn't about SME having trouble with those facilities, this is about 
>> using certain features (e.g. command line option processing) early in 
>> the boot. Any complex feature could have had that requirement, don't you 
>> think?
> 
> Sure, but then why do we need that patch at all then? Why do we need to
> disable instrumentation for SME early code and not for other early code?
> 
> I mean, if you grep around the tree you can see a bunch of
> KASAN_SANITIZE but in lib/ we only have
> 
> lib/Makefile:210:KASAN_SANITIZE_stackdepot.o := n
> 
> which is special. But the rest of the generic code in lib/ or
> arch/x86/lib/ isn't.
> 
> Now, there's this:
> 
> arch/x86/boot/Makefile:12:KASAN_SANITIZE                        := n
> arch/x86/boot/compressed/Makefile:20:KASAN_SANITIZE                     := n
> 
> which disables KASAN for all boot code.
> 
> And this is what you mean - all early boot code should not be sanitized.
> 
> Which also gives the right solution, IMO:
> 
> cmdline.o should not be sanitized only when used in the boot code. But
> that is already the case.
> 
> So why do you need to disable KASAN for arch/x86/lib/cmdline.c?
> 
> Because for those two:
> 
> arch/x86/boot/cmdline.c
> arch/x86/boot/compressed/cmdline.c
> 
> that should already be the case due to the Makefile defines above.

This isn't the code in arch/x86/boot. This is post decompression phase and
part of the actual kernel early boot code where the page tables are being
modified with the encryption mask and the actual kernel load address (see
the sme_enable() call from __startup_64() in arch/x86/kernel/head64.c).

> 
>> Right. My goal was to get a conversation started, because folks are
>> running into this problem when KASAN is enabled.
> 
> You say KASAN. Why is there KCOV_INSTRUMENT_cmdline.o too?
> 
>> N.B. Here's another facet of this problem: cmdline.c doesn't (today) 
>> contain anything that would trigger the stack protector. However, it's 
>> possible to enable the stack protector globally when building, right? In 
>> which case, a boot would fail, so we have the same issue: early boot 
>> code has special requirements / restrictions.
> 
> How so?
> 
> This .config boots here in a vm just fine.

The SME path that this addresses is bypassed when you boot in a VM. The
sme_enable() code detects that it is executing under a hypervisor and does
not check the command line or do a string compare.

Thanks,
Tom

> 
> $ grep STACKPROT .config
> CONFIG_CC_HAS_SANE_STACKPROTECTOR=y
> CONFIG_HAVE_STACKPROTECTOR=y
> CONFIG_CC_HAS_STACKPROTECTOR_NONE=y
> CONFIG_STACKPROTECTOR=y
> CONFIG_STACKPROTECTOR_STRONG=y
>