From c555152407c1001e49689dacc8fe2c308f2326c0 Mon Sep 17 00:00:00 2001 From: Matthias Rosenfelder Date: Thu, 14 Dec 2023 20:12:26 +0100 Subject: [PATCH] elfloader: fix UEFI integration bug: descriptor size mismatch. The UEFI specification 2.10 says in section 7 for EFI_BOOT_SERVICES.GetMemoryMap(): "The GetMemoryMap() function also returns the size and revision number of the EFI_MEMORY_DESCRIPTOR. The DescriptorSize represents the size in bytes of an EFI_MEMORY_DESCRIPTOR array element returned in MemoryMap. The size is returned to allow for future expansion of the EFI_MEMORY_DESCRIPTOR in response to hardware innovation. The structure of the EFI_MEMORY_DESCRIPTOR may be extended in the future but it will remain backwards compatible with the current definition. Thus OS software must use the DescriptorSize to find the start of each EFI_MEMORY_DESCRIPTOR in the MemoryMap array." This mismatch is the case on (our) Orin UEFI. The compiled size of a memory descriptor is 40 Bytes, but the Orin UEFI implementation uses 48 Bytes per descriptor. Thus, due to the requirement to use a larger size than the returned total size (due to the fact that the buffer allocation itself may lead to one more entry in the memory map), we must increase by the size (in terms of number of descriptors), but use the number of bytes that UEFI uses for one memory map entry, not what we think it might be. Some other people already stumbled over this: https://forum.osdev.org/viewtopic.php?f=1&t=32953 Based on the comment in the existing code, the author seems to not have understood how the size of the memory map can be determined. Just read the spec! So we better update that misleading comment. Signed-off-by: Matthias Rosenfelder --- elfloader-tool/src/binaries/efi/efi_init.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/elfloader-tool/src/binaries/efi/efi_init.c b/elfloader-tool/src/binaries/efi/efi_init.c index e938d037..47dc2651 100644 --- a/elfloader-tool/src/binaries/efi/efi_init.c +++ b/elfloader-tool/src/binaries/efi/efi_init.c @@ -63,8 +63,8 @@ static unsigned long exit_boot_services(void) /* * As the number of existing memory segments are unknown, - * we need to resort to a trial and error to guess that. - * We start from 32 and increase it by one until get a valid value. + * we need to start somewhere. The API then tells us how much space we need + * if it is not enough. */ map_size = sizeof(*memory_map) * 32; @@ -81,7 +81,12 @@ static unsigned long exit_boot_services(void) memory_map = NULL; if (status == EFI_BUFFER_TOO_SMALL) { - map_size += sizeof(*memory_map); + /* Note: "map_size" is an IN/OUT-parameter and has been updated to the + * required size. We still add one more entry ("desc_size" is in bytes) + * due to the hint from the spec ("since allocation of the new buffer + * may potentially increase memory map size."). + */ + map_size += desc_size; } else { /* some other error; bail out! */ return status;