Multi profile UKIs don't take into account that .linux section must be the last section in a UKI

[DaanDeMeyer]

In ukify there's this comment

        # linux shall be last to leave breathing room for decompression.
        # We'll add it later.

The current implementation of UKI profiles seems to completely ignore this fact and just appends profiles at the end. To accomodate this, we should probably special case the .linux and not allow it to be modified in a profile and just treat it as a global section.

This also means the entire ukify interface for multi profile UKIs is broken and needs to be reworked so all the profiles are built upfront and passed in when building the UKI so that ukify can put the .linux section last.

[poettering]

I don't understand that comment? Everything works fine here?

What precisely is the issue here?

[DaanDeMeyer]

cc @keszybz

I found
0fa2cac#commitcomment-84868898 where putting the kernel last addressed a boot issue. I don't think testing in a virtual machine is sufficient justification to say we can remove this unless we have conclusive evidence that decompressing the kernel in place can't override subsequent sections anymore.

[keszybz]

I don't really have much to add — that comment in ukify was added based on those comments and I didn't investigate further.

[poettering]

PE sections are mapped into memory by their VMA address, encoded in the section header, not by the order in the PE files. I think there's some confusion here about this?

Hence how ukify places the sections in memory can be entirely distinct from the order it places them in the PE file.

[DaanDeMeyer]

@poettering Sure but it doesn't do that at the moment, the virtual addresses starts at the end of the previous section aligned to the section alignment and is as large as the content of the section. So if the kernel image is decompressed in place there's a good chance it overrides data from the next section.

Relevant code from ukify:

        new_section.VirtualAddress = round_up(
            pe.sections[-1].VirtualAddress + pe.sections[-1].Misc_VirtualSize,
            pe.OPTIONAL_HEADER.SectionAlignment,
        )
        new_section.Misc_VirtualSize = len(data)

[DaanDeMeyer]

@ardbiesheuvel Any chance you can pitch in here, does the EFI stub decompress the kernel in place and could that override data from subsequent PE sections that are mapped into memory just after the compressed kernel image from the UKI?

[ardbiesheuvel]

@ardbiesheuvel Any chance you can pitch in here, does the EFI stub decompress the kernel in place and could that override data from subsequent PE sections that are mapped into memory just after the compressed kernel image from the UKI?

Recent kernels (v6.7 and newer) will no longer run the legacy decompressor when booting via the EFI stub, but allocate memory for a decompression buffer using the EFI boot services. So these should not be affected at all here, as long as the space includes space for uninitialized data (i.e., the delta between RawDataSize and VirtualSize of the last section in the bzImage PE/COFF image). This functionality was backported to v6.1 and newer as well.

Before that, the EFI stub would enter via the legacy decompressor, which relocates itself to the end of the memory region it assumes to be reserved for it, and decompresses its payload to the start of the buffer (i.e., where the decompressor program itself used to reside)

Linux commit 8eace5b3555606e684739bef5bcdfcfe68235257 removes this additional space from the PE/COFF header, given that EFI boot no longer needs it.

So the only potential issue I can see here is where a loader uses the PE/COFF metadata to size the allocation, but then calls the legacy decompressor instead of the EFI stub PE entrypoint. Otherwise, things should work fine as long as the SizeOfImage field in the PE header is respected.

[ardbiesheuvel]

Looking at the UKI booting my local work laptop:

  Section {
    Number: 12
    Name: .linux (2E 6C 69 6E 75 78 00 00)
    VirtualSize: 0x926200
    VirtualAddress: 0x5833000
    RawDataSize: 9593344

which matches

-rw-r--r-- 1 root root 9593344 Sep  4 09:50 /boot/vmlinuz-6.9.10-1rodete5-amd64

However, the VirtualSize of that section equals the RawDataSize, whereas the SizeOfImage is 9883648 so there is some space missing here.

The SizeOfImage of the UKI is 102080512, whereas the virtual base of that section plus the SizeOfImage of the PE image inside it is 102367232.

So there is definitely something broken here*. This is systemd-boot version 255.4-1+gl0 (Google internal Debian based distro)

* if the payload is expected to execute in place from where it was loaded when the UKI was loaded by the firmware

[DaanDeMeyer]

Looking at the UKI booting my local work laptop:

  Section {
    Number: 12
    Name: .linux (2E 6C 69 6E 75 78 00 00)
    VirtualSize: 0x926200
    VirtualAddress: 0x5833000
    RawDataSize: 9593344

which matches

-rw-r--r-- 1 root root 9593344 Sep  4 09:50 /boot/vmlinuz-6.9.10-1rodete5-amd64

However, the VirtualSize of that section equals the RawDataSize, whereas the SizeOfImage is 9883648 so there is some space missing here.

The SizeOfImage of the UKI is 102080512, whereas the virtual base of that section plus the SizeOfImage of the PE image inside it is 102367232.

So there is definitely something broken here*. This is systemd-boot version 255.4-1+gl0 (Google internal Debian based distro)

• if the payload is expected to execute in place from where it was loaded when the UKI was loaded by the firmware

Right looking at the ukify code we don't take SizeOfImage into account at all. Do I understand correctly that Misc_VirtualSize for the .linux section should be set to SizeOfImage of the bzimage?

[ardbiesheuvel]

Looking at the UKI booting my local work laptop:

  Section {
    Number: 12
    Name: .linux (2E 6C 69 6E 75 78 00 00)
    VirtualSize: 0x926200
    VirtualAddress: 0x5833000
    RawDataSize: 9593344

which matches

-rw-r--r-- 1 root root 9593344 Sep  4 09:50 /boot/vmlinuz-6.9.10-1rodete5-amd64

However, the VirtualSize of that section equals the RawDataSize, whereas the SizeOfImage is 9883648 so there is some space missing here.
The SizeOfImage of the UKI is 102080512, whereas the virtual base of that section plus the SizeOfImage of the PE image inside it is 102367232.
So there is definitely something broken here*. This is systemd-boot version 255.4-1+gl0 (Google internal Debian based distro)

• if the payload is expected to execute in place from where it was loaded when the UKI was loaded by the firmware

Right looking at the ukify code we don't take SizeOfImage into account at all. Do I understand correctly that Misc_VirtualSize for the .linux section should be set to SizeOfImage of the bzimage?

Yes, assuming the code might be called in place from the loaded UKI image, which I suspect is the case with older kernels and the EFI handover protocol.

[keszybz]

Yes, assuming the code might be called in place from the loaded UKI image, which I suspect is the case with older kernels and the EFI handover protocol.

We know the kernel version that we're packaging, so we could make this conditional. But I'm not sure if it's worth it… Is there any downside to using a larger VirtualSize value?

[ardbiesheuvel]

Yes, assuming the code might be called in place from the loaded UKI image, which I suspect is the case with older kernels and the EFI handover protocol.

We know the kernel version that we're packaging, so we could make this conditional. But I'm not sure if it's worth it… Is there any downside to using a larger VirtualSize value?

I don't think so, it is just padding, although some PE/COFF loaders may misbehave or complain if the padding is in the middle rather than at the end.

To @poettering's point,

PE sections are mapped into memory by their VMA address, encoded in the section header, not by the order in the PE files. I think there's some confusion here about this?

Hence how ukify places the sections in memory can be entirely distinct from the order it places them in the PE file.

According to the PE/COFF spec (section 13),

In an image file, the VAs for sections must be assigned by the linker so that they are
in ascending order and adjacent, and they must be a multiple of the SectionAlignment
value in the optional header.

so the order in the PE file must match the ordering based on the virtual address, even though the placement of the raw data described by those ordered section headers may be anywhere in the file. But again, it would be wise not to stray too far from how PE/COFF files are commonly laid out by the existing toolchains.