From 286581cf18ae3a08ecbb7fc4a6f08ba4f13cbbd3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 10 Jun 2026 14:39:25 +0200 Subject: [PATCH] F-5093: fix OOB memset in x86_paging_setup_ptp and non-looping panic Three related defects: - panic() halted with a single hlt instruction (no loop), so any resumable interrupt (LAPIC timer via iretq) caused it to return, allowing callers to continue executing. Add while(1) and declare __attribute__((noreturn)) in both definition and header. - x86_paging_setup_ptp guarded with == WOLFBOOT_PTP_NUM instead of >=, so if the counter ever exceeded that value (after a panic() return) the guard was permanently bypassed. - The ptp pointer was computed before the bounds check, creating an out-of-bounds pointer for one-past-end indices; move the assignment to after the guard so no invalid pointer is ever formed. Add unit-x86-paging-oob test that sets page_table_page_used to WOLFBOOT_PTP_NUM and verifies that every subsequent call to x86_paging_setup_ptp triggers panic (via longjmp stub) rather than silently proceeding with an out-of-bounds memset. --- include/x86/common.h | 2 +- src/x86/common.c | 6 +- src/x86/paging.c | 6 +- tools/unit-tests/Makefile | 5 ++ tools/unit-tests/unit-x86-paging-oob.c | 85 ++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 tools/unit-tests/unit-x86-paging-oob.c diff --git a/include/x86/common.h b/include/x86/common.h index 0bd371bc..5a407012 100644 --- a/include/x86/common.h +++ b/include/x86/common.h @@ -61,7 +61,7 @@ void io_write32(uint16_t port, uint32_t value); uint32_t io_read32(uint16_t port); void reset(uint8_t warm); void delay(int msec); -void panic(void); +__attribute__((noreturn)) void panic(void); void cpuid(uint32_t eax_param, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx); int cpuid_is_1gb_page_supported(void); diff --git a/src/x86/common.c b/src/x86/common.c index 80bdcd26..7e0db437 100644 --- a/src/x86/common.c +++ b/src/x86/common.c @@ -300,9 +300,11 @@ uint64_t hal_get_timer_us(void) * This function is used for error handling when the system encounters an unrecoverable issue. * It enters an infinite loop, causing a panic state. */ -void panic() +__attribute__((noreturn)) void panic() { - hlt(); + while(1) { + hlt(); + } } /** diff --git a/src/x86/paging.c b/src/x86/paging.c index 083b1c21..1e957e6b 100644 --- a/src/x86/paging.c +++ b/src/x86/paging.c @@ -156,12 +156,12 @@ static void x86_paging_setup_ptp(uint64_t* e) { uint8_t *ptp; - ptp = &page_table_pages[page_table_page_used * PAGE_TABLE_PAGE_SIZE]; - page_table_page_used++; - if (page_table_page_used == WOLFBOOT_PTP_NUM) { + if (page_table_page_used >= WOLFBOOT_PTP_NUM) { wolfBoot_printf("No more page table page structure\r\n"); panic(); } + ptp = &page_table_pages[page_table_page_used * PAGE_TABLE_PAGE_SIZE]; + page_table_page_used++; x86_paging_setup_entry(e, (uintptr_t)ptp); memset(ptp, 0, PAGE_TABLE_PAGE_SIZE); } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6bd15d38..1be8de3f 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -64,6 +64,7 @@ TESTS+=unit-fit-fpga TESTS+=unit-mpusize TESTS+=unit-flash-erase-h7 TESTS+=unit-otp-keystore +TESTS+=unit-x86-paging-oob # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -443,6 +444,10 @@ unit-disk: unit-disk.c gpt-sfdisk-test.h unit-multiboot: unit-multiboot.c gcc -o $@ unit-multiboot.c $(CFLAGS) $(LDFLAGS) +unit-x86-paging-oob: ../../include/target.h unit-x86-paging-oob.c + gcc -o $@ unit-x86-paging-oob.c $(CFLAGS) \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + %.o:%.c gcc -c -o $@ $^ $(CFLAGS) diff --git a/tools/unit-tests/unit-x86-paging-oob.c b/tools/unit-tests/unit-x86-paging-oob.c new file mode 100644 index 00000000..1f78c60c --- /dev/null +++ b/tools/unit-tests/unit-x86-paging-oob.c @@ -0,0 +1,85 @@ +/* unit-x86-paging-oob.c + * + * Regression test for OOB memset in x86_paging_setup_ptp when pool is + * exhausted. The guard must fire for any call where page_table_page_used + * >= WOLFBOOT_PTP_NUM, not only the exact == boundary. With the old + * single-hlt panic() the guard check can be bypassed on a second call, + * writing 4 KB one page past the end of page_table_pages[]. + */ + +#include +#include +#include +#include + +static jmp_buf panic_jmp; +static int panic_count = 0; + +/* Satisfies __attribute__((noreturn)) via longjmp; allows test to continue + * after a guarded panic call without executing the code that follows it. */ +__attribute__((noreturn)) void panic(void) +{ + panic_count++; + longjmp(panic_jmp, 1); +} + +/* paging.c includes which maps wolfBoot_printf to fprintf(stderr, + * ...) on Linux, so no extra stub is needed. The cr3-reading static function + * x86_paging_get_paget_table_root() is compiled but never called here. */ +#include "../../src/x86/paging.c" + +static void reset_pool(void) +{ + page_table_page_used = 0; + panic_count = 0; + memset(page_table_pages, 0, sizeof(page_table_pages)); +} + +/* Verify that x86_paging_setup_ptp triggers panic for ANY call when the pool + * is full (page_table_page_used >= WOLFBOOT_PTP_NUM), not only the first. */ +START_TEST(test_ptp_guard_triggers_on_full_pool) +{ + uint64_t e = 0; + + reset_pool(); + page_table_page_used = WOLFBOOT_PTP_NUM; + + /* An over-limit call must trigger panic (longjmp). */ + if (setjmp(panic_jmp) == 0) { + x86_paging_setup_ptp(&e); + /* Reaching here means the guard was bypassed — the bug is present. */ + ck_abort_msg("panic not triggered for over-limit allocation (== guard bypassed)"); + } + ck_assert_int_eq(panic_count, 1); + + /* page_table_page_used must not have advanced past WOLFBOOT_PTP_NUM; + * with the fix the counter is checked before being incremented. */ + ck_assert_int_le(page_table_page_used, WOLFBOOT_PTP_NUM); + + /* A second over-limit call must also trigger panic (>= guard). */ + if (setjmp(panic_jmp) == 0) { + x86_paging_setup_ptp(&e); + ck_abort_msg("panic not triggered on second over-limit allocation"); + } + ck_assert_int_eq(panic_count, 2); +} +END_TEST + +static Suite *paging_oob_suite(void) +{ + Suite *s = suite_create("x86_paging_oob"); + TCase *tc = tcase_create("setup_ptp_guard"); + tcase_add_test(tc, test_ptp_guard_triggers_on_full_pool); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite *s = paging_oob_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + int failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +}