Re: [PATCH RESEND v3] x86/efi: Implement support for embedding SBAT data for x86

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Vitaly Kuznetsov <vkuznets@redhat.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@kernel.org>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.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