2024-03-27 12:07:50

by Sasha Levin

[permalink] [raw]
Subject: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

The patch below does not apply to the 6.8-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <[email protected]>.

Thanks,
Sasha

------------------ original commit in Linus's tree ------------------

From cefcd4fe2e3aaf792c14c9e56dab89e3d7a65d02 Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <[email protected]>
Date: Fri, 22 Mar 2024 17:03:58 +0200
Subject: [PATCH] x86/efistub: Call mixed mode boot services on the firmware's
stack

Normally, the EFI stub calls into the EFI boot services using the stack
that was live when the stub was entered. According to the UEFI spec,
this stack needs to be at least 128k in size - this might seem large but
all asynchronous processing and event handling in EFI runs from the same
stack and so quite a lot of space may be used in practice.

In mixed mode, the situation is a bit different: the bootloader calls
the 32-bit EFI stub entry point, which calls the decompressor's 32-bit
entry point, where the boot stack is set up, using a fixed allocation
of 16k. This stack is still in use when the EFI stub is started in
64-bit mode, and so all calls back into the EFI firmware will be using
the decompressor's limited boot stack.

Due to the placement of the boot stack right after the boot heap, any
stack overruns have gone unnoticed. However, commit

5c4feadb0011983b ("x86/decompressor: Move global symbol references to C code")

moved the definition of the boot heap into C code, and now the boot
stack is placed right at the base of BSS, where any overruns will
corrupt the end of the .data section.

While it would be possible to work around this by increasing the size of
the boot stack, doing so would affect all x86 systems, and mixed mode
systems are a tiny (and shrinking) fraction of the x86 installed base.

So instead, record the firmware stack pointer value when entering from
the 32-bit firmware, and switch to this stack every time a EFI boot
service call is made.

Cc: <[email protected]> # v6.1+
Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/compressed/efi_mixed.S | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
index f4e22ef774ab6..719e939050cbf 100644
--- a/arch/x86/boot/compressed/efi_mixed.S
+++ b/arch/x86/boot/compressed/efi_mixed.S
@@ -49,6 +49,11 @@ SYM_FUNC_START(startup_64_mixed_mode)
lea efi32_boot_args(%rip), %rdx
mov 0(%rdx), %edi
mov 4(%rdx), %esi
+
+ /* Switch to the firmware's stack */
+ movl efi32_boot_sp(%rip), %esp
+ andl $~7, %esp
+
#ifdef CONFIG_EFI_HANDOVER_PROTOCOL
mov 8(%rdx), %edx // saved bootparams pointer
test %edx, %edx
@@ -254,6 +259,9 @@ SYM_FUNC_START_LOCAL(efi32_entry)
/* Store firmware IDT descriptor */
sidtl (efi32_boot_idt - 1b)(%ebx)

+ /* Store firmware stack pointer */
+ movl %esp, (efi32_boot_sp - 1b)(%ebx)
+
/* Store boot arguments */
leal (efi32_boot_args - 1b)(%ebx), %ebx
movl %ecx, 0(%ebx)
@@ -318,5 +326,6 @@ SYM_DATA_END(efi32_boot_idt)

SYM_DATA_LOCAL(efi32_boot_cs, .word 0)
SYM_DATA_LOCAL(efi32_boot_ds, .word 0)
+SYM_DATA_LOCAL(efi32_boot_sp, .long 0)
SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0, 0)
SYM_DATA(efi_is64, .byte 1)
--
2.43.0






2024-03-27 14:53:16

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

On Wed, 27 Mar 2024 at 14:06, Sasha Levin <[email protected]> wrote:
>
> The patch below does not apply to the 6.8-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <[email protected]>.
>

This applies fine on top of 6.8.2, 6.7.11 and 6.6.23.

On 6.1.83, it gave me a warning

Auto-merging arch/x86/boot/compressed/efi_mixed.S

but the change still applied without problems.

Not sure what is going on here .....


> ------------------ original commit in Linus's tree ------------------
>
> From cefcd4fe2e3aaf792c14c9e56dab89e3d7a65d02 Mon Sep 17 00:00:00 2001
> From: Ard Biesheuvel <[email protected]>
> Date: Fri, 22 Mar 2024 17:03:58 +0200
> Subject: [PATCH] x86/efistub: Call mixed mode boot services on the firmware's
> stack
>
> Normally, the EFI stub calls into the EFI boot services using the stack
> that was live when the stub was entered. According to the UEFI spec,
> this stack needs to be at least 128k in size - this might seem large but
> all asynchronous processing and event handling in EFI runs from the same
> stack and so quite a lot of space may be used in practice.
>
> In mixed mode, the situation is a bit different: the bootloader calls
> the 32-bit EFI stub entry point, which calls the decompressor's 32-bit
> entry point, where the boot stack is set up, using a fixed allocation
> of 16k. This stack is still in use when the EFI stub is started in
> 64-bit mode, and so all calls back into the EFI firmware will be using
> the decompressor's limited boot stack.
>
> Due to the placement of the boot stack right after the boot heap, any
> stack overruns have gone unnoticed. However, commit
>
> 5c4feadb0011983b ("x86/decompressor: Move global symbol references to C code")
>
> moved the definition of the boot heap into C code, and now the boot
> stack is placed right at the base of BSS, where any overruns will
> corrupt the end of the .data section.
>
> While it would be possible to work around this by increasing the size of
> the boot stack, doing so would affect all x86 systems, and mixed mode
> systems are a tiny (and shrinking) fraction of the x86 installed base.
>
> So instead, record the firmware stack pointer value when entering from
> the 32-bit firmware, and switch to this stack every time a EFI boot
> service call is made.
>
> Cc: <[email protected]> # v6.1+
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> arch/x86/boot/compressed/efi_mixed.S | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/efi_mixed.S b/arch/x86/boot/compressed/efi_mixed.S
> index f4e22ef774ab6..719e939050cbf 100644
> --- a/arch/x86/boot/compressed/efi_mixed.S
> +++ b/arch/x86/boot/compressed/efi_mixed.S
> @@ -49,6 +49,11 @@ SYM_FUNC_START(startup_64_mixed_mode)
> lea efi32_boot_args(%rip), %rdx
> mov 0(%rdx), %edi
> mov 4(%rdx), %esi
> +
> + /* Switch to the firmware's stack */
> + movl efi32_boot_sp(%rip), %esp
> + andl $~7, %esp
> +
> #ifdef CONFIG_EFI_HANDOVER_PROTOCOL
> mov 8(%rdx), %edx // saved bootparams pointer
> test %edx, %edx
> @@ -254,6 +259,9 @@ SYM_FUNC_START_LOCAL(efi32_entry)
> /* Store firmware IDT descriptor */
> sidtl (efi32_boot_idt - 1b)(%ebx)
>
> + /* Store firmware stack pointer */
> + movl %esp, (efi32_boot_sp - 1b)(%ebx)
> +
> /* Store boot arguments */
> leal (efi32_boot_args - 1b)(%ebx), %ebx
> movl %ecx, 0(%ebx)
> @@ -318,5 +326,6 @@ SYM_DATA_END(efi32_boot_idt)
>
> SYM_DATA_LOCAL(efi32_boot_cs, .word 0)
> SYM_DATA_LOCAL(efi32_boot_ds, .word 0)
> +SYM_DATA_LOCAL(efi32_boot_sp, .long 0)
> SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0, 0)
> SYM_DATA(efi_is64, .byte 1)
> --
> 2.43.0
>
>
>
>

2024-03-29 06:13:57

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

On Wed, 27 Mar 2024 at 15:46, Ard Biesheuvel <[email protected]> wrote:
>
> On Wed, 27 Mar 2024 at 14:06, Sasha Levin <[email protected]> wrote:
> >
> > The patch below does not apply to the 6.8-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <[email protected]>.
> >
>
> This applies fine on top of 6.8.2, 6.7.11 and 6.6.23.
>
> On 6.1.83, it gave me a warning
>
> Auto-merging arch/x86/boot/compressed/efi_mixed.S
>
> but the change still applied without problems.
>
> Not sure what is going on here .....
>

Ping?

2024-03-29 09:16:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

On Fri, Mar 29, 2024 at 08:13:36AM +0200, Ard Biesheuvel wrote:
> On Wed, 27 Mar 2024 at 15:46, Ard Biesheuvel <[email protected]> wrote:
> >
> > On Wed, 27 Mar 2024 at 14:06, Sasha Levin <[email protected]> wrote:
> > >
> > > The patch below does not apply to the 6.8-stable tree.
> > > If someone wants it applied there, or to any other stable or longterm
> > > tree, then please email the backport, including the original git commit
> > > id to <[email protected]>.
> > >
> >
> > This applies fine on top of 6.8.2, 6.7.11 and 6.6.23.
> >
> > On 6.1.83, it gave me a warning
> >
> > Auto-merging arch/x86/boot/compressed/efi_mixed.S
> >
> > but the change still applied without problems.
> >
> > Not sure what is going on here .....
> >
>
> Ping?
>

Give us time to catch up please, I'll go look at this now...

2024-03-29 09:18:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

On Wed, Mar 27, 2024 at 03:46:53PM +0200, Ard Biesheuvel wrote:
> On Wed, 27 Mar 2024 at 14:06, Sasha Levin <[email protected]> wrote:
> >
> > The patch below does not apply to the 6.8-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <[email protected]>.
> >
>
> This applies fine on top of 6.8.2, 6.7.11 and 6.6.23.
>
> On 6.1.83, it gave me a warning
>
> Auto-merging arch/x86/boot/compressed/efi_mixed.S
>
> but the change still applied without problems.
>
> Not sure what is going on here .....

Odd, it worked here forme, now queued up everywhere, sorry for the
noise.

greg k-h

2024-03-29 09:19:38

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: FAILED: Patch "x86/efistub: Call mixed mode boot services on the firmware's stack" failed to apply to 6.8-stable tree

On Fri, 29 Mar 2024 at 11:17, Greg KH <[email protected]> wrote:
>
> On Wed, Mar 27, 2024 at 03:46:53PM +0200, Ard Biesheuvel wrote:
> > On Wed, 27 Mar 2024 at 14:06, Sasha Levin <[email protected]> wrote:
> > >
> > > The patch below does not apply to the 6.8-stable tree.
> > > If someone wants it applied there, or to any other stable or longterm
> > > tree, then please email the backport, including the original git commit
> > > id to <[email protected]>.
> > >
> >
> > This applies fine on top of 6.8.2, 6.7.11 and 6.6.23.
> >
> > On 6.1.83, it gave me a warning
> >
> > Auto-merging arch/x86/boot/compressed/efi_mixed.S
> >
> > but the change still applied without problems.
> >
> > Not sure what is going on here .....
>
> Odd, it worked here forme, now queued up everywhere, sorry for the
> noise.
>

Thanks!