From 5c22c271e808ac9a98e41d5a0e5fce89d19e3264 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 18:02:08 +0200 Subject: [PATCH 01/18] F-12061: RP2350: RMW partial pages in hal_flash_write flash_range_program() requires a page-aligned address and a page-multiple length (pico-sdk ROM, invalid_params_if on both). The partition-state path without NVM_FLASH_WRITEONCE issues 1-byte (trailer) and 4-byte (magic) writes that violated the contract on every state transition. Keep the direct-program fast path for page-aligned page-multiple writes; otherwise read the page back from XIP, merge the write, and program the full page. The AND program keeps the trailer flag accumulation intact. Add unit-rp2350-flash-write: runs the extracted hal_flash_write against a mock flash_range_program() that enforces the ROM contract (4/6 checks fail pre-fix, 6/6 pass post-fix). --- hal/rp2350.c | 59 ++++- tools/unit-tests/Makefile | 14 +- tools/unit-tests/unit-rp2350-flash-write.c | 277 +++++++++++++++++++++ 3 files changed, 338 insertions(+), 12 deletions(-) create mode 100644 tools/unit-tests/unit-rp2350-flash-write.c diff --git a/hal/rp2350.c b/hal/rp2350.c index 85b1f06832..f67d104f08 100644 --- a/hal/rp2350.c +++ b/hal/rp2350.c @@ -225,20 +225,57 @@ void hal_prepare_boot(void) int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) { uint8_t cache[WOLFBOOT_SECTOR_SIZE]; + uint32_t flash_addr = address - XIP_BASE; uint32_t written = 0; uint32_t sz; - if (((uintptr_t)data & 0x20000000UL) == 0) { - /* Not in RAM: copy to cache before writing */ - while (written < len) { - sz = WOLFBOOT_SECTOR_SIZE; - if (sz > (len - written)) - sz = len - written; - memcpy(cache, data + written, sz); - flash_range_program(address - XIP_BASE + written, cache, sz); - written += sz; + uint32_t addr; + uint32_t page_off; + uint32_t page_addr; + uint32_t remaining; + + if (len > 0) { + if ((flash_addr & (FLASH_PAGE_SIZE - 1)) == 0 && + ((uint32_t)len & (FLASH_PAGE_SIZE - 1)) == 0) { + /* Page aligned start, page multiple length: program + * directly. */ + if (((uintptr_t)data & 0x20000000UL) == 0) { + /* Not in RAM: copy to cache before writing, XIP is + * disabled while the flash is programmed. */ + while (written < (uint32_t)len) { + sz = WOLFBOOT_SECTOR_SIZE; + if (sz > (uint32_t)len - written) + sz = (uint32_t)len - written; + memcpy(cache, data + written, sz); + flash_range_program(flash_addr + written, cache, sz); + written += sz; + } + } else { + flash_range_program(flash_addr, data, len); + } + } else { + /* Partial page at the start and/or end: read the page + * back from XIP, merge in the write, program the whole + * page. flash_range_program() only accepts page aligned + * addresses and page multiple lengths. The AND program + * keeps the trailer flag accumulation intact. */ + while (written < (uint32_t)len) { + addr = flash_addr + written; + page_off = addr & (FLASH_PAGE_SIZE - 1); + page_addr = addr & ~(FLASH_PAGE_SIZE - 1); + remaining = (uint32_t)len - written; + + sz = FLASH_PAGE_SIZE - page_off; + if (sz > remaining) + sz = remaining; + + memcpy(cache, (const uint8_t *)(XIP_BASE + page_addr), + FLASH_PAGE_SIZE); + memcpy(cache + page_off, data + written, sz); + flash_range_program(page_addr, cache, FLASH_PAGE_SIZE); + written += sz; + } } - } else - flash_range_program(address - XIP_BASE, data, len); + } return 0; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index fac6b42881..61fe0f7995 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -117,6 +117,7 @@ TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write +TESTS+=unit-rp2350-flash-write TESTS+=unit-fwtpm-rsp-overrun TESTS+=unit-fwtpm-cmd-toctou TESTS+=unit-fdt-memrsv-wrap @@ -1230,6 +1231,16 @@ hifive1_flash_write_extract.h: ../../hal/hifive1.c unit-hifive1-flash-write: unit-hifive1-flash-write.c hifive1_flash_write_extract.h gcc -o $@ unit-hifive1-flash-write.c $(CFLAGS) $(LDFLAGS) +# unit-rp2350-flash-write runs the real hal_flash_write() from +# hal/rp2350.c against a mock flash_range_program() that enforces the +# ROM contract (F-12061: unaligned / non page-multiple writes from +# the partition-state path violate the 256-byte page requirement). +rp2350_flash_write_extract.h: ../../hal/rp2350.c + sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< > $@ + +unit-rp2350-flash-write: unit-rp2350-flash-write.c rp2350_flash_write_extract.h + gcc -o $@ unit-rp2350-flash-write.c $(CFLAGS) $(LDFLAGS) + # unit-ecc-raw-der runs the real wolfCrypt raw-to-DER conversion and # verification (F-11024: the wolfHSM verify path in src/image.c passed # minimal field sizes with field-start pointers to @@ -1464,7 +1475,8 @@ covclean: GENERATED_SRC:=aurix_erased_extract.h \ hifive1_flash_write_extract.h nvm_cache_scrub_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ - p1021_erase_extract.h p1021_erase_fn_extract.h sdhci_host.c \ + p1021_erase_extract.h p1021_erase_fn_extract.h rp2350_flash_write_extract.h \ + sdhci_host.c \ stm32g4_write_extract.h stm32l5_write_extract.h \ stm32u5_write_extract.h \ t10xx_flash_status_extract.h t10xx_qe_firmware_extract.h \ diff --git a/tools/unit-tests/unit-rp2350-flash-write.c b/tools/unit-tests/unit-rp2350-flash-write.c new file mode 100644 index 0000000000..752d8112e2 --- /dev/null +++ b/tools/unit-tests/unit-rp2350-flash-write.c @@ -0,0 +1,277 @@ +/* unit-rp2350-flash-write.c + * + * Regression test for F-12061: hal_flash_write() in hal/rp2350.c + * chunked the write by WOLFBOOT_SECTOR_SIZE (8 KiB) and passed the + * resulting sizes straight to pico-sdk flash_range_program(), which + * requires a 256-byte page aligned address and a page multiple + * length (invalid_params_if on both, no partial-page support). The + * partition-state path without NVM_FLASH_WRITEONCE violates the + * contract on every state transition: trailer_write() issues a + * 1-byte write and partition_magic_write() a 4-byte write. + * + * The real function is extracted by the Makefile and run against a + * mock flash_range_program() that enforces the ROM contract (any + * unaligned or non page-multiple call is recorded as a violation) + * and programs with flash AND semantics. The flash image is mmap'd + * at XIP_BASE so the read-modify-write page reads hit real memory, + * exactly as the XIP mapping would on hardware. + * + * 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 + +/* Host stand-in for the ARM build attribute. */ +#define RAMFUNCTION + +/* RP2350 XIP flash base (pico-sdk hardware/regs/addressmap.h). */ +#define XIP_BASE 0x10000000UL +#define FLASH_PAGE_SIZE 256 +/* Sector size from config/examples/rp2350.config. */ +#define WOLFBOOT_SECTOR_SIZE 0x2000 + +#define FLASH_MEM_SZ (4 * FLASH_PAGE_SIZE) +static uint8_t *g_flash; /* mmap'd at XIP_BASE */ + +/* "SRAM" source buffer: bit 29 set, so the HAL takes the in-RAM + * direct-program branch, the same branch a stack/.bss pointer takes + * on the real target (SRAM at 0x20000000). */ +#define SRAM_ADDR 0x21000000UL +#define SRAM_SZ 1024 +static uint8_t *g_sram; + +/* Contract violations recorded by the mock ROM call. */ +static int g_violations; + +void flash_range_program(uint32_t flash_offs, const uint8_t *data, + size_t count) +{ + size_t i; + + if ((flash_offs & (FLASH_PAGE_SIZE - 1)) || + (count & (FLASH_PAGE_SIZE - 1)) || + (flash_offs + count > FLASH_MEM_SZ)) { + g_violations++; + return; + } + /* Flash can only clear bits: programmed = old & new. */ + for (i = 0; i < count; i++) + g_flash[flash_offs + i] &= data[i]; +} + +/* The real hal_flash_write() from hal/rp2350.c (extracted). */ +#include "rp2350_flash_write_extract.h" + +static void setup(void) +{ + memset(g_flash, 0xFF, FLASH_MEM_SZ); /* erased */ + memset(g_sram, 0x5A, SRAM_SZ); + g_violations = 0; +} + +static void teardown(void) +{ +} + +/* The exact partition-state write: trailer_write() without +* NVM_FLASH_WRITEONCE compiles to hal_flash_write(addr, &val, 1). +* Pre-fix the 1-byte length is passed to flash_range_program() and +* violates the ROM contract; post-fix the page is RMW'd. */ +START_TEST(test_one_byte_trailer_write){ + uint8_t val = 0x7E; + int i; + + /* Seed the page with an existing pattern; only the write offset + * is erased, so the programmed byte must land as requested while + * every other byte of the page is preserved. */ + for (i = 0; i < FLASH_PAGE_SIZE; i++) + g_flash[FLASH_PAGE_SIZE + i] = (i == 100) ? 0xFF : + (uint8_t)(0xA0 ^ (i & 0x0F)); + g_sram[0] = val; + + ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + FLASH_PAGE_SIZE + + 100), g_sram, 1), 0); + ck_assert_int_eq(g_violations, 0); + ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + 100], val); + for (i = 0; i < FLASH_PAGE_SIZE; i++) { + if (i != 100) + ck_assert_uint_eq(g_flash[FLASH_PAGE_SIZE + i], + (uint8_t)(0xA0 ^ (i & 0x0F))); + } +} +END_TEST + +/* partition_magic_write() compiles to a 4-byte write of the magic + * trailer. Same contract violation pre-fix. */ +START_TEST(test_four_byte_magic_write) +{ + uint32_t magic = 0x600DF00D; + uint8_t *m = (uint8_t *)&magic; + int i; + + for (i = 0; i < FLASH_PAGE_SIZE; i++) + g_flash[i] = (uint8_t)(0x0F ^ (i & 0x70)); + /* The magic region is erased, as on a fresh partition tail. */ + g_flash[200] = g_flash[201] = g_flash[202] = g_flash[203] = 0xFF; + memcpy(g_sram, m, 4); + + ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + 200), g_sram, 4), + 0); + ck_assert_int_eq(g_violations, 0); + ck_assert_uint_eq(*(uint32_t *)(g_flash + 200), magic); + for (i = 0; i < FLASH_PAGE_SIZE; i++) { + if (i >= 200 && i <= 203) + continue; + ck_assert_uint_eq(g_flash[i], (uint8_t)(0x0F ^ (i & 0x70))); + } +} +END_TEST + +/* An unaligned write spanning three pages: pre-fix the whole 600 + * bytes go to flash_range_program() in one non-page-multiple call; + * post-fix each page is RMW'd and the bytes outside the request are + * preserved. */ +START_TEST(test_unaligned_multi_page_write) +{ + int i; + int ret; + + for (i = 0; i < SRAM_SZ; i++) + g_sram[i] = (uint8_t)(0x30 + (i & 0x0F)); + + ret = hal_flash_write((uint32_t)(XIP_BASE + 100), g_sram, 600); + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(g_violations, 0); + ck_assert_mem_eq(g_flash + 100, g_sram, 600); + for (i = 0; i < 100; i++) + ck_assert_uint_eq(g_flash[i], 0xFF); + for (i = 700; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash[i], 0xFF); +} +END_TEST + +/* Page aligned start and page multiple length: the fast path. Data + * in RAM is programmed directly, unchanged by the fix. */ +START_TEST(test_aligned_page_multiple_write_sram) +{ + int i; + + for (i = 0; i < 512; i++) + g_sram[i] = (uint8_t)(0xC0 ^ (i & 0x1F)); + + ck_assert_int_eq(hal_flash_write((uint32_t)XIP_BASE, g_sram, 512), 0); + ck_assert_int_eq(g_violations, 0); + ck_assert_mem_eq(g_flash, g_sram, 512); + for (i = 512; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash[i], 0xFF); +} +END_TEST + +/* Fast path with the source in XIP flash: the data must be staged + * to RAM before programming (XIP is disabled while a page is + * programmed). */ +START_TEST(test_aligned_page_multiple_write_xip) +{ + int i; + + for (i = 0; i < 512; i++) + g_flash[3 * FLASH_PAGE_SIZE + i] = (uint8_t)(0x80 ^ (i & 0x3F)); + + ck_assert_int_eq(hal_flash_write((uint32_t)XIP_BASE, + (uint8_t *)(XIP_BASE + + 3 * FLASH_PAGE_SIZE), + 512), 0); + ck_assert_int_eq(g_violations, 0); + for (i = 0; i < 512; i++) + ck_assert_uint_eq(g_flash[i], (uint8_t)(0x80 ^ (i & 0x3F))); +} +END_TEST + +/* Aligned start, non page-multiple length, source in XIP: pre-fix + * the 300-byte tail is passed to flash_range_program() as-is; + * post-fix the last partial page is RMW'd. Source (pages 0-1) and + * destination (pages 2-3) do not overlap. */ +START_TEST(test_xip_partial_write) +{ + int i; + + for (i = 0; i < 300; i++) + g_flash[i] = (uint8_t)(0x40 + (i & 0x07)); + + ck_assert_int_eq(hal_flash_write((uint32_t)(XIP_BASE + 2 * FLASH_PAGE_SIZE), + (uint8_t *)XIP_BASE, + 300), 0); + ck_assert_int_eq(g_violations, 0); + ck_assert_mem_eq(g_flash + 2 * FLASH_PAGE_SIZE, g_flash, 300); + /* Source region untouched. */ + for (i = 0; i < 300; i++) + ck_assert_uint_eq(g_flash[i], (uint8_t)(0x40 + (i & 0x07))); + for (i = 300; i < 2 * FLASH_PAGE_SIZE; i++) + ck_assert_uint_eq(g_flash[i], 0xFF); + for (i = 2 * FLASH_PAGE_SIZE + 300; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash[i], 0xFF); +} +END_TEST + +Suite *rp2350_flash_write_suite(void) +{ + Suite *s = suite_create("rp2350 flash write"); + TCase *tc = tcase_create("page-contract"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_one_byte_trailer_write); + tcase_add_test(tc, test_four_byte_magic_write); + tcase_add_test(tc, test_unaligned_multi_page_write); + tcase_add_test(tc, test_aligned_page_multiple_write_sram); + tcase_add_test(tc, test_aligned_page_multiple_write_xip); + tcase_add_test(tc, test_xip_partial_write); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = rp2350_flash_write_suite(); + SRunner *sr = srunner_create(s); + + g_flash = mmap((void *)XIP_BASE, FLASH_MEM_SZ, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + if (g_flash == MAP_FAILED) + return 99; + g_sram = mmap((void *)SRAM_ADDR, SRAM_SZ, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + if (g_sram == MAP_FAILED) + return 99; + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + munmap(g_flash, FLASH_MEM_SZ); + munmap(g_sram, SRAM_SZ); + + return fails; +} From 40d448b771c0a6a90d9702e02bda7afa41c7fcba Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 18:17:02 +0200 Subject: [PATCH 02/18] F-12062: STM32L4: require a full double word in the fast write path The double-word fast path of hal_flash_write() was selected on "len - i > 3" but always reads and programs two 32-bit words, so an aligned 4-7 byte tail read up to four bytes past the caller's buffer and programmed them into flash. Require at least eight remaining bytes before taking the fast path; shorter tails fall to the RMW branch, which rewrites the unit with the out-of-range bytes read back from flash. Same fix as the STM32G4 twin (F-11023). Add unit-stm32l4-write: runs the extracted hal_flash_write() against a host register/flash model with a canary after the source (3/5 checks fail pre-fix, 5/5 pass post-fix). --- hal/stm32l4.c | 3 +- tools/unit-tests/Makefile | 16 +- tools/unit-tests/unit-stm32l4-write.c | 237 ++++++++++++++++++++++++++ 3 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-stm32l4-write.c diff --git a/hal/stm32l4.c b/hal/stm32l4.c index 999e1473fe..84bcc85842 100644 --- a/hal/stm32l4.c +++ b/hal/stm32l4.c @@ -133,7 +133,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { flash_clear_errors(); - if ((len - i > 3) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { + if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && + ((((uint32_t)data) + i) & 0x07) == 0)) { uint32_t idx = i >> 2; src = (uint32_t *)data; dst = (uint32_t *)(address); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 61fe0f7995..287f9374dc 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -126,6 +126,7 @@ TESTS+=unit-aurix-erased-fill-invert TESTS+=unit-t2080-fman-loader TESTS+=unit-ecc-raw-der TESTS+=unit-stm32g4-write +TESTS+=unit-stm32l4-write TESTS+=unit-stm32l5-write TESTS+=unit-stm32u5-write TESTS+=unit-nvm-cache-scrub @@ -1159,6 +1160,19 @@ stm32g4_write_extract.h: ../../hal/stm32g4.c unit-stm32g4-write: unit-stm32g4-write.c stm32g4_write_extract.h gcc -o $@ unit-stm32g4-write.c $(CFLAGS) $(LDFLAGS) +# unit-stm32l4-write runs the real hal_flash_write() from hal/stm32l4.c +# (F-12062: the double-word fast path was selected on "len - i > 3" but +# consumed eight bytes, so an aligned 4-7 byte tail over-read the +# caller's buffer and over-programmed flash). Same harness as the +# STM32G4 twin (F-11023); the l4 helpers are static and un-prefixed. +stm32l4_write_extract.h: ../../hal/stm32l4.c + sed -n '/^static RAMFUNCTION void flash_wait_complete/,/^}/p' $< > $@ + sed -n '/^static void RAMFUNCTION flash_clear_errors/,/^}/p' $< >> $@ + sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ + +unit-stm32l4-write: unit-stm32l4-write.c stm32l4_write_extract.h + gcc -o $@ unit-stm32l4-write.c $(CFLAGS) $(LDFLAGS) + # unit-t10xx-flash-status runs the real hal_flash_write()/hal_flash_erase() # and hal_flash_status_wait() from hal/nxp_t10xx.c against a mock QPI # status model (F-11033: a timed-out program/erase used to report @@ -1477,7 +1491,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ p1021_erase_extract.h p1021_erase_fn_extract.h rp2350_flash_write_extract.h \ sdhci_host.c \ - stm32g4_write_extract.h stm32l5_write_extract.h \ + stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ stm32u5_write_extract.h \ t10xx_flash_status_extract.h t10xx_qe_firmware_extract.h \ t2080_fman_extract.h \ diff --git a/tools/unit-tests/unit-stm32l4-write.c b/tools/unit-tests/unit-stm32l4-write.c new file mode 100644 index 0000000000..0e4ce13174 --- /dev/null +++ b/tools/unit-tests/unit-stm32l4-write.c @@ -0,0 +1,237 @@ +/* unit-stm32l4-write.c + * + * Regression test for F-12062: the double-word fast path of + * hal_flash_write() in hal/stm32l4.c was selected on "len - i > 3" + * but always reads and programs two 32-bit words (eight bytes), so + * an aligned 4-7 byte tail read up to four bytes past the caller's + * buffer and programmed them into flash. The fix requires at least + * eight remaining bytes before taking the fast path; shorter tails + * fall to the RMW branch, which rewrites the unit with the + * out-of-range bytes read back from flash. + * + * Same harness as the STM32G4 twin (F-11023): extracted functions, + * registers on a host file, stale destination flash, canary after + * the source. The source buffer is 8-byte aligned so the fast-path + * alignment test on the data pointer can pass. The L4 HAL clears the + * status error bits with a write-1-to-clear store; on the host the + * clear is a no-op, which models a program that raises no errors. + * 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 + +/* Host stand-in for the ARM build attribute. */ +#define RAMFUNCTION + +/* Host FLASH register file. SR/CR bit values per RM0394; the L4 + * flash controller has the same status/control layout as the G4. */ +typedef struct flash_reg { + volatile uint32_t SR; + volatile uint32_t CR; +} flash_reg_t; + +static flash_reg_t g_flash_regs; +#define FLASH (&g_flash_regs) + +#define FLASH_SR_EOP (1u << 0) +#define FLASH_SR_PROGERR (1u << 3) +#define FLASH_SR_BSY (1u << 16) +#define FLASH_CR_PG (1u << 0) +#define FLASH_CR_FSTPG (1u << 4) + +/* W1C clear: no-op on the host, models a program with no errors. */ +#define __HAL_FLASH_CLEAR_FLAG(flags) ((void)0) + +/* Destination flash: pre-filled with stale data (rewrite scenario). + * hal_flash_write() takes the address as uint32_t (32-bit MCU), so + * on the 64-bit host the flash must live at an address that fits in + * 32 bits: map it at a fixed low location. */ +#define FLASH_MEM_SZ 256 +#define FLASH_MEM_ADDR 0x10000000UL +static uint8_t *g_flash_mem; + +/* Source buffer followed by a canary: a pre-fix short write reads + * the canary and lands it in the destination flash. */ +#define DATA_SZ 64 +#define CANARY_SZ 32 +static uint8_t g_data[DATA_SZ + CANARY_SZ] __attribute__((aligned(8))); +#define g_canary (g_data + DATA_SZ) + +/* The real functions from hal/stm32l4.c (extracted by the Makefile). */ +#include "stm32l4_write_extract.h" + +static void setup(void) +{ + int i; + + memset(&g_flash_regs, 0, sizeof(g_flash_regs)); + for (i = 0; i < FLASH_MEM_SZ; i++) + g_flash_mem[i] = 0x12; /* stale */ + for (i = 0; i < DATA_SZ; i++) + g_data[i] = (uint8_t)(0x30 + i); + /* 0x70..0x8F: distinct from the data bytes (0x30..0x6F), the + * stale flash fill (0x12) and the erased-value padding (0xFF), + * so a canary hit means source bytes past len were really read. */ + for (i = 0; i < CANARY_SZ; i++) + g_canary[i] = (uint8_t)(0x70 + i); +} + +static void teardown(void) +{ +} + +static int canary_in_flash(void) +{ + int i; + + for (i = 0; i < CANARY_SZ; i++) + if (memchr(g_flash_mem, g_canary[i], FLASH_MEM_SZ) != NULL) + return 1; + return 0; +} + +/* A write of 60 bytes: seven full double words, then a 4-byte tail. + * Pre-fix the tail took the fast path and programmed bytes 60..63 + * from source bytes past len. Post-fix the tail is RMW'd and nothing + * past len is read or written. */ +START_TEST(test_write_60_no_overread){ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 60), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 60), 0); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 63 bytes: the longest 4-7 byte tail (7). Pre-fix the + * fast path over-reads data[60..63] and programs byte 63, which the + * request does not cover. */ +START_TEST(test_write_63_max_tail) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 63), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 63), 0); + /* byte 63 keeps its flash content */ + ck_assert_uint_eq(g_flash_mem[63], 0x12); + for (i = 64; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A 4-byte aligned write: the partition magic write shape. Pre-fix + * the whole request took the fast path and programmed four canary + * bytes after the magic. */ +START_TEST(test_write_4_magic) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 4), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 4), 0); + /* bytes 4..7 keep their flash content */ + for (i = 4; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 56 bytes, a multiple of 8: the fast path is taken for + * every unit and behaves exactly as before the fix. */ +START_TEST(test_write_56_full_units) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 56), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 56), 0); + for (i = 56; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); +} +END_TEST + +/* A write of 58 bytes: the final unit is partial (bytes 58,59 are + * outside the request); they are read back from flash and rewritten + * unchanged, and nothing past len is read. */ +START_TEST(test_write_58_partial_word_padded) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 58), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 58), 0); + /* word 14 (bytes 56..59): 58,59 keep their flash content */ + ck_assert_uint_eq(g_flash_mem[58], 0x12); + ck_assert_uint_eq(g_flash_mem[59], 0x12); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +Suite *stm32l4_write_suite(void) +{ + Suite *s = suite_create("stm32l4-write"); + TCase *tc = tcase_create("stm32l4-write"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_write_60_no_overread); + tcase_add_test(tc, test_write_63_max_tail); + tcase_add_test(tc, test_write_4_magic); + tcase_add_test(tc, test_write_56_full_units); + tcase_add_test(tc, test_write_58_partial_word_padded); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = stm32l4_write_suite(); + SRunner *sr = srunner_create(s); + + g_flash_mem = mmap((void *)FLASH_MEM_ADDR, FLASH_MEM_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | + MAP_FIXED, + -1, 0); + if (g_flash_mem == MAP_FAILED) + return 99; + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + munmap(g_flash_mem, FLASH_MEM_SZ); + + return fails; +} From 6792b5da0190a56c1498b7871f7d556adf848c61 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 18:26:44 +0200 Subject: [PATCH 03/18] F-12063: STM32WB: require a full double word in the fast write path The double-word fast path was selected on 'len - i > 3' but always reads and programs two 32-bit words, so an aligned 4-7 byte tail read up to four bytes past the caller's buffer and programmed them into flash. Require at least eight remaining bytes; shorter tails fall to the RMW branch, which rewrites the unit with the out-of-range bytes read back from flash. Add unit-stm32wb-write: runs the extracted hal_flash_write() against a host register file with stale destination flash and a source canary (3/5 checks fail pre-fix, 5/5 pass post-fix). --- hal/stm32wb.c | 3 +- tools/unit-tests/Makefile | 16 +- tools/unit-tests/unit-stm32wb-write.c | 238 ++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-stm32wb-write.c diff --git a/hal/stm32wb.c b/hal/stm32wb.c index b17e2bfaa0..6e41a6760f 100644 --- a/hal/stm32wb.c +++ b/hal/stm32wb.c @@ -188,7 +188,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { flash_clear_errors(); - if ((len - i > 3) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { + if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && + ((((uint32_t)data) + i) & 0x07) == 0)) { uint32_t idx = i >> 2; src = (uint32_t *)data; dst = (uint32_t *)(address); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 287f9374dc..7c854ba8eb 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -127,6 +127,7 @@ TESTS+=unit-t2080-fman-loader TESTS+=unit-ecc-raw-der TESTS+=unit-stm32g4-write TESTS+=unit-stm32l4-write +TESTS+=unit-stm32wb-write TESTS+=unit-stm32l5-write TESTS+=unit-stm32u5-write TESTS+=unit-nvm-cache-scrub @@ -1173,6 +1174,19 @@ stm32l4_write_extract.h: ../../hal/stm32l4.c unit-stm32l4-write: unit-stm32l4-write.c stm32l4_write_extract.h gcc -o $@ unit-stm32l4-write.c $(CFLAGS) $(LDFLAGS) +# unit-stm32wb-write runs the real hal_flash_write() from hal/stm32wb.c +# (F-12063: the double-word fast path was selected on "len - i > 3" but +# consumed eight bytes, so an aligned 4-7 byte tail over-read the +# caller's buffer and over-programmed flash). Same harness as the +# STM32G4/STM32L4 twins; the wb helpers are static and un-prefixed. +stm32wb_write_extract.h: ../../hal/stm32wb.c + sed -n '/^static RAMFUNCTION void flash_wait_complete/,/^}/p' $< > $@ + sed -n '/^static void RAMFUNCTION flash_clear_errors/,/^}/p' $< >> $@ + sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ + +unit-stm32wb-write: unit-stm32wb-write.c stm32wb_write_extract.h + gcc -o $@ unit-stm32wb-write.c $(CFLAGS) $(LDFLAGS) + # unit-t10xx-flash-status runs the real hal_flash_write()/hal_flash_erase() # and hal_flash_status_wait() from hal/nxp_t10xx.c against a mock QPI # status model (F-11033: a timed-out program/erase used to report @@ -1492,7 +1506,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ p1021_erase_extract.h p1021_erase_fn_extract.h rp2350_flash_write_extract.h \ sdhci_host.c \ stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ - stm32u5_write_extract.h \ + stm32u5_write_extract.h stm32wb_write_extract.h \ t10xx_flash_status_extract.h t10xx_qe_firmware_extract.h \ t2080_fman_extract.h \ ti_hercules_write_extract.h versal_ext_write_extract.h versal_host.c \ diff --git a/tools/unit-tests/unit-stm32wb-write.c b/tools/unit-tests/unit-stm32wb-write.c new file mode 100644 index 0000000000..dd58f22d8b --- /dev/null +++ b/tools/unit-tests/unit-stm32wb-write.c @@ -0,0 +1,238 @@ +/* unit-stm32wb-write.c + * + * Regression test for F-12063: the double-word fast path of + * hal_flash_write() in hal/stm32wb.c was selected on "len - i > 3" + * but always reads and programs two 32-bit words (eight bytes), so + * an aligned 4-7 byte tail read up to four bytes past the caller's + * buffer and programmed them into flash. The fix requires at least + * eight remaining bytes before taking the fast path; shorter tails + * fall to the RMW branch, which rewrites the unit with the + * out-of-range bytes read back from flash. + * + * Same harness as the STM32G4 (F-11023) and STM32L4 (F-12062) + * twins: extracted functions, registers on a host file, stale + * destination flash, canary after the source. The source buffer is + * 8-byte aligned so the fast-path alignment test on the data + * pointer can pass. The WB HAL uses bare FLASH_SR/FLASH_CR macros + * like the G4; the error clear is a write-1-to-clear store that + * leaves no error visible on the host, modeling a program that + * raises no errors. + * 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 + +/* Host stand-in for the ARM build attribute. */ +#define RAMFUNCTION + +/* Host FLASH register file (offsets as in the WB CMSIS header). The + * WB flash controller has the same status/control layout as the G4 + * and L4. */ +static uint32_t g_flash_regs[0x20 / sizeof(uint32_t)]; +#define FLASH_BASE ((uintptr_t)g_flash_regs) +#define FLASH_SR (*(volatile uint32_t *)(FLASH_BASE + 0x10)) +#define FLASH_CR (*(volatile uint32_t *)(FLASH_BASE + 0x14)) + +#define FLASH_SR_EOP (1u << 0) +#define FLASH_SR_PROGERR (1u << 3) +#define FLASH_SR_WRPERR (1u << 4) +#define FLASH_SR_PGAERR (1u << 5) +#define FLASH_SR_SIZERR (1u << 6) +#define FLASH_SR_BSY (1u << 16) +#define FLASH_SR_CFGBSY (1u << 18) +#define FLASH_CR_PG (1u << 0) +#define FLASH_CR_FSTPG (1u << 4) + +/* Destination flash: pre-filled with stale data (rewrite scenario). + * hal_flash_write() takes the address as uint32_t (32-bit MCU), so + * on the 64-bit host the flash must live at an address that fits in + * 32 bits: map it at a fixed low location. */ +#define FLASH_MEM_SZ 256 +#define FLASH_MEM_ADDR 0x10000000UL +static uint8_t *g_flash_mem; + +/* Source buffer followed by a canary: a pre-fix short write reads + * the canary and lands it in the destination flash. */ +#define DATA_SZ 64 +#define CANARY_SZ 32 +static uint8_t g_data[DATA_SZ + CANARY_SZ] __attribute__((aligned(8))); +#define g_canary (g_data + DATA_SZ) + +/* The real functions from hal/stm32wb.c (extracted by the Makefile). */ +#include "stm32wb_write_extract.h" + +static void setup(void) +{ + int i; + + memset(g_flash_regs, 0, sizeof(g_flash_regs)); + for (i = 0; i < FLASH_MEM_SZ; i++) + g_flash_mem[i] = 0x12; /* stale */ + for (i = 0; i < DATA_SZ; i++) + g_data[i] = (uint8_t)(0x30 + i); + /* 0x70..0x8F: distinct from the data bytes (0x30..0x6F), the + * stale flash fill (0x12) and the erased-value padding (0xFF), + * so a canary hit means source bytes past len were really read. */ + for (i = 0; i < CANARY_SZ; i++) + g_canary[i] = (uint8_t)(0x70 + i); +} + +static void teardown(void) +{ +} + +static int canary_in_flash(void) +{ + int i; + + for (i = 0; i < CANARY_SZ; i++) + if (memchr(g_flash_mem, g_canary[i], FLASH_MEM_SZ) != NULL) + return 1; + return 0; +} + +/* A write of 60 bytes: seven full double words, then a 4-byte tail. + * Pre-fix the tail took the fast path and programmed bytes 60..63 + * from source bytes past len. Post-fix the tail is RMW'd and nothing + * past len is read or written. */ +START_TEST(test_write_60_no_overread){ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 60), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 60), 0); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 63 bytes: the longest 4-7 byte tail (7). Pre-fix the + * fast path over-reads data[60..63] and programs byte 63, which the + * request does not cover. */ +START_TEST(test_write_63_max_tail) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 63), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 63), 0); + /* byte 63 keeps its flash content */ + ck_assert_uint_eq(g_flash_mem[63], 0x12); + for (i = 64; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A 4-byte aligned write: the partition magic write shape. Pre-fix + * the whole request took the fast path and programmed four canary + * bytes after the magic. */ +START_TEST(test_write_4_magic) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 4), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 4), 0); + /* bytes 4..7 keep their flash content */ + for (i = 4; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write of 56 bytes, a multiple of 8: the fast path is taken for + * every unit and behaves exactly as before the fix. */ +START_TEST(test_write_56_full_units) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 56), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 56), 0); + for (i = 56; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); +} +END_TEST + +/* A write of 58 bytes: the final unit is partial (bytes 58,59 are + * outside the request); they are read back from flash and rewritten + * unchanged, and nothing past len is read. */ +START_TEST(test_write_58_partial_word_padded) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 58), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 58), 0); + /* word 14 (bytes 56..59): 58,59 keep their flash content */ + ck_assert_uint_eq(g_flash_mem[58], 0x12); + ck_assert_uint_eq(g_flash_mem[59], 0x12); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +Suite *stm32wb_write_suite(void) +{ + Suite *s = suite_create("stm32wb-write"); + TCase *tc = tcase_create("stm32wb-write"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_write_60_no_overread); + tcase_add_test(tc, test_write_63_max_tail); + tcase_add_test(tc, test_write_4_magic); + tcase_add_test(tc, test_write_56_full_units); + tcase_add_test(tc, test_write_58_partial_word_padded); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = stm32wb_write_suite(); + SRunner *sr = srunner_create(s); + + g_flash_mem = mmap((void *)FLASH_MEM_ADDR, FLASH_MEM_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | + MAP_FIXED, + -1, 0); + if (g_flash_mem == MAP_FAILED) + return 99; + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + munmap(g_flash_mem, FLASH_MEM_SZ); + + return fails; +} From 20c9031965a0db1706dc9b77044417d7d1906d8b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 18:46:18 +0200 Subject: [PATCH 04/18] F-12064: P1021: check bad-block markers per erase block ext_flash_read() initialized its bad-block page counter once per request, so the marker was inspected only on the first two pages read and a bad erase block later in the request was delivered as valid data. Restart the counter at the start of each erase block. The skip path also rewound the logical position to a block boundary without rewinding the output pointer, so a marker found after some pages had been delivered continued the read past the end of the caller's buffer. pos and data already agree (data = original + pos) after any delivered pages, so the skip only advances the source address. Add unit-p1021-read-badblock: runs the extracted ext_flash_read() against a mocked ELBC on a simulated NAND with three 16 KiB blocks (2/5 checks fail pre-fix, 5/5 pass post-fix). --- hal/nxp_p1021.c | 10 +- tools/unit-tests/Makefile | 35 ++- tools/unit-tests/unit-p1021-read-badblock.c | 275 ++++++++++++++++++++ 3 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 tools/unit-tests/unit-p1021-read-badblock.c diff --git a/hal/nxp_p1021.c b/hal/nxp_p1021.c index 763f14761a..b0cf9036a2 100644 --- a/hal/nxp_p1021.c +++ b/hal/nxp_p1021.c @@ -1738,6 +1738,10 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) /* total download loop */ while (pos < len) { + /* the bad-block marker only exists on the first pages of each + * erase block: restart the per-block page counter */ + i = 0; + /* block loop */ do { /* Calculate page address */ @@ -1765,9 +1769,11 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) /* check for bad page. if either of the first two pages are bad then * skip to next block */ if (i++ < 2 && flash_buf[bad_marker] != 0xFF) { - /* skip block - advance address by block and restart position */ + /* skip block: the bad block's bytes are not delivered + * and the read continues at the next block. pos and + * data already agree (data = original + pos), so only + * the source address moves. */ address = (address + block_size) & ~(block_size - 1); - pos &= ~(block_size - 1); break; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 7c854ba8eb..b3fd5b0742 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -115,6 +115,7 @@ TESTS+=unit-versal-ext-write TESTS+=unit-t10xx-qe-firmware TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance +TESTS+=unit-p1021-read-badblock TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-rp2350-flash-write @@ -1228,6 +1229,36 @@ unit-p1021-erase-advance: unit-p1021-erase-advance.c p1021_erase_extract.h \ p1021_erase_fn_extract.h gcc -o $@ unit-p1021-erase-advance.c $(CFLAGS) $(LDFLAGS) +# unit-p1021-read-badblock runs the real ext_flash_read() from +# hal/nxp_p1021.c against a simulated NAND (F-12064: the bad-block +# marker counter was kept for the whole request, so only the first +# two pages read were checked, and a skip rewound the position but +# not the output pointer). +p1021_read_extract.h: ../../hal/nxp_p1021.c + sed -n '/#define ELBC_BASE /p' $< > $@ + sed -n '/#define ELBC_MDR /p' $< >> $@ + sed -n '/#define ELBC_FIR /p' $< >> $@ + sed -n '/#define ELBC_FCR /p' $< >> $@ + sed -n '/#define ELBC_FBCR /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP(/p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_PA /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_CA /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_CM0 /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_CM1 /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_CW0 /p' $< >> $@ + sed -n '/#define ELBC_FIR_OP_RBW /p' $< >> $@ + sed -n '/#define ELBC_FCR_CMD(/p' $< >> $@ + sed -n '/#define NAND_CMD_READA /p' $< >> $@ + sed -n '/#define NAND_CMD_READSTART /p' $< >> $@ + sed -n '/#define FLASH_PAGE_SIZE /p' $< >> $@ + +p1021_read_fn_extract.h: ../../hal/nxp_p1021.c + sed -n '/^int ext_flash_read(uintptr_t address, uint8_t \*data, int len)$$/,/^}/p' $< > $@ + +unit-p1021-read-badblock: unit-p1021-read-badblock.c p1021_read_extract.h \ + p1021_read_fn_extract.h + gcc -o $@ unit-p1021-read-badblock.c $(CFLAGS) $(LDFLAGS) + # unit-samr21-erase-advance runs the real hal_flash_erase() from # hal/samr21.c against a host NVMCTRL register window (F-11036: the # length decrement was the body of the NVMREADY wait and the address @@ -1503,7 +1534,9 @@ covclean: GENERATED_SRC:=aurix_erased_extract.h \ hifive1_flash_write_extract.h nvm_cache_scrub_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ - p1021_erase_extract.h p1021_erase_fn_extract.h rp2350_flash_write_extract.h \ + p1021_erase_extract.h p1021_erase_fn_extract.h \ + p1021_read_extract.h p1021_read_fn_extract.h \ + rp2350_flash_write_extract.h \ sdhci_host.c \ stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ stm32u5_write_extract.h stm32wb_write_extract.h \ diff --git a/tools/unit-tests/unit-p1021-read-badblock.c b/tools/unit-tests/unit-p1021-read-badblock.c new file mode 100644 index 0000000000..317b66369e --- /dev/null +++ b/tools/unit-tests/unit-p1021-read-badblock.c @@ -0,0 +1,275 @@ +/* unit-p1021-read-badblock.c + * + * Regression test for F-12064: ext_flash_read() in hal/nxp_p1021.c + * kept its bad-block page counter for the whole request, so the + * marker was inspected only on the first two pages read and a bad + * block later in the request was copied as valid data. When a + * marker did cause a skip, the logical position was rewound to a + * block boundary while the output pointer was not, so the read + * continued past the end of the caller's buffer. + * + * The real function is extracted by the Makefile together with the + * ELBC register macros it uses; the ELBC register access and the + * flash helpers are mocked on top of a small simulated NAND (three + * 16 KiB blocks of 512-byte pages, bad-block marker in spare byte 5 + * of the block's first pages). + * 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 + +/* ELBC_BASE is built from CCSRBAR in the real file; pin it here. */ +#define CCSRBAR 0x0 + +/* ELBC register macros + NAND command codes from hal/nxp_p1021.c + * (extracted by the Makefile). */ +#include "p1021_read_extract.h" + +/* Simulated NAND: three 16 KiB blocks of 512-byte pages. The + * bad-block marker lives in spare byte 5 of the block's first + * pages, matching bad_marker = page_size + 5 for small pages. */ +#define SIM_PAGES 96 +#define SIM_PAGE_SIZE 512 +#define SIM_SPARE_SIZE 64 +#define SIM_BLOCK_PAGES 32 +#define SIM_BLOCK_SIZE (SIM_BLOCK_PAGES * SIM_PAGE_SIZE) +#define SIM_MARKER_SPARE 5 + +static uint8_t g_nand[SIM_PAGES][SIM_PAGE_SIZE]; +static uint8_t g_spare[SIM_PAGES][SIM_SPARE_SIZE]; +static int g_cmd_ret; +static int g_cmd_calls; +static int g_last_page; + +/* FCM buffer + copy offset, as in the real file. */ +static volatile uint8_t g_fcm[SIM_PAGE_SIZE + SIM_SPARE_SIZE]; +static volatile uint8_t *flash_buf = g_fcm; +static uint32_t flash_idx; + +static void sim_reset(void) +{ + int p, i; + + for (p = 0; p < SIM_PAGES; p++) { + for (i = 0; i < SIM_PAGE_SIZE; i++) + g_nand[p][i] = (uint8_t)((p * 7 + i) & 0xFF); + for (i = 0; i < SIM_SPARE_SIZE; i++) + g_spare[p][i] = 0xFF; + } + g_cmd_ret = 0; + g_cmd_calls = 0; + g_last_page = -1; +} + +static void sim_mark_bad_block(int block, int marker_pages) +{ + int p; + + for (p = 0; p < marker_pages; p++) + g_spare[block * SIM_BLOCK_PAGES + p][SIM_MARKER_SPARE] = 0x00; +} + +/* Mocks for the ELBC register access helpers (hal/nxp_ppc.h). */ +static void set32(volatile unsigned int *addr, unsigned int val) +{ + (void)addr; + (void)val; +} + +static uint32_t get32(volatile unsigned int *addr) +{ + (void)addr; + return 0; /* MDR status: no error */ +} + +/* Loads the full page + spare into the FCM buffer (BC = 0) and + * points the copy at the requested column, as the real helper does. */ +static void hal_flash_set_addr(int page, int col) +{ + int i; + + g_last_page = page; + for (i = 0; i < SIM_PAGE_SIZE; i++) + flash_buf[i] = g_nand[page][i]; + for (i = 0; i < SIM_SPARE_SIZE; i++) + flash_buf[SIM_PAGE_SIZE + i] = g_spare[page][i]; + flash_idx = (uint32_t)col; +} + +static int hal_flash_command(uint8_t iswrite) +{ + (void)iswrite; + + g_cmd_calls++; + return g_cmd_ret; +} + +static void hal_flash_read_bytes(uint8_t *data, size_t len) +{ + memcpy(data, (const void *)&flash_buf[flash_idx], len); +} + +/* The real ext_flash_read() from hal/nxp_p1021.c (extracted). */ +#include "p1021_read_fn_extract.h" + +/* Two-block read with a bad second block: the bad block's data must + * not reach the output; the read continues at the third block. + * Pre-fix the marker counter was never reset, so the bad block was + * copied as valid data. */ +START_TEST (test_bad_block_in_later_block_skipped){ + uint8_t out[2 * SIM_BLOCK_SIZE]; + int ret, p; + + sim_reset(); + sim_mark_bad_block(1, 1); + + ret = ext_flash_read(0, out, 2 * SIM_BLOCK_SIZE); + + ck_assert_int_eq(ret, 2 * SIM_BLOCK_SIZE); + /* block 0 delivered as-is */ + for (p = 0; p < SIM_BLOCK_PAGES; p++) + ck_assert_int_eq(memcmp(out + p * SIM_PAGE_SIZE, g_nand[p], + SIM_PAGE_SIZE), 0); + /* block 1 skipped, block 2 takes its place */ + for (p = 0; p < SIM_BLOCK_PAGES; p++) + ck_assert_int_eq(memcmp(out + (SIM_BLOCK_PAGES + p) * + SIM_PAGE_SIZE, + g_nand[2 * SIM_BLOCK_PAGES + p], + SIM_PAGE_SIZE), 0); +} +END_TEST + +/* Bad marker on the second page of the first block: one page was + * already delivered when the skip fires. Post-fix the output + * pointer and the position stay consistent and nothing is written + * past the buffer; pre-fix the position was rewound while the + * pointer was not, overflowing the buffer by one page. */ +START_TEST(test_bad_marker_second_page_no_overflow) +{ + uint8_t out[1024 + 64]; + int ret, i; + + sim_reset(); + g_spare[1][SIM_MARKER_SPARE] = 0x00; /* marker on page 1 only */ + for (i = 1024; i < (int)sizeof(out); i++) + out[i] = 0xEE; + + ret = ext_flash_read(0, out, 1024); + + ck_assert_int_eq(ret, 1024); + /* page 0 of block 0, then block 1 from its first page */ + ck_assert_int_eq(memcmp(out, g_nand[0], SIM_PAGE_SIZE), 0); + ck_assert_int_eq(memcmp(out + SIM_PAGE_SIZE, g_nand[SIM_BLOCK_PAGES], + SIM_PAGE_SIZE), 0); + for (i = 1024; i < (int)sizeof(out); i++) + ck_assert_uint_eq(out[i], 0xEE); +} +END_TEST + +/* Bad first block, marker on its first page: the classic skip. The + * whole request is satisfied from the second block. */ +START_TEST(test_bad_first_block_page0) +{ + uint8_t out[SIM_BLOCK_SIZE]; + int ret, p; + + sim_reset(); + sim_mark_bad_block(0, 1); + + ret = ext_flash_read(0, out, SIM_BLOCK_SIZE); + + ck_assert_int_eq(ret, SIM_BLOCK_SIZE); + for (p = 0; p < SIM_BLOCK_PAGES; p++) + ck_assert_int_eq(memcmp(out + p * SIM_PAGE_SIZE, + g_nand[SIM_BLOCK_PAGES + p], + SIM_PAGE_SIZE), 0); +} +END_TEST + +/* All blocks good: a plain two-block read is byte-exact. */ +START_TEST(test_all_good_two_blocks) +{ + uint8_t out[2 * SIM_BLOCK_SIZE]; + int ret, p; + + sim_reset(); + + ret = ext_flash_read(0, out, 2 * SIM_BLOCK_SIZE); + + ck_assert_int_eq(ret, 2 * SIM_BLOCK_SIZE); + for (p = 0; p < 2 * SIM_BLOCK_PAGES; p++) + ck_assert_int_eq(memcmp(out + p * SIM_PAGE_SIZE, g_nand[p], + SIM_PAGE_SIZE), 0); +} +END_TEST + +/* Unaligned start mid-page spanning four pages: the column offset + * and the per-page copy size stay correct across the read. */ +START_TEST(test_unaligned_start_across_pages) +{ + uint8_t out[2000]; + int ret, off, p, i; + + sim_reset(); + + ret = ext_flash_read(256, out, 2000); + + ck_assert_int_eq(ret, 2000); + off = 0; + for (p = 0; off < 2000; p++) { + int start = (p == 0) ? 256 : 0; + int take = SIM_PAGE_SIZE - start; + + if (take > 2000 - off) + take = 2000 - off; + for (i = 0; i < take; i++) + ck_assert_uint_eq(out[off + i], g_nand[p][start + i]); + off += take; + } +} +END_TEST + +Suite *p1021_read_badblock_suite(void) +{ + Suite *s = suite_create("p1021 read badblock"); + TCase *tc = tcase_create("bad-block"); + + tcase_add_test(tc, test_bad_block_in_later_block_skipped); + tcase_add_test(tc, test_bad_marker_second_page_no_overflow); + tcase_add_test(tc, test_bad_first_block_page0); + tcase_add_test(tc, test_all_good_two_blocks); + tcase_add_test(tc, test_unaligned_start_across_pages); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = p1021_read_badblock_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From 4c447f4bf9df60d94442914fd4e64d1a0a4b009b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:14:42 +0200 Subject: [PATCH 05/18] F-12104: Kontron TGL: apply the SPI BIOS-region lock through the BAR tgl_lock_bios_region() wrote the protected range and the FLOCKDN value through PCI configuration space (offsets 0x48 and 0x04, the status/command dword) instead of the SPI controller's memory-mapped registers at the BAR0 base, and took the range from FREG1 (non-BIOS) instead of FREG0 (BIOS). The lock now writes FPR0 and BIOS/H SFSTS/CTL through mmio_write32(), verifies both by readback, and returns an error if the bits do not stick. The helper had no callers: no hal_flash_protect() override existed, so the weak no-op default ran before handoff and the BIOS region stayed writable. Add the override routing to tgl_lock_bios_region(). Including for the hook signature also exposes the hal_flash_write/hal_flash_erase stubs as mismatching the HAL contract; fix their address parameter to haladdr_t. Add unit-kontron-tgl-spi: runs the extracted tgl_lock_bios_region() and hal_flash_protect() against mocked PCI config space and an MMIO array at the BAR address (build fails pre-fix - hal_flash_protect undefined - 4/4 pass post-fix). --- hal/kontron_vx3060_s2.c | 47 +++-- tools/unit-tests/Makefile | 18 ++ tools/unit-tests/unit-kontron-tgl-spi.c | 229 ++++++++++++++++++++++++ 3 files changed, 282 insertions(+), 12 deletions(-) create mode 100644 tools/unit-tests/unit-kontron-tgl-spi.c diff --git a/hal/kontron_vx3060_s2.c b/hal/kontron_vx3060_s2.c index 1ca9752c76..2211662979 100644 --- a/hal/kontron_vx3060_s2.c +++ b/hal/kontron_vx3060_s2.c @@ -20,6 +20,7 @@ */ #include +#include #include #include #include @@ -33,7 +34,10 @@ #define SPI_PCI_DEV 31 #define SPI_PCI_FUN 5 #define SPI_BAR_OFF 0x10 -#define SPI_FREG1 0x58 +/* Tiger Lake SPI controller register offsets, memory-mapped at the + * BAR0 base. FREG0 holds the BIOS flash region base/limit; FPR0 is + * the protected range register with the same base/limit layout. */ +#define SPI_FREG0 0x50 #define SPI_FREG_BASE_MASK (0x7fffU << 0) #define SPI_FREG_LIMIT_MASK (0x7fffU << 16) #define SPI_FREG_LIMIT_SHIFT (16) @@ -48,6 +52,7 @@ int tgl_lock_bios_region() { uint32_t spi_bar, spi_cmd; uint32_t reg; + int ret = 0; #if defined(DEBUG) uint32_t bios_reg_base, bios_reg_lim; @@ -60,7 +65,12 @@ int tgl_lock_bios_region() pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, PCI_COMMAND_OFFSET, spi_cmd | PCI_COMMAND_MEM_SPACE); - reg = mmio_read32(spi_bar + SPI_FREG1); + /* The Flash Protected Range register has the same base/limit + * layout as the Flash Region register: take the BIOS region + * (FREG0) and enable read and write protection on it. The SPI + * registers live in the BAR's memory-mapped space, not in PCI + * configuration space. */ + reg = mmio_read32(spi_bar + SPI_FREG0); #if defined(DEBUG) bios_reg_base = (reg & SPI_FREG_BASE_MASK) << SPI_FREG_ADDR_SHIFT; bios_reg_lim = ((reg & SPI_FREG_LIMIT_MASK) >> SPI_FREG_LIMIT_SHIFT) @@ -68,21 +78,34 @@ int tgl_lock_bios_region() wolfBoot_printf("Bios reg base: 0x%x lim: 0x%x\r\n", bios_reg_base, bios_reg_lim); #endif - /* Flash Protected Range register has very similar layout of the Flash - * Region Register, so we can reuse it and just enable read and write - * protection - */ reg |= (SPI_FPR_RPE) | (SPI_FPR_WPE); - pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_FPR0, reg); + mmio_write32(spi_bar + SPI_FPR0, reg); + if ((mmio_read32(spi_bar + SPI_FPR0) & + (SPI_FPR_RPE | SPI_FPR_WPE)) != (SPI_FPR_RPE | SPI_FPR_WPE)) { + ret = -1; + } /* lock down BIOS register configuration */ - reg = pci_config_read32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_BIOS_HSFSTS_CTL); + reg = mmio_read32(spi_bar + SPI_BIOS_HSFSTS_CTL); reg |= SPI_FLOCKDN; - pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, SPI_BIOS_HSFSTS_CTL, reg); + mmio_write32(spi_bar + SPI_BIOS_HSFSTS_CTL, reg); + if ((mmio_read32(spi_bar + SPI_BIOS_HSFSTS_CTL) & SPI_FLOCKDN) == 0) { + ret = -1; + } /* restore original cmd */ pci_config_write32(0, SPI_PCI_DEV, SPI_PCI_FUN, PCI_COMMAND_OFFSET, spi_cmd); - return 0; + return ret; +} + +int hal_flash_protect(haladdr_t address, int len) +{ + (void)address; + (void)len; + + /* The TGL BIOS region covers the bootloader partition, so the + * hook's address/len are the same range FREG0 describes. */ + return tgl_lock_bios_region(); } void hal_init(void) @@ -97,7 +120,7 @@ void hal_prepare_boot(void) } #endif -int hal_flash_write(uint32_t address, const uint8_t *data, int len) +int hal_flash_write(haladdr_t address, const uint8_t *data, int len) { return 0; } @@ -110,7 +133,7 @@ void hal_flash_lock(void) { } -int hal_flash_erase(uint32_t address, int len) +int hal_flash_erase(haladdr_t address, int len) { return 0; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index b3fd5b0742..e2fca0067c 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -116,6 +116,7 @@ TESTS+=unit-t10xx-qe-firmware TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance TESTS+=unit-p1021-read-badblock +TESTS+=unit-kontron-tgl-spi TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-rp2350-flash-write @@ -1259,6 +1260,22 @@ unit-p1021-read-badblock: unit-p1021-read-badblock.c p1021_read_extract.h \ p1021_read_fn_extract.h gcc -o $@ unit-p1021-read-badblock.c $(CFLAGS) $(LDFLAGS) +# unit-kontron-tgl-spi runs the real tgl_lock_bios_region() from +# hal/kontron_vx3060_s2.c against mocked PCI config space and an +# MMIO array at the SPI BAR address (F-12104: the lock was never +# applied - no hal_flash_protect() override - and targeted the +# wrong register space). +kontron_spi_extract.h: ../../hal/kontron_vx3060_s2.c + sed -n '/^#define SPI_PCI_DEV/,/^#define SPI_FLOCKDN/p' $< > $@ + +kontron_spi_fn_extract.h: ../../hal/kontron_vx3060_s2.c + sed -n '/^int tgl_lock_bios_region/,/^}/p' $< > $@ + sed -n '/^int hal_flash_protect/,/^}/p' $< >> $@ + +unit-kontron-tgl-spi: unit-kontron-tgl-spi.c kontron_spi_extract.h \ + kontron_spi_fn_extract.h + gcc -o $@ unit-kontron-tgl-spi.c $(CFLAGS) $(LDFLAGS) + # unit-samr21-erase-advance runs the real hal_flash_erase() from # hal/samr21.c against a host NVMCTRL register window (F-11036: the # length decrement was the body of the NVMREADY wait and the address @@ -1536,6 +1553,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ nxp_ls1028a_host.c nxp_p1021_host.c nxp_t10xx_fixup_extract.h \ p1021_erase_extract.h p1021_erase_fn_extract.h \ p1021_read_extract.h p1021_read_fn_extract.h \ + kontron_spi_extract.h kontron_spi_fn_extract.h \ rp2350_flash_write_extract.h \ sdhci_host.c \ stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ diff --git a/tools/unit-tests/unit-kontron-tgl-spi.c b/tools/unit-tests/unit-kontron-tgl-spi.c new file mode 100644 index 0000000000..b1ae0c6555 --- /dev/null +++ b/tools/unit-tests/unit-kontron-tgl-spi.c @@ -0,0 +1,229 @@ +/* unit-kontron-tgl-spi.c + * + * Regression test for F-12104: the Kontron VX3060 S2 (Tiger Lake) + * SPI BIOS-region lock was never applied - no hal_flash_protect() + * override existed, so the weak no-op default ran before handoff - + * and the only helper, tgl_lock_bios_region(), wrote the protected + * range and lock values through PCI configuration space (offsets + * 0x48/0x04) instead of the SPI BAR's memory-mapped register space, + * and took the range from FREG1 (non-BIOS) instead of FREG0 (BIOS). + * + * The real function is extracted by the Makefile and run against + * mocked PCI config space and an MMIO array at the BAR address. + * 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 + +typedef uintptr_t haladdr_t; + +#define MMIO_BASE 0xFED40000UL + +/* SPI PCI device constants + register offsets from + * hal/kontron_vx3060_s2.c (extracted by the Makefile). */ +#include "kontron_spi_extract.h" + +/* TGL register offsets used by the model. */ +#define TGL_FREG0_OFF 0x50 +#define TGL_FREG1_OFF 0x58 +#define TGL_FPR0_OFF 0x48 +#define TGL_SFSTS_CTL_OFF 0x04 + +/* FREG0: BIOS region, base/limit fields (14 bits each, shifted 12). + * FREG1: a different range, so a test that compares the FPR0 value + * against FREG0 also proves FREG1 was not the source. */ +#define FREG0_INIT 0x7FFF7400U +#define FREG1_INIT 0x3FFF0000U + +static uint32_t g_pci_cfg[16]; /* 64 bytes of config space */ +static uint8_t g_mmio[256]; +static uint8_t g_cfg_write_off[16]; +static int g_cfg_write_count; +static int g_fail_fpr_readback; +static int g_fail_lockdn_readback; + +static void sim_reset(void) +{ + memset(g_mmio, 0, sizeof(g_mmio)); + memset(g_pci_cfg, 0, sizeof(g_pci_cfg)); + g_pci_cfg[PCI_COMMAND_OFFSET / 4] = 0x0001; /* IO space only */ + /* memory BAR, read-write (type bit 0 set), 32-bit: the code's + * PCI_BAR_MASK strips the type bits */ + g_pci_cfg[PCI_BAR0_OFFSET / 4] = (uint32_t)(MMIO_BASE) | 0x1; + g_mmio[TGL_FREG0_OFF] = 0; + *(uint32_t *)&g_mmio[TGL_FREG0_OFF] = FREG0_INIT; + *(uint32_t *)&g_mmio[TGL_FREG1_OFF] = FREG1_INIT; + g_cfg_write_count = 0; + g_fail_fpr_readback = 0; + g_fail_lockdn_readback = 0; +} + +/* Mocks for the PCI config accessors (src/pci.c). */ +uint32_t pci_config_read32(uint8_t bus, uint8_t dev, uint8_t fun, + uint8_t off) +{ + (void)bus; + (void)dev; + (void)fun; + + return g_pci_cfg[off / 4]; +} + +void pci_config_write32(uint8_t bus, uint8_t dev, uint8_t fun, + uint8_t off, uint32_t val) +{ + (void)bus; + (void)dev; + (void)fun; + + if (g_cfg_write_count < (int)(sizeof(g_cfg_write_off) / + sizeof(g_cfg_write_off[0]))) + g_cfg_write_off[g_cfg_write_count] = off; + g_cfg_write_count++; + if (off == PCI_COMMAND_OFFSET) + g_pci_cfg[off / 4] = val; +} + +/* Mocks for the MMIO accessors (src/x86/common.c). */ +static void mmio_write32(uintptr_t address, uint32_t value) +{ + uint32_t *slot = (uint32_t *)&g_mmio[address - MMIO_BASE]; + + *slot = value; +} + +static uint32_t mmio_read32(uintptr_t address) +{ + uint32_t val = *(uint32_t *)&g_mmio[address - MMIO_BASE]; + + if (g_fail_fpr_readback && (address - MMIO_BASE) == TGL_FPR0_OFF) + val &= ~SPI_FPR_WPE; + if (g_fail_lockdn_readback && + (address - MMIO_BASE) == TGL_SFSTS_CTL_OFF) + val &= ~SPI_FLOCKDN; + return val; +} + +/* The real tgl_lock_bios_region() + hal_flash_protect() from + * hal/kontron_vx3060_s2.c (extracted). */ +#include "kontron_spi_fn_extract.h" + +/* The lock must land in the MMIO space: FPR0 carries the FREG0 + * (BIOS region) base/limit with RPE/WPE set, FLOCKDN is set in + * BIOS/H SFSTS/CTL, and PCI config space sees only the COMMAND + * enable/restore. Pre-fix the values went to config offsets 0x48 + * and 0x04 and FPR0 was never written. */ +START_TEST (test_lock_written_to_mmio){ + uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + int i, ret; + int cmd_restored = 1; + + sim_reset(); + ret = tgl_lock_bios_region(); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + SPI_FLOCKDN, SPI_FLOCKDN); + /* FREG0 itself is not modified */ + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FREG0_OFF], FREG0_INIT); + /* config space: only the COMMAND register is written, and the + * final write restores the original value */ + for (i = 0; i < g_cfg_write_count; i++) + ck_assert_int_eq(g_cfg_write_off[i], PCI_COMMAND_OFFSET); + if (g_cfg_write_count > 0) + cmd_restored = (g_pci_cfg[PCI_COMMAND_OFFSET / 4] == 0x0001); + ck_assert_int_eq(cmd_restored, 1); +} +END_TEST + +/* The hal_flash_protect() override must route to the TGL lock with + * the hook's address/len, so the update paths' + * hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) + * call actually establishes protection. */ +START_TEST(test_hal_flash_protect_wires_lock) +{ + uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + int ret; + + sim_reset(); + ret = hal_flash_protect(0xFFF00000, 0x600000); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); + ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + SPI_FLOCKDN, SPI_FLOCKDN); +} +END_TEST + +/* A protected range that does not stick (readback missing WPE) + * must be reported as an error, not silently accepted. */ +START_TEST(test_fpr_readback_mismatch) +{ + int ret; + + sim_reset(); + g_fail_fpr_readback = 1; + ret = tgl_lock_bios_region(); + + ck_assert_int_lt(ret, 0); +} +END_TEST + +/* Same for the FLOCKDN bit. */ +START_TEST(test_flockdn_readback_mismatch) +{ + int ret; + + sim_reset(); + g_fail_lockdn_readback = 1; + ret = tgl_lock_bios_region(); + + ck_assert_int_lt(ret, 0); +} +END_TEST + +Suite *kontron_tgl_spi_suite(void) +{ + Suite *s = suite_create("kontron tgl spi"); + TCase *tc = tcase_create("bios-region lock"); + + tcase_add_test(tc, test_lock_written_to_mmio); + tcase_add_test(tc, test_hal_flash_protect_wires_lock); + tcase_add_test(tc, test_fpr_readback_mismatch); + tcase_add_test(tc, test_flockdn_readback_mismatch); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = kontron_tgl_spi_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From c55795b5a4efa0722d8a345eac1ef53172f65799 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:33:38 +0200 Subject: [PATCH 06/18] F-12065: update_ram: reject short ext flash reads on the RAM load The non-RAMBOOT copy-to-RAM path only rejected negative ext_flash_read() results. Backends return the number of bytes read on success (filesystem.c forwards XFREAD's count), so a positive short read was accepted and boot continued with a truncated RAM image. Require the read to return exactly os_image.fw_size; any other result aborts the boot, matching the header-copy check already in the same file (ret != IMAGE_HEADER_SIZE). Note: the RAMBOOT image-load path (WOLFBOOT_USE_RAMBOOT) has the same `ret < 0` pattern on its img_size read; outside this finding's scope, left as-is. Test: unit-update-ram-noramboot gains a short-read case (mock ext_flash_read withholds 1 byte from the full-image copy only). Pre-fix the truncated image staged for boot (staged_ok == 1); post-fix the boot is aborted with "Error loading image ... (ret 5299)". --- src/update_ram.c | 4 ++- tools/unit-tests/unit-mock-flash.c | 14 +++++++++-- tools/unit-tests/unit-update-ram-noramboot.c | 26 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/update_ram.c b/src/update_ram.c index 649a3c51eb..ee7b884525 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -573,7 +573,9 @@ void RAMFUNCTION wolfBoot_start(void) os_image.fw_base, load_address, os_image.fw_size); ret = ext_flash_read((uintptr_t)os_image.fw_base, (uint8_t*)load_address, os_image.fw_size); - if (ret < 0){ + /* Backends return the number of bytes read: a positive short read + * leaves a truncated image in RAM, so require the full size. */ + if (ret != os_image.fw_size) { wolfBoot_printf("Error loading image at %p (ret %d)\n", os_image.fw_base, ret); return; diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index 3dd9e44b56..c12aa3472b 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -212,14 +212,24 @@ int ext_flash_write(uintptr_t address, const uint8_t *data, int len) return 0; } +/* When mock_ext_flash_short_len > 0, ext_flash_read() calls of + * exactly that length return len - mock_ext_flash_short_bytes (a + * short positive read, F-12065). Other lengths read in full. */ +int mock_ext_flash_short_len = 0; +int mock_ext_flash_short_bytes = 0; + int ext_flash_read(uintptr_t address, uint8_t *data, int len) { int i; + int ret = len; uint8_t *a = (uint8_t *)address; - for (i = 0; i < len; i++) { + + if (mock_ext_flash_short_len == len && mock_ext_flash_short_bytes > 0) + ret = len - mock_ext_flash_short_bytes; + for (i = 0; i < ret; i++) { data[i] = a[i]; } - return len; + return ret; } void ext_flash_unlock(void) diff --git a/tools/unit-tests/unit-update-ram-noramboot.c b/tools/unit-tests/unit-update-ram-noramboot.c index 8561830611..88635b8cba 100644 --- a/tools/unit-tests/unit-update-ram-noramboot.c +++ b/tools/unit-tests/unit-update-ram-noramboot.c @@ -220,18 +220,44 @@ START_TEST (test_noramboot_highversion_rollback_denied) { } END_TEST +/* F-12065: a short positive ext_flash_read() result on the +copy-to-RAM path must abort the boot, not continue with a +truncated image. Only the full-image copy read (fw_size bytes) +is short; the smaller integrity reads are unaffected. */ +START_TEST (test_noramboot_ext_flash_short_read_rejected) { + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + mock_ext_flash_short_len = TEST_SIZE_SMALL; + mock_ext_flash_short_bytes = 1; + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_staged_ok, 0); + mock_ext_flash_short_len = 0; + mock_ext_flash_short_bytes = 0; + cleanup_flash(); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfboot-noramboot"); TCase *sunnyday = tcase_create("Non-RAMBOOT sunny day"); + TCase *ext_short_read = + tcase_create("Non-RAMBOOT short ext flash read rejected"); TCase *rollback_denied = tcase_create("Non-RAMBOOT high-version rollback denied"); tcase_add_test(sunnyday, test_noramboot_sunnyday); + tcase_add_test(ext_short_read, + test_noramboot_ext_flash_short_read_rejected); tcase_add_test(rollback_denied, test_noramboot_highversion_rollback_denied); suite_add_tcase(s, sunnyday); + suite_add_tcase(s, ext_short_read); suite_add_tcase(s, rollback_denied); tcase_set_timeout(sunnyday, 5); + tcase_set_timeout(ext_short_read, 5); tcase_set_timeout(rollback_denied, 5); return s; } From 7736d975f11b6581af812cc7bf66707a81b0ebf1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:42:18 +0200 Subject: [PATCH 07/18] F-12060: multiboot2: terminate the boot info tag list with an end tag mb2_build_boot_info_header() emitted the requested tags and set total_size without appending the Multiboot2 end tag (type 0, size 8). A strict consumer walking the output section sees no terminator inside total_size (or a zero-sized pseudo-terminator only if the destination buffer happens to be zero-filled), so the handoff is structurally invalid. Reserve eight bytes for the end tag after the requested tags, write {type 0, flags 0, size 8}, and include it in total_size. The write is bounds-checked against the caller's max_size like the other tag builders, and idx is already 8-byte aligned since every tag size is a multiple of 8. Tests: the existing layout assertions now expect the end tag inside total_size (basic mem info: 24 -> 32, mem map with one entry: 48 -> 56), and a new consumer-style test walks the generated output section the way a strict Multiboot2 consumer would: both requested tags found, well-formed end tag (type 0, size 8), and the end tag is the last structure in total_size. Pre-fix: 3 failures (total_size short by 8, walk finds no end tag); post-fix: 36/36. --- src/multiboot.c | 15 +++++ tools/unit-tests/unit-multiboot.c | 98 ++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/src/multiboot.c b/src/multiboot.c index 9d3677be70..ef5528934c 100644 --- a/src/multiboot.c +++ b/src/multiboot.c @@ -253,6 +253,7 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info, struct mb2_boot_info_header *hdr = (struct mb2_boot_info_header *)mb2_boot_info; struct mb2_tag_info_req *info_req_tag; + struct mb2_tag *end_tag; int requested_tags, i, r; uint32_t header_length; uint8_t *idx; @@ -299,6 +300,20 @@ int mb2_build_boot_info_header(uint8_t *mb2_boot_info, } } + /* The Multiboot2 spec requires the tag list to be terminated by an + * end tag (type 0, size 8); reserve its space and include it in + * total_size. */ + if (max_size < sizeof(struct mb2_tag)) { + MB2_DEBUG_PRINTF("Not enough size to build mb2 end tag\r\n"); + return -1; + } + max_size -= sizeof(struct mb2_tag); + end_tag = (struct mb2_tag *)idx; + end_tag->type = 0; + end_tag->flags = 0; + end_tag->size = sizeof(*end_tag); + idx += sizeof(*end_tag); + hdr->total_size = idx - (uint8_t*)hdr; return 0; diff --git a/tools/unit-tests/unit-multiboot.c b/tools/unit-tests/unit-multiboot.c index 40b365218b..0ac826d992 100644 --- a/tools/unit-tests/unit-multiboot.c +++ b/tools/unit-tests/unit-multiboot.c @@ -699,6 +699,7 @@ START_TEST(test_build_info_basic_mem) struct stage2_parameter p = make_stage2(); struct mb2_boot_info_header *bih; struct mb2_basic_memory_info *meminfo; + struct mb2_tag *end_tag; struct mock_mem_region regions[] = { {0, 640 * 1024, EFI_RESOURCE_SYSTEM_MEMORY}, {1024 * 1024, 127ULL * 1024 * 1024, EFI_RESOURCE_SYSTEM_MEMORY} @@ -713,14 +714,23 @@ START_TEST(test_build_info_basic_mem) sizeof(boot_info)), 0); bih = (struct mb2_boot_info_header *)boot_info; + /* header + basic mem info + end tag (F-12060) */ ck_assert_uint_eq(bih->total_size, - sizeof(*bih) + sizeof(struct mb2_basic_memory_info)); + sizeof(*bih) + sizeof(struct mb2_basic_memory_info) + + sizeof(struct mb2_tag)); meminfo = (struct mb2_basic_memory_info *)(boot_info + sizeof(*bih)); ck_assert_uint_eq(meminfo->type, 4); ck_assert_uint_eq(meminfo->mem_lower, 640); ck_assert_uint_eq(meminfo->mem_upper, 127 * 1024); + /* F-12060: the tag list must end with a proper end tag */ + end_tag = (struct mb2_tag *)(boot_info + sizeof(*bih) + + sizeof(struct mb2_basic_memory_info)); + ck_assert_uint_eq(end_tag->type, 0); + ck_assert_uint_eq(end_tag->flags, 0); + ck_assert_uint_eq(end_tag->size, sizeof(*end_tag)); + mock_regions = NULL; mock_region_count = 0; } @@ -747,6 +757,11 @@ START_TEST(test_build_info_mem_map) sizeof(boot_info)), 0); bih = (struct mb2_boot_info_header *)boot_info; + /* header + mem map (1 entry) + end tag (F-12060) */ + ck_assert_uint_eq(bih->total_size, + sizeof(*bih) + sizeof(struct mb2_mem_map_header) + + sizeof(struct mb2_mem_map_entry) + + sizeof(struct mb2_tag)); map_hdr = (struct mb2_mem_map_header *)(boot_info + sizeof(*bih)); ck_assert_uint_eq(map_hdr->type, 6); ck_assert_uint_eq(map_hdr->entry_size, sizeof(struct mb2_mem_map_entry)); @@ -764,6 +779,86 @@ START_TEST(test_build_info_mem_map) } END_TEST +/* F-12060: consumer-style tag walk over the generated output section. + * A strict Multiboot2 consumer walks 8-byte-aligned tags until the end + * tag (type 0, size 8); the walk must find both requested tags, stop at + * a well-formed end tag, and the end tag must be the last structure + * inside total_size. */ +START_TEST(test_build_info_end_tag_walk) +{ + uint8_t header[64] __attribute__((aligned(8))); + uint8_t boot_info[256] __attribute__((aligned(8))); + struct stage2_parameter p = make_stage2(); + struct mb2_header *h; + struct mb2_tag_info_req *info; + struct mb2_tag *term; + struct mb2_boot_info_header *bih; + struct mb2_tag *tag; + uint8_t *end; + int seen_basic = 0; + int seen_mmap = 0; + int seen_end = 0; + struct mock_mem_region regions[] = { + {0, 640 * 1024, EFI_RESOURCE_SYSTEM_MEMORY}, + {1024 * 1024, 127ULL * 1024 * 1024, EFI_RESOURCE_SYSTEM_MEMORY} + }; + mock_regions = regions; + mock_region_count = 2; + + /* header requesting both basic mem info (4) and mem map (6) */ + memset(header, 0, sizeof(header)); + h = (struct mb2_header *)header; + h->magic = MB2_MAGIC; + h->architecture = 0; + h->header_length = 40; /* header(16) + info_req(16) + term(8) */ + h->checksum = 0; + info = (struct mb2_tag_info_req *)(header + 16); + info->type = 1; + info->flags = 0; + info->size = 16; /* 8 + two uint32_t */ + info->mbi_tag_types[0] = 4; /* MB2_REQ_TAG_BASIC_MEM_INFO */ + info->mbi_tag_types[1] = 6; /* MB2_REQ_TAG_MEM_MAP */ + term = (struct mb2_tag *)(header + 32); + term->type = 0; + term->flags = 0; + term->size = 8; + + memset(boot_info, 0, sizeof(boot_info)); + ck_assert_int_eq( + mb2_build_boot_info_header(boot_info, header, &p, + sizeof(boot_info)), 0); + + bih = (struct mb2_boot_info_header *)boot_info; + end = boot_info + bih->total_size; + tag = (struct mb2_tag *)(boot_info + sizeof(*bih)); + while ((uint8_t *)tag + sizeof(*tag) <= end) { + if (tag->type == 0) { + seen_end = 1; + ck_assert_uint_eq(tag->size, sizeof(*tag)); + break; + } + if (tag->size < sizeof(*tag) || + (uint8_t *)tag + tag->size > end) + fail("tag overruns total_size"); + if (tag->type == 4) + seen_basic = 1; + if (tag->type == 6) + seen_mmap = 1; + tag = (struct mb2_tag *)mb2_align_address_up( + (uint8_t *)tag + tag->size, 8); + } + + ck_assert_int_eq(seen_basic, 1); + ck_assert_int_eq(seen_mmap, 1); + ck_assert_int_eq(seen_end, 1); + /* the end tag must be the last structure in total_size */ + ck_assert_ptr_eq((uint8_t *)tag + tag->size, end); + + mock_regions = NULL; + mock_region_count = 0; +} +END_TEST + /* ---- Suite ---- */ Suite *wolfboot_suite(void) @@ -824,6 +919,7 @@ Suite *wolfboot_suite(void) TCase *tc_hob = tcase_create("mb2_build_boot_info_with_hob"); tcase_add_test(tc_hob, test_build_info_basic_mem); tcase_add_test(tc_hob, test_build_info_mem_map); + tcase_add_test(tc_hob, test_build_info_end_tag_walk); suite_add_tcase(s, tc_hob); return s; From 0da41145ea40ecb657cf4c83d411e5463a901ec5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 19:50:10 +0200 Subject: [PATCH 08/18] F-12066: pci: restore original COMMAND and clear windows on bridge error pci_program_bridge() used orig_cmd both as the saved COMMAND register value and as the accumulator for the decode bits enabled while programming. Error paths after a window was programmed (post-enum MMIO or IO alignment failures) restored that mutated value and left the programmed bridge windows active, so the bridge kept decoding address ranges the allocator rollback had just returned. Keep the two values separate: saved_cmd holds the original register content for the error path, new_cmd accumulates the decode bits and is written on success (seeded from saved_cmd, so the success path preserves the bits it did not manage, exactly as before). The error path now disables every bridge window (prefetch, MMIO, IO) before restoring saved_cmd. Test: test_program_bridge_oom_late_restore programs a prefetch window behind the bridge, then exhausts the MMIO pool so the post-enum MMIO alignment fails. Pre-fix the restored COMMAND was 0x0006 (original 0x0004 plus the MEM_SPACE bit for the discarded window) and the prefetch window stayed programmed (0x9000-0x900F); post-fix the original COMMAND is restored and all windows are disabled. --- src/pci.c | 32 ++++++++++++----- tools/unit-tests/unit-pci.c | 71 +++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/pci.c b/src/pci.c index d87b6b964e..934f8ed38a 100644 --- a/src/pci.c +++ b/src/pci.c @@ -623,7 +623,8 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, uint64_t prefetch_start; uint64_t mem_start; uint64_t io_start; - uint32_t orig_cmd; + uint32_t saved_cmd; + uint32_t new_cmd; uint8_t saved_bus; uint64_t saved_mem; uint64_t saved_pf; @@ -635,8 +636,12 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, saved_pf = info->mem_pf; saved_io = info->io; - orig_cmd = pci_config_read16(bus, dev, fun, PCI_COMMAND_OFFSET); + saved_cmd = pci_config_read16(bus, dev, fun, PCI_COMMAND_OFFSET); pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, 0); + /* decode bits are accumulated from the original value so the + * success path preserves the bits it did not manage; the error + * path restores saved_cmd itself */ + new_cmd = saved_cmd; /* curr_bus_number is one bus per bridge level; at 0xFF the next * increment wraps to 0, which would write SECONDARY_BUS 0 and @@ -699,7 +704,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, prefetch_start >> 16); pci_config_write16(bus, dev, fun, PCI_PREFETCH_LIMIT_OFF, (info->mem_pf - 1) >> 16); - orig_cmd |= PCI_COMMAND_MEM_SPACE; + new_cmd |= PCI_COMMAND_MEM_SPACE; } else { /* disable prefetch */ pci_config_write16(bus, dev, fun, PCI_PREFETCH_BASE_OFF, @@ -719,7 +724,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, mem_start >> 16); pci_config_write16(bus, dev, fun, PCI_MMIO_LIMIT_OFF, (info->mem - 1) >> 16); - orig_cmd |= PCI_COMMAND_MEM_SPACE; + new_cmd |= PCI_COMMAND_MEM_SPACE; } else { /* disable mem */ pci_config_write16(bus, dev, fun, PCI_MMIO_BASE_OFF, @@ -739,7 +744,7 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, io_start >> 8); pci_config_write8(bus, dev, fun, PCI_IO_LIMIT_OFF, (info->io - 1) >> 8); - orig_cmd |= PCI_COMMAND_IO_SPACE; + new_cmd |= PCI_COMMAND_IO_SPACE; } else { pci_config_write8(bus, dev, fun, PCI_IO_BASE_OFF, @@ -748,8 +753,8 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, 0x0); } - orig_cmd |= PCI_COMMAND_BUS_MASTER; - pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, orig_cmd); + new_cmd |= PCI_COMMAND_BUS_MASTER; + pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, new_cmd); pci_dump_bridge(bus,dev,fun); return 0; @@ -759,10 +764,21 @@ static int pci_program_bridge(uint8_t bus, uint8_t dev, uint8_t fun, info->mem = saved_mem; info->mem_pf = saved_pf; info->io = saved_io; + /* Disable every window that may have been programmed before the + * error: the allocator cursors are rolled back, so the bridge must + * not keep decoding the returned address ranges. */ + pci_config_write16(bus, dev, fun, PCI_PREFETCH_BASE_OFF, 0xffff); + pci_config_write16(bus, dev, fun, PCI_PREFETCH_LIMIT_OFF, 0x0); + pci_config_write16(bus, dev, fun, PCI_MMIO_BASE_OFF, 0xffff); + pci_config_write16(bus, dev, fun, PCI_MMIO_LIMIT_OFF, 0x0); + pci_config_write8(bus, dev, fun, PCI_IO_BASE_OFF, 0xff); + pci_config_write8(bus, dev, fun, PCI_IO_LIMIT_OFF, 0x0); pci_config_write8(bus, dev, fun, PCI_PRIMARY_BUS, 0); pci_config_write8(bus, dev, fun, PCI_SECONDARY_BUS, 0); pci_config_write8(bus, dev, fun, PCI_SUB_SEC_BUS, 0); - pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, orig_cmd); + /* restore the original COMMAND value, not the decode bits + * accumulated for the discarded windows */ + pci_config_write16(bus, dev, fun, PCI_COMMAND_OFFSET, saved_cmd); return -1; } diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index 67d5dd39e8..6d967d3e5e 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -1552,6 +1552,73 @@ START_TEST(test_program_bridge_oom_post_enum) } END_TEST +/* F-12066: on a post-enum error the bridge must be left fully + * disabled. The prefetch window is programmed (and the MEM_SPACE + * decode bit accumulated) before the MMIO post-enum alignment fails; + * the error path must restore the original COMMAND register value and + * disable every window, not restore a mutated COMMAND with the + * programmed prefetch window still active for an address range the + * allocator rollback just returned. */ +START_TEST(test_program_bridge_oom_late_restore) +{ + struct test_pci_topology t; + struct pci_enum_info info; + int br, ep, ret; + uint16_t cmd_before = 0x0004; /* master only: no decode bits */ + uint16_t cmd, pfbase, pflimit, mbase, mlimit; + uint8_t iobase, iolimit; + + test_pci_init(&t); + br = test_pci_add_bridge(&t, 1, 0, 0xAAAA, 0xBBBB, TEST_PCI_ROOT_BUS); + ep = test_pci_add_dev(&t, 0, 0, 0xCCCC, 0xDDDD, br); + /* 64KB prefetchable BAR: programs the bridge prefetch window */ + test_pci_dev_set_bar(&t, ep, 0, 0x10000, TEST_PCI_BAR_PF); + /* 64KB MMIO BAR: consumes the whole 1MB pool, so the post-enum + * MMIO alignment (aligned start == limit) fails after the + * prefetch window was programmed. */ + test_pci_dev_set_bar(&t, ep, 1, 0x10000, TEST_PCI_BAR_MMIO); + test_pci_commit(&t); + memcpy(&t.nodes[br].cfg[PCI_COMMAND_OFFSET], &cmd_before, 2); + + memset(&info, 0, sizeof(info)); + info.mem = 0x80000000; + info.mem_limit = 0x80100000; + info.mem_pf = 0x90000000; + info.mem_pf_limit = 0xFFFFFFFF; + info.io = 0x2000; + info.curr_bus_number = 0; + + ret = pci_program_bridge(0, 1, 0, &info); + ck_assert_int_eq(ret, -1); + + /* original COMMAND restored, not the value with MEM_SPACE added */ + cmd = pci_config_read16(0, 1, 0, PCI_COMMAND_OFFSET); + ck_assert_uint_eq(cmd, cmd_before); + + /* every bridge window disabled */ + pfbase = pci_config_read16(0, 1, 0, PCI_PREFETCH_BASE_OFF); + pflimit = pci_config_read16(0, 1, 0, PCI_PREFETCH_LIMIT_OFF); + ck_assert_uint_eq(pfbase, 0xFFFF); + ck_assert_uint_eq(pflimit, 0x0000); + mbase = pci_config_read16(0, 1, 0, PCI_MMIO_BASE_OFF); + mlimit = pci_config_read16(0, 1, 0, PCI_MMIO_LIMIT_OFF); + ck_assert_uint_eq(mbase, 0xFFFF); + ck_assert_uint_eq(mlimit, 0x0000); + iobase = pci_config_read8(0, 1, 0, PCI_IO_BASE_OFF); + iolimit = pci_config_read8(0, 1, 0, PCI_IO_LIMIT_OFF); + ck_assert_uint_eq(iobase, 0xFF); + ck_assert_uint_eq(iolimit, 0x00); + + /* allocator cursors rolled back */ + ck_assert_uint_eq(info.mem, 0x80000000); + ck_assert_uint_eq(info.mem_pf, 0x90000000); + ck_assert_uint_eq(info.io, 0x2000); + ck_assert_uint_eq(info.curr_bus_number, 0); + + test_pci_cleanup(&t); +} +END_TEST + /* test_program_bridge_bus_exhaustion: curr_bus_number is one bus per * bridge level; at 0xFF the next increment wraps to 0, writing * SECONDARY_BUS 0 and re-enumerating bus 0 over the already configured @@ -2078,6 +2145,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_oom_post, test_program_bridge_oom_post_enum); suite_add_tcase(s, tc_oom_post); + TCase *tc_oom_late = tcase_create("bridge-oom-late-restore"); + tcase_add_test(tc_oom_late, test_program_bridge_oom_late_restore); + suite_add_tcase(s, tc_oom_late); + TCase *tc_bus_exhaust = tcase_create("bridge-bus-exhaustion"); tcase_add_test(tc_bus_exhaust, test_program_bridge_bus_exhaustion); suite_add_tcase(s, tc_bus_exhaust); From 6dabdad01ea1a6c926e8bc0e1b458ea228ac5186 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 20:12:48 +0200 Subject: [PATCH 09/18] F-12114: pkcs11: wipe the login PIN in crypto deinit pkcs11_pin is a file-scope copy of the compile-time credential passed to C_Login() for the token holding the firmware-decryption key. pkcs11_crypto_deinit() runs on the pre-handoff path but only closed the session, leaving the credential in retained bootloader memory where a post-handoff attacker could recover it and authenticate to the token. Wipe the copy (volatile zeroize) after the final C_CloseSession(). No re-init path exists after deinit in the product flow (init is only called from the verification paths), so wiping inside the deinit is safe. Add unit-pkcs11-pin-zeroize: a full init/deinit cycle with a stubbed PKCS#11 backend that asserts the pin copy is all zero after deinit and the session was closed, plus a no-session deinit safety case. Verification: unit-pkcs11-pin-zeroize 1/2 pre-fix (pin byte 0 not wiped), 2/2 post-fix; full unit suite green; sim build green; kontron_vx3060_s2 CI build green. --- src/libwolfboot.c | 16 ++ tools/unit-tests/Makefile | 11 + tools/unit-tests/unit-pkcs11-pin-zeroize.c | 242 +++++++++++++++++++++ 3 files changed, 269 insertions(+) create mode 100644 tools/unit-tests/unit-pkcs11-pin-zeroize.c diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 46c8e7eeaa..9e6e9b11e6 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2585,11 +2585,27 @@ int pkcs11_crypto_decrypt(uint8_t *out, uint8_t *in, size_t size) return 0; } +/* Erase the live copy of the login credential: bootloader memory is + * retained after the handoff, and the credential must not survive in + * it. The volatile store keeps the zeroize from being optimized + * away. */ +static void pkcs11_pin_wipe(void) +{ + volatile uint8_t *pin; + size_t i; + + pin = (volatile uint8_t *)pkcs11_pin; + for (i = 0; i < sizeof(pkcs11_pin); i++) { + pin[i] = 0; + } +} + void pkcs11_crypto_deinit(void) { if (encrypt_initialized) { pkcs11_function_list->C_CloseSession(pkcs11_session); encrypt_initialized = 0; + pkcs11_pin_wipe(); } } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index e2fca0067c..c34d235e40 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -78,6 +78,7 @@ TESTS+=unit-tpm-nsc-cert TESTS+=unit-tpm-advio-zeroize TESTS+=unit-tpm-mfgid-eh-zeroize TESTS+=unit-pkcs11-nsc-zeroize +TESTS+=unit-pkcs11-pin-zeroize TESTS+=unit-ubootenv TESTS+=unit-diagnostics TESTS+=unit-diagnostics-256 @@ -411,6 +412,16 @@ unit-pkcs11-nsc-zeroize: ../../include/target.h unit-pkcs11-nsc-zeroize.c -DWOLFPKCS11_USER_SETTINGS -DWOLFCRYPT_SECURE_MODE \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +# The pin-zeroize test pulls libwolfboot.c into the test translation +# unit; --gc-sections drops the unused boot paths so only the crypto +# init/deinit dependency set must link. +unit-pkcs11-pin-zeroize: ../../include/target.h unit-pkcs11-pin-zeroize.c + gcc -o $@ unit-pkcs11-pin-zeroize.c $(CFLAGS) -DMOCK_PARTITIONS \ + -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH -DWOLFBOOT_HASH_SHA256 \ + -DPRINTF_ENABLED -DEXT_FLASH -I$(WOLFBOOT_LIB_WOLFPKCS11) \ + -DSECURE_PKCS11 -DWOLFPKCS11_USER_SETTINGS -ffunction-sections \ + -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-fwtpm-stub: ../../include/target.h unit-fwtpm-stub.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ -DWOLFTPM_USER_SETTINGS -ffunction-sections -fdata-sections \ diff --git a/tools/unit-tests/unit-pkcs11-pin-zeroize.c b/tools/unit-tests/unit-pkcs11-pin-zeroize.c new file mode 100644 index 0000000000..c0bec78f52 --- /dev/null +++ b/tools/unit-tests/unit-pkcs11-pin-zeroize.c @@ -0,0 +1,242 @@ +/* unit-pkcs11-pin-zeroize.c + * + * Unit test for the PKCS#11 login credential lifetime (F-12114). + * + * pkcs11_pin is a file-scope copy of the credential supplied to + * C_Login() for the token holding the firmware-decryption key. + * pkcs11_crypto_deinit() runs on the pre-handoff path and must + * erase that copy: the bootloader memory is retained after the + * handoff, and a live credential there lets an attacker + * authenticate to the token. + * + * + * 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 +#include + +#include "target.h" + +#define EXT_ENCRYPTED +#define ENCRYPT_PKCS11 1 +#define ENCRYPT_PKCS11_PIN "w0lfboot-pin" +#define ENCRYPT_PKCS11_MECHANISM CKM_AES_CTR +#define ENCRYPT_PKCS11_BLOCK_SIZE 16 +#define ENCRYPT_PKCS11_KEY_ID_SIZE 16 +#define ENCRYPT_PKCS11_NONCE_SIZE 16 + +#include "user_settings.h" +#include "wolfboot/wolfboot.h" +/* pulls in the PKCS#11 types (via wcs_pkcs11.h) before the stubs */ +#include "encrypt.h" + +/* the key id object the UNIT_TEST build points key_id at; the nonce + * lives directly after the key id, as in the real layout */ +static uint8_t test_encrypt_key[ENCRYPT_PKCS11_KEY_ID_SIZE + + ENCRYPT_PKCS11_NONCE_SIZE]; +#define ENCRYPT_KEY test_encrypt_key + +/* ---- PKCS#11 stubs ---- */ + +static int stub_close_session_calls; + +static CK_RV stub_C_Initialize(CK_VOID_PTR pInitArgs) +{ + (void)pInitArgs; + return CKR_OK; +} + +static CK_RV stub_C_Finalize(CK_VOID_PTR pReserved) +{ + (void)pReserved; + return CKR_OK; +} + +static CK_RV stub_C_OpenSession(CK_SLOT_ID slotID, CK_FLAGS flags, + CK_VOID_PTR pApplication, + CK_NOTIFY Notify, + CK_SESSION_HANDLE_PTR phSession) +{ + (void)slotID; + (void)flags; + (void)pApplication; + (void)Notify; + *phSession = 1; + return CKR_OK; +} + +static CK_RV stub_C_CloseSession(CK_SESSION_HANDLE hSession) +{ + (void)hSession; + stub_close_session_calls++; + return CKR_OK; +} + +static CK_RV stub_C_Login(CK_SESSION_HANDLE hSession, CK_USER_TYPE userType, + CK_UTF8CHAR_PTR pPin, CK_ULONG ulPinLen) +{ + (void)hSession; + (void)userType; + (void)pPin; + (void)ulPinLen; + return CKR_OK; +} + +static CK_RV stub_C_Logout(CK_SESSION_HANDLE hSession) +{ + (void)hSession; + return CKR_OK; +} + +static CK_RV stub_C_FindObjectsInit(CK_SESSION_HANDLE hSession, + CK_ATTRIBUTE_PTR pTemplate, + CK_ULONG ulCount) +{ + (void)hSession; + (void)pTemplate; + (void)ulCount; + return CKR_OK; +} + +static CK_RV stub_C_FindObjects(CK_SESSION_HANDLE hSession, + CK_OBJECT_HANDLE_PTR phObject, + CK_ULONG maxObjectSize, + CK_ULONG_PTR pulObjectCount) +{ + (void)hSession; + (void)phObject; + (void)maxObjectSize; + *pulObjectCount = 1; + return CKR_OK; +} + +static CK_RV stub_C_FindObjectsFinal(CK_SESSION_HANDLE hSession) +{ + (void)hSession; + return CKR_OK; +} + +static CK_FUNCTION_LIST stub_function_list; + +static void init_stub_function_list(void) +{ + memset(&stub_function_list, 0, sizeof(stub_function_list)); + stub_function_list.C_Initialize = stub_C_Initialize; + stub_function_list.C_Finalize = stub_C_Finalize; + stub_function_list.C_OpenSession = stub_C_OpenSession; + stub_function_list.C_CloseSession = stub_C_CloseSession; + stub_function_list.C_Login = stub_C_Login; + stub_function_list.C_Logout = stub_C_Logout; + stub_function_list.C_FindObjectsInit = stub_C_FindObjectsInit; + stub_function_list.C_FindObjects = stub_C_FindObjects; + stub_function_list.C_FindObjectsFinal = stub_C_FindObjectsFinal; +} + +CK_RV C_GetFunctionList(CK_FUNCTION_LIST_PTR *ppFunctionList) +{ + *ppFunctionList = &stub_function_list; + return CKR_OK; +} + +void hal_trng_init(void) +{ +} + +void panic(void) +{ + ck_abort_msg("panic!"); +} + +#include "libwolfboot.c" + +static void reset_stub_state(void) +{ + stub_close_session_calls = 0; +} + +/* F-12114: the pre-handoff deinitializer must erase the PKCS#11 + * login credential. The pin copy starts with the compile-time + * credential; after a full init/deinit cycle every byte of it must + * be zero, and the session must have been closed. */ +START_TEST(test_pkcs11_pin_wiped_on_deinit){ + int ret; + size_t i; + + reset_stub_state(); + ck_assert_mem_eq(pkcs11_pin, ENCRYPT_PKCS11_PIN, + sizeof(ENCRYPT_PKCS11_PIN)); + + ret = pkcs11_crypto_init(); + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(encrypt_initialized, 1); + + pkcs11_crypto_deinit(); + + ck_assert_int_eq(encrypt_initialized, 0); + ck_assert_int_eq(stub_close_session_calls, 1); + for (i = 0; i < sizeof(pkcs11_pin); i++) { + ck_assert_msg(pkcs11_pin[i] == 0, + "pkcs11_pin byte %zu not wiped", i); + } +} +END_TEST + +/* deinit without an established session must not touch the token + * (no C_CloseSession) and is safe to call repeatedly. */ +START_TEST(test_pkcs11_deinit_no_session) +{ + reset_stub_state(); + encrypt_initialized = 0; + + pkcs11_crypto_deinit(); + pkcs11_crypto_deinit(); + + ck_assert_int_eq(encrypt_initialized, 0); + ck_assert_int_eq(stub_close_session_calls, 0); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("wolfboot-pkcs11-pin"); + TCase *tc = tcase_create("pkcs11-pin-zeroize"); + + tcase_add_test(tc, test_pkcs11_pin_wiped_on_deinit); + tcase_add_test(tc, test_pkcs11_deinit_no_session); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s; + SRunner *sr; + + init_stub_function_list(); + s = wolfboot_suite(); + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From b548341e9b16f451b7e5bb4bc870e9efbbe33bb7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 21:36:46 +0200 Subject: [PATCH 10/18] F-12114: pkcs11: wipe the login PIN on all pre-handoff paths pkcs11_crypto_deinit() - the only caller of pkcs11_pin_wipe() - was invoked from the update_flash path alone (src/update_flash.c:1715). On the RAMBOOT, hwswap and disk pre-handoff paths the PKCS#11 login credential stayed in retained bootloader memory after handoff. Add the same #ifdef ENCRYPT_PKCS11 deinit block after the WOLFHSM cleanup in src/update_ram.c, src/update_flash_hwswap.c and src/update_disk.c, in the same position as the existing update_flash call (before hal_flash_protect/hal_prepare_boot). update_ram.c and update_flash_hwswap.c did not include encrypt.h, where pkcs11_crypto_deinit() is declared - add the include (update_flash.c and update_disk.c already had it). The deinit is a no-op when crypto was never initialized, so the calls are safe on every build. Verification: full build with PKCS11 enabled (sim config + CFLAGS_EXTRA: ENCRYPT_PKCS11, EXT_ENCRYPTED, EXT_FLASH, WOLFCRYPT_SECURE_MODE, SECURE_PKCS11, WOLFPKCS11_USER_SETTINGS + mechanism/sizes/PIN) compiles all sources cleanly; the link stops on pre-existing externals (token library + secure-mode wolfssl objects that a real target's link config supplies) - a control build of the unpatched tree fails identically with the same undefined-symbol set. sim and kontron_vx3060_s2 builds green (PKCS11 disabled, hunks inactive). cstyle clean on the changed hunks. --- src/update_disk.c | 4 ++++ src/update_flash_hwswap.c | 5 +++++ src/update_ram.c | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/src/update_disk.c b/src/update_disk.c index b880658e3f..a0a87cc95c 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -928,6 +928,10 @@ void RAMFUNCTION wolfBoot_start(void) #elif defined(WOLFBOOT_ENABLE_WOLFHSM_SERVER) (void)hal_hsm_server_cleanup(); #endif + +#ifdef ENCRYPT_PKCS11 + pkcs11_crypto_deinit(); +#endif #ifndef TZEN if (hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) < 0) { wolfBoot_printf("Error protecting bootloader flash region\r\n"); diff --git a/src/update_flash_hwswap.c b/src/update_flash_hwswap.c index 6e86a5a0be..888dface14 100644 --- a/src/update_flash_hwswap.c +++ b/src/update_flash_hwswap.c @@ -29,6 +29,7 @@ #include "spi_flash.h" #include "wolfboot/wolfboot.h" #include "printf.h" +#include "encrypt.h" #ifdef SECURE_PKCS11 int WP11_Library_Init(void); #endif @@ -125,6 +126,10 @@ void RAMFUNCTION wolfBoot_start(void) #elif defined(WOLFBOOT_ENABLE_WOLFHSM_SERVER) (void)hal_hsm_server_cleanup(); #endif + +#ifdef ENCRYPT_PKCS11 + pkcs11_crypto_deinit(); +#endif #ifndef TZEN if (hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) < 0) boot_panic(); diff --git a/src/update_ram.c b/src/update_ram.c index ee7b884525..d9f1f3d344 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -30,6 +30,7 @@ #include "printf.h" #include "wolfboot/wolfboot.h" #include +#include "encrypt.h" #ifdef WOLFBOOT_UBOOT_LEGACY #include "gpt.h" /* gpt_crc32_* helpers (reflected CRC-32, poly 0xEDB88320) */ @@ -807,6 +808,10 @@ void RAMFUNCTION wolfBoot_start(void) (void)hal_hsm_server_cleanup(); #endif +#ifdef ENCRYPT_PKCS11 + pkcs11_crypto_deinit(); +#endif + #ifndef TZEN if (hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) < 0) { wolfBoot_printf("Error protecting bootloader flash region\n"); From 1c57f8c06b40405cbbc6c021301c084da77b94d8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 10:16:35 +0200 Subject: [PATCH 11/18] F-12114: pkcs11: wipe the PIN even when no session was established pkcs11_pin is pre-populated from the compile-time credential (ENCRYPT_PKCS11_PIN), so the RAM copy exists from image load, not from a successful C_Login. pkcs11_crypto_deinit() only wiped it inside the encrypt_initialized branch, so on a target where init never completed the credential stayed in retained memory after the pre-handoff path ran. Move pkcs11_pin_wipe() out of the branch: the token interaction (C_CloseSession) stays conditional on an established session, the credential wipe is unconditional. deinit only runs on the terminal pre-handoff paths, so this cannot break the init retry in wolfBoot_initialize_encryption, which runs at decryption time, well before handoff. test_pkcs11_deinit_no_session now re-populates the pin and asserts every byte is zero after deinit without init (plus no C_CloseSession and repeat-call safety). Pre-fix it failed with "pkcs11_pin byte 0 not wiped" (1/2); post-fix 2/2. Verification: unit-pkcs11-pin-zeroize 2/2, full unit suite green, sim build green, cstyle clean on changed hunks. --- src/libwolfboot.c | 5 ++++- tools/unit-tests/unit-pkcs11-pin-zeroize.c | 10 +++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 9e6e9b11e6..dca3f52338 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2605,8 +2605,11 @@ void pkcs11_crypto_deinit(void) if (encrypt_initialized) { pkcs11_function_list->C_CloseSession(pkcs11_session); encrypt_initialized = 0; - pkcs11_pin_wipe(); } + /* pkcs11_pin is pre-populated from the compile-time credential, + * so wipe it even when no session was ever established: the + * pre-handoff paths must not leave it in retained memory. */ + pkcs11_pin_wipe(); } #endif diff --git a/tools/unit-tests/unit-pkcs11-pin-zeroize.c b/tools/unit-tests/unit-pkcs11-pin-zeroize.c index c0bec78f52..591cbe250a 100644 --- a/tools/unit-tests/unit-pkcs11-pin-zeroize.c +++ b/tools/unit-tests/unit-pkcs11-pin-zeroize.c @@ -201,17 +201,25 @@ START_TEST(test_pkcs11_pin_wiped_on_deinit){ END_TEST /* deinit without an established session must not touch the token - * (no C_CloseSession) and is safe to call repeatedly. */ + * (no C_CloseSession), is safe to call repeatedly, and still wipes + * the pre-populated credential copy. */ START_TEST(test_pkcs11_deinit_no_session) { + size_t i; + reset_stub_state(); encrypt_initialized = 0; + memcpy(pkcs11_pin, ENCRYPT_PKCS11_PIN, sizeof(ENCRYPT_PKCS11_PIN)); pkcs11_crypto_deinit(); pkcs11_crypto_deinit(); ck_assert_int_eq(encrypt_initialized, 0); ck_assert_int_eq(stub_close_session_calls, 0); + for (i = 0; i < sizeof(pkcs11_pin); i++) { + ck_assert_msg(pkcs11_pin[i] == 0, + "pkcs11_pin byte %zu not wiped", i); + } } END_TEST From 65cab0a0a7e7d6eaaa1d1378e6e753e8c617da0a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 10:16:35 +0200 Subject: [PATCH 12/18] F-12104: unit test: model the TGL SPI MMIO as 32-bit registers The Kontron TGL SPI regression test kept the MMIO shadow in a uint8_t array and reached into it through uint32_t * casts. The accesses are all 32-bit at 4-aligned register offsets (0x04, 0x48, 0x50, 0x58), so the model is now a uint32_t array indexed by offset/4: no casts, no alignment or strict-aliasing doubt, and the redundant byte-clear before the 32-bit FREG0 store is gone. Verification: unit-kontron-tgl-spi 4/4, full unit suite green, cstyle clean. --- tools/unit-tests/unit-kontron-tgl-spi.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tools/unit-tests/unit-kontron-tgl-spi.c b/tools/unit-tests/unit-kontron-tgl-spi.c index b1ae0c6555..65f34ba82e 100644 --- a/tools/unit-tests/unit-kontron-tgl-spi.c +++ b/tools/unit-tests/unit-kontron-tgl-spi.c @@ -55,7 +55,7 @@ typedef uintptr_t haladdr_t; #define FREG1_INIT 0x3FFF0000U static uint32_t g_pci_cfg[16]; /* 64 bytes of config space */ -static uint8_t g_mmio[256]; +static uint32_t g_mmio[64]; /* 256 bytes of SPI BAR MMIO, 32-bit regs */ static uint8_t g_cfg_write_off[16]; static int g_cfg_write_count; static int g_fail_fpr_readback; @@ -69,9 +69,8 @@ static void sim_reset(void) /* memory BAR, read-write (type bit 0 set), 32-bit: the code's * PCI_BAR_MASK strips the type bits */ g_pci_cfg[PCI_BAR0_OFFSET / 4] = (uint32_t)(MMIO_BASE) | 0x1; - g_mmio[TGL_FREG0_OFF] = 0; - *(uint32_t *)&g_mmio[TGL_FREG0_OFF] = FREG0_INIT; - *(uint32_t *)&g_mmio[TGL_FREG1_OFF] = FREG1_INIT; + g_mmio[TGL_FREG0_OFF / 4] = FREG0_INIT; + g_mmio[TGL_FREG1_OFF / 4] = FREG1_INIT; g_cfg_write_count = 0; g_fail_fpr_readback = 0; g_fail_lockdn_readback = 0; @@ -106,14 +105,12 @@ void pci_config_write32(uint8_t bus, uint8_t dev, uint8_t fun, /* Mocks for the MMIO accessors (src/x86/common.c). */ static void mmio_write32(uintptr_t address, uint32_t value) { - uint32_t *slot = (uint32_t *)&g_mmio[address - MMIO_BASE]; - - *slot = value; + g_mmio[(address - MMIO_BASE) / 4] = value; } static uint32_t mmio_read32(uintptr_t address) { - uint32_t val = *(uint32_t *)&g_mmio[address - MMIO_BASE]; + uint32_t val = g_mmio[(address - MMIO_BASE) / 4]; if (g_fail_fpr_readback && (address - MMIO_BASE) == TGL_FPR0_OFF) val &= ~SPI_FPR_WPE; @@ -141,11 +138,11 @@ START_TEST (test_lock_written_to_mmio){ ret = tgl_lock_bios_region(); ck_assert_int_eq(ret, 0); - ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); - ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + ck_assert_uint_eq(g_mmio[TGL_FPR0_OFF / 4], expected_fpr0); + ck_assert_uint_eq(g_mmio[TGL_SFSTS_CTL_OFF / 4] & SPI_FLOCKDN, SPI_FLOCKDN); /* FREG0 itself is not modified */ - ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FREG0_OFF], FREG0_INIT); + ck_assert_uint_eq(g_mmio[TGL_FREG0_OFF / 4], FREG0_INIT); /* config space: only the COMMAND register is written, and the * final write restores the original value */ for (i = 0; i < g_cfg_write_count; i++) @@ -169,8 +166,8 @@ START_TEST(test_hal_flash_protect_wires_lock) ret = hal_flash_protect(0xFFF00000, 0x600000); ck_assert_int_eq(ret, 0); - ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_FPR0_OFF], expected_fpr0); - ck_assert_uint_eq(*(uint32_t *)&g_mmio[TGL_SFSTS_CTL_OFF] & + ck_assert_uint_eq(g_mmio[TGL_FPR0_OFF / 4], expected_fpr0); + ck_assert_uint_eq(g_mmio[TGL_SFSTS_CTL_OFF / 4] & SPI_FLOCKDN, SPI_FLOCKDN); } END_TEST From 5e809031038a452a3313adfce13bfb569cb2375a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 11:54:50 +0200 Subject: [PATCH 13/18] F-12104: kontron tgl: correct the SPIBAR FREG/FPR register offsets PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that the register offsets introduced by the F-12104 fix are wrong, and the Linux kernel's Intel PCH SPI driver (drivers/spi/spi-intel.c) confirms it: FDATA(n) = 0x10 + 4n -> 0x48 is FDATA14, a scratch data register FRACC = 0x50 -> not FREG0 FREG(n) = 0x54 + 4n -> FREG0 = 0x54, FREG1 = 0x58 FPR0-4 = 0x84-0x9C (BXT/CNL protection-range base) Two consequences. First, the BIOS range source: Intel flash region numbering is region 0 = flash descriptor, region 1 = BIOS, and the kernel driver's partition code reflects that ("start from the mandatory descriptor region", then iterate FREG(1..)). The original pre-F-12104 code read FREG1 (0x58) for the BIOS range and was right on that point; the F-12104 fix regressed it to FREG0 (0x50), which is the FRACC register. Restore FREG1. Second, FPR0: the original 0x48 came from the buggy PCI-config-space write path and is a FDATA scratch register in the SPIBAR map, so the readback check passed on a register that never programs protection. FPR0 is 0x84 (BXT/CNL PR base; JSL is not in the kernel's platform table but follows the same-generation layout). HSFSTS_CTL at 0x04 and the RPE (bit 15) / WPE (bit 31) / base / limit fields match the kernel's PR_ definitions and are unchanged. unit-kontron-tgl-spi.c: mirror the corrected offsets (FREG0 decoy at 0x54, FREG1 BIOS source at 0x58, FPR0 at 0x84) and assert FPR0 carries the FREG1 range; the test runs the real extracted tgl_lock_bios_region(), so it fails if the HAL offsets drift. Verification: unit-kontron-tgl-spi 4/4, full unit suite green, sim build green, cstyle clean on changed hunks. --- hal/kontron_vx3060_s2.c | 22 +++++++++++------- tools/unit-tests/unit-kontron-tgl-spi.c | 31 +++++++++++++------------ 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/hal/kontron_vx3060_s2.c b/hal/kontron_vx3060_s2.c index 2211662979..fcdefab5cc 100644 --- a/hal/kontron_vx3060_s2.c +++ b/hal/kontron_vx3060_s2.c @@ -35,14 +35,18 @@ #define SPI_PCI_FUN 5 #define SPI_BAR_OFF 0x10 /* Tiger Lake SPI controller register offsets, memory-mapped at the - * BAR0 base. FREG0 holds the BIOS flash region base/limit; FPR0 is - * the protected range register with the same base/limit layout. */ -#define SPI_FREG0 0x50 + * BAR0 base. FREG1 holds the BIOS flash region base/limit (region 0 + * is the flash descriptor); FPR0 is the protected range register + * with the same base/limit layout. Offsets per the Intel PCH SPI + * register map (see drivers/spi/spi-intel.c in the Linux kernel): + * FDATA0-15 at 0x10-0x4C, FRACC at 0x50, FREG0-7 at 0x54-0x74, + * FPR0-4 at 0x84-0x9C. */ +#define SPI_FREG1 0x58 #define SPI_FREG_BASE_MASK (0x7fffU << 0) #define SPI_FREG_LIMIT_MASK (0x7fffU << 16) #define SPI_FREG_LIMIT_SHIFT (16) #define SPI_FREG_ADDR_SHIFT (12) -#define SPI_FPR0 (0x48) +#define SPI_FPR0 (0x84) #define SPI_FPR_WPE (1U << 31) #define SPI_FPR_RPE (1U << 15) #define SPI_BIOS_HSFSTS_CTL (0x4) @@ -67,10 +71,10 @@ int tgl_lock_bios_region() /* The Flash Protected Range register has the same base/limit * layout as the Flash Region register: take the BIOS region - * (FREG0) and enable read and write protection on it. The SPI - * registers live in the BAR's memory-mapped space, not in PCI - * configuration space. */ - reg = mmio_read32(spi_bar + SPI_FREG0); + * (FREG1, flash region 1) and enable read and write protection + * on it. The SPI registers live in the BAR's memory-mapped + * space, not in PCI configuration space. */ + reg = mmio_read32(spi_bar + SPI_FREG1); #if defined(DEBUG) bios_reg_base = (reg & SPI_FREG_BASE_MASK) << SPI_FREG_ADDR_SHIFT; bios_reg_lim = ((reg & SPI_FREG_LIMIT_MASK) >> SPI_FREG_LIMIT_SHIFT) @@ -104,7 +108,7 @@ int hal_flash_protect(haladdr_t address, int len) (void)len; /* The TGL BIOS region covers the bootloader partition, so the - * hook's address/len are the same range FREG0 describes. */ + * hook's address/len are the same range FREG1 describes. */ return tgl_lock_bios_region(); } diff --git a/tools/unit-tests/unit-kontron-tgl-spi.c b/tools/unit-tests/unit-kontron-tgl-spi.c index 65f34ba82e..13c984b526 100644 --- a/tools/unit-tests/unit-kontron-tgl-spi.c +++ b/tools/unit-tests/unit-kontron-tgl-spi.c @@ -4,9 +4,9 @@ * SPI BIOS-region lock was never applied - no hal_flash_protect() * override existed, so the weak no-op default ran before handoff - * and the only helper, tgl_lock_bios_region(), wrote the protected - * range and lock values through PCI configuration space (offsets - * 0x48/0x04) instead of the SPI BAR's memory-mapped register space, - * and took the range from FREG1 (non-BIOS) instead of FREG0 (BIOS). + * range and lock values through PCI configuration space instead of + * the SPI BAR's memory-mapped register space, using 0x48 for FPR0 + * (a FDATA scratch register in the SPI map, FPR0 lives at 0x84). * * The real function is extracted by the Makefile and run against * mocked PCI config space and an MMIO array at the BAR address. @@ -43,14 +43,15 @@ typedef uintptr_t haladdr_t; #include "kontron_spi_extract.h" /* TGL register offsets used by the model. */ -#define TGL_FREG0_OFF 0x50 +#define TGL_FREG0_OFF 0x54 #define TGL_FREG1_OFF 0x58 -#define TGL_FPR0_OFF 0x48 +#define TGL_FPR0_OFF 0x84 #define TGL_SFSTS_CTL_OFF 0x04 -/* FREG0: BIOS region, base/limit fields (14 bits each, shifted 12). - * FREG1: a different range, so a test that compares the FPR0 value - * against FREG0 also proves FREG1 was not the source. */ +/* FREG1: BIOS region (Intel flash region 1; region 0 is the flash + * descriptor), base/limit fields (15 bits each, shifted 12). FREG0: + * a different range, so a test that compares the FPR0 value against + * FREG1 also proves FREG0 was not the source. */ #define FREG0_INIT 0x7FFF7400U #define FREG1_INIT 0x3FFF0000U @@ -124,13 +125,13 @@ static uint32_t mmio_read32(uintptr_t address) * hal/kontron_vx3060_s2.c (extracted). */ #include "kontron_spi_fn_extract.h" -/* The lock must land in the MMIO space: FPR0 carries the FREG0 +/* The lock must land in the MMIO space: FPR0 carries the FREG1 * (BIOS region) base/limit with RPE/WPE set, FLOCKDN is set in * BIOS/H SFSTS/CTL, and PCI config space sees only the COMMAND - * enable/restore. Pre-fix the values went to config offsets 0x48 - * and 0x04 and FPR0 was never written. */ + * enable/restore. Pre-fix the values went to PCI config offsets + * 0x48 and 0x04 and FPR0 was never written. */ START_TEST (test_lock_written_to_mmio){ - uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + uint32_t expected_fpr0 = FREG1_INIT | SPI_FPR_RPE | SPI_FPR_WPE; int i, ret; int cmd_restored = 1; @@ -141,8 +142,8 @@ START_TEST (test_lock_written_to_mmio){ ck_assert_uint_eq(g_mmio[TGL_FPR0_OFF / 4], expected_fpr0); ck_assert_uint_eq(g_mmio[TGL_SFSTS_CTL_OFF / 4] & SPI_FLOCKDN, SPI_FLOCKDN); - /* FREG0 itself is not modified */ - ck_assert_uint_eq(g_mmio[TGL_FREG0_OFF / 4], FREG0_INIT); + /* FREG1 itself is not modified */ + ck_assert_uint_eq(g_mmio[TGL_FREG1_OFF / 4], FREG1_INIT); /* config space: only the COMMAND register is written, and the * final write restores the original value */ for (i = 0; i < g_cfg_write_count; i++) @@ -159,7 +160,7 @@ END_TEST * call actually establishes protection. */ START_TEST(test_hal_flash_protect_wires_lock) { - uint32_t expected_fpr0 = FREG0_INIT | SPI_FPR_RPE | SPI_FPR_WPE; + uint32_t expected_fpr0 = FREG1_INIT | SPI_FPR_RPE | SPI_FPR_WPE; int ret; sim_reset(); From 634d67904cec75c73e55fce9a1f2f02982328db0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 11:54:50 +0200 Subject: [PATCH 14/18] F-12061: unit test: keep the XIP fast-path source in the flash model PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that test_aligned_page_multiple_write_xip filled its 512-byte source at offset 3 * FLASH_PAGE_SIZE (768) of the 1024-byte flash model, so both the fixture and the HAL's XIP staging read 256 bytes past the modeled region; it only passed because mmap rounds the mapping up to a full page. Move the source to pages 2-3 (offset 2 * FLASH_PAGE_SIZE) with the destination at pages 0-1: non-overlapping, fully inside the model. Verification: unit-rp2350-flash-write 6/6, full unit suite green. --- tools/unit-tests/unit-rp2350-flash-write.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/unit-tests/unit-rp2350-flash-write.c b/tools/unit-tests/unit-rp2350-flash-write.c index 752d8112e2..438ab21c71 100644 --- a/tools/unit-tests/unit-rp2350-flash-write.c +++ b/tools/unit-tests/unit-rp2350-flash-write.c @@ -194,11 +194,11 @@ START_TEST(test_aligned_page_multiple_write_xip) int i; for (i = 0; i < 512; i++) - g_flash[3 * FLASH_PAGE_SIZE + i] = (uint8_t)(0x80 ^ (i & 0x3F)); + g_flash[2 * FLASH_PAGE_SIZE + i] = (uint8_t)(0x80 ^ (i & 0x3F)); ck_assert_int_eq(hal_flash_write((uint32_t)XIP_BASE, (uint8_t *)(XIP_BASE + - 3 * FLASH_PAGE_SIZE), + 2 * FLASH_PAGE_SIZE), 512), 0); ck_assert_int_eq(g_violations, 0); for (i = 0; i < 512; i++) From a504552883e5b14fb154e3c5e6dc35a0968d6df3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 12:19:13 +0200 Subject: [PATCH 15/18] unit test: match wolfBoot_get_dts_size mock to the 2-arg signature 95227f82 (fdt: rewrite device tree parser with capacity bound and full validation) changed wolfBoot_get_dts_size() to take a capacity argument but left the mock in unit-update-disk-fs.c with the old 1-arg signature, so the test no longer compiles (conflicting types). Update the mock; behavior is unchanged (always -1, no DTS in this test). --- tools/unit-tests/unit-update-disk-fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-update-disk-fs.c b/tools/unit-tests/unit-update-disk-fs.c index 287b23eb13..98f06ac8f5 100644 --- a/tools/unit-tests/unit-update-disk-fs.c +++ b/tools/unit-tests/unit-update-disk-fs.c @@ -682,9 +682,10 @@ uint32_t wolfBoot_get_blob_version(uint8_t *blob) return version; } -int wolfBoot_get_dts_size(void *dts_addr) +int wolfBoot_get_dts_size(void *dts_addr, uint32_t capacity) { (void)dts_addr; + (void)capacity; return -1; } From fa0c329a02926b54171a4adc5b205cf876aa062a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 13:17:48 +0200 Subject: [PATCH 16/18] F-12064: p1021: drop already-delivered pages of a bad block When the bad-block marker is found on the block's second page, the first page has already been copied to the caller's buffer and counted in pos; the skip advanced only the source address, so the bad block's page stayed in the output and the read returned len with the bad block's content mixed into the image. Record the output position at the start of each erase block and, on a bad block, rewind both pos and the data pointer to it before advancing the source address. This preserves the data = original + pos invariant (no out-of-bounds write) and discards the bad block's delivered pages. test_bad_marker_second_page_dropped now asserts the bad block is fully skipped (output starts at the next block's first page); it fails against the previous code. --- hal/nxp_p1021.c | 21 ++++++++--- tools/unit-tests/unit-p1021-read-badblock.c | 39 ++++++++++++--------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/hal/nxp_p1021.c b/hal/nxp_p1021.c index b0cf9036a2..774be3bc26 100644 --- a/hal/nxp_p1021.c +++ b/hal/nxp_p1021.c @@ -1706,6 +1706,8 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) uint32_t block_size, page_size, read_size; int ret = 0, pos = 0, i = 0; int bad_marker; + uint8_t *block_start_data; + int block_start_pos; #ifdef DEBUG_EXT_FLASH wolfBoot_printf("ext read: addr 0x%x, dst 0x%x, len %d\n", @@ -1739,8 +1741,13 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) /* total download loop */ while (pos < len) { /* the bad-block marker only exists on the first pages of each - * erase block: restart the per-block page counter */ + * erase block: restart the per-block page counter. Record the + * output position at the start of the block so that, if the + * block turns out to be bad, the pages already copied from it + * can be discarded. */ i = 0; + block_start_data = data; + block_start_pos = pos; /* block loop */ do { @@ -1769,10 +1776,14 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) /* check for bad page. if either of the first two pages are bad then * skip to next block */ if (i++ < 2 && flash_buf[bad_marker] != 0xFF) { - /* skip block: the bad block's bytes are not delivered - * and the read continues at the next block. pos and - * data already agree (data = original + pos), so only - * the source address moves. */ + /* bad block: discard the pages already copied from it + * (the marker is only checked on the first two pages, so + * a page may have been delivered before detection) and + * continue at the next block. Rewind pos and data to the + * block start (data = original + pos is preserved) and + * move the source address past the bad block. */ + pos = block_start_pos; + data = block_start_data; address = (address + block_size) & ~(block_size - 1); break; } diff --git a/tools/unit-tests/unit-p1021-read-badblock.c b/tools/unit-tests/unit-p1021-read-badblock.c index 317b66369e..b0948c7cee 100644 --- a/tools/unit-tests/unit-p1021-read-badblock.c +++ b/tools/unit-tests/unit-p1021-read-badblock.c @@ -1,12 +1,15 @@ /* unit-p1021-read-badblock.c * - * Regression test for F-12064: ext_flash_read() in hal/nxp_p1021.c - * kept its bad-block page counter for the whole request, so the - * marker was inspected only on the first two pages read and a bad - * block later in the request was copied as valid data. When a - * marker did cause a skip, the logical position was rewound to a - * block boundary while the output pointer was not, so the read - * continued past the end of the caller's buffer. + * Regression test for F-12064 (and the follow-up bad-block skip + * review): ext_flash_read() in hal/nxp_p1021.c kept its bad-block + * page counter for the whole request, so the marker was inspected + * only on the first two pages read and a bad block later in the + * request was copied as valid data. When a marker did cause a skip, + * the logical position was rewound to a block boundary while the + * output pointer was not, so the read continued past the end of the + * caller's buffer. A related defect kept the pages of a bad block + * that were already delivered before the marker was found on its + * second page; those pages must be discarded as well. * * The real function is extracted by the Makefile together with the * ELBC register macros it uses; the ELBC register access and the @@ -157,12 +160,12 @@ START_TEST (test_bad_block_in_later_block_skipped){ } END_TEST -/* Bad marker on the second page of the first block: one page was - * already delivered when the skip fires. Post-fix the output - * pointer and the position stay consistent and nothing is written - * past the buffer; pre-fix the position was rewound while the - * pointer was not, overflowing the buffer by one page. */ -START_TEST(test_bad_marker_second_page_no_overflow) +/* Bad marker on the second page of the first block: page 0 was + * already delivered when the skip fires, so it must be discarded + * along with the rest of the bad block. The output is the first two + * pages of block 1, nothing is written past the buffer, and the + * return value is still the full requested length. */ +START_TEST(test_bad_marker_second_page_dropped) { uint8_t out[1024 + 64]; int ret, i; @@ -175,9 +178,11 @@ START_TEST(test_bad_marker_second_page_no_overflow) ret = ext_flash_read(0, out, 1024); ck_assert_int_eq(ret, 1024); - /* page 0 of block 0, then block 1 from its first page */ - ck_assert_int_eq(memcmp(out, g_nand[0], SIM_PAGE_SIZE), 0); - ck_assert_int_eq(memcmp(out + SIM_PAGE_SIZE, g_nand[SIM_BLOCK_PAGES], + /* block 0 fully skipped: output starts at block 1 page 0 */ + ck_assert_int_eq(memcmp(out, g_nand[SIM_BLOCK_PAGES], + SIM_PAGE_SIZE), 0); + ck_assert_int_eq(memcmp(out + SIM_PAGE_SIZE, + g_nand[SIM_BLOCK_PAGES + 1], SIM_PAGE_SIZE), 0); for (i = 1024; i < (int)sizeof(out); i++) ck_assert_uint_eq(out[i], 0xEE); @@ -253,7 +258,7 @@ Suite *p1021_read_badblock_suite(void) TCase *tc = tcase_create("bad-block"); tcase_add_test(tc, test_bad_block_in_later_block_skipped); - tcase_add_test(tc, test_bad_marker_second_page_no_overflow); + tcase_add_test(tc, test_bad_marker_second_page_dropped); tcase_add_test(tc, test_bad_first_block_page0); tcase_add_test(tc, test_all_good_two_blocks); tcase_add_test(tc, test_unaligned_start_across_pages); From 80e92ea7286f8df90be294319e305d60eafa8b1b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 2 Sep 2026 18:21:54 +0200 Subject: [PATCH 17/18] F-12065: update_ram: type-safe short-read check for -Wsign-compare The F-12065 fix compared the int return of ext_flash_read() directly against the uint32_t os_image.fw_size. That int-vs-unsigned comparison triggers -Wsign-compare, which is a hard error under the default -Werror -Wextra for every EXT_FLASH+NO_XIP update_ram target (e.g. zynqmp) at -O0 and on host x86_64 gcc. Check the error range explicitly and cast for the size comparison, matching the established pattern in src/disk_fs.c (ret < 0 check followed by (uint32_t)ret != len). Semantics are unchanged: negative returns and positive short reads are both rejected. Verified: host gcc -Werror -Wextra -fsyntax-only warns on the old line and is clean on this one; unit-update-ram-noramboot 3/3. --- src/update_ram.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/update_ram.c b/src/update_ram.c index d9f1f3d344..4b55dec581 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -575,8 +575,10 @@ void RAMFUNCTION wolfBoot_start(void) ret = ext_flash_read((uintptr_t)os_image.fw_base, (uint8_t*)load_address, os_image.fw_size); /* Backends return the number of bytes read: a positive short read - * leaves a truncated image in RAM, so require the full size. */ - if (ret != os_image.fw_size) { + * leaves a truncated image in RAM, so require the full size. + * ret is int, fw_size uint32_t: check the error range first and + * cast for the size comparison to keep -Wsign-compare quiet. */ + if (ret < 0 || (uint32_t)ret != os_image.fw_size) { wolfBoot_printf("Error loading image at %p (ret %d)\n", os_image.fw_base, ret); return; From 931d2246522d6ffcdb88e5b3e1d6576b3413a6b2 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 3 Sep 2026 08:54:51 +0200 Subject: [PATCH 18/18] F-12065: unit test: clear the short-read mock before the assertion PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that test_noramboot_ext_flash_short_read_rejected set the shared mock globals mock_ext_flash_short_len/mock_ext_flash_short_bytes and only cleared them after the ck_assert. The suite runs CK_NOFORK, so a failing assertion longjmps out of the test and leaves every full-size ext_flash_read truncated for test_noramboot_highversion_rollback_denied, which then fails for an unrelated reason. Clear the mock globals immediately after wolfBoot_start() returns, before the assertion: the assert only checks wolfBoot_staged_ok, which is set during wolfBoot_start(), so the ordering changes nothing about what is tested and the mock state can no longer leak into the next test on a failure path. Verification: full unit suite in wolfboot-ci-sim (make -C tools/unit-tests; make run) exit 0; unit-update-ram-noramboot 3/3. --- tools/unit-tests/unit-update-ram-noramboot.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-update-ram-noramboot.c b/tools/unit-tests/unit-update-ram-noramboot.c index 88635b8cba..b8dca806ef 100644 --- a/tools/unit-tests/unit-update-ram-noramboot.c +++ b/tools/unit-tests/unit-update-ram-noramboot.c @@ -233,9 +233,12 @@ START_TEST (test_noramboot_ext_flash_short_read_rejected) { wolfBoot_start(); - ck_assert_int_eq(wolfBoot_staged_ok, 0); + /* Clear the short-read mock before asserting: the suite runs CK_NOFORK, + * so a failing ck_assert longjmps past any cleanup below and would leave + * every full-size ext_flash_read truncated for the next test. */ mock_ext_flash_short_len = 0; mock_ext_flash_short_bytes = 0; + ck_assert_int_eq(wolfBoot_staged_ok, 0); cleanup_flash(); } END_TEST