| Message ID | 20250505154523.231233-3-vkuznets@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | efi: Add a mechanism for embedding SBAT section | expand |
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.
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 .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
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 +++++++ 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