1. Patchew
  2. Xen
  3. View series

[XEN PATCH v2] sbat: Add SBAT section to the Xen EFI binary

Gerald Elder-Vass posted 1 patch 6 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
[XEN PATCH v2] sbat: Add SBAT section to the Xen EFI binary
Posted by Gerald Elder-Vass 6 months ago 
SBAT is a revocation scheme for UEFI SecureBoot, and is mandated by Microsoft
for signing.

The SBAT section provides a way for the binary to declare a generation
id for its upstream source and any vendor changes applied. A compatible
loader can then revoke vulnerable binaries by generation, using the
binary's declared generation id(s) to determine if it is safe to load.

More information about SBAT is available here:
https://github.com/rhboot/shim/blob/main/SBAT.md

Vendors should append a custom line onto sbat.csv(.in) with their vendor
specific sbat data.

Populate the SBAT section in the Xen binary by using the information
in xen/arch/x86/sbat.csv

Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@cloud.com>
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Tested-by: Gerald Elder-Vass <gerald.elder-vass@cloud.com>
---
Changed since v1:
 * Updated commit message to explain why SBAT is needed
 * Renamed sbat_data.o rule to sbat.o
 * Moved sbat.o rule into alphabetical order
 * Removed xen specific entry from sbat.csv (and rule for auto filling version)
   - The alternative of adding a "customise me" line would result in more
     overhead for anyone else building Xen, regardless of UEFI SecureBoot usage

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d902fb7accd9..8fc5c69111d5 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -58,6 +58,7 @@ obj-y += percpu.o
 obj-y += physdev.o
 obj-$(CONFIG_COMPAT) += x86_64/physdev.o
 obj-y += psr.o
+obj-y += sbat.o
 obj-y += setup.o
 obj-y += shutdown.o
 obj-y += smp.o
@@ -277,6 +278,9 @@ $(obj)/efi.lds: AFLAGS-y += -DEFI
 $(obj)/xen.lds $(obj)/efi.lds: $(src)/xen.lds.S FORCE
 	$(call if_changed_dep,cpp_lds_S)
 
+$(obj)/sbat.o: $(src)/sbat.csv
+	$(OBJCOPY) -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents $< $@
+
 clean-files := \
     include/asm/asm-macros.* \
     $(objtree)/.xen-syms.[0-9]* \
diff --git a/xen/arch/x86/sbat.csv b/xen/arch/x86/sbat.csv
new file mode 100644
index 000000000000..1f262b5f038b
--- /dev/null
+++ b/xen/arch/x86/sbat.csv
@@ -0,0 +1 @@
+sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 9a1dfe1b340a..e6405941e1b7 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -343,6 +343,8 @@ SECTIONS
     *(.reloc)
     __base_relocs_end = .;
   }
+
+  .sbat (NOLOAD) : { *(.sbat) }
 #elif defined(XEN_BUILD_EFI)
   /*
    * Due to the way EFI support is currently implemented, these two symbols
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index a17810bb286f..756f97d48183 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -92,7 +92,8 @@
        *(.comment.*) \
        *(.note.*)
 #else
-#define DISCARD_EFI_SECTIONS
+#define DISCARD_EFI_SECTIONS \
+       *(.sbat)
 #endif
 
 /* Sections to be discarded. */
Re: [XEN PATCH v2] sbat: Add SBAT section to the Xen EFI binary
Posted by Jan Beulich 6 months ago 
On 01.05.2025 14:23, Gerald Elder-Vass wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -58,6 +58,7 @@ obj-y += percpu.o
>  obj-y += physdev.o
>  obj-$(CONFIG_COMPAT) += x86_64/physdev.o
>  obj-y += psr.o
> +obj-y += sbat.o
>  obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smp.o

This being x86-specific here without there really being anything x86-specific
about it - what about Arm?

It being EFI-specific, why put it here rather than in x86/efi/ (and/or, as
per above, in common/efi/, at least for the source file)?

> @@ -277,6 +278,9 @@ $(obj)/efi.lds: AFLAGS-y += -DEFI
>  $(obj)/xen.lds $(obj)/efi.lds: $(src)/xen.lds.S FORCE
>  	$(call if_changed_dep,cpp_lds_S)
>  
> +$(obj)/sbat.o: $(src)/sbat.csv
> +	$(OBJCOPY) -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents $< $@

While it may seem unlikely now, both rule and dependencies may change going
forward. So perhaps better to use the if_changed_deps model right away?
Which of course will raise a source file naming question: We can't really
assume a .csv -> .o rule to be generically applicable. Therefore maybe the
source file extension would better be something less generic, like .sbat.

Jan
Re: [XEN PATCH v2] sbat: Add SBAT section to the Xen EFI binary
Posted by Andrew Cooper 5 months, 3 weeks ago 
On 02/05/2025 8:01 am, Jan Beulich wrote:
> On 01.05.2025 14:23, Gerald Elder-Vass wrote:
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -58,6 +58,7 @@ obj-y += percpu.o
>>  obj-y += physdev.o
>>  obj-$(CONFIG_COMPAT) += x86_64/physdev.o
>>  obj-y += psr.o
>> +obj-y += sbat.o
>>  obj-y += setup.o
>>  obj-y += shutdown.o
>>  obj-y += smp.o
> This being x86-specific here without there really being anything x86-specific
> about it - what about Arm?
>
> It being EFI-specific, why put it here rather than in x86/efi/ (and/or, as
> per above, in common/efi/, at least for the source file)?

Having just spent a morning debugging, I've told Gerald to keep it in
x86.   common/efi is an intractable mess and needs turning into a
regular part of the build before this suggestion can be actioned.

~Andrew
Re: [XEN PATCH v2] sbat: Add SBAT section to the Xen EFI binary
Posted by Frediano Ziglio 6 months ago 
On Thu, May 1, 2025 at 1:23 PM Gerald Elder-Vass
<gerald.elder-vass@cloud.com> wrote:
>
> SBAT is a revocation scheme for UEFI SecureBoot, and is mandated by Microsoft
> for signing.
>
> The SBAT section provides a way for the binary to declare a generation
> id for its upstream source and any vendor changes applied. A compatible
> loader can then revoke vulnerable binaries by generation, using the
> binary's declared generation id(s) to determine if it is safe to load.
>
> More information about SBAT is available here:
> https://github.com/rhboot/shim/blob/main/SBAT.md
>
> Vendors should append a custom line onto sbat.csv(.in) with their vendor
> specific sbat data.
>
> Populate the SBAT section in the Xen binary by using the information
> in xen/arch/x86/sbat.csv
>
> Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@cloud.com>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> Tested-by: Gerald Elder-Vass <gerald.elder-vass@cloud.com>
> ---
> Changed since v1:
>  * Updated commit message to explain why SBAT is needed
>  * Renamed sbat_data.o rule to sbat.o
>  * Moved sbat.o rule into alphabetical order
>  * Removed xen specific entry from sbat.csv (and rule for auto filling version)
>    - The alternative of adding a "customise me" line would result in more
>      overhead for anyone else building Xen, regardless of UEFI SecureBoot usage
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index d902fb7accd9..8fc5c69111d5 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -58,6 +58,7 @@ obj-y += percpu.o
>  obj-y += physdev.o
>  obj-$(CONFIG_COMPAT) += x86_64/physdev.o
>  obj-y += psr.o
> +obj-y += sbat.o
>  obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smp.o
> @@ -277,6 +278,9 @@ $(obj)/efi.lds: AFLAGS-y += -DEFI
>  $(obj)/xen.lds $(obj)/efi.lds: $(src)/xen.lds.S FORCE
>         $(call if_changed_dep,cpp_lds_S)
>
> +$(obj)/sbat.o: $(src)/sbat.csv
> +       $(OBJCOPY) -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents $< $@
> +
>  clean-files := \
>      include/asm/asm-macros.* \
>      $(objtree)/.xen-syms.[0-9]* \
> diff --git a/xen/arch/x86/sbat.csv b/xen/arch/x86/sbat.csv
> new file mode 100644
> index 000000000000..1f262b5f038b
> --- /dev/null
> +++ b/xen/arch/x86/sbat.csv
> @@ -0,0 +1 @@
> +sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 9a1dfe1b340a..e6405941e1b7 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -343,6 +343,8 @@ SECTIONS
>      *(.reloc)
>      __base_relocs_end = .;
>    }
> +
> +  .sbat (NOLOAD) : { *(.sbat) }
>  #elif defined(XEN_BUILD_EFI)
>    /*
>     * Due to the way EFI support is currently implemented, these two symbols
> diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
> index a17810bb286f..756f97d48183 100644
> --- a/xen/include/xen/xen.lds.h
> +++ b/xen/include/xen/xen.lds.h
> @@ -92,7 +92,8 @@
>         *(.comment.*) \
>         *(.note.*)
>  #else
> -#define DISCARD_EFI_SECTIONS
> +#define DISCARD_EFI_SECTIONS \
> +       *(.sbat)
>  #endif
>
>  /* Sections to be discarded. */

Reviewed-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Frediano

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.