diff mbox series

efi/x86: Clear BSS when entering in mixed mode via compat entrypoint

Message ID 20240610215304.2953611-2-ardb+git@google.com
State New
Headers show
Series efi/x86: Clear BSS when entering in mixed mode via compat entrypoint | expand

Commit Message

Ard Biesheuvel June 10, 2024, 9:53 p.m. UTC 
From: Ard Biesheuvel <ardb@kernel.org>

Nicholas reports that since commit

  df7ecce842b8 ("x86/efistub: Don't clear BSS twice in mixed mode")

booting in mixed mode via the compat entrypoint is broken on firmware/loader
combinations where the PE/COFF executable is entered with BSS containing
garbage.

The above commit changed the logic because clearing BSS in the 64-bit
entrypoint is too late when booting in mixed mode, as at this point, BSS
variables may have already been modified by the program, and so
reverting it is not an option.

So instead, fix this issue by moving the existing mixed mode BSS
clearing logic to the 32-bit startup routine that is shared between the
different mixed mode boot protocols, so that it is called immediately
after entering from the firmware in all cases.

Fixes: df7ecce842b8 ("x86/efistub: Don't clear BSS twice in mixed mode")
Reported-by: Nicholas Bishop <nicholasbishop@google.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/efi_mixed.S | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Nicholas Bishop June 11, 2024, 4:49 p.m. UTC | #1 
Works for me, tested both my Compute Stick and VM.

Tested-by: Nicholas Bishop <nicholasbishop@google.com>
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
index 876fc6d46a13..a4035c426229 100644
--- a/arch/x86/boot/compressed/efi_mixed.S
+++ b/arch/x86/boot/compressed/efi_mixed.S
@@ -152,16 +152,6 @@  SYM_FUNC_START(efi32_stub_entry)
 	call	1f
 1:	popl	%ecx
 	leal	(efi32_boot_args - 1b)(%ecx), %ebx
-
-	/* Clear BSS */
-	xorl	%eax, %eax
-	leal	(_bss - 1b)(%ecx), %edi
-	leal	(_ebss - 1b)(%ecx), %ecx
-	subl	%edi, %ecx
-	shrl	$2, %ecx
-	cld
-	rep	stosl
-
 	add	$0x4, %esp		/* Discard return address */
 	popl	%ecx
 	popl	%edx
@@ -264,12 +254,24 @@  SYM_FUNC_START_LOCAL(efi32_entry)
 	/* Store firmware stack pointer */
 	movl	%esp, (efi32_boot_sp - 1b)(%ebx)
 
+	/* Take the address of _bss in %edi */
+	movl	$_bss - 1b, %esi
+	leal	(%ebx, %esi), %edi
+
 	/* Store boot arguments */
 	leal	(efi32_boot_args - 1b)(%ebx), %ebx
 	movl	%ecx, 0(%ebx)
 	movl	%edx, 4(%ebx)
 	movb	$0x0, 12(%ebx)          // efi_is64
 
+	/* Clear BSS */
+	xorl	%eax, %eax
+	movl	$_ebss - 1b, %ecx
+	subl	%esi, %ecx
+	shrl	$2, %ecx
+	cld
+	rep	stosl
+
 	/*
 	 * Allocate some memory for a temporary struct boot_params, which only
 	 * needs the minimal pieces that startup_32() relies on.