linux-kernel - Re: [Patch] Do not clear BSS region in x86 stub

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <A2AEC38D-56FB-4687-BDC2-54F1600F14A1@zytor.com>
Date: Wed, 17 Jul 2024 23:15:21 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: "Shao, Marshall" <Marshall.Shao@...l.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "bp@...en8.de" <bp@...en8.de>, "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>
CC: "Mishra, Ashish" <Ashish.Mishra4@...l.com>,
        "Dion, Christopher" <Christopher.Dion@...l.com>,
        "Caisse, Joe" <Joe.Caisse@...l.com>,
        "Chia, Jia Yuan" <JiaYuan.Chia@...l.com>,
        "Mukundan, Govind" <Govind.Mukundan@...l.com>
Subject: Re: [Patch] Do not clear BSS region in x86 stub

On July 17, 2024 12:33:08 AM PDT, "Shao, Marshall" <Marshall.Shao@...l.com> wrote:
>From: Marshall Shao <marshall.shao@...l.com>
>
>Clearing the BSS region may cause the UEFI firmware to malfunction
>during boot.
>
>When booting the kernel from an older firmware version that has TPM
>enabled and the MemoryOverwriteRequestControl bit set to 1, the
>firmware's boot service might encounter an exception if it attempts
>to initialize the BSS region within the x86 stub.
>
>To circumvent the firmware exception, it is advisable to enlarge the
>BOOT_STACK_SIZE and to perform the initialization of static variables
>prior to the decompression of the bzImage.
>
>Signed-off-by: Marshall Shao <marshall.shao@...l.com>
>---
> arch/x86/boot/compressed/misc.c         | 8 +++-----
> arch/x86/include/asm/boot.h             | 2 +-
> drivers/firmware/efi/libstub/x86-stub.c | 5 -----
> 3 files changed, 4 insertions(+), 11 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
>index b70e4a21c15f..bac5a3c55c2c 100644
>--- a/arch/x86/boot/compressed/misc.c
>+++ b/arch/x86/boot/compressed/misc.c
>@@ -356,11 +356,9 @@ unsigned long decompress_kernel(unsigned char *outbuf, unsigned long virt_addr,
>                                void (*error)(char *x))
> {
>        unsigned long entry;
>-
>-       if (!free_mem_ptr) {
>-               free_mem_ptr     = (unsigned long)boot_heap;
>-               free_mem_end_ptr = (unsigned long)boot_heap + sizeof(boot_heap);
>-       }
>+       free_mem_ptr     = (unsigned long)boot_heap;
>+       free_mem_end_ptr = (unsigned long)boot_heap + sizeof(boot_heap);
>+       malloc_ptr = free_mem_ptr;
>
>        if (__decompress(input_data, input_len, NULL, NULL, outbuf, output_len,
>                         NULL, error) < 0)
>diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
>index 3e5b111e619d..312bc87ab027 100644
>--- a/arch/x86/include/asm/boot.h
>+++ b/arch/x86/include/asm/boot.h
>@@ -33,7 +33,7 @@
> #endif
>
> #ifdef CONFIG_X86_64
>-# define BOOT_STACK_SIZE       0x4000
>+# define BOOT_STACK_SIZE       0x10000
>
> /*
>  * Used by decompressor's startup_32() to allocate page tables for identity
>diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
>index 1983fd3bf392..d92d2ccc709b 100644
>--- a/drivers/firmware/efi/libstub/x86-stub.c
>+++ b/drivers/firmware/efi/libstub/x86-stub.c
>@@ -21,7 +21,6 @@
> #include "efistub.h"
> #include "x86-stub.h"
>
>-extern char _bss[], _ebss[];
>
> const efi_system_table_t *efi_system_table;
> const efi_dxe_services_table_t *efi_dxe_table;
>@@ -476,9 +475,6 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
>        efi_status_t status;
>        char *cmdline_ptr;
>
>-       if (efi_is_native())
>-               memset(_bss, 0, _ebss - _bss);
>-
>        efi_system_table = sys_table_arg;
>
>        /* Check if we were booted by the EFI firmware */
>@@ -1000,7 +996,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> void efi_handover_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg,
>                        struct boot_params *boot_params)
> {
>-       memset(_bss, 0, _ebss - _bss);
>        efi_stub_entry(handle, sys_table_arg, boot_params);
> }
>
>--
>2.34.1
>
>
>Internal Use - Confidential
>

This is strange. Is the problem that the BSS is not properly reflected in the UEFI PECOFF header?