* [PATCH v2 0/2] efi: Add a mechanism for embedding SBAT section
@ 2025-05-05 15:45 Vitaly Kuznetsov
2025-05-05 15:45 ` [PATCH v2 1/2] efi: zboot specific " Vitaly Kuznetsov
2025-05-05 15:45 ` [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86 Vitaly Kuznetsov
0 siblings, 2 replies; 7+ messages in thread
From: Vitaly Kuznetsov @ 2025-05-05 15:45 UTC (permalink / raw)
To: x86, linux-efi
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Ard Biesheuvel, Peter Jones, Daniel Berrange,
Emanuele Giuseppe Esposito, Gerd Hoffmann, Greg KH, Luca Boccassi,
Peter Zijlstra, Matthew Garrett, James Bottomley, Eric Snowberg,
Paolo Bonzini, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, linux-riscv, linux-kernel
Changes since v1 (Ard):
(https://lore.kernel.org/linux-efi/20250424080950.289864-1-vkuznets@redhat.com/)
- Get back to the idea of putting '.sbat' in between '.text' and '.data' as
the later also covers '.bss' (and '.pgtable' on x86) which are not present
in the file but must be accounted for in memory (see the discussion on v1
for additional details).
- CONFIG_EFI_SBAT is now automatically set when CONFIG_EFI_SBAT_FILE is set.
- Simplified make magic for tracing possible sbat data changes.
- Dropped 'sbat.S' for zboot (x86 stays as there's no good place for it in
the existing 'vmlinux' compilation units).
- Other necessary tweaks.
Original description:
SBAT is a mechanism which improves SecureBoot revocations of UEFI binaries
by introducing a generation-based technique. Compromised or vulnerable UEFI
binaries can be prevented from booting by bumping the minimal required
generation for the specific component in the bootloader. More information
on the SBAT can be obtained here:
https://github.com/rhboot/shim/blob/main/SBAT.md
Currently, shim checks .sbat data for itself in self-test and for second
stage bootloaders (grub, sd-boot, UKIs with sd-stub, ...) but kernel
revocations require cycling signing keys or adding kernel hashes to shim's
internal dbx. Adding .sbat to kernel and enforcing it on kernel loading
will allow to do the same tracking and revocation distros are already
doing with a simplified mechanism, and without having to keep lists of
kernels outside of the git repos.
Previously, an attempt was made to add ".sbat" section to the linux kernel:
https://lwn.net/Articles/938422/
The approach was rejected mainly because currently there's no policy on how
to update SBAT generation number when a new vulnerability is discovered. In
particular, it is unclear what to do with stable kernels which may or may
not backport certain patches making it impossible to describe the current
state with a simple number.
This series suggests a different approach: instead of defining SBAT
information, provide a mechanism for downstream kernel builders (distros)
to include their own SBAT data. This leaves the decision on the policy to
the distro vendors. Basically, each distro implementing SecureBoot today,
will have an option to inject their own SBAT data during kernel build and
before it gets signed by their SecureBoot CA. Different distro do not need
to agree on the common SBAT component names or generation numbers as each
distro ships its own 'shim' with their own 'vendor_cert'/'vendor_db'. Linux
upstream will never, ever need to care about the data unless they choose in
the future to participate in that way.
Vitaly Kuznetsov (2):
efi: zboot specific mechanism for embedding SBAT section
x86/efi: Implement support for embedding SBAT data for x86
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 | 33 +++++++++++++++------
drivers/firmware/efi/Kconfig | 24 +++++++++++++++
drivers/firmware/efi/libstub/Makefile.zboot | 4 +++
drivers/firmware/efi/libstub/zboot-header.S | 22 ++++++++++++--
drivers/firmware/efi/libstub/zboot.lds | 11 +++++++
9 files changed, 104 insertions(+), 12 deletions(-)
create mode 100644 arch/x86/boot/compressed/sbat.S
--
2.49.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] efi: zboot specific mechanism for embedding SBAT section
2025-05-05 15:45 [PATCH v2 0/2] efi: Add a mechanism for embedding SBAT section Vitaly Kuznetsov
@ 2025-05-05 15:45 ` Vitaly Kuznetsov
2025-05-09 9:16 ` Ard Biesheuvel
2025-05-05 15:45 ` [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86 Vitaly Kuznetsov
1 sibling, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2025-05-05 15:45 UTC (permalink / raw)
To: x86, linux-efi
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Ard Biesheuvel, Peter Jones, Daniel Berrange,
Emanuele Giuseppe Esposito, Gerd Hoffmann, Greg KH, Luca Boccassi,
Peter Zijlstra, Matthew Garrett, James Bottomley, Eric Snowberg,
Paolo Bonzini, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, linux-riscv, linux-kernel
SBAT is a mechanism which improves SecureBoot revocations of UEFI binaries
by introducing a generation-based technique. Compromised or vulnerable UEFI
binaries can be prevented from booting by bumping the minimal required
generation for the specific component in the bootloader. More information
on the SBAT can be obtained here:
https://github.com/rhboot/shim/blob/main/SBAT.md
Upstream Linux kernel does not currently participate in any way in SBAT as
there's no existing policy in how SBAT generation number should be
defined. Keep the status quo and provide a mechanism for distro vendors and
anyone else who signs their kernel for SecureBoot to include their own SBAT
data. This leaves the decision on the policy to the vendor. Basically, each
distro implementing SecureBoot today, will have an option to inject their
own SBAT data during kernel build and before it gets signed by their
SecureBoot CA. Different distro do not need to agree on the common SBAT
component names or generation numbers as each distro ships its own 'shim'
with their own 'vendor_cert'/'vendor_db'
Implement support for embedding SBAT data for architectures using
zboot (arm64, loongarch, riscv). Put '.sbat' section in between '.data' and
'.text' as the former also covers '.bss' and thus must be the last one.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/firmware/efi/Kconfig | 24 +++++++++++++++++++++
drivers/firmware/efi/libstub/Makefile.zboot | 4 ++++
| 22 +++++++++++++++++--
drivers/firmware/efi/libstub/zboot.lds | 11 ++++++++++
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 5fe61b9ab5f9..db8c5c03d3a2 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -281,6 +281,30 @@ config EFI_EMBEDDED_FIRMWARE
bool
select CRYPTO_LIB_SHA256
+config EFI_SBAT
+ def_bool y if EFI_SBAT_FILE!=""
+
+config EFI_SBAT_FILE
+ string "Embedded SBAT section file path"
+ depends on EFI_ZBOOT
+ help
+ SBAT section provides a way to improve SecureBoot revocations of UEFI
+ binaries by introducing a generation-based mechanism. With SBAT, older
+ UEFI binaries can be prevented from booting by bumping the minimal
+ required generation for the specific component in the bootloader.
+
+ Note: SBAT information is distribution specific, i.e. the owner of the
+ signing SecureBoot certificate must define the SBAT policy. Linux
+ kernel upstream does not define SBAT components and their generations.
+
+ See https://github.com/rhboot/shim/blob/main/SBAT.md for the additional
+ details.
+
+ Specify a file with SBAT data which is going to be embedded as '.sbat'
+ section into the kernel.
+
+ If unsure, leave blank.
+
endmenu
config UEFI_CPER
diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
index 48842b5c106b..92e3c73502ba 100644
--- a/drivers/firmware/efi/libstub/Makefile.zboot
+++ b/drivers/firmware/efi/libstub/Makefile.zboot
@@ -44,6 +44,10 @@ AFLAGS_zboot-header.o += -DMACHINE_TYPE=IMAGE_FILE_MACHINE_$(EFI_ZBOOT_MACH_TYPE
$(obj)/zboot-header.o: $(srctree)/drivers/firmware/efi/libstub/zboot-header.S FORCE
$(call if_changed_rule,as_o_S)
+ifneq ($(CONFIG_EFI_SBAT_FILE),)
+$(obj)/zboot-header.o: $(CONFIG_EFI_SBAT_FILE)
+endif
+
ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a
LDFLAGS_vmlinuz.efi.elf := -T $(srctree)/drivers/firmware/efi/libstub/zboot.lds
--git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
index fb676ded47fa..e02247458b65 100644
--- a/drivers/firmware/efi/libstub/zboot-header.S
+++ b/drivers/firmware/efi/libstub/zboot-header.S
@@ -123,11 +123,29 @@ __efistub_efi_zboot_header:
IMAGE_SCN_MEM_READ | \
IMAGE_SCN_MEM_EXECUTE
+#ifdef CONFIG_EFI_SBAT
+ .ascii ".sbat\0\0\0"
+ .long __sbat_size
+ .long _sbat - .Ldoshdr
+ .long __sbat_size
+ .long _sbat - .Ldoshdr
+
+ .long 0, 0
+ .short 0, 0
+ .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
+ IMAGE_SCN_MEM_READ | \
+ IMAGE_SCN_MEM_DISCARDABLE
+
+ .pushsection ".sbat", "a", @progbits
+ .incbin CONFIG_EFI_SBAT_FILE
+ .popsection
+#endif
+
.ascii ".data\0\0\0"
.long __data_size
- .long _etext - .Ldoshdr
+ .long _data - .Ldoshdr
.long __data_rawsize
- .long _etext - .Ldoshdr
+ .long _data - .Ldoshdr
.long 0, 0
.short 0, 0
diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
index 9ecc57ff5b45..c3a166675450 100644
--- a/drivers/firmware/efi/libstub/zboot.lds
+++ b/drivers/firmware/efi/libstub/zboot.lds
@@ -29,7 +29,17 @@ SECTIONS
. = _etext;
}
+#ifdef CONFIG_EFI_SBAT
+ .sbat : ALIGN(4096) {
+ _sbat = .;
+ *(.sbat)
+ _esbat = ALIGN(4096);
+ . = _esbat;
+ }
+#endif
+
.data : ALIGN(4096) {
+ _data = .;
*(.data* .init.data*)
_edata = ALIGN(512);
. = _edata;
@@ -52,3 +62,4 @@ PROVIDE(__efistub__gzdata_size =
PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
PROVIDE(__data_size = ABSOLUTE(_end - _etext));
+PROVIDE(__sbat_size = ABSOLUTE(_esbat - _sbat));
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86
2025-05-05 15:45 [PATCH v2 0/2] efi: Add a mechanism for embedding SBAT section Vitaly Kuznetsov
2025-05-05 15:45 ` [PATCH v2 1/2] efi: zboot specific " Vitaly Kuznetsov
@ 2025-05-05 15:45 ` Vitaly Kuznetsov
2025-05-09 9:20 ` Ard Biesheuvel
1 sibling, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2025-05-05 15:45 UTC (permalink / raw)
To: x86, linux-efi
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Ard Biesheuvel, Peter Jones, Daniel Berrange,
Emanuele Giuseppe Esposito, Gerd Hoffmann, Greg KH, Luca Boccassi,
Peter Zijlstra, Matthew Garrett, James Bottomley, Eric Snowberg,
Paolo Bonzini, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, linux-riscv, linux-kernel
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.
Note, the obsolete CRC-32 checksum (see commit 9c54baab4401 ("x86/boot:
Drop CRC-32 checksum and the build tool that generates it")) is gone and
while it would've been possible to reserve the last 4 bytes in '.sbat'
section too (like it's done today in '.data'), it seems to be a pointless
exercise: SBAT makes zero sense without a signature on the EFI binary so
'.sbat' won't be at the very end of the file anyway. Any tool which uses
the last 4 bytes of the file as a checksum is broken with signed EFI
binaries already.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
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 +++++++
| 33 +++++++++++++++++++-------
drivers/firmware/efi/Kconfig | 2 +-
6 files changed, 46 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 81f55da81967..5f7b52f0e7f5 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 fdbce022db55..1441435869cc 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-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)
--git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index b5c79f43359b..91964818bf50 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -179,15 +179,17 @@ 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"
+#ifdef CONFIG_EFI_SBAT
+ .long ZO__sbat # VirtualSize
+ .long setup_size # VirtualAddress
+ .long ZO__sbat # SizeOfRawData
+#else
+ .long ZO__data # VirtualSize
+ .long setup_size # VirtualAddress
+ .long ZO__data # SizeOfRawData
+#endif
+ .long setup_size # PointerToRawData
.long 0 # PointerToRelocations
.long 0 # PointerToLineNumbers
.word 0 # NumberOfRelocations
@@ -196,6 +198,19 @@ 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
+#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
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] efi: zboot specific mechanism for embedding SBAT section
2025-05-05 15:45 ` [PATCH v2 1/2] efi: zboot specific " Vitaly Kuznetsov
@ 2025-05-09 9:16 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2025-05-09 9:16 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: x86, linux-efi, Thomas Gleixner, Ingo Molnar, Dave Hansen,
H. Peter Anvin, Peter Jones, Daniel Berrange,
Emanuele Giuseppe Esposito, Gerd Hoffmann, Greg KH, Luca Boccassi,
Peter Zijlstra, Matthew Garrett, James Bottomley, Eric Snowberg,
Paolo Bonzini, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, linux-riscv, linux-kernel
On Mon, 5 May 2025 at 17:45, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> SBAT is a mechanism which improves SecureBoot revocations of UEFI binaries
> by introducing a generation-based technique. Compromised or vulnerable UEFI
> binaries can be prevented from booting by bumping the minimal required
> generation for the specific component in the bootloader. More information
> on the SBAT can be obtained here:
>
> https://github.com/rhboot/shim/blob/main/SBAT.md
>
> Upstream Linux kernel does not currently participate in any way in SBAT as
> there's no existing policy in how SBAT generation number should be
> defined. Keep the status quo and provide a mechanism for distro vendors and
> anyone else who signs their kernel for SecureBoot to include their own SBAT
> data. This leaves the decision on the policy to the vendor. Basically, each
> distro implementing SecureBoot today, will have an option to inject their
> own SBAT data during kernel build and before it gets signed by their
> SecureBoot CA. Different distro do not need to agree on the common SBAT
> component names or generation numbers as each distro ships its own 'shim'
> with their own 'vendor_cert'/'vendor_db'
>
> Implement support for embedding SBAT data for architectures using
> zboot (arm64, loongarch, riscv). Put '.sbat' section in between '.data' and
> '.text' as the former also covers '.bss' and thus must be the last one.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/firmware/efi/Kconfig | 24 +++++++++++++++++++++
> drivers/firmware/efi/libstub/Makefile.zboot | 4 ++++
> drivers/firmware/efi/libstub/zboot-header.S | 22 +++++++++++++++++--
> drivers/firmware/efi/libstub/zboot.lds | 11 ++++++++++
> 4 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 5fe61b9ab5f9..db8c5c03d3a2 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -281,6 +281,30 @@ config EFI_EMBEDDED_FIRMWARE
> bool
> select CRYPTO_LIB_SHA256
>
> +config EFI_SBAT
> + def_bool y if EFI_SBAT_FILE!=""
> +
> +config EFI_SBAT_FILE
> + string "Embedded SBAT section file path"
> + depends on EFI_ZBOOT
> + help
> + SBAT section provides a way to improve SecureBoot revocations of UEFI
> + binaries by introducing a generation-based mechanism. With SBAT, older
> + UEFI binaries can be prevented from booting by bumping the minimal
> + required generation for the specific component in the bootloader.
> +
> + Note: SBAT information is distribution specific, i.e. the owner of the
> + signing SecureBoot certificate must define the SBAT policy. Linux
> + kernel upstream does not define SBAT components and their generations.
> +
> + See https://github.com/rhboot/shim/blob/main/SBAT.md for the additional
> + details.
> +
> + Specify a file with SBAT data which is going to be embedded as '.sbat'
> + section into the kernel.
> +
> + If unsure, leave blank.
> +
> endmenu
>
> config UEFI_CPER
> diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
> index 48842b5c106b..92e3c73502ba 100644
> --- a/drivers/firmware/efi/libstub/Makefile.zboot
> +++ b/drivers/firmware/efi/libstub/Makefile.zboot
> @@ -44,6 +44,10 @@ AFLAGS_zboot-header.o += -DMACHINE_TYPE=IMAGE_FILE_MACHINE_$(EFI_ZBOOT_MACH_TYPE
> $(obj)/zboot-header.o: $(srctree)/drivers/firmware/efi/libstub/zboot-header.S FORCE
> $(call if_changed_rule,as_o_S)
>
> +ifneq ($(CONFIG_EFI_SBAT_FILE),)
> +$(obj)/zboot-header.o: $(CONFIG_EFI_SBAT_FILE)
> +endif
> +
> ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a
>
> LDFLAGS_vmlinuz.efi.elf := -T $(srctree)/drivers/firmware/efi/libstub/zboot.lds
> diff --git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
> index fb676ded47fa..e02247458b65 100644
> --- a/drivers/firmware/efi/libstub/zboot-header.S
> +++ b/drivers/firmware/efi/libstub/zboot-header.S
> @@ -123,11 +123,29 @@ __efistub_efi_zboot_header:
> IMAGE_SCN_MEM_READ | \
> IMAGE_SCN_MEM_EXECUTE
>
> +#ifdef CONFIG_EFI_SBAT
> + .ascii ".sbat\0\0\0"
> + .long __sbat_size
> + .long _sbat - .Ldoshdr
> + .long __sbat_size
> + .long _sbat - .Ldoshdr
> +
> + .long 0, 0
> + .short 0, 0
> + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> + IMAGE_SCN_MEM_READ | \
> + IMAGE_SCN_MEM_DISCARDABLE
> +
> + .pushsection ".sbat", "a", @progbits
> + .incbin CONFIG_EFI_SBAT_FILE
> + .popsection
> +#endif
> +
> .ascii ".data\0\0\0"
> .long __data_size
> - .long _etext - .Ldoshdr
> + .long _data - .Ldoshdr
> .long __data_rawsize
> - .long _etext - .Ldoshdr
> + .long _data - .Ldoshdr
>
> .long 0, 0
> .short 0, 0
> diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
> index 9ecc57ff5b45..c3a166675450 100644
> --- a/drivers/firmware/efi/libstub/zboot.lds
> +++ b/drivers/firmware/efi/libstub/zboot.lds
> @@ -29,7 +29,17 @@ SECTIONS
> . = _etext;
> }
>
> +#ifdef CONFIG_EFI_SBAT
> + .sbat : ALIGN(4096) {
> + _sbat = .;
> + *(.sbat)
> + _esbat = ALIGN(4096);
> + . = _esbat;
> + }
> +#endif
> +
> .data : ALIGN(4096) {
> + _data = .;
> *(.data* .init.data*)
> _edata = ALIGN(512);
> . = _edata;
> @@ -52,3 +62,4 @@ PROVIDE(__efistub__gzdata_size =
>
> PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> PROVIDE(__data_size = ABSOLUTE(_end - _etext));
> +PROVIDE(__sbat_size = ABSOLUTE(_esbat - _sbat));
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86
2025-05-05 15:45 ` [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86 Vitaly Kuznetsov
@ 2025-05-09 9:20 ` Ard Biesheuvel
2025-05-12 15:02 ` Vitaly Kuznetsov
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2025-05-09 9:20 UTC (permalink / raw)
To: Vitaly Kuznetsov, Ingo Molnar, Borislav Petkov
Cc: x86, linux-efi, Thomas Gleixner, Dave Hansen, H. Peter Anvin,
Peter Jones, Daniel Berrange, Emanuele Giuseppe Esposito,
Gerd Hoffmann, Luca Boccassi, Matthew Garrett, James Bottomley,
Eric Snowberg, Paolo Bonzini
On Mon, 5 May 2025 at 17:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> 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.
>
> Note, the obsolete CRC-32 checksum (see commit 9c54baab4401 ("x86/boot:
> Drop CRC-32 checksum and the build tool that generates it")) is gone and
> while it would've been possible to reserve the last 4 bytes in '.sbat'
> section too (like it's done today in '.data'), it seems to be a pointless
> exercise: SBAT makes zero sense without a signature on the EFI binary so
> '.sbat' won't be at the very end of the file anyway. Any tool which uses
> the last 4 bytes of the file as a checksum is broken with signed EFI
> binaries already.
>
Is this last paragraph still relevant? If not, please drop it.
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> 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 | 33 +++++++++++++++++++-------
> drivers/firmware/efi/Kconfig | 2 +-
> 6 files changed, 46 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 81f55da81967..5f7b52f0e7f5 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 fdbce022db55..1441435869cc 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-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 b5c79f43359b..91964818bf50 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -179,15 +179,17 @@ 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"
> +#ifdef CONFIG_EFI_SBAT
> + .long ZO__sbat # VirtualSize
> + .long setup_size # VirtualAddress
> + .long ZO__sbat # SizeOfRawData
> +#else
> + .long ZO__data # VirtualSize
> + .long setup_size # VirtualAddress
> + .long ZO__data # SizeOfRawData
> +#endif
> + .long setup_size # PointerToRawData
Would it work if we do the following here
#ifdef CONFIG_EFI_SBAT
.set .Ltextsize, ZO__sbat
#else
.set .Ltextsize, ZO__data
#endif
and keep a single section definition for .text
.ascii ".text\0\0\0"
.long .Ltextsize # VirtualSize
.long setup_size # VirtualAddress
.long .Ltextsize # SizeOfRawData
.long setup_size # PointerToRawData
> .long 0 # PointerToRelocations
> .long 0 # PointerToLineNumbers
> .word 0 # NumberOfRelocations
> @@ -196,6 +198,19 @@ pecompat_fstart:
> IMAGE_SCN_MEM_READ | \
> IMAGE_SCN_MEM_EXECUTE # Characteristics
>
> +#ifdef CONFIG_EFI_SBAT
> + .ascii ".sbat\0\0\0"
Inconsistent indentation? ^^^
> + .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
> +#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
> --
> 2.49.0
>
Modulo the nits, I think this patch looks fine, but it will need to go
through the -tip tree.
So with the changes,
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Ingo, Boris, given that this depends on the previous patch, mind
taking both via the -tip tree? I can take them too, but it doesn't
make sense splitting them up.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86
2025-05-09 9:20 ` Ard Biesheuvel
@ 2025-05-12 15:02 ` Vitaly Kuznetsov
2025-05-13 12:22 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Vitaly Kuznetsov @ 2025-05-12 15:02 UTC (permalink / raw)
To: Ard Biesheuvel, Ingo Molnar, Borislav Petkov
Cc: x86, linux-efi, Thomas Gleixner, Dave Hansen, H. Peter Anvin,
Peter Jones, Daniel Berrange, Emanuele Giuseppe Esposito,
Gerd Hoffmann, Luca Boccassi, Matthew Garrett, James Bottomley,
Eric Snowberg, Paolo Bonzini
Ard Biesheuvel <ardb@kernel.org> writes:
> On Mon, 5 May 2025 at 17:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> 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.
>>
>> Note, the obsolete CRC-32 checksum (see commit 9c54baab4401 ("x86/boot:
>> Drop CRC-32 checksum and the build tool that generates it")) is gone and
>> while it would've been possible to reserve the last 4 bytes in '.sbat'
>> section too (like it's done today in '.data'), it seems to be a pointless
>> exercise: SBAT makes zero sense without a signature on the EFI binary so
>> '.sbat' won't be at the very end of the file anyway. Any tool which uses
>> the last 4 bytes of the file as a checksum is broken with signed EFI
>> binaries already.
>>
>
> Is this last paragraph still relevant? If not, please drop it.
>
Ceratinly not relevant anymore, will drop.
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> 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 | 33 +++++++++++++++++++-------
>> drivers/firmware/efi/Kconfig | 2 +-
>> 6 files changed, 46 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 81f55da81967..5f7b52f0e7f5 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 fdbce022db55..1441435869cc 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-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 b5c79f43359b..91964818bf50 100644
>> --- a/arch/x86/boot/header.S
>> +++ b/arch/x86/boot/header.S
>> @@ -179,15 +179,17 @@ 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"
>> +#ifdef CONFIG_EFI_SBAT
>> + .long ZO__sbat # VirtualSize
>> + .long setup_size # VirtualAddress
>> + .long ZO__sbat # SizeOfRawData
>> +#else
>> + .long ZO__data # VirtualSize
>> + .long setup_size # VirtualAddress
>> + .long ZO__data # SizeOfRawData
>> +#endif
>> + .long setup_size # PointerToRawData
>
> Would it work if we do the following here
>
> #ifdef CONFIG_EFI_SBAT
> .set .Ltextsize, ZO__sbat
> #else
> .set .Ltextsize, ZO__data
> #endif
>
> and keep a single section definition for .text
>
> .ascii ".text\0\0\0"
> .long .Ltextsize # VirtualSize
> .long setup_size # VirtualAddress
> .long .Ltextsize # SizeOfRawData
> .long setup_size # PointerToRawData
>
As we already have '#ifdef CONFIG_EFI_SBAT' below I'd suggest we set
textsize there, basically:
@@ -199,16 +194,20 @@ pecompat_fstart:
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
+ .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"
and nobody seems to care that we use it first and define/set it later.
BTW, does '.L' prefix you suggest has a meaning here? I see we don't use
it for e.g. 'pecompat_fstart', 'section_count'.
>
>> .long 0 # PointerToRelocations
>> .long 0 # PointerToLineNumbers
>> .word 0 # NumberOfRelocations
>> @@ -196,6 +198,19 @@ pecompat_fstart:
>> IMAGE_SCN_MEM_READ | \
>> IMAGE_SCN_MEM_EXECUTE # Characteristics
>>
>> +#ifdef CONFIG_EFI_SBAT
>> + .ascii ".sbat\0\0\0"
>
> Inconsistent indentation? ^^^
>
Yep, fixing.
>> + .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
>> +#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
>> --
>> 2.49.0
>>
>
> Modulo the nits, I think this patch looks fine, but it will need to go
> through the -tip tree.
>
> So with the changes,
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Thanks for the review!
>
> Ingo, Boris, given that this depends on the previous patch, mind
> taking both via the -tip tree? I can take them too, but it doesn't
> make sense splitting them up.
>
--
Vitaly
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86
2025-05-12 15:02 ` Vitaly Kuznetsov
@ 2025-05-13 12:22 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2025-05-13 12:22 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Ingo Molnar, Borislav Petkov, x86, linux-efi, Thomas Gleixner,
Dave Hansen, H. Peter Anvin, Peter Jones, Daniel Berrange,
Emanuele Giuseppe Esposito, Gerd Hoffmann, Luca Boccassi,
Matthew Garrett, James Bottomley, Eric Snowberg, Paolo Bonzini
On Mon, 12 May 2025 at 16:02, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Ard Biesheuvel <ardb@kernel.org> writes:
>
> > On Mon, 5 May 2025 at 17:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >>
...
> >> + .ascii ".text\0\0\0"
> >> +#ifdef CONFIG_EFI_SBAT
> >> + .long ZO__sbat # VirtualSize
> >> + .long setup_size # VirtualAddress
> >> + .long ZO__sbat # SizeOfRawData
> >> +#else
> >> + .long ZO__data # VirtualSize
> >> + .long setup_size # VirtualAddress
> >> + .long ZO__data # SizeOfRawData
> >> +#endif
> >> + .long setup_size # PointerToRawData
> >
> > Would it work if we do the following here
> >
> > #ifdef CONFIG_EFI_SBAT
> > .set .Ltextsize, ZO__sbat
> > #else
> > .set .Ltextsize, ZO__data
> > #endif
> >
> > and keep a single section definition for .text
> >
> > .ascii ".text\0\0\0"
> > .long .Ltextsize # VirtualSize
> > .long setup_size # VirtualAddress
> > .long .Ltextsize # SizeOfRawData
> > .long setup_size # PointerToRawData
> >
>
> As we already have '#ifdef CONFIG_EFI_SBAT' below I'd suggest we set
> textsize there, basically:
>
> @@ -199,16 +194,20 @@ pecompat_fstart:
> 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
> + .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"
>
> and nobody seems to care that we use it first and define/set it later.
>
Yeah that looks fine.
> BTW, does '.L' prefix you suggest has a meaning here? I see we don't use
> it for e.g. 'pecompat_fstart', 'section_count'.
>
The .L prefix prevents the symbols from ending up in the ELF file -
you can drop it if you like, it doesn't really matter one way or the
other.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-05-13 12:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-05 15:45 [PATCH v2 0/2] efi: Add a mechanism for embedding SBAT section Vitaly Kuznetsov
2025-05-05 15:45 ` [PATCH v2 1/2] efi: zboot specific " Vitaly Kuznetsov
2025-05-09 9:16 ` Ard Biesheuvel
2025-05-05 15:45 ` [PATCH v2 2/2] x86/efi: Implement support for embedding SBAT data for x86 Vitaly Kuznetsov
2025-05-09 9:20 ` Ard Biesheuvel
2025-05-12 15:02 ` Vitaly Kuznetsov
2025-05-13 12:22 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox