efi/arm64: unmap the kernel during runtime service calls

Whether or not we will ever decide to start using x18 as a platform register in Linux is uncertain, but by that time, we will need to ensure that UEFI runtime services calls don't corrupt it. So let's start issuing warnings now for this, and increase the likelihood that these firmware images have all been replaced by that time.

This has been fixed on the EDK2 side in commit 6d73863b5464 ("BaseTools/tools_def AARCH64: mark register x18 as reserved")., dated July 13, 2017.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/efi.h       |  4 +-
 arch/arm64/kernel/Makefile         |  3 +-
 arch/arm64/kernel/efi-rt-wrapper.S | 41 ++++++++++++++++++++
 arch/arm64/kernel/efi.c            |  6 +++
 4 files changed, 52 insertions(+), 2 deletions(-)

arch/arm64/include/asm/efi.h +3 -1

index 8389050328bb..192d791f1103 100644
@@ -31,7 +31,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
 ({									\
 	efi_##f##_t *__f;						\
 	__f = p->f;							\
-	__f(args);							\
+	__efi_rt_asm_wrapper(__f, #f, args);				\
 })
 
 #define arch_efi_call_virt_teardown()					\

@@ -40,6 +40,8 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
 	efi_virtmap_unload();						\
 })
 
+efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
+
 #define ARCH_EFI_IRQ_FLAGS_MASK (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT)
 
 /* arch specific definitions used by the stub code */

arch/arm64/kernel/Makefile +2 -1

index b87541360f43..6a4bd80c75bd 100644
@@ -38,7 +38,8 @@ arm64-obj-$(CONFIG_CPU_PM)		+= sleep.o suspend.o
 arm64-obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
 arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
-arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
+arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o		\
+					   efi-rt-wrapper.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o

arch/arm64/kernel/efi-rt-wrapper.S new file +41 -0

new file mode 100644
index 000000000000..05235ebb336d
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2018 Linaro Ltd <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+
+ENTRY(__efi_rt_asm_wrapper)
+	stp	x29, x30, [sp, #-32]!
+	mov	x29, sp
+
+	/*
+	 * Register x18 is designated as the 'platform' register by the AAPCS,
+	 * which means firmware running at the same exception level as the OS
+	 * (such as UEFI) should never touch it.
+	 */
+	stp	x1, x18, [sp, #16]
+
+	/*
+	 * We are lucky enough that no EFI runtime services take more than
+	 * 5 arguments, so all are passed in registers rather than via the
+	 * stack.
+	 */
+	mov	x8, x0
+	mov	x0, x2
+	mov	x1, x3
+	mov	x2, x4
+	mov	x3, x5
+	mov	x4, x6
+	blr	x8
+
+	ldp	x1, x2, [sp, #16]
+	cmp	x2, x18
+	ldp	x29, x30, [sp], #32
+	b.ne	0f
+	ret
+0:	b	efi_handle_corrupted_x18	// tail call
+ENDPROC(__efi_rt_asm_wrapper)

arch/arm64/kernel/efi.c +6 -0

index 82cd07592519..af4f943cffac 100644
@@ -124,3 +124,9 @@ bool efi_poweroff_required(void)
 {
 	return efi_enabled(EFI_RUNTIME_SERVICES);
 }
+
+asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
+{
+	pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
+	return s;
+}

Signature · 1 line

2.11.0

As a preparatory step towards unmapping the kernel entirely while executing UEFI runtime services, move the stack and the entry wrapper routine mappings into the EFI page tables. Also, create a vector table that overrides the main one while executing in the firmware so we will be able to remap/unmap the kernel while taking interrupts.

arch/arm64/kernel/efi-rt-wrapper.S +4 -0

+	/*
+	 * EFI runtime services never drop to EL0, so the
+	 * remaining vector table entries are not needed.
+	 */

arch/arm64/kernel/efi.c +0 -0

index af4f943cffac..68c920b2f4f0 100644
@@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
 	pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
 	return s;

arch/arm64/kernel/efi.c +15 -0

+
+	/* map the stack */
+	create_pgd_mapping(mm, __pa_symbol(stack),
+			   EFI_STACK_BASE, EFI_STACK_SIZE,
+			   __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+			   false);
+
+	/* map the runtime wrapper pivot function */
+	create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
+			   EFI_CODE_BASE, EFI_CODE_SIZE,
+			   __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
+			   false);
+
+	return 0;
+}

arch/arm64/kernel/entry.S +0 -0

index b34e717d7597..3bab6c60a12b 100644

Hi Ard,

Quoted 6 lines · Ard Biesheuvel

As a preparatory step towards unmapping the kernel entirely while executing UEFI runtime services, move the stack and the entry wrapper routine mappings into the EFI page tables. Also, create a vector table that overrides the main one while executing in the firmware so we will be able to remap/unmap the kernel while taking interrupts.

[...]

arch/arm64/kernel/efi-rt-wrapper.S ENTRY(__efi_rt_asm_wrapper) · line 67

Quoted 6 lines

arch/arm64/kernel/efi-rt-wrapper.S +6 -0

+     .macro  ventry
+     .align  7
+.Lv\@ :      stp     x29, x30, [sp, #-16]!           // preserve x29 and x30
+     mrs     x29, elr_el1                    // preserve ELR
+     adr     x30, .Lret                      // take return address
+     msr     elr_el1, x30                    // set ELR to return address

Oh man, are you really doing two ERETs for a single exception, or am I missing something?

arch/arm64/kernel/efi-rt-wrapper.S ENTRY(__efi_rt_asm_wrapper) · line 75

Quoted 8 lines

arch/arm64/kernel/efi-rt-wrapper.S +8 -0

+     ldr     x30, 2b                         // take address of 'vectors'
+     msr     vbar_el1, x30                   // set VBAR to 'vectors'
+     isb
+     add     x30, x30, #.Lv\@ - __efi_rt_vectors
+     br      x30
+     .endm
+
+.Lret:       msr     elr_el1, x29

If you take an IRQ here, aren't you toast?

Quoted 6 lines

arch/arm64/kernel/efi-rt-wrapper.S +6 -0

+     adr     x30, __efi_rt_vectors
+     msr     vbar_el1, x30
+     isb
+     ldp     x29, x30, [sp], #16
+     eret
+

arch/arm64/kernel/efi-rt-wrapper.S ENTRY(__efi_rt_asm_wrapper) · line 86

Quoted 5 lines

arch/arm64/kernel/efi-rt-wrapper.S +5 -0

+     .align  11
+__efi_rt_vectors:
+     .rept   8
+     ventry
+     .endr

Have you thought about SDEI at all? I can't see any code here to handle that.

Quoted 17 lines

arch/arm64/kernel/efi-rt-wrapper.S +4 -0

+     /*
+      * EFI runtime services never drop to EL0, so the
+      * remaining vector table entries are not needed.
+      */

arch/arm64/kernel/efi.c +5 -0

index af4f943cffac..68c920b2f4f0 100644
@@ -130,3 +130,27 @@ asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f)
      pr_err_ratelimited(FW_BUG "register x18 corrupted by EFI %s\n", f);
      return s;
 }
+
+bool on_efi_stack(unsigned long sp)
+{
+     return sp >= EFI_STACK_BASE && sp < (EFI_STACK_BASE + EFI_STACK_SIZE);
+}

arch/arm64/kernel/efi.c asmlinkage efi_status_t efi_handle_corrupted_x18(efi_status_t s, const char *f) · line 141

Quoted 4 lines

arch/arm64/kernel/efi.c +4 -0

+
+int __init efi_allocate_runtime_regions(struct mm_struct *mm)
+{
+     static u8 stack[EFI_STACK_SIZE] __page_aligned_bss;

Probably just me, but the use of a function static variable in a function annotated with __init just makes me feel uneasy. Could we move it out into wider scope?

Quoted 22 lines

arch/arm64/kernel/efi.c +15 -0

+
+     /* map the stack */
+     create_pgd_mapping(mm, __pa_symbol(stack),
+                        EFI_STACK_BASE, EFI_STACK_SIZE,
+                        __pgprot(pgprot_val(PAGE_KERNEL) | PTE_NG),
+                        false);
+
+     /* map the runtime wrapper pivot function */
+     create_pgd_mapping(mm, __pa_symbol(__efi_rt_asm_wrapper),
+                        EFI_CODE_BASE, EFI_CODE_SIZE,
+                        __pgprot(pgprot_val(PAGE_KERNEL_ROX) | PTE_NG),
+                        false);
+
+     return 0;
+}

arch/arm64/kernel/entry.S +0 -0

index b34e717d7597..3bab6c60a12b 100644
@@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
 alternative_else_nop_endif
 

arch/arm64/kernel/entry.S alternative_if ARM64_HAS_PAN · line 207

Quoted 2 lines

arch/arm64/kernel/entry.S +1 -0

      .if     \el != 0
+     tbz     x21, #63, 1f                    // skip if TTBR0 covers the stack

So this is really a "detect EFI" check, right? Maybe comment it as such. Also, probably want to check bit 55 just in case tagging ever takes off.

Quoted 17 lines · Will Deacon

Quoted 6 lines · Ard Biesheuvel

As a preparatory step towards unmapping the kernel entirely while executing UEFI runtime services, move the stack and the entry wrapper routine mappings into the EFI page tables. Also, create a vector table that overrides the main one while executing in the firmware so we will be able to remap/unmap the kernel while taking interrupts.

[...]

Quoted 6 lines

arch/arm64/kernel/efi-rt-wrapper.S +6 -0

+     .macro  ventry
+     .align  7
+.Lv\@ :      stp     x29, x30, [sp, #-16]!           // preserve x29 and x30
+     mrs     x29, elr_el1                    // preserve ELR
+     adr     x30, .Lret                      // take return address
+     msr     elr_el1, x30                    // set ELR to return address

Quoted 3 lines

Oh man, are you really doing two ERETs for a single exception, or am I missing something?

Yes. I told you it was poetry.

Quoted 9 lines

Quoted 8 lines

arch/arm64/kernel/efi-rt-wrapper.S +8 -0

+     ldr     x30, 2b                         // take address of 'vectors'
+     msr     vbar_el1, x30                   // set VBAR to 'vectors'
+     isb
+     add     x30, x30, #.Lv\@ - __efi_rt_vectors
+     br      x30
+     .endm
+
+.Lret:       msr     elr_el1, x29

Quoted 2 lines

If you take an IRQ here, aren't you toast?

Yep. So we need to switch this with setting the VBAR below then.

Quoted 8 lines

arch/arm64/kernel/efi-rt-wrapper.S ENTRY(__efi_rt_asm_wrapper) · line 86

Quoted 4 lines

arch/arm64/kernel/efi-rt-wrapper.S +4 -0

+__efi_rt_vectors:
+     .rept   8
+     ventry
+     .endr

Have you thought about SDEI at all? I can't see any code here to handle that.

Nope

Quoted 7 lines

Quoted 7 lines

arch/arm64/kernel/entry.S +0 -0

index b34e717d7597..3bab6c60a12b 100644
@@ -204,6 +204,7 @@ alternative_if ARM64_HAS_PAN
 alternative_else_nop_endif
 

Quoted 6 lines

arch/arm64/kernel/entry.S alternative_if ARM64_HAS_PAN · line 207

Quoted 2 lines

arch/arm64/kernel/entry.S +1 -0

      .if     \el != 0
+     tbz     x21, #63, 1f                    // skip if TTBR0 covers the stack

So this is really a "detect EFI" check, right? Maybe comment it as such. Also, probably want to check bit 55 just in case tagging ever takes off.

Right. So just bit #55 should be sufficient then, right?

Now that all UEFI runtime service wrappers ensure that byref arguments are moved into the UEFI marshalling buffer (which is not part of the kernel mapping), we can proceed and unmap the kernel while UEFI runtime service calls are in progress.

This is done by setting the EPD1 bit and flushing the TLB of the local CPU. This makes it independent of KPTI or whether non-global mappings are being used.

Quoted 5 lines · Ard Biesheuvel

Now that all UEFI runtime service wrappers ensure that byref arguments are moved into the UEFI marshalling buffer (which is not part of the kernel mapping), we can proceed and unmap the kernel while UEFI runtime service calls are in progress.

Quoted 3 lines

This is done by setting the EPD1 bit and flushing the TLB of the local CPU. This makes it independent of KPTI or whether non-global mappings are being used.

One snag with this is that it will break SPE, so I'd prefer this behaviour to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check in drivers/perf/arm_spe_pmu.c remains valid.

Quoted 10 lines · Will Deacon

Quoted 8 lines · Ard Biesheuvel

Now that all UEFI runtime service wrappers ensure that byref arguments are moved into the UEFI marshalling buffer (which is not part of the kernel mapping), we can proceed and unmap the kernel while UEFI runtime service calls are in progress.

This is done by setting the EPD1 bit and flushing the TLB of the local CPU. This makes it independent of KPTI or whether non-global mappings are being used.

Quoted 4 lines

One snag with this is that it will break SPE, so I'd prefer this behaviour to be predicated on kpti so that the arm64_kernel_unmapped_at_el0() check in drivers/perf/arm_spe_pmu.c remains valid.

The problem with that is that they serve two different purposes: kpti protects against meltdown, this protects against Spectre variant 1.

Note that capsule update has been omitted. This is a bit involved, and I'd rather get some feedback before burning too many cycles on that. All other services should be functional, with the caveat that EFI variable names are now limited to 1024 [wide] characters, and UEFI variables themselves to 64 KB.

Quoted 6 lines · Ard Biesheuvel

Note that capsule update has been omitted. This is a bit involved, and I'd rather get some feedback before burning too many cycles on that. All other services should be functional, with the caveat that EFI variable names are now limited to 1024 [wide] characters, and UEFI variables themselves to 64 KB.

I'm curious, why these limitations?

We don't have any concern about the name length limitation at this time, but the variable size one is concerning.

We have a feature which creates a variable greater than 64KB that we want to utilize from Linux. The size limitation would seem to prevent that.