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

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

[Gerald Elder-Vass]

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/xs86/efi/sbat.sbat

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 v2:
 * Moved sbat files and rules to arch/x86/efi
 * Updated sbat rule to reuse existing objcopy command

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/efi/Makefile b/xen/arch/x86/efi/Makefile
index 24dfecfad184..75aa35870a9a 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -6,11 +6,17 @@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
 $(obj)/%.o: $(src)/%.ihex FORCE
 	$(call if_changed,objcopy_o_ihex)
 
+$(obj)/sbat.o: OBJCOPYFLAGS := -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents
+$(obj)/sbat.o: $(src)/sbat.sbat FORCE
+	$(call if_changed,objcopy)
+
 $(obj)/boot.init.o: $(obj)/buildid.o
 
 $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4)
 $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := $(cflags-stack-boundary)
 
+EFIOBJ-y += sbat.o
+
 obj-y := common-stub.o stub.o
 obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y))
 obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y))
diff --git a/xen/arch/x86/efi/sbat.sbat b/xen/arch/x86/efi/sbat.sbat
new file mode 100644
index 000000000000..1f262b5f038b
--- /dev/null
+++ b/xen/arch/x86/efi/sbat.sbat
@@ -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

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

[Andrew Cooper]

On 07/05/2025 2:54 pm, Gerald Elder-Vass wrote:
> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
> index 24dfecfad184..75aa35870a9a 100644
> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -6,11 +6,17 @@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
>  $(obj)/%.o: $(src)/%.ihex FORCE
>  	$(call if_changed,objcopy_o_ihex)
>  
> +$(obj)/sbat.o: OBJCOPYFLAGS := -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents
> +$(obj)/sbat.o: $(src)/sbat.sbat FORCE
> +	$(call if_changed,objcopy)
> +

Doing a build locally with this, I've found two issues.  One is:

> ld: warning: arch/x86/efi/sbat.o: missing .note.GNU-stack section implies executable stack
> ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
> ld: warning: arch/x86/efi/built_in.o: requires executable stack (because the .note.GNU-stack section is executable)
> ld: warning: arch/x86/built_in.o: requires executable stack (because the .note.GNU-stack section is executable)
> ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)
> ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)
> ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)

which isn't a terribly good look on a "higher security" feature.  The
easiest way to fix this is:

$(obj)/sbat.o: OBJCOPYFLAGS := -I binary -O elf64-x86-64 \
        --rename-section .data=.sbat,readonly,data,contents \
        --add-section .note.GNU-stack=/dev/null

to add the required section.



>  $(obj)/boot.init.o: $(obj)/buildid.o
>  
>  $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4)
>  $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := $(cflags-stack-boundary)
>  
> +EFIOBJ-y += sbat.o

Also,

> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'

This is because sbat.o is getting linked into the non-EFI build of Xen too.

I'm less sure how to go about fixing this.  There's no nice way I can
see of of getting sbat.o only in the EFI build.  The other option is to
discard it for the ELF build.

~Andrew

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

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On Thu, May 08, 2025 at 09:51:59AM +0100, Andrew Cooper wrote:
> Also,
> 
> > ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
> 
> This is because sbat.o is getting linked into the non-EFI build of Xen too.
> 
> I'm less sure how to go about fixing this.  There's no nice way I can
> see of of getting sbat.o only in the EFI build.  The other option is to
> discard it for the ELF build.

This is kinda related to my question on Matrix - is multiboot2 binary
also supposed to (eventually) support UEFI SB?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Frediano Ziglio]

On Thu, May 8, 2025 at 9:52 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 07/05/2025 2:54 pm, Gerald Elder-Vass wrote:
> > diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
> > index 24dfecfad184..75aa35870a9a 100644
> > --- a/xen/arch/x86/efi/Makefile
> > +++ b/xen/arch/x86/efi/Makefile
> > @@ -6,11 +6,17 @@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
> >  $(obj)/%.o: $(src)/%.ihex FORCE
> >       $(call if_changed,objcopy_o_ihex)
> >
> > +$(obj)/sbat.o: OBJCOPYFLAGS := -I binary -O elf64-x86-64 --rename-section .data=.sbat,readonly,data,contents
> > +$(obj)/sbat.o: $(src)/sbat.sbat FORCE
> > +     $(call if_changed,objcopy)
> > +
>
> Doing a build locally with this, I've found two issues.  One is:
>
> > ld: warning: arch/x86/efi/sbat.o: missing .note.GNU-stack section implies executable stack
> > ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
> > ld: warning: arch/x86/efi/built_in.o: requires executable stack (because the .note.GNU-stack section is executable)
> > ld: warning: arch/x86/built_in.o: requires executable stack (because the .note.GNU-stack section is executable)
> > ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)
> > ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)
> > ld: warning: prelink.o: requires executable stack (because the .note.GNU-stack section is executable)
>
> which isn't a terribly good look on a "higher security" feature.  The
> easiest way to fix this is:
>
> $(obj)/sbat.o: OBJCOPYFLAGS := -I binary -O elf64-x86-64 \
>         --rename-section .data=.sbat,readonly,data,contents \
>         --add-section .note.GNU-stack=/dev/null
>
> to add the required section.
>
>
>
> >  $(obj)/boot.init.o: $(obj)/buildid.o
> >
> >  $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4)
> >  $(addprefix $(obj)/,$(EFIOBJ-y)): CFLAGS_stack_boundary := $(cflags-stack-boundary)
> >
> > +EFIOBJ-y += sbat.o
>
> Also,
>
> > ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
>
> This is because sbat.o is getting linked into the non-EFI build of Xen too.
>
> I'm less sure how to go about fixing this.  There's no nice way I can
> see of of getting sbat.o only in the EFI build.  The other option is to
> discard it for the ELF build.
>

I don't see the point of having this section in the ELF file, it's
used only when in PE by secure boot.
It should not be hard to add it to the disard list. Specifically in
xen/include/xen/xen.lds.h file look at DISCARD_SECTIONS and
DISCARD_EFI_SECTIONS definitions (I think just add .sbat to the
DISCARD_EFI_SECTIONS list if EFI is not defined).

Frediano

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

[Andrew Cooper]

On 08/05/2025 11:31 am, Marek Marczykowski-Górecki wrote:
> On Thu, May 08, 2025 at 09:51:59AM +0100, Andrew Cooper wrote:
>> Also,
>>
>>> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
>> This is because sbat.o is getting linked into the non-EFI build of Xen too.
>>
>> I'm less sure how to go about fixing this.  There's no nice way I can
>> see of of getting sbat.o only in the EFI build.  The other option is to
>> discard it for the ELF build.
> This is kinda related to my question on Matrix - is multiboot2 binary
> also supposed to (eventually) support UEFI SB?

This is mixing two things.

Xen is either an ELF binary (ultimately zipped, so xen.gz) or is an EFI
binary (xen.efi).

Both of these binaries currently have an MB2 header.  This was by
accident, as xen.efi is a strict superset of the ELF build.

AIUI, SBAT only makes sense to exist in the EFI binary.

~Andrew

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

[Frediano Ziglio]

On Thu, May 8, 2025 at 12:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 08/05/2025 11:31 am, Marek Marczykowski-Górecki wrote:
> > On Thu, May 08, 2025 at 09:51:59AM +0100, Andrew Cooper wrote:
> >> Also,
> >>
> >>> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
> >> This is because sbat.o is getting linked into the non-EFI build of Xen too.
> >>
> >> I'm less sure how to go about fixing this.  There's no nice way I can
> >> see of of getting sbat.o only in the EFI build.  The other option is to
> >> discard it for the ELF build.
> > This is kinda related to my question on Matrix - is multiboot2 binary
> > also supposed to (eventually) support UEFI SB?
>
> This is mixing two things.
>
> Xen is either an ELF binary (ultimately zipped, so xen.gz) or is an EFI
> binary (xen.efi).
>
> Both of these binaries currently have an MB2 header.  This was by
> accident, as xen.efi is a strict superset of the ELF build.
>

We are planning to use multiboot2 booting. The reason is the way we
want some parameters (like command line) to be passed. We are going to
use grub2.

> AIUI, SBAT only makes sense to exist in the EFI binary.
>
> ~Andrew

Frediano

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

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On Thu, May 08, 2025 at 01:28:21PM +0100, Frediano Ziglio wrote:
> On Thu, May 8, 2025 at 12:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >
> > On 08/05/2025 11:31 am, Marek Marczykowski-Górecki wrote:
> > > On Thu, May 08, 2025 at 09:51:59AM +0100, Andrew Cooper wrote:
> > >> Also,
> > >>
> > >>> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
> > >> This is because sbat.o is getting linked into the non-EFI build of Xen too.
> > >>
> > >> I'm less sure how to go about fixing this.  There's no nice way I can
> > >> see of of getting sbat.o only in the EFI build.  The other option is to
> > >> discard it for the ELF build.
> > > This is kinda related to my question on Matrix - is multiboot2 binary
> > > also supposed to (eventually) support UEFI SB?
> >
> > This is mixing two things.
> >
> > Xen is either an ELF binary (ultimately zipped, so xen.gz) or is an EFI
> > binary (xen.efi).
> >
> > Both of these binaries currently have an MB2 header.  This was by
> > accident, as xen.efi is a strict superset of the ELF build.
> >
> 
> We are planning to use multiboot2 booting. The reason is the way we
> want some parameters (like command line) to be passed. We are going to
> use grub2.

Which means that multiboot2 binary needs to be signed somehow, and for
MS to be happy, needs to include SBAT too.

Relevant series:
https://lore.kernel.org/xen-devel/20240328151106.1451104-1-ross.lagerwall@citrix.com/
I don't recall seeing v3 posted.

And relevant grub series:
https://lore.kernel.org/xen-devel/20240328151302.1451158-1-ross.lagerwall@citrix.com/

> > AIUI, SBAT only makes sense to exist in the EFI binary.
> >
> > ~Andrew
> 
> Frediano

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Jan Beulich]

On 08.05.2025 10:51, Andrew Cooper wrote:
> On 07/05/2025 2:54 pm, Gerald Elder-Vass wrote:
> Also,
> 
>> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
> 
> This is because sbat.o is getting linked into the non-EFI build of Xen too.
> 
> I'm less sure how to go about fixing this.  There's no nice way I can
> see of of getting sbat.o only in the EFI build.  The other option is to
> discard it for the ELF build.

We already link $(note_file) into just xen.efi; I'm pretty sure the same could
be done for this new object.

Jan

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

[Andrew Cooper]

On 12/05/2025 11:50 am, Jan Beulich wrote:
> On 08.05.2025 10:51, Andrew Cooper wrote:
>> On 07/05/2025 2:54 pm, Gerald Elder-Vass wrote:
>> Also,
>>
>>> ld: warning: orphan section `.sbat' from `prelink.o' being placed in section `.sbat'
>> This is because sbat.o is getting linked into the non-EFI build of Xen too.
>>
>> I'm less sure how to go about fixing this.  There's no nice way I can
>> see of of getting sbat.o only in the EFI build.  The other option is to
>> discard it for the ELF build.
> We already link $(note_file) into just xen.efi; I'm pretty sure the same could
> be done for this new object.

Ah - that was the string I was failing to search for.

But, note_file is special and complicated.  sbat.o would be a regular
EFI-only object, but that can't be expressed currently, with how
prelink.o works.

I think in the short term, the v4 patch is the way to go.  Reworking the
build system to have EFI-only and ELF-only objects is a larger task.

~Andrew