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<[email protected]>:
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
