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.
pull/792/head
Daniele Lacamera 2026-06-10 14:39:25 +02:00
parent 712f38988f
commit 286581cf18
5 changed files with 98 additions and 6 deletions

View File

@ -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);

View File

@ -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();
}
}
/**

View File

@ -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);
}

View File

@ -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)

View File

@ -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 <check.h>
#include <stdint.h>
#include <string.h>
#include <setjmp.h>
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 <printf.h> 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;
}