Skip to contentSkip to conversation

More EFI updates for v5.7

GIT PULL New Ard Biesheuvel 1 participant 8 messages Feb 28, 2020 – Mar 4, 2020

Subsystems (4)
lore
Share & tools
lore

Ard Biesheuvel

author maintainer
#0 lore raw

Hello Ingo, Thomas,

A small set of EFI followup changes for v5.7. The last one fixes a boot regression in linux-next on x86 machines booting without EFI but with the IMA security subsystem enabled, which is why I am sending out the next batch a bit earlier than intended.

Thanks,
Ard.

The following changes since commit e9765680a31b22ca6703936c000ce5cc46192e10:

Merge tag 'efi-next' of git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi into efi/core (2020-02-26 15:21:22 +0100)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to be15278269343ec0e4d0e41bab5f64b49b0edb6b:

efi: mark all EFI runtime services as unsupported on non-EFI boot (2020-02-28 12:54:46 +0100)

----------------------------------------------------------------
More EFI updates for v5.7

A couple of followup fixes for the EFI changes queued for v5.7:
- a fix for a boot regression on x86 booting without UEFI
- memory encryption fixes for x86, so that the TPM tables and the RNG config table created by the stub are correctly identified as living in unencrypted memory
- style tweak from Heinrich
- followup to the ARM EFI entry code simplifications to ensure that we don't rely on EFI_LOADER_DATA memory being RWX

----------------------------------------------------------------
Ard Biesheuvel (3):
efi/arm: clean EFI stub exit code from cache instead of avoiding it efi/arm64: clean EFI stub exit code from cache instead of avoiding it
efi: mark all EFI runtime services as unsupported on non-EFI boot

Heinrich Schuchardt (1):
efi: don't shadow i in efi_config_parse_tables()

Tom Lendacky (2):
efi/x86: Add TPM related EFI tables to unencrypted mapping checks efi/x86: Add RNG seed EFI table to unencrypted mapping check

 arch/arm/boot/compressed/head.S | 18 ++++++++----------
 arch/arm64/kernel/efi-entry.S   | 26 +++++++++++++-------------
 arch/arm64/kernel/image-vars.h  |  4 ++--
 arch/x86/platform/efi/efi.c     |  3 +++
 drivers/firmware/efi/efi.c      | 25 +++++++++++++------------
 include/linux/efi.h             |  2 ++
 6 files changed, 41 insertions(+), 37 deletions(-)
author maintainer
#1 lore raw

PATCH 1/6 efi/x86: Add TPM related EFI tables to unencrypted mapping checks

From: Tom Lendacky <[email protected]>

When booting with SME active, EFI tables must be mapped unencrypted since they were built by UEFI in unencrypted memory. Update the list of tables to be checked during early_memremap() processing to account for the EFI TPM tables.

This fixes a bug where an EFI TPM log table has been created by UEFI, but it lives in memory that has been marked as usable rather than reserved.

---
 arch/x86/platform/efi/efi.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/platform/efi/efi.c +2 -0
index 43b24e149312..0a8117865430 100644
@@ -88,6 +88,8 @@ static const unsigned long * const efi_tables[] = {
 #ifdef CONFIG_EFI_RCI2_TABLE
 	&rci2_table_phys,
 #endif
+	&efi.tpm_log,
+	&efi.tpm_final_log,
 };
 
 u64 efi_setup;		/* efi setup_data physical address */
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH 2/6 efi/x86: Add RNG seed EFI table to unencrypted mapping check

In reply to Ard Biesheuvel

From: Tom Lendacky <[email protected]>

When booting with SME active, EFI tables must be mapped unencrypted since they were built by UEFI in unencrypted memory. Update the list of tables to be checked during early_memremap() processing to account for the EFI RNG seed table.

---
 arch/x86/platform/efi/efi.c |  1 +
 drivers/firmware/efi/efi.c  | 18 ++++++++++--------
 include/linux/efi.h         |  2 ++
 3 files changed, 13 insertions(+), 8 deletions(-)
arch/x86/platform/efi/efi.c +1 -0
index 0a8117865430..aca9bdd87bca 100644
@@ -90,6 +90,7 @@ static const unsigned long * const efi_tables[] = {
 #endif
 	&efi.tpm_log,
 	&efi.tpm_final_log,
+	&efi_rng_seed,
 };
 
 u64 efi_setup;		/* efi setup_data physical address */
drivers/firmware/efi/efi.c +10 -8
index 69a585106d30..1e79f77d4e6c 100644
@@ -45,7 +45,7 @@ struct efi __read_mostly efi = {
 };
 EXPORT_SYMBOL(efi);
 
-static unsigned long __ro_after_init rng_seed = EFI_INVALID_TABLE_ADDR;
+unsigned long __ro_after_init efi_rng_seed = EFI_INVALID_TABLE_ADDR;
 static unsigned long __initdata mem_reserve = EFI_INVALID_TABLE_ADDR;
 static unsigned long __initdata rt_prop = EFI_INVALID_TABLE_ADDR;
 
@@ -451,7 +451,7 @@ static const efi_config_table_type_t common_tables[] __initconst = {
 	{SMBIOS3_TABLE_GUID, "SMBIOS 3.0", &efi.smbios3},
 	{EFI_SYSTEM_RESOURCE_TABLE_GUID, "ESRT", &efi.esrt},
 	{EFI_MEMORY_ATTRIBUTES_TABLE_GUID, "MEMATTR", &efi_mem_attr_table},
-	{LINUX_EFI_RANDOM_SEED_TABLE_GUID, "RNG", &rng_seed},
+	{LINUX_EFI_RANDOM_SEED_TABLE_GUID, "RNG", &efi_rng_seed},
 	{LINUX_EFI_TPM_EVENT_LOG_GUID, "TPMEventLog", &efi.tpm_log},
 	{LINUX_EFI_TPM_FINAL_LOG_GUID, "TPMFinalLog", &efi.tpm_final_log},
 	{LINUX_EFI_MEMRESERVE_TABLE_GUID, "MEMRESERVE", &mem_reserve},
@@ -519,11 +519,11 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 	pr_cont("\n");
 	set_bit(EFI_CONFIG_TABLES, &efi.flags);
 
-	if (rng_seed != EFI_INVALID_TABLE_ADDR) {
+	if (efi_rng_seed != EFI_INVALID_TABLE_ADDR) {
 		struct linux_efi_random_seed *seed;
 		u32 size = 0;
 
-		seed = early_memremap(rng_seed, sizeof(*seed));
+		seed = early_memremap(efi_rng_seed, sizeof(*seed));
 		if (seed != NULL) {
 			size = seed->size;
 			early_memunmap(seed, sizeof(*seed));
@@ -531,7 +531,8 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 			pr_err("Could not map UEFI random seed!\n");
 		}
 		if (size > 0) {
-			seed = early_memremap(rng_seed, sizeof(*seed) + size);
+			seed = early_memremap(efi_rng_seed,
+					      sizeof(*seed) + size);
 			if (seed != NULL) {
 				pr_notice("seeding entropy pool\n");
 				add_bootloader_randomness(seed->bits, seed->size);
@@ -923,7 +924,7 @@ static int update_efi_random_seed(struct notifier_block *nb,
 	if (!kexec_in_progress)
 		return NOTIFY_DONE;
 
-	seed = memremap(rng_seed, sizeof(*seed), MEMREMAP_WB);
+	seed = memremap(efi_rng_seed, sizeof(*seed), MEMREMAP_WB);
 	if (seed != NULL) {
 		size = min(seed->size, EFI_RANDOM_SEED_SIZE);
 		memunmap(seed);
@@ -931,7 +932,8 @@ static int update_efi_random_seed(struct notifier_block *nb,
 		pr_err("Could not map UEFI random seed!\n");
 	}
 	if (size > 0) {
-		seed = memremap(rng_seed, sizeof(*seed) + size, MEMREMAP_WB);
+		seed = memremap(efi_rng_seed, sizeof(*seed) + size,
+				MEMREMAP_WB);
 		if (seed != NULL) {
 			seed->size = size;
 			get_random_bytes(seed->bits, seed->size);
@@ -949,7 +951,7 @@ static struct notifier_block efi_random_seed_nb = {
 
 static int __init register_update_efi_random_seed(void)
 {
-	if (rng_seed == EFI_INVALID_TABLE_ADDR)
+	if (efi_rng_seed == EFI_INVALID_TABLE_ADDR)
 		return 0;
 	return register_reboot_notifier(&efi_random_seed_nb);
 }
include/linux/efi.h +2 -0
index 2ab33d5d6ca5..e8a08a499131 100644
@@ -526,6 +526,8 @@ typedef struct {
 	efi_time_t time_of_revocation;
 } efi_cert_x509_sha256_t;
 
+extern unsigned long __ro_after_init efi_rng_seed;		/* RNG Seed table */
+
 /*
  * All runtime access to EFI goes through this structure:
  */
Signature · 1 line
2.17.1
author maintainer
#3 lore raw

PATCH 3/6 efi: don't shadow i in efi_config_parse_tables()

In reply to Ard Biesheuvel

From: Heinrich Schuchardt <[email protected]>

Shadowing variables is generally frowned upon.

Let's simply reuse the existing loop counter i instead of shadowing it.

---
 drivers/firmware/efi/efi.c | 1 -
 1 file changed, 1 deletion(-)
drivers/firmware/efi/efi.c +0 -1
index 1e79f77d4e6c..41269a95ff85 100644
@@ -554,7 +554,6 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
 		while (prsv) {
 			struct linux_efi_memreserve *rsv;
 			u8 *p;
-			int i;
 
 			/*
 			 * Just map a full page: that is what we will get
Signature · 1 line
2.17.1
author maintainer
#4 lore raw

PATCH 4/6 efi/arm: clean EFI stub exit code from cache instead of avoiding it

In reply to Ard Biesheuvel

Commit c7225494b ("efi/arm: Work around missing cache maintenance in decompressor handover") modified the handover code written in assembler to work around the missing cache maintenance of the piece of code that is executed after the MMU and caches are turned off. Due to the fact that this sequence incorporates a subroutine call, cleaning that code from the cache is not a matter of simply passing the start and end of the currently running subroutine into cache_clean_flush(), which is why instead, the code jumps across into the cleaned copy of the image.

However, this assumes that this copy is executable, and this means we expect EFI_LOADER_DATA regions to be executable as well, which is not a reasonable assumption to make, even if this is true for most UEFI implementations today.

So change this back, and add a cache_clean_flush() call to cover the remaining code in the subroutine, and any code it may execute in the context of cache_off().

---
 arch/arm/boot/compressed/head.S | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
arch/arm/boot/compressed/head.S +8 -10
index 36ffbeecd30b..04f77214f050 100644
@@ -1452,13 +1452,11 @@ ENTRY(efi_enter_kernel)
 
 		@ The PE/COFF loader might not have cleaned the code we are
 		@ running beyond the PoU, and so calling cache_off below from
-		@ inside the PE/COFF loader allocated region is unsafe. Let's
-		@ assume our own zImage relocation code did a better job, and
-		@ jump into its version of this routine before proceeding.
-		ldr	r1, .Ljmp
-		sub	r1, r7, r1
-		mov	pc, r1				@ no mode switch
-0:
+		@ inside the PE/COFF loader allocated region is unsafe unless
+		@ we explicitly clean it to the PoC.
+		adr	r0, call_cache_fn		@ region of code we will
+		adr	r1, 0f				@ run with MMU off
+		bl	cache_clean_flush
 		bl	cache_off
 
 		@ Set parameters for booting zImage according to boot protocol
@@ -1467,10 +1465,10 @@ ENTRY(efi_enter_kernel)
 		mov	r0, #0
 		mov	r1, #0xFFFFFFFF
 		mov	r2, r4
-		b	__efi_start
+		add	r7, r7, #(__efi_start - start)
+		mov	pc, r7				@ no mode switch
 ENDPROC(efi_enter_kernel)
-		.align	2
-.Ljmp:		.long	start - 0b
+0:
 #endif
 
 		.align
Signature · 1 line
2.17.1
author maintainer
#5 lore raw

PATCH 5/6 efi/arm64: clean EFI stub exit code from cache instead of avoiding it

In reply to Ard Biesheuvel

Commit 9f9223778 ("efi/libstub/arm: Make efi_entry() an ordinary PE/COFF entrypoint") modified the handover code written in assembler, and for maintainability, aligned the logic with the logic used in the 32-bit ARM version, which is to avoid cache maintenance on the remaining instructions in the subroutine that will be executed with the MMU and caches off, and instead, branch into the relocated copy of the kernel image.

However, this assumes that this copy is executable, and this means we expect EFI_LOADER_DATA regions to be executable as well, which is not a reasonable assumption to make, even if this is true for most UEFI implementations today.

So change this back, and add a __clean_dcache_area_poc() call to cover the remaining code in the subroutine. While at it, switch the other call site over to __clean_dcache_area_poc() as well, and clean up the terminology in comments to avoid using 'flush' in the context of cache maintenance. Also, let's switch to the new style asm annotations.

---
 arch/arm64/kernel/efi-entry.S  | 26 +++++++++++++-------------
 arch/arm64/kernel/image-vars.h |  4 ++--
 2 files changed, 15 insertions(+), 15 deletions(-)
arch/arm64/kernel/efi-entry.S +13 -13
index 4cfd03c35c49..1a03618df0df 100644
@@ -12,32 +12,32 @@
 
 	__INIT
 
-ENTRY(efi_enter_kernel)
+SYM_CODE_START(efi_enter_kernel)
 	/*
 	 * efi_entry() will have copied the kernel image if necessary and we
 	 * end up here with device tree address in x1 and the kernel entry
 	 * point stored in x0. Save those values in registers which are
 	 * callee preserved.
 	 */
-	mov	x19, x0			// relocated Image address
+	ldr	w2, =stext_offset
+	add	x19, x0, x2		// relocated Image entrypoint
 	mov	x20, x1			// DTB address
 
 	/*
-	 * Flush the copied Image to the PoC, and ensure it is not shadowed by
+	 * Clean the copied Image to the PoC, and ensure it is not shadowed by
 	 * stale icache entries from before relocation.
 	 */
 	ldr	w1, =kernel_size
-	bl	__flush_dcache_area
+	bl	__clean_dcache_area_poc
 	ic	ialluis
-	dsb	sy
 
 	/*
-	 * Jump across, into the copy of the image that we just cleaned
-	 * to the PoC, so that we can safely disable the MMU and caches.
+	 * Clean the remainder of this routine to the PoC
+	 * so that we can safely disable the MMU and caches.
 	 */
-	ldr	w0, .Ljmp
-	sub	x0, x19, w0, sxtw
-	br	x0
+	adr	x0, 0f
+	ldr	w1, 3f
+	bl	__clean_dcache_area_poc
 0:
 	/* Turn off Dcache and MMU */
 	mrs	x0, CurrentEL
@@ -63,6 +63,6 @@ ENTRY(efi_enter_kernel)
 	mov	x1, xzr
 	mov	x2, xzr
 	mov	x3, xzr
-	b	stext
-ENDPROC(efi_enter_kernel)
-.Ljmp:	.long	_text - 0b
+	br	x19
+SYM_CODE_END(efi_enter_kernel)
+3:	.long	. - 0b
arch/arm64/kernel/image-vars.h +2 -2
index 9a7aef0d6f70..7f06ad93fc95 100644
@@ -13,6 +13,7 @@
 #ifdef CONFIG_EFI
 
 __efistub_kernel_size		= _edata - _text;
+__efistub_stext_offset		= stext - _text;
 
 
 /*
@@ -34,7 +35,7 @@ __efistub_strnlen		= __pi_strnlen;
 __efistub_strcmp		= __pi_strcmp;
 __efistub_strncmp		= __pi_strncmp;
 __efistub_strrchr		= __pi_strrchr;
-__efistub___flush_dcache_area	= __pi___flush_dcache_area;
+__efistub___clean_dcache_area_poc = __pi___clean_dcache_area_poc;
 
 #ifdef CONFIG_KASAN
 __efistub___memcpy		= __pi_memcpy;
@@ -43,7 +44,6 @@ __efistub___memset		= __pi_memset;
 #endif
 
 __efistub__text			= _text;
-__efistub_stext			= stext;
 __efistub__end			= _end;
 __efistub__edata		= _edata;
 __efistub_screen_info		= screen_info;
Signature · 1 line
2.17.1
author maintainer
#6 lore raw

PATCH 6/6 efi: mark all EFI runtime services as unsupported on non-EFI boot

In reply to Ard Biesheuvel

Recent changes to the way we deal with EFI runtime services that are marked as unsupported by the firmware resulted in a regression for non-EFI boot. The problem is that all EFI runtime services are marked as available by default, and any non-NULL checks on the EFI service function pointers (which will be non-NULL even for runtime services that are unsupported on an EFI boot) were replaced with checks against the mask stored in efi.runtime_supported_mask.

When doing a non-EFI boot, this check against the mask will return a false positive, given the fact that all runtime services are marked as enabled by default. Since we dropped the non-NULL check of the runtime service function pointer in favor of the mask check, we will now unconditionally dereference the function pointer, even if it is NULL, and go boom.

So let's ensure that the mask reflects reality on a non-EFI boot, which is that all EFI runtime services are unsupported.

---
 drivers/firmware/efi/efi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/firmware/efi/efi.c +3 -3
index 41269a95ff85..d1746a579c99 100644
@@ -300,12 +300,12 @@ static int __init efisubsys_init(void)
 {
 	int error;
 
-	if (!efi_enabled(EFI_BOOT))
-		return 0;
-
 	if (!efi_enabled(EFI_RUNTIME_SERVICES))
 		efi.runtime_supported_mask = 0;
 
+	if (!efi_enabled(EFI_BOOT))
+		return 0;
+
 	if (efi.runtime_supported_mask) {
 		/*
 		 * Since we process only one efi_runtime_service() at a time, an
Signature · 1 line
2.17.1
author maintainer
#7 lore raw
In reply to Ard Biesheuvel
Quoted 8 lines · Ard Biesheuvel

Hello Ingo, Thomas,

A small set of EFI followup changes for v5.7. The last one fixes a boot regression in linux-next on x86 machines booting without EFI but with the IMA security subsystem enabled, which is why I am sending out the next batch a bit earlier than intended.

Please disregard this for now. I will send a v2 tomorrow or Friday which will contain a few more fixes for the changes that are queued for v5.7 already.

Quoted 45 lines

The following changes since commit e9765680a31b22ca6703936c000ce5cc46192e10:

Merge tag 'efi-next' of git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi into efi/core (2020-02-26 15:21:22 +0100)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to be15278269343ec0e4d0e41bab5f64b49b0edb6b:

efi: mark all EFI runtime services as unsupported on non-EFI boot (2020-02-28 12:54:46 +0100)

----------------------------------------------------------------
More EFI updates for v5.7

A couple of followup fixes for the EFI changes queued for v5.7:
- a fix for a boot regression on x86 booting without UEFI
- memory encryption fixes for x86, so that the TPM tables and the RNG config table created by the stub are correctly identified as living in unencrypted memory
- style tweak from Heinrich
- followup to the ARM EFI entry code simplifications to ensure that we don't rely on EFI_LOADER_DATA memory being RWX

----------------------------------------------------------------
Ard Biesheuvel (3):
efi/arm: clean EFI stub exit code from cache instead of avoiding it efi/arm64: clean EFI stub exit code from cache instead of avoiding it
efi: mark all EFI runtime services as unsupported on non-EFI boot

Heinrich Schuchardt (1):
efi: don't shadow i in efi_config_parse_tables()

Tom Lendacky (2):
efi/x86: Add TPM related EFI tables to unencrypted mapping checks efi/x86: Add RNG seed EFI table to unencrypted mapping check

 arch/arm/boot/compressed/head.S | 18 ++++++++----------
 arch/arm64/kernel/efi-entry.S   | 26 +++++++++++++-------------
 arch/arm64/kernel/image-vars.h  |  4 ++--
 arch/x86/platform/efi/efi.c     |  3 +++
 drivers/firmware/efi/efi.c      | 25 +++++++++++++------------
 include/linux/efi.h             |  2 ++
 6 files changed, 41 insertions(+), 37 deletions(-)