[PATCH v8 1/5] efi: add EFI_SYSTEM_TABLE_POINTER for debug

[Heinrich Schuchardt]

On 7/12/25 02:17, Heinrich Schuchardt wrote:
> On 7/11/25 22:55, Heinrich Schuchardt wrote:
>> Am 4. Juli 2025 13:59:53 MESZ schrieb Paul Liu<paul.liu at linaro.org>:
>>> Hi Heinrich, Ilias,
>>>
>>> Sorry for the late reply. I'm a bit worried about this change. This
>>> structure (efi_system_table_pointer), is not packed.
>>> So its actual size is a bit larger. The crc32 field is not at the end of
>>> the structure. Actually there are 4 bytes padding after it. But the 
>>> CRC32
>>> function is calculating the crc sum on the whole structure. So it might
>>> produce random value if we don't zero those padding bytes.
>> Thanks for highlighting this.
>>
>> Structures in UEFI are typically packed.
>>
>> Should padding bytes really be included in the crc32?
>>
>> We should add a test of the crc32 tohttps://github.com/trini/u-boot/ 
>> blob/master/lib/efi_loader/dbginfodump.c so that we can compare U-Boot 
>> to EDK2.
>>
>> Best regards
>>
>> Heinrich
> 
> 
> EDK II only zeros out the crc32 field.
> 
> BaseTools/Scripts/efi_debugging.py has this logic:
> 
> if system_table_pointer.Signature == efi_system_table_signature:
>      buf1 = bytearray(system_table_pointer)[:16]
>      crc32_value = zlib.crc32(buf1)
>      crc32_value = zlib.crc32(b'\0' * (sizeof(EFI_SYSTEM_TABLE_POINTER) 
> - len(buf1)), crc32_value)
>      if crc32_value == system_table_pointer.Crc32:
>          break
> 
> sizeof(EFI_SYSTEM_TABLE_POINTER) evaluates to 24. So it includes padding 
> bytes. The logic calculates the crc32 as if the padding bytes were zero.
> 
> In contrast to your contribution EDK II uses BootServicesData for 
> storing the system table pointer.
> 
> Here is a dump of what I saw on EDK II:
> 
> System Table Pointer @ 0x000000017fc00000
> 49 42 49 20 53 59 53 54 18 e0 ff 7f 01 00 00 00 56 d8 89 d0 00 00 00 00
> 
> The crc32 contains the trailing NULs.
> 
> So you are right we should zero out the padding bytes.
> 
> Best regards
> 
> Heinrich

I have opened an issue https://mantis.uefi.org/mantis/view.php?id=2521:

0002521: CRC32 field of System Table Pointer not well defined

The UEFI spec has this sentence concerning structure 
EFI_SYSTEM_TABLE_POINTER:
"The 32-bit CRC of the entire structure is calculated assuming the Crc32 
field is zero. This value is then written to the Crc32 field."

EDK II has implemented the structure as non-packed. Using the GCC 
compiler this implies on 64-bit systems that sizeof() reports the 
structure size as 24 bytes including 4 padding bytes after CRC32. These 
padding bytes are not zeroed out by the EDK II code and could take 
random values.

MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c hence produces a random 
Crc32 in CoreUpdateDebugTableCrc32():
gBS->CalculateCrc32 ((VOID *)mDebugTable, sizeof 
(EFI_SYSTEM_TABLE_POINTER), &mDebugTable->Crc32);

BaseTools/Scripts/efi_debugging.py:1824 calculates the CRC32 as if the 
padding bytes were zero even if they are not non-zero. This may lead to 
not finding the System Table Pointer at all.

The specification should explicitly state how the Crc32 field is to be 
calculated in the presence of padding bytes.

My suggestion would be change the specification to define the structure 
as packed so that we don't have to worry about any padding bytes:

"Eventual padding bytes after the Crc32 field are not considered in the 
calculation of the Crc32 value."

Best regards

Heinrich