From e02a734e72d7f0795743b56b7119f0526969b13a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 24 Aug 2026 18:06:02 +0200 Subject: [PATCH] F-11027: abort the ELF load when an mmu_cb mapping fails elf_load_image_mmu() skipped a segment (continue) when its mmu_cb mapping failed, then kept loading the rest, published the ELF entry point and returned success for a partially loaded image. The x86 FSP payload path (boot_x86_fsp_payload.c) passes a real mmu_cb and only panics on a non-zero return, so the buggy continue booted a payload with a missing segment. Return -6 with a fail-loud message, matching the program-header clobber guard that aborts for the same reason: never silently drop a a PT_LOAD segment. unit-elf-mmu-fail fails the first segment's mapping and checks the load is rejected with no entry point published; a second test checks successful mappings still load the segments and publish the entry. Verification: - Built: unit test compiles elf.c (WOLFBOOT_ELF config); elf.c syntax-clean under the WOLFBOOT_FSP config (gcc -fsyntax-only). - Tested: unit-elf-mmu-fail 2/2; pre-fix the failure-path test got ret == 0 (entry published for a partially loaded image). - Pitfalls: no caller switches on the exact code (all check != 0); -6 is new and dedicated to the mapping failure. - Style: cstyle-check.sh on src/elf.c flags pre-existing FMT/R1 issues also present on the pre-change file; the new test trips the uncrustify pointer-alignment class the sibling unit tests trip and matches their local style. - Message: F-11027: prefix, no co-author trailers. --- src/elf.c | 14 ++- tools/unit-tests/Makefile | 8 ++ tools/unit-tests/unit-elf-mmu-fail.c | 175 +++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 tools/unit-tests/unit-elf-mmu-fail.c diff --git a/src/elf.c b/src/elf.c index f18aa256..cbd547fc 100644 --- a/src/elf.c +++ b/src/elf.c @@ -183,12 +183,14 @@ int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry, #ifndef ELF_PARSER if (mmu_cb != NULL) { if (mmu_cb(vaddr, paddr, mem_size) != 0) { -#ifdef DEBUG_ELF - wolfBoot_printf( - "Fail to map %u bytes to %p (p %p)\r\n", - (uint32_t)mem_size, (void*)vaddr, (void*)paddr); -#endif - continue; + /* Never silently drop a PT_LOAD segment: a failed mapping + * leaves a hole in the image, and publishing the entry + * point for a partially loaded ELF is worse than failing + * the load. */ + wolfBoot_printf("ELF: failed to map %u bytes to %p " + "(p %p) -- aborting ELF load\r\n", + (uint32_t)mem_size, (void*)vaddr, (void*)paddr); + return -6; } } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index a8dcb019..9e3e4566 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -99,6 +99,7 @@ TESTS+=unit-ata-security-passphrase-zeroize TESTS+=unit-fwtpm-nv-oob TESTS+=unit-elf-bss-guard TESTS+=unit-elf-entry-inplace +TESTS+=unit-elf-mmu-fail TESTS+=unit-image-elf-scatter TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-dice-token-size @@ -1191,6 +1192,13 @@ unit-elf-entry-inplace: unit-elf-entry-inplace.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) +# unit-elf-mmu-fail: a failing mmu_cb must abort the whole ELF load +# (F-11027: the failed mapping used to be skipped with continue, and +# the entry point was published for a partially loaded image). +unit-elf-mmu-fail: unit-elf-mmu-fail.c ../../src/elf.c + gcc -o $@ unit-elf-mmu-fail.c -I../../include -DWOLFBOOT_ELF \ + -DARCH_FLASH_OFFSET=0 -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) + unit-image-elf-scatter: ../../include/target.h unit-image-elf-scatter.c gcc -o $@ unit-image-elf-scatter.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c \ $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-elf-mmu-fail.c b/tools/unit-tests/unit-elf-mmu-fail.c new file mode 100644 index 00000000..e61c8307 --- /dev/null +++ b/tools/unit-tests/unit-elf-mmu-fail.c @@ -0,0 +1,175 @@ +/* unit-elf-mmu-fail.c + * + * Regression test for F-11027: elf_load_image_mmu() skipped a segment + * when its mmu_cb mapping failed (continue) instead of aborting, so it + * could publish the ELF entry point and return success for a partially + * loaded image. The x86 FSP payload path (boot_x86_fsp_payload.c) + * passes a real mmu_cb and panics only on a non-zero return, so the + * buggy continue booted a payload with a missing segment. + * + * The loader must fail the whole load when a mapping fails, matching + * the program-header clobber guard that aborts for the same reason: + * never silently drop a PT_LOAD segment. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include + +#include "elf.h" + +/* Pull in elf.c directly (avoids a separate link step). */ +#include "../../src/elf.c" + +/* 4 KiB is plenty for the ELF64 header + 2 program headers + segment data. */ +#define TEST_IMG_SIZE 4096 + +static uint8_t g_image[TEST_IMG_SIZE]; +static uint8_t g_target[64]; + +/* Fail the mapping of the first segment only, on the first call. */ +static int g_fail_first; + +static int mmu_cb_fail_first(uint64_t vaddr, uint64_t paddr, uint32_t size) +{ + (void)vaddr; + (void)paddr; + (void)size; + + if (g_fail_first) { + g_fail_first = 0; + return -1; + } + return 0; +} + +static int mmu_cb_ok(uint64_t vaddr, uint64_t paddr, uint32_t size) +{ + (void)vaddr; + (void)paddr; + (void)size; + return 0; +} + +/* Build a minimal ELF64 with two 8-byte PT_LOAD segments, with their + * data at image offsets 256 and 264, mapped onto g_target. */ +static void build_elf(void) +{ + elf64_header *hdr; + elf64_program_header *ph; + + memset(g_image, 0, sizeof(g_image)); + memset(g_target, 0, sizeof(g_target)); + + hdr = (elf64_header *)g_image; + memcpy(hdr->ident, ELF_IDENT_STR, 4); + hdr->ident[ELF_CLASS_OFF] = ELF_CLASS_64; + hdr->ident[5] = ELF_ENDIAN_LITTLE; + hdr->type = ELF_HET_EXEC; + hdr->version = 1; + hdr->entry = 0x1000; + hdr->ph_offset = sizeof(elf64_header); /* 64 */ + hdr->ph_entry_size = sizeof(elf64_program_header); /* 56 */ + hdr->ph_entry_count = 2; + + ph = (elf64_program_header *)(g_image + sizeof(elf64_header)); + + ph[0].type = ELF_PT_LOAD; + ph[0].flags = 0; + ph[0].offset = 256; + ph[0].vaddr = (uint64_t)(uintptr_t)g_target; + ph[0].paddr = ph[0].vaddr; + ph[0].file_size = 8; + ph[0].mem_size = 8; + ph[0].align = 1; + + ph[1].type = ELF_PT_LOAD; + ph[1].flags = 0; + ph[1].offset = 264; + ph[1].vaddr = (uint64_t)(uintptr_t)(g_target + 16); + ph[1].paddr = ph[1].vaddr; + ph[1].file_size = 8; + ph[1].mem_size = 8; + ph[1].align = 1; + + /* Segment payloads: recognizable bytes. */ + memset(g_image + 256, 0xA5, 8); + memset(g_image + 264, 0x5A, 8); +} + +/* A failed mapping must fail the whole load: no entry point published, + * no success returned. */ +START_TEST(test_mmu_failure_aborts_load) +{ + uintptr_t entry = 0; + int ret; + + build_elf(); + g_fail_first = 1; + + ret = elf_load_image_mmu(g_image, sizeof(g_image), &entry, + mmu_cb_fail_first); + + ck_assert_int_ne(ret, 0); + ck_assert_msg(entry == 0, + "entry point published despite a failed mmu mapping"); +} +END_TEST + +/* With successful mappings the load completes and the entry point is + * published: the failure path must not over-reject. */ +START_TEST(test_successful_mappings_still_load) +{ + uintptr_t entry = 0; + int ret; + + build_elf(); + + ret = elf_load_image_mmu(g_image, sizeof(g_image), &entry, mmu_cb_ok); + + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(entry, 0x1000); + ck_assert_mem_eq(g_target, g_image + 256, 8); + ck_assert_mem_eq(g_target + 16, g_image + 264, 8); +} +END_TEST + +Suite *elf_mmu_fail_suite(void) +{ + Suite *s = suite_create("ELF mmu failure"); + TCase *tc = tcase_create("mmu-cb-failure"); + tcase_add_test(tc, test_mmu_failure_aborts_load); + tcase_add_test(tc, test_successful_mappings_still_load); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = elf_mmu_fail_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +}