[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <877c17pxsz.fsf@redhat.com>
Date: Thu, 19 Jun 2025 17:15:40 +0200
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: x86@...nel.org, Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, Ingo
Molnar <mingo@...hat.com>, Dave Hansen <dave.hansen@...ux.intel.com>, "H.
Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>, Peter Jones
<pjones@...hat.com>, Daniel Berrange <berrange@...hat.com>, Emanuele
Giuseppe Esposito <eesposit@...hat.com>, Gerd Hoffmann
<kraxel@...hat.com>, Greg KH <gregkh@...uxfoundation.org>, Luca Boccassi
<bluca@...ian.org>, Peter Zijlstra <peterz@...radead.org>, Matthew Garrett
<mjg59@...f.ucam.org>, James Bottomley
<James.Bottomley@...senpartnership.com>, Eric Snowberg
<eric.snowberg@...cle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v3] x86/efi: Implement support for embedding SBAT
data for x86
Vitaly Kuznetsov <vkuznets@...hat.com> writes:
> Similar to zboot architectures, implement support for embedding SBAT data
> for x86. Put '.sbat' section in between '.data' and '.text' as the former
> also covers '.bss' and '.pgtable' and thus must be the last one in the
> file.
>
> Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
> ---
> The patch was previously sent as part of "[PATCH v3 0/2] efi: Add a
> mechanism for embedding SBAT section" series. The main patch has
> already made it upstream through Ard's 'efi' tree (see commit 0f9a1739dd0e
> "efi: zboot specific mechanism for embedding SBAT section") but x86 part
> is still pending. Resending so it doesn't get lost.
>
I was going to ping again but while working on a (mostly unrelated)
zboot issue:
https://lore.kernel.org/linux-efi/20250618122008.264294-1-vkuznets@redhat.com/T/#u
I noticed that in x86 SBAT patch I neglected to update optional PE
header and '.sbat' section is accounted as part of 'SizeOfCode' instead
of 'SizeOfInitializedData'. OVMF doesn't seem to care about the optional
header and I don't really know who does but as the patch is not merged
yet, I think it would be better to fix. The differential change is:
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 9bea5a1e2c52..f57c45d8584a 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -78,9 +78,9 @@ optional_header:
.byte 0x02 # MajorLinkerVersion
.byte 0x14 # MinorLinkerVersion
- .long ZO__data # SizeOfCode
+ .long textsize # SizeOfCode
- .long ZO__end - ZO__data # SizeOfInitializedData
+ .long ZO__end - textsize # SizeOfInitializedData
.long 0 # SizeOfUninitializedData
.long setup_size + ZO_efi_pe_entry # AddressOfEntryPoint
I'm going to send v4 with this change included.
> arch/x86/boot/Makefile | 2 +-
> arch/x86/boot/compressed/Makefile | 5 +++++
> arch/x86/boot/compressed/sbat.S | 7 ++++++
> arch/x86/boot/compressed/vmlinux.lds.S | 8 +++++++
> arch/x86/boot/header.S | 31 ++++++++++++++++++--------
> drivers/firmware/efi/Kconfig | 2 +-
> 6 files changed, 44 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/boot/compressed/sbat.S
>
> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> index 640fcac3af74..3f9fb3698d66 100644
> --- a/arch/x86/boot/Makefile
> +++ b/arch/x86/boot/Makefile
> @@ -71,7 +71,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE
>
> SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))
>
> -sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi.._stub_entry\|efi\(32\)\?_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|_e\?data\|z_.*\)$$/\#define ZO_\2 0x\1/p'
> +sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi.._stub_entry\|efi\(32\)\?_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|_e\?data\|_e\?sbat\|z_.*\)$$/\#define ZO_\2 0x\1/p'
>
> quiet_cmd_zoffset = ZOFFSET $@
> cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index f4f7b22d8113..3a38fdcdb9bd 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -106,6 +106,11 @@ vmlinux-objs-$(CONFIG_UNACCEPTED_MEMORY) += $(obj)/mem.o
> vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
> vmlinux-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> vmlinux-libs-$(CONFIG_X86_64) += $(objtree)/arch/x86/boot/startup/lib.a
> +vmlinux-objs-$(CONFIG_EFI_SBAT) += $(obj)/sbat.o
> +
> +ifdef CONFIG_EFI_SBAT
> +$(obj)/sbat.o: $(CONFIG_EFI_SBAT_FILE)
> +endif
>
> $(obj)/vmlinux: $(vmlinux-objs-y) $(vmlinux-libs-y) FORCE
> $(call if_changed,ld)
> diff --git a/arch/x86/boot/compressed/sbat.S b/arch/x86/boot/compressed/sbat.S
> new file mode 100644
> index 000000000000..838f70a997dd
> --- /dev/null
> +++ b/arch/x86/boot/compressed/sbat.S
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Embed SBAT data in the kernel.
> + */
> + .pushsection ".sbat", "a", @progbits
> + .incbin CONFIG_EFI_SBAT_FILE
> + .popsection
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 3b2bc61c9408..587ce3e7c504 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -43,6 +43,14 @@ SECTIONS
> *(.rodata.*)
> _erodata = . ;
> }
> +#ifdef CONFIG_EFI_SBAT
> + .sbat : ALIGN(0x1000) {
> + _sbat = . ;
> + *(.sbat)
> + _esbat = ALIGN(0x1000);
> + . = _esbat;
> + }
> +#endif
> .data : ALIGN(0x1000) {
> _data = . ;
> *(.data)
> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
> index e1f4fd5bc8ee..9bea5a1e2c52 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -179,15 +179,11 @@ pecompat_fstart:
> #else
> .set pecompat_fstart, setup_size
> #endif
> - .ascii ".text"
> - .byte 0
> - .byte 0
> - .byte 0
> - .long ZO__data
> - .long setup_size
> - .long ZO__data # Size of initialized data
> - # on disk
> - .long setup_size
> + .ascii ".text\0\0\0"
> + .long textsize # VirtualSize
> + .long setup_size # VirtualAddress
> + .long textsize # SizeOfRawData
> + .long setup_size # PointerToRawData
> .long 0 # PointerToRelocations
> .long 0 # PointerToLineNumbers
> .word 0 # NumberOfRelocations
> @@ -196,6 +192,23 @@ pecompat_fstart:
> IMAGE_SCN_MEM_READ | \
> IMAGE_SCN_MEM_EXECUTE # Characteristics
>
> +#ifdef CONFIG_EFI_SBAT
> + .ascii ".sbat\0\0\0"
> + .long ZO__esbat - ZO__sbat # VirtualSize
> + .long setup_size + ZO__sbat # VirtualAddress
> + .long ZO__esbat - ZO__sbat # SizeOfRawData
> + .long setup_size + ZO__sbat # PointerToRawData
> +
> + .long 0, 0, 0
> + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> + IMAGE_SCN_MEM_READ | \
> + IMAGE_SCN_MEM_DISCARDABLE # Characteristics
> +
> + .set textsize, ZO__sbat
> +#else
> + .set textsize, ZO__data
> +#endif
> +
> .ascii ".data\0\0\0"
> .long ZO__end - ZO__data # VirtualSize
> .long setup_size + ZO__data # VirtualAddress
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index db8c5c03d3a2..16baa038d412 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -286,7 +286,7 @@ config EFI_SBAT
>
> config EFI_SBAT_FILE
> string "Embedded SBAT section file path"
> - depends on EFI_ZBOOT
> + depends on EFI_ZBOOT || (EFI_STUB && X86)
> help
> SBAT section provides a way to improve SecureBoot revocations of UEFI
> binaries by introducing a generation-based mechanism. With SBAT, older
--
Vitaly
Powered by blists - more mailing lists