EFI fixes for -next
Subsystems (3)
Ard Biesheuvel
maintainercover EFI fixes for -next
A couple of fixes for more unusual configurations that weren't caught in my testing before. One for KASAN, one for the LLVM linker and one for the old EFI memory map when running on mixed mode systems.
These apply onto the Git pull request [0[ that I sent out at the beginning of the week.
[0] https://lore.kernel.org/linux-efi/[email protected]/
Ard Biesheuvel (3):
efi/x86: avoid KASAN false positives when accessing the 1:1 mapping x86/boot/compressed: relax sed symbol type regex for LLVM ld.lld efi/x86: disallow efi=old_map in mixed mode
arch/x86/boot/Makefile | 2 +- arch/x86/platform/efi/efi_64.c | 11 ++++++----- arch/x86/platform/uv/bios_uv.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-)
Signature · 1 line
2.17.1
Ard Biesheuvel
maintainerPATCH 1/3 efi/x86: avoid KASAN false positives when accessing the 1:1 mapping
When installing the EFI virtual address map during early boot, we access the EFI system table to retrieve the 1:1 mapped address of the SetVirtualAddressMap() EFI runtime service. This memory is not known to KASAN, so on KASAN enabled builds, this may result in a splat like
==================================================================
BUG: KASAN: user-memory-access in efi_set_virtual_address_map+0x141/0x354 Read of size 4 at addr 000000003fbeef38 by task swapper/0/0 CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.5.0-rc5+ #758 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 Call Trace: dump_stack+0x8b/0xbb ? efi_set_virtual_address_map+0x141/0x354 ? efi_set_virtual_address_map+0x141/0x354 __kasan_report+0x176/0x192 ? efi_set_virtual_address_map+0x141/0x354 kasan_report+0xe/0x20 efi_set_virtual_address_map+0x141/0x354 ? efi_thunk_runtime_setup+0x148/0x148 ? __inc_numa_state+0x19/0x90 ? memcpy+0x34/0x50 efi_enter_virtual_mode+0x5fd/0x67d start_kernel+0x5cd/0x682 ? mem_encrypt_init+0x6/0x6 ? x86_family+0x5/0x20 ? load_ucode_bsp+0x46/0x154 secondary_startup_64+0xa4/0xb0 ==================================================================
Since this code runs only a single time during early boot, let's annotate it as __no_sanitize_address so KASAN disregards it entirely.
--- arch/x86/platform/efi/efi_64.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
arch/x86/platform/efi/efi_64.c +6 -5
@@ -543,7 +543,7 @@ static DEFINE_SPINLOCK(efi_runtime_lock); __s; \ }) -static efi_status_t __init +static efi_status_t __init __no_sanitize_address efi_thunk_set_virtual_address_map(unsigned long memory_map_size, unsigned long descriptor_size, u32 descriptor_version,
@@ -882,10 +882,11 @@ void __init efi_thunk_runtime_setup(void) efi.query_capsule_caps = efi_thunk_query_capsule_caps; } -efi_status_t __init efi_set_virtual_address_map (unsigned long memory_map_size, -unsigned long descriptor_size, -u32 descriptor_version, -efi_memory_desc_t *virtual_map) +efi_status_t __init __no_sanitize_address +efi_set_virtual_address_map (unsigned long memory_map_size, +unsigned long descriptor_size, +u32 descriptor_version, +efi_memory_desc_t *virtual_map) {efi_status_t status;unsigned long flags;
Signature · 1 line
2.17.1
Ard Biesheuvel
maintainerPATCH 2/3 x86/boot/compressed: relax sed symbol type regex for LLVM ld.lld
In reply to Ard BiesheuvelThe final build stage of the x86 kernel captures some symbol addresses from the decompressor binary and copies them into zoffset.h. It uses sed with a regular expression that matches the address, symbol type and symbol name, and mangles the captured addresses and the names of symbols of interest into #define directives that are added to zoffset.h
The symbol type is indicated by a single letter, which we match
strictly: only letters in the set 'ABCDGRSTVW' are matched, even though the actual symbol type is relevant and therefore ignored.
Commit bc7c9d620 ("efi/libstub/x86: Force 'hidden' visibility for extern declarations") made a change to the way external symbol references are classified, resulting in 'startup_32' now being emitted as a hidden symbol. This prevents the use of GOT entries to refer to this symbol via its absolute address, which recent toolchains (including Clang based ones) already avoid by default, making this change a no-op in the majority of cases.
However, as it turns out, the LLVM linker classifies such hidden symbols as symbols with static linkage in fully linked ELF binaries, causing tools such as NM to output a lowercase 't' rather than an upper case 'T' for the type of such symbols. Since our sed expression only matches upper case letters for the symbol type, the line describing startup_32 is disregarded, resulting in a build error like the following
arch/x86/boot/header.S:568:18: error: symbol 'ZO_startup_32' can not be
undefined in a subtraction expression
init_size: .long (0x00000000008fd000 - ZO_startup_32 +
(((0x0000000001f6361c + ((0x0000000001f6361c >> 8) + 65536)
- 0x00000000008c32e5) + 4095) & ~4095)) # kernel initialization size
Given that we are only interested in the value of the symbol, let's match any character in the set 'a-zA-Z' instead.
--- arch/x86/boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/boot/Makefile +1 -1
@@ -88,7 +88,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE SETUP_OBJS = $(addprefix $(obj)/,$(setup-y)) -sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' +sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' quiet_cmd_zoffset = ZOFFSET $@ cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
Signature · 1 line
2.17.1
Re: [PATCH 2/3] x86/boot/compressed: relax sed symbol type regex for LLVM ld.lld
Quoted 33 lines · Ard Biesheuvel
The final build stage of the x86 kernel captures some symbol addresses from the decompressor binary and copies them into
zoffset.h. It uses sed with a regular expression that matches the address, symbol type and symbol name, and mangles the captured addresses and the names of symbols of interest into #define directives that are added tozoffset.hThe symbol type is indicated by a single letter, which we match
strictly: only letters in the set 'ABCDGRSTVW' are matched, even though the actual symbol type is relevant and therefore ignored.Commit
bc7c9d620("efi/libstub/x86: Force 'hidden' visibility for extern declarations") made a change to the way external symbol references are classified, resulting in 'startup_32' now being emitted as a hidden symbol. This prevents the use of GOT entries to refer to this symbol via its absolute address, which recent toolchains (including Clang based ones) already avoid by default, making this change a no-op in the majority of cases.However, as it turns out, the LLVM linker classifies such hidden symbols as symbols with static linkage in fully linked ELF binaries, causing tools such as NM to output a lowercase 't' rather than an upper case 'T' for the type of such symbols. Since our sed expression only matches upper case letters for the symbol type, the line describing
startup_32is disregarded, resulting in a build error like the following
arch/x86/boot/header.S:568:18: error: symbol 'ZO_startup_32' can not be
undefined in a subtraction expression
init_size: .long (0x00000000008fd000-ZO_startup_32+
(((0x0000000001f6361c+ ((0x0000000001f6361c>> 8) + 65536)
-0x00000000008c32e5) + 4095) & ~4095)) # kernel initialization size
Quoted 5 lines
Given that we are only interested in the value of the symbol, let's match any character in the set 'a-zA-Z' instead.
Tested-by Nathan Chancellor <[email protected]>Cc Nick Desaulniers <[email protected]>
Thanks for the patch! This fixes a build breakage for us.
Quoted 21 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/x86/boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/boot/Makefile +1
-2@@ -88,7 +88,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE SETUP_OBJS = $(addprefix $(obj)/,$(setup-y)) -sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' +sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' quiet_cmd_zoffset = ZOFFSET $@ cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@ --2.17.1
Signature · 2 lines
Thanks, ~Nick Desaulniers
Ard Biesheuvel
maintainerPATCH 3/3 efi/x86: disallow efi=old_map in mixed mode
In reply to Ard BiesheuvelBefore commit ... ("efi/x86: limit EFI old memory map to SGI UV machines"), enabling the old EFI memory map on mixed mode systems disabled EFI runtime services altogether. Given that efi=old_map is a debug feature designed to work around firmware problems related to EFI runtime services, and disabling them can be achieved more straightforwardly using 'noefi' or 'efi=noruntime', it makes more sense to ignore efi=old_map on mixed mode systems.
Currently, we do neither, and try to use the old memory map in combination with mixed mode routines, which results in crashes, so let's fix this by making efi=old_map functional on native systems only.
--- arch/x86/platform/uv/bios_uv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/platform/uv/bios_uv.c +1 -1
@@ -371,7 +371,7 @@ static int __init arch_parse_efi_cmdline(char *str)return -EINVAL ; } -if (parse_option_str (str,"old_map" )) +if (!efi_is_mixed () &&parse_option_str (str,"old_map" ))set_bit (EFI_UV1_MEMMAP , &efi.flags);return 0 ;
Signature · 1 line
2.17.1
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 31 lines · Ingo Molnar
Quoted 14 lines · Ard Biesheuvel
A couple of fixes for more unusual configurations that weren't caught in my testing before. One for KASAN, one for the LLVM linker and one for the old EFI memory map when running on mixed mode systems.
These apply onto the Git pull request [0[ that I sent out at the beginning of the week.
[0] https://lore.kernel.org/linux-efi/[email protected]/
Ard Biesheuvel (3):
efi/x86: avoid KASAN false positives when accessing the 1:1 mapping x86/boot/compressed: relax sed symbol type regex for LLVM ld.lld efi/x86: disallow efi=old_map in mixed modeQuoted 4 lines
arch/x86/boot/Makefile | 2 +- arch/x86/platform/efi/efi_64.c | 11 ++++++----- arch/x86/platform/uv/bios_uv.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-)Just a minor bugreport, in some (weird) config combinations we now get this build failure:
ld:
arch/x86/platform/efi/efi_64.o: in function `efi_set_virtual_address_map':
efi_64.c:(.init.text+0x1419): undefined reference to `__efi64_thunk'
ld: efi_64.c:(.init.text+0x1530): undefined reference to `efi_uv1_memmap_phys_prolog'
ld: efi_64.c:(.init.text+0x1706): undefined reference to `efi_uv1_memmap_phys_epilog'
Quoted 7 lines
Config attached.
I believe the trigger condition is:
!CONFIG_X86_UV
CONFIG_EFI=y
Strange.
Those references to missing symbols are guarded by efi_is_mixed() and efi_have_uv1_memmap(), both of which are static inline bool() functions wrapping IS_ENABLED() checks against CONFIG_EFI_MIXED and CONFIG_X86_UV, respectively. IOW, it is unexpected that the compiler doesn't const-propagate those expressions and optimize away the references entirely.
Is there any special debug or diagnostic options enabled?
Quoted 41 lines · Ard Biesheuvel
Quoted 31 lines · Ingo Molnar
Quoted 14 lines · Ard Biesheuvel
A couple of fixes for more unusual configurations that weren't caught in my testing before. One for KASAN, one for the LLVM linker and one for the old EFI memory map when running on mixed mode systems.
These apply onto the Git pull request [0[ that I sent out at the beginning of the week.
[0] https://lore.kernel.org/linux-efi/[email protected]/
Ard Biesheuvel (3):
efi/x86: avoid KASAN false positives when accessing the 1:1 mapping x86/boot/compressed: relax sed symbol type regex for LLVM ld.lld efi/x86: disallow efi=old_map in mixed modeQuoted 4 lines
arch/x86/boot/Makefile | 2 +- arch/x86/platform/efi/efi_64.c | 11 ++++++----- arch/x86/platform/uv/bios_uv.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-)Just a minor bugreport, in some (weird) config combinations we now get this build failure:
ld:
arch/x86/platform/efi/efi_64.o: in function `efi_set_virtual_address_map':
efi_64.c:(.init.text+0x1419): undefined reference to `__efi64_thunk'
ld: efi_64.c:(.init.text+0x1530): undefined reference to `efi_uv1_memmap_phys_prolog'
ld: efi_64.c:(.init.text+0x1706): undefined reference to `efi_uv1_memmap_phys_epilog'Quoted 7 lines
Config attached.
I believe the trigger condition is:
!CONFIG_X86_UV
CONFIG_EFI=y
Quoted 9 lines
Strange.
Those references to missing symbols are guarded by
efi_is_mixed()andefi_have_uv1_memmap(), both of which are static inlinebool()functions wrappingIS_ENABLED()checks againstCONFIG_EFI_MIXEDandCONFIG_X86_UV, respectively. IOW, it is unexpected that the compiler doesn't const-propagate those expressions and optimize away the references entirely.
I think we should simply add the following - this should also cover the remaining mixed mode KASAN issue that I mentioned.
arch/x86/platform/efi/Makefile +2 -0
@@ -1,5 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y +KASAN_SANITIZE := n +GCOV_PROFILE := n obj-$(CONFIG_EFI) += quirks.o efi.o efi_$(BITS).o
efi_stub_$(BITS).o
obj-$(CONFIG_EFI_MIXED) += efi_thunk_$(BITS).o