Re: [PATCH 1/2] efi/libstub: zboot specific mechanism for embedding SBAT section

Ard Biesheuvel
2025-04-24T16:47:51Z

Hi Vitaly,

On Thu, 24 Apr 2025 at 10:10, 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). Build '.sbat' section along with libstub
> so it can be reused by x86 implementation later.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> drivers/firmware/efi/Kconfig | 25 +++++++++++++++++++++
> drivers/firmware/efi/libstub/Makefile | 7 ++++++
> drivers/firmware/efi/libstub/Makefile.zboot | 3 ++-
> drivers/firmware/efi/libstub/sbat.S | 7 ++++++
> drivers/firmware/efi/libstub/zboot-header.S | 14 ++++++++++++
> drivers/firmware/efi/libstub/zboot.lds | 17 ++++++++++++++
> 6 files changed, 72 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/efi/libstub/sbat.S
>
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 5fe61b9ab5f9..2edb0167ba49 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -281,6 +281,31 @@ config EFI_EMBEDDED_FIRMWARE
> bool
> select CRYPTO_LIB_SHA256
>
> +config EFI_SBAT
> + bool "Embed SBAT section in the kernel"
> + 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.
> +
> + If unsure, say N.
> +
> +config EFI_SBAT_FILE
> + string "Embedded SBAT section file path"
> + depends on EFI_SBAT
> + help
> + Specify a file with SBAT data which is going to be embedded as '.sbat'
> + section into the kernel.
> +

Can we simplify this? CONFIG_EFI_SBAT makes no sense if
CONFIG_EFI_SBAT_FILE is left empty. If you really need both symbols,
set EFI_SBAT automatically based on whether EFI_SBAT_FILE is
non-empty.

> endmenu
>
> config UEFI_CPER
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index d23a1b9fed75..5113cbdadf9a 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -105,6 +105,13 @@ lib-$(CONFIG_UNACCEPTED_MEMORY) += unaccepted_memory.o bitmap.o find.o
> extra-y := $(lib-y)
> lib-y := $(patsubst %.o,%.stub.o,$(lib-y))
>
> +extra-$(CONFIG_EFI_SBAT) += sbat.o
> +$(obj)/sbat.o: $(obj)/sbat.bin
> +targets += sbat.bin
> +filechk_sbat.bin = cat $(or $(real-prereqs), /dev/null)
> +$(obj)/sbat.bin: $(CONFIG_EFI_SBAT_FILE) FORCE
> + $(call filechk,sbat.bin)
> +

Please get rid of all of this, and move the .incbin into zboot-header.S

> # Even when -mbranch-protection=none is set, Clang will generate a
> # .note.gnu.property for code-less object files (like lib/ctype.c),
> # so work around this by explicitly removing the unwanted section.
> diff --git a/drivers/firmware/efi/libstub/Makefile.zboot b/drivers/firmware/efi/libstub/Makefile.zboot
> index 48842b5c106b..3d2d0b326f7c 100644
> --- a/drivers/firmware/efi/libstub/Makefile.zboot
> +++ b/drivers/firmware/efi/libstub/Makefile.zboot
> @@ -44,7 +44,8 @@ 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)
>
> -ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a
> +ZBOOT_DEPS := $(obj)/zboot-header.o $(objtree)/drivers/firmware/efi/libstub/lib.a \
> + $(if $(CONFIG_EFI_SBAT),$(objtree)/drivers/firmware/efi/libstub/sbat.o)
>

Drop this too

> LDFLAGS_vmlinuz.efi.elf := -T $(srctree)/drivers/firmware/efi/libstub/zboot.lds
> $(obj)/vmlinuz.efi.elf: $(obj)/vmlinuz.o $(ZBOOT_DEPS) FORCE
> diff --git a/drivers/firmware/efi/libstub/sbat.S b/drivers/firmware/efi/libstub/sbat.S
> new file mode 100644
> index 000000000000..4e99a1bac794
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/sbat.S
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Embed SBAT data in the kernel.
> + */
> + .pushsection ".sbat","a",@progbits
> + .incbin "drivers/firmware/efi/libstub/sbat.bin"
> + .popsection
> diff --git a/drivers/firmware/efi/libstub/zboot-header.S b/drivers/firmware/efi/libstub/zboot-header.S
> index fb676ded47fa..f2df24504fc5 100644
> --- a/drivers/firmware/efi/libstub/zboot-header.S
> +++ b/drivers/firmware/efi/libstub/zboot-header.S
> @@ -135,6 +135,20 @@ __efistub_efi_zboot_header:
> IMAGE_SCN_MEM_READ | \
> IMAGE_SCN_MEM_WRITE
>
> +#ifdef CONFIG_EFI_SBAT
> + .ascii ".sbat\0\0\0"
> + .long __sbat_size
> + .long _edata - .Ldoshdr
> + .long __sbat_size
> + .long _edata - .Ldoshdr
> +
> + .long 0, 0
> + .short 0, 0
> + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> + IMAGE_SCN_MEM_READ | \
> + IMAGE_SCN_MEM_DISCARDABLE

You can put the pushsection/popsection right here.

> +#endif
> +
> .set .Lsection_count, (. - .Lsection_table) / 40
>
> #ifdef PE_DLL_CHAR_EX
> diff --git a/drivers/firmware/efi/libstub/zboot.lds b/drivers/firmware/efi/libstub/zboot.lds
> index 9ecc57ff5b45..2cd5015c70ce 100644
> --- a/drivers/firmware/efi/libstub/zboot.lds
> +++ b/drivers/firmware/efi/libstub/zboot.lds
> @@ -31,10 +31,24 @@ SECTIONS
>
> .data : ALIGN(4096) {
> *(.data* .init.data*)
> +#ifndef CONFIG_EFI_SBAT
> _edata = ALIGN(512);
> +#else
> + /* Avoid gap between '.data' and '.sbat' */
> + _edata = ALIGN(4096);
> +#endif

Just use 4096 in all cases.

> . = _edata;
> }
>
> +#ifdef CONFIG_EFI_SBAT
> + .sbat : ALIGN(4096) {
> + _sbat = . ;
> + *(.sbat)
> + _esbat = ALIGN(512);
> + . = _esbat;
> + }
> +#endif
> +
> .bss : {
> *(.bss* .init.bss*)
> _end = ALIGN(512);
> @@ -52,3 +66,6 @@ PROVIDE(__efistub__gzdata_size =
>
> PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> PROVIDE(__data_size = ABSOLUTE(_end - _etext));
> +#ifdef CONFIG_EFI_SBAT
> +PROVIDE(__sbat_size = ABSOLUTE(_esbat - _sbat));
> +#endif

This can be unconditional - it is only evaluated when a reference to it exists.

> --
> 2.49.0