From eb3d587fe27f8f46fc284c8b665e5ddf939d707f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 4 Sep 2026 10:57:08 +0200 Subject: [PATCH] F-9746: x86_64_efi: fix do_boot prototype mismatch, drop unused param boot_x86_64.c declared x86_64_efi_do_boot(uint8_t *) while the HAL defines (uint32_t *, uint8_t *): the linker connected the incompatible pair and the call was undefined behavior, mostly latent because the second parameter was discarded. Match the AArch64 sibling: single const uint32_t *boot_addr in the declaration, definition and call, and remove the unused dts_address parameter. --- hal/x86_64_efi.c | 15 ++++----- src/boot_x86_64.c | 4 +-- tools/unit-tests/unit-efi-x86-open-image.c | 39 ++++++++++++++++++++-- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/hal/x86_64_efi.c b/hal/x86_64_efi.c index 8b80af0a..c99dcbd6 100644 --- a/hal/x86_64_efi.c +++ b/hal/x86_64_efi.c @@ -106,12 +106,10 @@ static void panic() #endif } -void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) +void RAMFUNCTION x86_64_efi_do_boot(const uint32_t *boot_addr) { - uint32_t *size; - uint8_t* manifest = ((uint8_t*)boot_addr) - IMAGE_HEADER_SIZE; - - (void)dts_address; /* Unused for now */ + const uint32_t *size; + const uint8_t* manifest = ((const uint8_t*)boot_addr) - IMAGE_HEADER_SIZE; MEMMAP_DEVICE_PATH mem_path_device[2]; EFI_HANDLE kernelImageHandle; @@ -121,7 +119,7 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) EFI_LOADED_IMAGE *kernel_li = NULL; EFI_GUID lipGuid = EFI_LOADED_IMAGE_PROTOCOL_GUID; - size = (uint32_t *)(manifest + 4); + size = (const uint32_t *)(manifest + 4); /* Guard against a zero-size image: EndingAddress below would underflow * and an empty range would be handed to LoadImage. */ @@ -138,7 +136,8 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) mem_path_device->Header.Type = EFI_DEVICE_PATH_PROTOCOL_HW_TYPE; mem_path_device->Header.SubType = EFI_DEVICE_PATH_PROTOCOL_MEM_SUBTYPE; mem_path_device->MemoryType = EfiLoaderData; - mem_path_device->StartingAddress = (EFI_PHYSICAL_ADDRESS)boot_addr; + mem_path_device->StartingAddress = + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr; /* MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid byte). */ mem_path_device->EndingAddress = (EFI_PHYSICAL_ADDRESS)((uintptr_t)boot_addr + *size - 1); @@ -153,7 +152,7 @@ void RAMFUNCTION x86_64_efi_do_boot(uint32_t *boot_addr, uint8_t *dts_address) 0, /* bool */ gImageHandle, (EFI_DEVICE_PATH*)mem_path_device, - boot_addr, + (void*)(uintptr_t)boot_addr, *size, &kernelImageHandle); if (status != EFI_SUCCESS) { diff --git a/src/boot_x86_64.c b/src/boot_x86_64.c index 865d8665..6b5b4c7c 100644 --- a/src/boot_x86_64.c +++ b/src/boot_x86_64.c @@ -34,7 +34,7 @@ extern unsigned int __bss_end__; static volatile unsigned int cpu_id; extern unsigned int *END_STACK; -extern void RAMFUNCTION x86_64_efi_do_boot(uint8_t *kernel); +extern void RAMFUNCTION x86_64_efi_do_boot(const uint32_t *boot_addr); #if defined(MMU) || defined(WOLFBOOT_FDT) void RAMFUNCTION do_boot(const uint32_t *app_offset, const uint32_t* dts_offset) @@ -42,7 +42,7 @@ void RAMFUNCTION do_boot(const uint32_t *app_offset, const uint32_t* dts_offset) void RAMFUNCTION do_boot(const uint32_t *app_offset) #endif { - x86_64_efi_do_boot((uint8_t *)app_offset); + x86_64_efi_do_boot(app_offset); } #endif /* TARGET_X86_64_EFI */ diff --git a/tools/unit-tests/unit-efi-x86-open-image.c b/tools/unit-tests/unit-efi-x86-open-image.c index 5caa045b..58cb7b08 100644 --- a/tools/unit-tests/unit-efi-x86-open-image.c +++ b/tools/unit-tests/unit-efi-x86-open-image.c @@ -388,6 +388,11 @@ uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr) /* Pull in the code under test (its statics become visible here). */ #include "../../hal/x86_64_efi.c" +/* The caller too: with the definition already in this translation unit, + * its extern declaration is checked against it, so a prototype drift + * between the two files is a compile error. */ +#include "../../src/boot_x86_64.c" + /* The tests pass their own CHAR16 filename (the mock ignores the content). * The build rule uses -fshort-wchar like the real x86_64_efi build, so * efi_main's L"..." literals are valid 16-bit CHAR16 strings here too. */ @@ -653,7 +658,7 @@ START_TEST(test_do_boot_mem_path_end_inclusive) image[IMAGE_HEADER_SIZE + i] = (uint8_t)(i & 0xFF); boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); - x86_64_efi_do_boot(boot_addr, NULL); + x86_64_efi_do_boot(boot_addr); ck_assert_int_eq(wolfBoot_panicked, 0); ck_assert_int_eq(mock_load_image_calls, 1); @@ -689,7 +694,7 @@ START_TEST(test_do_boot_zero_size_panics) memcpy(image + 4, &fw_size, sizeof(fw_size)); boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); - x86_64_efi_do_boot(boot_addr, NULL); + x86_64_efi_do_boot(boot_addr); ck_assert_int_gt(wolfBoot_panicked, 0); ck_assert_int_eq(mock_load_image_calls, 0); @@ -697,6 +702,35 @@ START_TEST(test_do_boot_zero_size_panics) } END_TEST +/* The caller (do_boot in src/boot_x86_64.c) must hand its app_offset to + * the HAL unmodified: same pointer in LoadImage's SourceBuffer and in the + * memory device path. A caller/callee prototype mismatch (e.g. the old + * uint8_t * declaration) would truncate or reinterpret it. */ +START_TEST(test_do_boot_transfers_app_offset) +{ + uint32_t fw_size = 1024; + uint32_t *boot_addr; + uint8_t image[IMAGE_HEADER_SIZE + 1024]; + int i; + + memset(image, 0, sizeof(image)); + memcpy(image, "WOLF", 4); + memcpy(image + 4, &fw_size, sizeof(fw_size)); + for (i = 0; i < 1024; i++) + image[IMAGE_HEADER_SIZE + i] = (uint8_t)(i & 0xFF); + boot_addr = (uint32_t *)(image + IMAGE_HEADER_SIZE); + + do_boot(boot_addr); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_load_image_calls, 1); + ck_assert_uint_eq(captured_src_addr, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); + ck_assert_uint_eq(captured_dp[0].StartingAddress, + (EFI_PHYSICAL_ADDRESS)(uintptr_t)boot_addr); +} +END_TEST + Suite *efi_x86_open_image_suite(void) { Suite *s = suite_create("efi-x86-open-image"); @@ -711,6 +745,7 @@ Suite *efi_x86_open_image_suite(void) tcase_add_test(tc, test_open_image_header_boundary); tcase_add_test(tc, test_do_boot_mem_path_end_inclusive); tcase_add_test(tc, test_do_boot_zero_size_panics); + tcase_add_test(tc, test_do_boot_transfers_app_offset); suite_add_tcase(s, tc); return s;