[PATCH 2/2] x86/efi: Implement support for embedding SBAT data for x86
Vitaly Kuznetsov
vkuznets at redhat.com
Fri May 2 05:09:59 PDT 2025
Ard Biesheuvel <ardb at kernel.org> writes:
> On Mon, 28 Apr 2025 at 12:59, Vitaly Kuznetsov <vkuznets at redhat.com> wrote:
>>
>> Ard Biesheuvel <ardb at kernel.org> writes:
>>
>> > On Thu, 24 Apr 2025 at 10:10, Vitaly Kuznetsov <vkuznets at redhat.com> wrote:
...
>> >> $(obj)/vmlinux: $(vmlinux-objs-y) $(vmlinux-libs-y) FORCE
>> >> $(call if_changed,ld)
>> >>
>> >> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>> >> index 3b2bc61c9408..d0a27905de90 100644
>> >> --- a/arch/x86/boot/compressed/vmlinux.lds.S
>> >> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
>> >> @@ -49,9 +49,22 @@ SECTIONS
>> >> *(.data.*)
>> >>
>> >> /* Add 4 bytes of extra space for the obsolete CRC-32 checksum */
>> >> +#ifndef CONFIG_EFI_SBAT
>> >> . = ALIGN(. + 4, 0x200);
>> >> +#else
>> >> + /* Avoid gap between '.data' and '.sbat' */
>> >> + . = ALIGN(. + 4, 0x1000);
>> >> +#endif
>> >> _edata = . ;
>> >> }
>> >> +#ifdef CONFIG_EFI_SBAT
>> >> + .sbat : ALIGN(0x1000) {
>> >> + _sbat = . ;
>> >> + *(.sbat)
>> >> + _esbat = ALIGN(0x200);
>> >> + . = _esbat;
>> >> + }
>> >> +#endif
>> >> . = ALIGN(L1_CACHE_BYTES);
>> >> .bss : {
>> >> _bss = . ;
>> >
>> > This looks a bit odd - see below
>> >
>> >> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>> >> index b5c79f43359b..ab851490ef74 100644
>> >> --- a/arch/x86/boot/header.S
>> >> +++ b/arch/x86/boot/header.S
>> >> @@ -207,6 +207,19 @@ pecompat_fstart:
>> >> IMAGE_SCN_MEM_READ | \
>> >> IMAGE_SCN_MEM_WRITE # 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
>> >> +
>> >
>> > This puts the .sbat section at the very end of the file. However, the
>> > virtual size of .data is 'ZO__end - ZO__data' not 'ZO__edata -
>> > ZO__data', and so the .sbat section will overlap with .bss in the
>> > memory view of the image.
>>
>> Missed that, will fix, thanks! A stupid question though: does this
>> matter in practice for SBAT? I don't think anyone needs SBAT data after
>> kernel starts booting so we can consider it 'discarded'. BSS data can
>> then do whatever it wants.
>>
>
> It violates the PE/COFF spec, and some PE loaders and signing tools
> are very pedantic about the layout.
Turns out it the problem is slightly harder to address then I initially
thought. On x86, arch/x86/boot/bzImage is composed of setup.bin and
vmlinux.bin and the later is produced from vmlinux with
objcopy -O binary -R .note -R .comment -S arch/x86/boot/compressed/vmlinux arch/x86/boot/vmlinux.bin
"objcopy -O binary" basically dumps memory representation of vmlinux. In
case we put '.sbat' before '.bss' we get:
Sections:
Idx Name Size VMA LMA File off Algn
...
4 .data 00002000 0000000000aef000 0000000000aef000 00af0000 2**12
CONTENTS, ALLOC, LOAD, DATA
5 .sbat 00001000 0000000000af1000 0000000000af1000 00af2000 2**12
CONTENTS, ALLOC, LOAD, READONLY, DATA
6 .bss 00023078 0000000000af2000 0000000000af2000 00af3000 2**12
ALLOC
7 .pgtable 00021000 0000000000b16000 0000000000b16000 00af3000 2**12
ALLOC
...
so we can't have a single '.data' section in PE covering both '.data'
and '.bss'/'.pgtable' as '.sbat' is in the middle and we want it to be a
separate section. If we try putting '.sbat' after '.bss' we are going to
get a hole size of '.bss' + '.pgtable' in arch/x86/boot/vmlinux.bin
because 'objcopy -O binary' is not going to squeeze anything (and it has
no idea what '.sbat' is and if it can be moved).
The problem is similar for zboot. I have two ideas:
1) Get back to the idea of putting '.sbat' between '.text' and '.data'
(was in my RFC).
2) Introduce a separate '.bss' section to the PE binary, basically:
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index b5c79f43359b..dae8202705c4 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -197,7 +197,7 @@ pecompat_fstart:
IMAGE_SCN_MEM_EXECUTE # Characteristics
.ascii ".data\0\0\0"
- .long ZO__end - ZO__data # VirtualSize
+ .long ZO__edata - ZO__data # VirtualSize
.long setup_size + ZO__data # VirtualAddress
.long ZO__edata - ZO__data # SizeOfRawData
.long setup_size + ZO__data # PointerToRawData
@@ -207,6 +207,17 @@ pecompat_fstart:
IMAGE_SCN_MEM_READ | \
IMAGE_SCN_MEM_WRITE # Characteristics
+ .ascii ".bss\0\0\0\0"
+ .long ZO__end - ZO__edata # VirtualSize
+ .long setup_size + ZO__edata # VirtualAddress
+ .long 0 # SizeOfRawData
+ .long 0 # PointerToRawData
+
+ .long 0, 0, 0
+ .long IMAGE_SCN_CNT_UNINITIALIZED_DATA| \
+ IMAGE_SCN_MEM_READ | \
+ IMAGE_SCN_MEM_WRITE # Characteristics
+
.set section_count, (. - section_table) / 40
#endif /* CONFIG_EFI_STUB */
This way '.data' doesn't need to be the last meaninful section in
'vmlinux' and we can then pub '.sbat' right after it. I'm going to give
it a try but also I'm wondering why do we have '.data' covering '.bss'
and '.pgtable' in the first place.
--
Vitaly
More information about the linux-riscv
mailing list