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

[Ard Biesheuvel]

On Wed, 27 Mar 2024 at 14:06, Sasha Levin <sashal@xxxxxxxxxx> 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 <stable@xxxxxxxxxxxxxxx>.
>

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 <ardb@xxxxxxxxxx>
> 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: <stable@xxxxxxxxxx> # v6.1+
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
>  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
>
>
>
>