Re: [PATCH 1/6] Add SBAT section to the PE binary

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/06/2026 1:06 pm, Jan Beulich wrote:
> On 29.05.2026 17:35, Frediano Ziglio wrote:
>> From: Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>
>>
>> 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
>>
>> Populate the SBAT section in the Xen binary by using the information
>> in xen/arch/x86/sbat.csv.
>>
>> On XenServer, the version and release fields are populated by the spec
>> file during the build process.
>>
>> Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
>> ---
>>  xen/arch/x86/Makefile     | 4 ++++
>>  xen/arch/x86/xen.lds.S    | 2 ++
>>  xen/include/xen/xen.lds.h | 3 ++-
>>  3 files changed, 8 insertions(+), 1 deletion(-)
> This gives the impression of being an entirely new patch, when really the
> (standalone) patch was already at v4, and my comment there wasn't addressed
> (perhaps merely by extending the description some). It also feels like there
> were other pending comments, or else why would this not have gone in long
> ago?

The content of the SBAT table is still pending the security document,
and in particular the decision over whether upstream Xen maintains a
global revision or not.

Although the global revision could be appended later.

>  Additionally, ...
>
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -71,6 +71,7 @@ obj-$(CONFIG_TBOOT) += tboot.o
>>  obj-y += hpet.o
>>  obj-$(CONFIG_VM_EVENT) += vm_event.o
>>  obj-y += xstate.o
>> +obj-y += sbat_data.o
> ... like elsewhere: New files' names should prefer dashes over underscores.
> Question of course is why this isn't simply sbat.o in the first place.

Also sorted.

>
>> @@ -275,6 +276,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_data.o: $(src)/sbat.csv
>> +    $(OBJCOPY) -I binary -O elf64-x86-64 --rename-section 
>> .data=.sbat,readonly,data,contents --add-section .note.GNU-stack=/dev/null 
>> $(srcdir)/sbat.csv $@
> That'll be an SHT_PROGBITS .note.GNU-stack, won't it? When it really wants to
> be SHT_NOTE at least for bleeding edge GNU binutils (see relatively recent
> changes there).

That was my addition to prevent there being a warning about RWX stacks
cascade all the way up the build.

Maybe a better option is to have sbat.S with an .incbin sbat.csv ?

~Andrew