From 228c9d6d8028d41a8d588db5b466a0fe956a1830 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:03:24 +0200 Subject: [PATCH 01/23] F-6407: zeroize static cached_sector after each flash commit in psa_store cache_commit() is the single choke point through which every psa_store helper (create_object, delete_object, wolfPSA_Store_Write's sector loop, update_store_size, erase_object_payload, check_vault) flushes the static cached_sector staging buffer to flash. The buffer was never cleared, so key-object plaintext staged there could remain resident in static SRAM after the store operation returned. Wipe cached_sector with wc_ForceZero right after the flash write completes, mirroring the existing pattern in src/wolfhsm_flash_hal.c. --- src/psa_store.c | 5 +++++ tools/unit-tests/unit-psa_store.c | 27 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/psa_store.c b/src/psa_store.c index 5be5570831..5c3daaad5a 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -184,6 +184,11 @@ static void cache_commit(uint32_t offset) hal_flash_write((uintptr_t)vault_base + offset, cached_sector, WOLFBOOT_SECTOR_SIZE); hal_flash_lock(); + + /* cached_sector may hold key-object plaintext; every caller commits it + * to flash and then either overwrites it before reuse or returns, so + * it is safe to wipe it here right after the last use. */ + wc_ForceZero(cached_sector, sizeof(cached_sector)); } static void restore_backup(uint32_t offset) diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index e66261b8d3..b6bc280023 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -266,6 +266,30 @@ START_TEST(test_shorter_overwrite_clears_tail) } END_TEST +START_TEST(test_cache_commit_zeroizes_cached_sector) +{ + int ret; + int i; + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xFF, keyvault_size); + + /* Simulate cached_sector staging key-object plaintext, as every + * caller of cache_commit() does before committing to flash. */ + memset(cached_sector, 0x5A, sizeof(cached_sector)); + + cache_commit(0); + + for (i = 0; i < (int)sizeof(cached_sector); i++) { + ck_assert_msg(cached_sector[i] == 0, + "cached_sector retains stale data at offset %d: 0x%02x", + i, cached_sector[i]); + } +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); @@ -275,6 +299,7 @@ Suite *wolfboot_suite(void) TCase *tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase *tcase_find_bounds = tcase_create("find_bounds"); TCase *tcase_tail = tcase_create("shorter_overwrite_clears_tail"); + TCase *tcase_zeroize = tcase_create("cache_commit_zeroizes_cached_sector"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -282,12 +307,14 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); tcase_add_test(tcase_tail, test_shorter_overwrite_clears_tail); + tcase_add_test(tcase_zeroize, test_cache_commit_zeroizes_cached_sector); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_tail); + suite_add_tcase(s, tcase_zeroize); return s; } From 60acdc231ec491d11600e9eeb851f26b25232c91 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:10:44 +0200 Subject: [PATCH 02/23] F-6402: add negative tests pinning wolfBoot_check_rot digest comparison Add tests asserting rejection on a mismatched pubkey hint and on a short digestSz from wolfTPM2_NVReadAuth, so a weakened comparison (e.g. == 0 -> >= 0, or a dropped clause) is caught. --- tools/unit-tests/unit-tpm-rsa-exp.c | 34 ++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-tpm-rsa-exp.c b/tools/unit-tests/unit-tpm-rsa-exp.c index a66c2d358e..f5909d55e8 100644 --- a/tools/unit-tests/unit-tpm-rsa-exp.c +++ b/tools/unit-tests/unit-tpm-rsa-exp.c @@ -29,6 +29,7 @@ static uint8_t test_exponent_der[] = { 0xAA, 0x01, 0x00, 0x01, 0x7B }; static uint8_t test_nv_digest[WOLFBOOT_SHA_DIGEST_SIZE]; static uint32_t captured_exponent; static int forbidden_memcmp_calls; +static uint32_t mock_nv_digest_sz; int keyslot_id_by_sha(const uint8_t* pubkey_hint) { @@ -127,7 +128,7 @@ int wolfTPM2_NVReadAuth(WOLFTPM2_DEV* dev, WOLFTPM2_NV* nv, (void)offset; ck_assert_uint_eq(*pDataSz, WOLFBOOT_SHA_DIGEST_SIZE); memcpy(dataBuf, test_nv_digest, WOLFBOOT_SHA_DIGEST_SIZE); - *pDataSz = WOLFBOOT_SHA_DIGEST_SIZE; + *pDataSz = mock_nv_digest_sz; return 0; } @@ -173,6 +174,7 @@ static void setup(void) memset(test_nv_digest, 0x7C, sizeof(test_nv_digest)); captured_exponent = 0; forbidden_memcmp_calls = 0; + mock_nv_digest_sz = WOLFBOOT_SHA_DIGEST_SIZE; } START_TEST(test_wolfBoot_load_pubkey_decodes_der_exponent_bytes) @@ -206,6 +208,34 @@ START_TEST(test_wolfBoot_check_rot_avoids_memcmp_on_digest_compare) } END_TEST +START_TEST(test_wolfBoot_check_rot_rejects_mismatched_digest) +{ + uint8_t hint[WOLFBOOT_SHA_DIGEST_SIZE]; + int rc; + + memcpy(hint, test_nv_digest, sizeof(hint)); + hint[0] ^= 0x01; + + rc = wolfBoot_check_rot(0, hint); + + ck_assert_int_ne(rc, 0); +} +END_TEST + +START_TEST(test_wolfBoot_check_rot_rejects_wrong_digest_size) +{ + uint8_t hint[WOLFBOOT_SHA_DIGEST_SIZE]; + int rc; + + memcpy(hint, test_nv_digest, sizeof(hint)); + mock_nv_digest_sz = WOLFBOOT_SHA_DIGEST_SIZE - 1; + + rc = wolfBoot_check_rot(0, hint); + + ck_assert_int_ne(rc, 0); +} +END_TEST + static Suite *tpm_suite(void) { Suite *s; @@ -216,6 +246,8 @@ static Suite *tpm_suite(void) tcase_add_checked_fixture(tc, setup, NULL); tcase_add_test(tc, test_wolfBoot_load_pubkey_decodes_der_exponent_bytes); tcase_add_test(tc, test_wolfBoot_check_rot_avoids_memcmp_on_digest_compare); + tcase_add_test(tc, test_wolfBoot_check_rot_rejects_mismatched_digest); + tcase_add_test(tc, test_wolfBoot_check_rot_rejects_wrong_digest_size); suite_add_tcase(s, tc); return s; } From 20f11ae854d610a31de293dca4344f3a4ff34909 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:18:03 +0200 Subject: [PATCH 03/23] F-6129: zeroize disk-unlock secret before the state-mismatch panic in sata_unlock_disk sata_unlock_disk() calls panic() directly when the post-unlock ATA security state doesn't match the expected SEC5/SEC6, bypassing the cleanup: label that zeroizes the plaintext secret[] stack buffer. On x86, panic() is an infinite hlt() loop, so the plaintext disk-unlock secret (TPM-unsealed key or password) remains readable in DRAM for as long as the device stays halted. Zeroize secret[] before panic() on this path, mirroring the existing cleanup zeroization. --- .gitignore | 1 + src/x86/ahci.c | 1 + tools/unit-tests/Makefile | 5 + tools/unit-tests/unit-ahci-unlock-panic.c | 153 ++++++++++++++++++++++ 4 files changed, 160 insertions(+) create mode 100644 tools/unit-tests/unit-ahci-unlock-panic.c diff --git a/.gitignore b/.gitignore index 0cc97322c4..4e58cb9214 100644 --- a/.gitignore +++ b/.gitignore @@ -210,6 +210,7 @@ tools/unit-tests/unit-flash-erase-u3 tools/unit-tests/unit-flash-erase-wb tools/unit-tests/unit-fwtpm-nv-oob tools/unit-tests/unit-x86-paging-oob +tools/unit-tests/unit-ahci-unlock-panic diff --git a/src/x86/ahci.c b/src/x86/ahci.c index bb87a7326e..b7bef12968 100644 --- a/src/x86/ahci.c +++ b/src/x86/ahci.c @@ -504,6 +504,7 @@ int sata_unlock_disk(int drv, int freeze) if ((freeze && ata_st != ATA_SEC6) || (!freeze && ata_st != ATA_SEC5)) { AHCI_DEBUG_PRINTF("ATA: Security is not enabled/locked (State SEC%d)\r\n", ata_st); + ahci_secret_zeroize(secret, sizeof(secret)); panic(); } AHCI_DEBUG_PRINTF("ATA: Security enabled. State SEC%d\r\n", ata_st); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index bd83eb9fca..7bec24b9d5 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -75,6 +75,7 @@ TESTS+=unit-flash-erase-c0 TESTS+=unit-flash-erase-u3 TESTS+=unit-otp-keystore TESTS+=unit-x86-paging-oob +TESTS+=unit-ahci-unlock-panic TESTS+=unit-fwtpm-nv-oob TESTS+=unit-elf-bss-guard TESTS+=unit-arm-tee-psa-ipc @@ -564,6 +565,10 @@ unit-x86-paging-oob: ../../include/target.h unit-x86-paging-oob.c gcc -o $@ unit-x86-paging-oob.c $(CFLAGS) \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-ahci-unlock-panic: ../../include/target.h unit-ahci-unlock-panic.c + gcc -o $@ unit-ahci-unlock-panic.c $(CFLAGS) \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-elf-bss-guard: unit-elf-bss-guard.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) diff --git a/tools/unit-tests/unit-ahci-unlock-panic.c b/tools/unit-tests/unit-ahci-unlock-panic.c new file mode 100644 index 0000000000..e49273b306 --- /dev/null +++ b/tools/unit-tests/unit-ahci-unlock-panic.c @@ -0,0 +1,153 @@ +/* unit-ahci-unlock-panic.c + * + * Regression test for sata_unlock_disk() leaking the plaintext disk-unlock + * secret on the stack when the post-unlock security-state check fails. + * That path calls panic() directly, bypassing the cleanup: label that + * zeroizes the secret buffer, so the secret survives on the stack for as + * long as the (real) panic() halt loop keeps the DRAM contents alive. + */ + +#include +#include +#include +#include +#include + +#define WOLFBOOT_ATA_DISK_LOCK +#define WOLFBOOT_ATA_DISK_LOCK_PASSWORD "unit-test-secret" + +static jmp_buf panic_jmp; +static int panic_count = 0; + +/* Captured by the ata_security_unlock_device() mock below: the address and + * length of sata_unlock_disk()'s local `secret` buffer. panic() runs + * synchronously from inside sata_unlock_disk() (before any stack unwinding), + * so reading through this pointer from panic() observes the exact stack + * contents an attacker halting the device at that point would see. */ +static const uint8_t *mock_secret_ptr; +static size_t mock_secret_len; +static uint8_t panic_secret_snapshot[64]; +static size_t panic_secret_snapshot_len; + +/* Satisfies __attribute__((noreturn)) via longjmp, like unit-x86-paging-oob.c + * does for the same reason: allows the test to resume after a panic() call + * without executing any code that follows it. */ +__attribute__((noreturn)) void panic(void) +{ + panic_count++; + if (mock_secret_ptr != NULL && mock_secret_len <= sizeof(panic_secret_snapshot)) { + memcpy(panic_secret_snapshot, mock_secret_ptr, mock_secret_len); + panic_secret_snapshot_len = mock_secret_len; + } + longjmp(panic_jmp, 1); +} + +/* Mocked ATA security layer: ahci.c calls these as extern functions + * (defined in src/x86/ata.c, not included here) to drive the security + * state machine. Controlled by the test to reach the state-mismatch path + * at the end of sata_unlock_disk() without any real hardware. */ +static enum ata_security_state mock_states[8]; +static int mock_states_idx; +static int mock_unlock_ret; +static int mock_identify_ret; +static int mock_freeze_ret; + +enum ata_security_state ata_security_get_state(int drv) +{ + (void)drv; + ck_assert_int_lt(mock_states_idx, 8); + return mock_states[mock_states_idx++]; +} + +int ata_security_unlock_device(int drv, const char *passphrase, int master) +{ + (void)drv; (void)master; + mock_secret_ptr = (const uint8_t *)passphrase; + mock_secret_len = strlen(WOLFBOOT_ATA_DISK_LOCK_PASSWORD); + return mock_unlock_ret; +} + +int ata_identify_device(int drv) +{ + (void)drv; + return mock_identify_ret; +} + +int ata_security_freeze_lock(int drv) +{ + (void)drv; + return mock_freeze_ret; +} + +int ata_security_set_password(int drv, int master, const char *passphrase) +{ + (void)drv; (void)master; (void)passphrase; + return 0; +} + +#include "../../src/x86/ahci.c" + +static void reset_mocks(void) +{ + memset(mock_states, 0, sizeof(mock_states)); + mock_states_idx = 0; + mock_unlock_ret = 0; + mock_identify_ret = 0; + mock_freeze_ret = 0; + mock_secret_ptr = NULL; + mock_secret_len = 0; + memset(panic_secret_snapshot, 0, sizeof(panic_secret_snapshot)); + panic_secret_snapshot_len = 0; + panic_count = 0; +} + +/* Drive under ATA_SEC4 (locked), unlock command succeeds, but the drive + * never actually reaches ATA_SEC5/SEC6 (e.g. a faulty/hostile drive that + * acknowledges the unlock command without changing state). This forces + * sata_unlock_disk() into the state-mismatch branch that calls panic() + * directly at src/x86/ahci.c:507, per the report's description: "This path + * is reachable when the drive does not reach the expected SEC5/SEC6 + * security state after a successful secret retrieval." */ +START_TEST(test_unlock_zeroizes_secret_before_state_mismatch_panic) +{ + int r; + + reset_mocks(); + mock_states[0] = ATA_SEC4; /* initial state: locked */ + mock_states[1] = ATA_SEC4; /* state after unlock+identify: still locked */ + mock_states[2] = ATA_SEC4; /* final check: still not SEC6 with freeze=1 */ + + if (setjmp(panic_jmp) == 0) { + r = sata_unlock_disk(0, 1); + ck_abort_msg("sata_unlock_disk returned %d instead of panicking " + "on state mismatch", r); + } + + ck_assert_int_eq(panic_count, 1); + ck_assert_uint_eq(panic_secret_snapshot_len, strlen(WOLFBOOT_ATA_DISK_LOCK_PASSWORD)); + for (r = 0; r < (int)panic_secret_snapshot_len; r++) { + ck_assert_msg(panic_secret_snapshot[r] == 0, + "plaintext unlock secret still on the stack at panic(): " + "byte %d = 0x%02x", r, panic_secret_snapshot[r]); + } +} +END_TEST + +static Suite *ahci_unlock_panic_suite(void) +{ + Suite *s = suite_create("ahci_unlock_panic"); + TCase *tc = tcase_create("state_mismatch_zeroize"); + tcase_add_test(tc, test_unlock_zeroizes_secret_before_state_mismatch_panic); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite *s = ahci_unlock_panic_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + int failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From cd4e4be2f654af69c9a215608c77b3a6f6cd30fd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:27:04 +0200 Subject: [PATCH 04/23] F-5968: zeroize disk-unlock passphrase from static ATA command buffer security_command_passphrase() copies the plaintext disk-unlock secret into the file-static DMA buffer `buffer` at ATA_SECURITY_PASSWORD_OFFSET but never wipes it. On the slot<0 early return no ATA command is even dispatched, and on failures of ata_security_set_password()/ ata_security_unlock_device() (which sata_unlock_disk() turns into a panic()) no later IDENTIFY DMA ever overwrites it, so the secret is left resident in BSS for as long as the device stays powered. Zeroize the password field on the slot<0 path and after synchronous command completion. The async path (used only by the currently unreachable ata_security_erase_unit()) is left untouched because the HBA may still be DMAing out of the buffer when exec_cmd_slot_ex() returns ATA_ERR_BUSY; wiping it there would race the transfer. --- .gitignore | 1 + src/x86/ata.c | 22 +++ tools/unit-tests/Makefile | 5 + .../unit-ata-security-passphrase-zeroize.c | 142 ++++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 tools/unit-tests/unit-ata-security-passphrase-zeroize.c diff --git a/.gitignore b/.gitignore index 4e58cb9214..bca5feab91 100644 --- a/.gitignore +++ b/.gitignore @@ -211,6 +211,7 @@ tools/unit-tests/unit-flash-erase-wb tools/unit-tests/unit-fwtpm-nv-oob tools/unit-tests/unit-x86-paging-oob tools/unit-tests/unit-ahci-unlock-panic +tools/unit-tests/unit-ata-security-passphrase-zeroize diff --git a/src/x86/ata.c b/src/x86/ata.c index bb6dd1fae2..5858c4d394 100644 --- a/src/x86/ata.c +++ b/src/x86/ata.c @@ -433,6 +433,19 @@ static int security_command(int drv, uint8_t ata_cmd) return ret; } +/** + * @brief Wipe the plaintext passphrase bytes out of the static command + * buffer once the HBA no longer needs them. + */ +static void ata_security_buffer_zeroize(void) +{ + volatile uint8_t *p = (volatile uint8_t *)buffer + ATA_SECURITY_PASSWORD_OFFSET; + size_t len = ATA_SECURITY_PASSWORD_LEN; + while (len-- > 0U) { + *p++ = 0U; + } +} + /** * @brief Helper function to execute an ATA command from the security set that * require transmitting a passphrase. @@ -471,6 +484,7 @@ static int security_command_passphrase(int drv, uint8_t ata_cmd, memcpy(buffer + ATA_SECURITY_PASSWORD_OFFSET, passphrase, passphrase_len); if (slot < 0) { + ata_security_buffer_zeroize(); return slot; } cmd = (struct hba_cmd_header *)(uintptr_t)ata->clb_port; @@ -482,6 +496,14 @@ static int security_command_passphrase(int drv, uint8_t ata_cmd, cmdfis->command = ata_cmd; cmdfis->count = 1; ret = exec_cmd_slot_ex(drv, slot, async); + /* exec_cmd_slot_ex() only returns once the HBA has finished the DMA + * transfer of this buffer when running synchronously (async = 0), so + * it is safe to wipe the passphrase here. In async mode the transfer + * may still be in flight when we return (the caller polls completion + * via ata_cmd_complete_async()), so clearing the buffer now would race + * the HBA and could corrupt the command still in progress. */ + if (!async) + ata_security_buffer_zeroize(); return ret; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 7bec24b9d5..352e179687 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -76,6 +76,7 @@ TESTS+=unit-flash-erase-u3 TESTS+=unit-otp-keystore TESTS+=unit-x86-paging-oob TESTS+=unit-ahci-unlock-panic +TESTS+=unit-ata-security-passphrase-zeroize TESTS+=unit-fwtpm-nv-oob TESTS+=unit-elf-bss-guard TESTS+=unit-arm-tee-psa-ipc @@ -569,6 +570,10 @@ unit-ahci-unlock-panic: ../../include/target.h unit-ahci-unlock-panic.c gcc -o $@ unit-ahci-unlock-panic.c $(CFLAGS) \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-passphrase-zeroize.c + gcc -o $@ unit-ata-security-passphrase-zeroize.c $(CFLAGS) \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-elf-bss-guard: unit-elf-bss-guard.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) diff --git a/tools/unit-tests/unit-ata-security-passphrase-zeroize.c b/tools/unit-tests/unit-ata-security-passphrase-zeroize.c new file mode 100644 index 0000000000..f0df0d6c68 --- /dev/null +++ b/tools/unit-tests/unit-ata-security-passphrase-zeroize.c @@ -0,0 +1,142 @@ +/* unit-ata-security-passphrase-zeroize.c + * + * Regression test for security_command_passphrase() leaving the plaintext + * disk-unlock passphrase resident in the file-static ATA command DMA buffer + * ("buffer" in src/x86/ata.c) after it returns, instead of wiping it like + * ahci.c does for its own copies of the same secret (ahci_secret_zeroize()). + * Exercised through the public ata_security_unlock_device() wrapper, which + * is exactly how sata_unlock_disk() reaches it. + */ + +#include +#include +#include +#include +#include + +#define WOLFBOOT_ATA_DISK_LOCK + +/* Mocked AHCI port registers. security_command_passphrase() reaches these + * only through find_cmd_slot() (SACT/CI, to allocate a slot) and + * exec_cmd_slot_ex() (TFD/IS/CI, to wait for command completion). An + * always-idle model is enough to drive a real command to synchronous + * completion without simulating actual AHCI hardware; mock_slots_full + * additionally lets a test force prepare_cmd_h2d_slot() down its "no free + * slot" error path. */ +static int mock_slots_full; + +uint32_t mmio_read32(uintptr_t address) +{ + (void)address; + return mock_slots_full ? 0xFFFFFFFF : 0; +} + +void mmio_write32(uintptr_t address, uint32_t value) +{ + (void)address; + (void)value; +} + +void panic(void) +{ + ck_abort_msg("panic!"); +} + +#include "../../src/x86/ata.c" + +/* struct ata_drive stores clb_port/ctable_port as uint32_t "physical" + * addresses, matching the real x86 target's 32-bit DMA pointers. MAP_32BIT + * keeps these allocations inside that range on a 64-bit test host so the + * truncating uint32_t assignment below doesn't lose address bits. */ +static uint8_t *clb_mem; +static uint8_t *ctable_mem; + +static void setup(void) +{ + mock_slots_full = 0; + clb_mem = mmap(NULL, sizeof(struct hba_cmd_header) * 32, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(clb_mem, MAP_FAILED); + ctable_mem = mmap(NULL, sizeof(struct hba_cmd_table), + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(ctable_mem, MAP_FAILED); + + memset(&ATA_Drv[0], 0, sizeof(ATA_Drv[0])); + ATA_Drv[0].ahci_base = 0x10000; /* arbitrary: mmio_* mocks ignore it */ + ATA_Drv[0].ahci_port = 0; + ATA_Drv[0].clb_port = (uint32_t)(uintptr_t)clb_mem; + ATA_Drv[0].ctable_port = (uint32_t)(uintptr_t)ctable_mem; +} + +static void teardown(void) +{ + munmap(clb_mem, sizeof(struct hba_cmd_header) * 32); + munmap(ctable_mem, sizeof(struct hba_cmd_table)); +} + +static void assert_password_field_zero(const char *ctx) +{ + int i; + for (i = 0; i < ATA_SECURITY_PASSWORD_LEN; i++) { + ck_assert_msg(buffer[ATA_SECURITY_PASSWORD_OFFSET + i] == 0, + "%s: plaintext passphrase still resident in static ATA command " + "buffer: byte %d = 0x%02x", ctx, i, + buffer[ATA_SECURITY_PASSWORD_OFFSET + i]); + } +} + +/* Reachable path taken every time sata_unlock_disk() unlocks a drive + * (ata_st == ATA_SEC4): the command dispatches and completes synchronously. + * Per the report, nothing overwrites the password bytes on return other + * than the next unrelated command that happens to reuse the buffer. */ +START_TEST(test_unlock_zeroizes_passphrase_after_command_completes) +{ + static const char passphrase[] = "unit-test-disk-secret"; + int r; + + r = ata_security_unlock_device(0, passphrase, 0); + ck_assert_int_eq(r, 0); + + assert_password_field_zero("after successful SECURITY UNLOCK"); +} +END_TEST + +/* Reachable when the HBA has no free command slot: security_command_passphrase() + * still memcpy()s the passphrase into the static buffer before checking + * `slot < 0`, so the secret is written even though no ATA command is ever + * dispatched. */ +START_TEST(test_unlock_zeroizes_passphrase_on_no_free_slot) +{ + static const char passphrase[] = "unit-test-disk-secret"; + int r; + + mock_slots_full = 1; + r = ata_security_unlock_device(0, passphrase, 0); + ck_assert_int_eq(r, -1); + + assert_password_field_zero("after no-free-slot error return"); +} +END_TEST + +static Suite *ata_security_passphrase_zeroize_suite(void) +{ + Suite *s = suite_create("ata_security_passphrase_zeroize"); + TCase *tc = tcase_create("zeroize"); + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_unlock_zeroizes_passphrase_after_command_completes); + tcase_add_test(tc, test_unlock_zeroizes_passphrase_on_no_free_slot); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite *s = ata_security_passphrase_zeroize_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + int failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From 9ce750fdc6f6e1fc752dd30c537de364c7c45b75 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:35:07 +0200 Subject: [PATCH 05/23] F-5965: skip wb_diff match candidates whose offset MSB collides with ESC A matched block's source offset is encoded as off[0..2] right after the ESC (0x7f) header marker. When the match offset falls in [0x7f0000, 0x7fffff], off[0] is 0x7f, so the header begins with ESC ESC -- the same two bytes wb_patch uses to decode an escaped literal 0x7f. The decoder then emits a single literal byte, consumes only 2 of the 6 header bytes, and desyncs the rest of the stream, breaking the wb_patch(wb_diff(A,B)) == B roundtrip for base images >= ~8MB (a supported MMU/Linux delta-update configuration). Make wb_diff skip any candidate match whose offset's most-significant byte equals ESC, in both the forward (base-image) and backward (previously-patched-image) search paths, so the ambiguous header is never produced; the position falls back to literal encoding instead. --- src/delta.c | 16 ++++++++++++++++ tools/unit-tests/unit-delta.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/delta.c b/src/delta.c index 236a6c09f5..67c44e5f6f 100644 --- a/src/delta.c +++ b/src/delta.c @@ -303,6 +303,15 @@ int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len) blk_start = pa - ctx->src_a; if (blk_start > BLOCK_OFF_MAX) return -1; + if (((blk_start >> 16) & 0xFF) == ESC) { + /* The most-significant offset byte would collide with + * the escaped-literal marker (ESC ESC), making the + * header indistinguishable from a literal ESC byte on + * decode. Skip this candidate and keep searching. + */ + pa++; + continue; + } b_start = ctx->off_b; pa+= BLOCK_HDR_SIZE; ctx->off_b += BLOCK_HDR_SIZE; @@ -367,6 +376,13 @@ int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len) blk_start = pb - ctx->src_b; if (blk_start > BLOCK_OFF_MAX) return -1; + if (((blk_start >> 16) & 0xFF) == ESC) { + /* Same ESC-collision hazard as the forward-match + * path: skip this candidate and keep searching. + */ + pb++; + continue; + } pb+= BLOCK_HDR_SIZE; ctx->off_b += BLOCK_HDR_SIZE; while ((pb < pb_limit) && diff --git a/tools/unit-tests/unit-delta.c b/tools/unit-tests/unit-delta.c index b870b32ca5..6f630ac4c1 100644 --- a/tools/unit-tests/unit-delta.c +++ b/tools/unit-tests/unit-delta.c @@ -533,6 +533,34 @@ START_TEST(test_wb_patch_and_diff_multi_sector_images) } END_TEST +START_TEST(test_wb_patch_and_diff_match_offset_msb_equals_esc) +{ + /* A matched block whose 24-bit source offset has 0x7f (ESC) as its + * most-significant byte would encode a header starting with the same + * two bytes (ESC, ESC) used to escape a literal ESC byte, making it + * indistinguishable on decode. wb_diff must never emit such a header; + * base images large enough to expose an offset in [0x7f0000,0x7fffff] + * are a supported configuration (e.g. MMU/Linux delta updates). + */ + static uint8_t src_a[0x800000]; + uint8_t src_b[7]; + const uint32_t src_off = 0x7f0010; + + memset(src_a, 0, sizeof(src_a)); + src_a[src_off] = 0x11; + src_a[src_off + 1] = 0x22; + src_a[src_off + 2] = 0x33; + src_a[src_off + 3] = 0x44; + src_a[src_off + 4] = 0x55; + src_a[src_off + 5] = 0x66; + + memcpy(src_b, src_a + src_off, 6); + src_b[6] = 0xAA; + + (void)run_roundtrip_case(src_a, sizeof(src_a), src_b, sizeof(src_b), 64); +} +END_TEST + START_TEST(test_wb_diff_get_sector_size_rejects_values_above_16bit) { #if HAVE_POSIX_FORK @@ -690,6 +718,7 @@ Suite *patch_diff_suite(void) tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff_completely_different_images); tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff_all_escape_images); tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff_multi_sector_images); + tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff_match_offset_msb_equals_esc); tcase_add_test(tc_wolfboot_delta, test_wb_diff_get_sector_size_accepts_16bit_limit); tcase_add_test(tc_wolfboot_delta, test_wb_diff_get_sector_size_rejects_values_above_16bit); tcase_add_test(tc_wolfboot_delta, test_wb_patch_and_diff_size_changing_update); From fea97c55f7956d2cef7176872ae4b972d5d31d14 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:47:38 +0200 Subject: [PATCH 06/23] F-5963: fix over-advance of address/len in unaligned hal_flash_write (kinetis.c, mcxa.c, mcxw.c) In the unaligned/partial-word path, address/len were advanced by the full flash-word-relative loop index "i" (which starts at start_off), instead of by the number of data bytes actually consumed (i - start_off). On a write spanning more than one flash word this drops start_off bytes of input data and misdirects the following word write. Mirrors the already-correct form in hal/kinetis_kl26.c. --- hal/kinetis.c | 4 +- hal/mcxa.c | 4 +- hal/mcxw.c | 4 +- tools/unit-tests/Makefile | 6 + tools/unit-tests/mcxa_fsl_stub/fsl_clock.h | 5 + tools/unit-tests/mcxa_fsl_stub/fsl_common.h | 17 ++ tools/unit-tests/mcxa_fsl_stub/fsl_romapi.h | 14 ++ tools/unit-tests/mcxa_fsl_stub/fsl_spc.h | 5 + tools/unit-tests/unit-flash-write-mcxa.c | 163 ++++++++++++++++++++ 9 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 tools/unit-tests/mcxa_fsl_stub/fsl_clock.h create mode 100644 tools/unit-tests/mcxa_fsl_stub/fsl_common.h create mode 100644 tools/unit-tests/mcxa_fsl_stub/fsl_romapi.h create mode 100644 tools/unit-tests/mcxa_fsl_stub/fsl_spc.h create mode 100644 tools/unit-tests/unit-flash-write-mcxa.c diff --git a/hal/kinetis.c b/hal/kinetis.c index f5467a1257..c0f86c5b38 100644 --- a/hal/kinetis.c +++ b/hal/kinetis.c @@ -330,8 +330,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) if (ret != kStatus_FTFx_Success) return -1; } - address += i; - len -= i; + address = address_align + i; + len -= (int)(i - start_off); } else { uint32_t len_align = len - (len & 0x07); ret = FLASH_Program(&pflash, address, (uint8_t*)data + w, len_align); diff --git a/hal/mcxa.c b/hal/mcxa.c index 390803b18c..f1dd73c904 100644 --- a/hal/mcxa.c +++ b/hal/mcxa.c @@ -93,8 +93,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) if (ret != kStatus_Success) return -1; } - address += i; - len -= i; + address = address_align + i; + len -= (int)(i - start_off); } else { uint32_t len_align = len - (len & 0x0F); diff --git a/hal/mcxw.c b/hal/mcxw.c index 9c66d622cf..0d3d352a9b 100644 --- a/hal/mcxw.c +++ b/hal/mcxw.c @@ -172,8 +172,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) if (memcmp(aligned_qword, empty_qword, flash_word_size) != 0) { write_flash_qword((uint32_t *)address_align, aligned_qword); } - address += i; - len -= i; + address = address_align + i; + len -= (int)(i - start_off); } else { uint32_t i; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 352e179687..9a1e0c3e9d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -81,6 +81,7 @@ TESTS+=unit-fwtpm-nv-oob TESTS+=unit-elf-bss-guard TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-va416x0-fram +TESTS+=unit-flash-write-mcxa # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -574,6 +575,11 @@ unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-p gcc -o $@ unit-ata-security-passphrase-zeroize.c $(CFLAGS) \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +# unit-flash-write-mcxa includes hal/mcxa.c directly, with mcxa_fsl_stub/ +# standing in for the (not vendored) NXP MCUXpresso SDK headers it includes. +unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c + gcc -o $@ unit-flash-write-mcxa.c -Imcxa_fsl_stub $(CFLAGS) $(LDFLAGS) + unit-elf-bss-guard: unit-elf-bss-guard.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) diff --git a/tools/unit-tests/mcxa_fsl_stub/fsl_clock.h b/tools/unit-tests/mcxa_fsl_stub/fsl_clock.h new file mode 100644 index 0000000000..fa2b0b71e3 --- /dev/null +++ b/tools/unit-tests/mcxa_fsl_stub/fsl_clock.h @@ -0,0 +1,5 @@ +/* Stub: hal/mcxa.c only uses this under #ifdef __WOLFBOOT, which the unit + * test does not define. */ +#ifndef FSL_CLOCK_STUB_H +#define FSL_CLOCK_STUB_H +#endif /* FSL_CLOCK_STUB_H */ diff --git a/tools/unit-tests/mcxa_fsl_stub/fsl_common.h b/tools/unit-tests/mcxa_fsl_stub/fsl_common.h new file mode 100644 index 0000000000..bce2daa2ec --- /dev/null +++ b/tools/unit-tests/mcxa_fsl_stub/fsl_common.h @@ -0,0 +1,17 @@ +/* Minimal stand-in for the NXP MCUXpresso SDK header of the same name. + * hal/mcxa.c only needs the flash config type and a status code from it; + * the real SDK is not vendored in this source tree. */ +#ifndef FSL_COMMON_STUB_H +#define FSL_COMMON_STUB_H + +#include +#include + +typedef int status_t; +#define kStatus_Success 0 + +typedef struct { + int dummy; +} flash_config_t; + +#endif /* FSL_COMMON_STUB_H */ diff --git a/tools/unit-tests/mcxa_fsl_stub/fsl_romapi.h b/tools/unit-tests/mcxa_fsl_stub/fsl_romapi.h new file mode 100644 index 0000000000..bd864a6abb --- /dev/null +++ b/tools/unit-tests/mcxa_fsl_stub/fsl_romapi.h @@ -0,0 +1,14 @@ +/* Minimal stand-in for the NXP MCUXpresso SDK ROM flash API declarations + * used by hal/mcxa.c's hal_flash_write()/hal_flash_erase(). The unit test + * provides the definitions of these functions. */ +#ifndef FSL_ROMAPI_STUB_H +#define FSL_ROMAPI_STUB_H + +status_t FLASH_ProgramPhrase(flash_config_t *config, uint32_t start, + uint8_t *src, uint32_t len); +status_t FLASH_EraseSector(flash_config_t *config, uint32_t start, + uint32_t len, uint32_t key); + +#define kFLASH_ApiEraseKey 0x6b65796b + +#endif /* FSL_ROMAPI_STUB_H */ diff --git a/tools/unit-tests/mcxa_fsl_stub/fsl_spc.h b/tools/unit-tests/mcxa_fsl_stub/fsl_spc.h new file mode 100644 index 0000000000..b6d2b1538d --- /dev/null +++ b/tools/unit-tests/mcxa_fsl_stub/fsl_spc.h @@ -0,0 +1,5 @@ +/* Stub: hal/mcxa.c only uses this under #ifdef __WOLFBOOT, which the unit + * test does not define. */ +#ifndef FSL_SPC_STUB_H +#define FSL_SPC_STUB_H +#endif /* FSL_SPC_STUB_H */ diff --git a/tools/unit-tests/unit-flash-write-mcxa.c b/tools/unit-tests/unit-flash-write-mcxa.c new file mode 100644 index 0000000000..29eaa2f86b --- /dev/null +++ b/tools/unit-tests/unit-flash-write-mcxa.c @@ -0,0 +1,163 @@ +/* unit-flash-write-mcxa.c + * + * Regression test for F-5963: in the unaligned/partial-word path of + * hal_flash_write() (hal/mcxa.c), the post-program bookkeeping did + * address += i; + * len -= i; + * where "i" is the flash-word-relative loop index (it starts at start_off, + * the byte offset of "address" within its 16-byte flash word), not the + * number of data bytes actually consumed. That is (i - start_off), which is + * how far "w" (the data source index) really advanced. Whenever an unaligned + * write spans more than one flash word, both address and len end up + * over-advanced by start_off: the next word is targeted start_off bytes too + * high, and len is start_off too small, so start_off bytes of the input are + * silently dropped instead of being written. + * + * hal/kinetis_kl26.c already has the correct form: + * address = address_align + i; + * len -= (int)(i - start_off); + * + * 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 + +/* NXP MCUXpresso SDK stand-ins (mcxa_fsl_stub/) provide flash_config_t, + * status_t and the FLASH_* prototypes hal/mcxa.c expects; this test supplies + * their bodies below, simulating flash programming as a plain memcpy onto + * the mock buffer that "address" points into. */ +#include "fsl_common.h" +#include "fsl_romapi.h" +#include "image.h" + +status_t FLASH_ProgramPhrase(flash_config_t *config, uint32_t start, + uint8_t *src, uint32_t len) +{ + (void)config; + memcpy((void*)(uintptr_t)start, src, len); + return kStatus_Success; +} + +status_t FLASH_EraseSector(flash_config_t *config, uint32_t start, + uint32_t len, uint32_t key) +{ + (void)config; (void)start; (void)len; (void)key; + return kStatus_Success; +} + +#include "../../hal/mcxa.c" + +/* hal_flash_write() treats "address" as a real pointer into memory-mapped + * flash (it memcpy()s from it directly). struct fields carrying such + * addresses are uint32_t, matching the real 32-bit target. MAP_32BIT keeps + * the mock flash buffer inside that range on a 64-bit test host, exactly as + * unit-ata-security-passphrase-zeroize.c does for 32-bit DMA pointers. */ +#define MOCK_FLASH_SIZE 64 +static uint8_t *mock_flash; + +static void setup(void) +{ + mock_flash = mmap(NULL, MOCK_FLASH_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(mock_flash, MAP_FAILED); + memset(mock_flash, 0xFF, MOCK_FLASH_SIZE); +} + +static void teardown(void) +{ + munmap(mock_flash, MOCK_FLASH_SIZE); +} + +/* Write 20 bytes starting 5 bytes into a 16-byte flash word: the write spans + * two flash words (word 0 bytes 5..15, word 1 bytes 0..8). Byte-for-byte, + * mock_flash[5 .. 24] must equal the 20 input bytes, and everything outside + * that range must be untouched. Before the fix, address/len over-advanced by + * start_off (5) after the first word, so mock_flash[16..20] were left as + * 0xFF and the last 5 input bytes were never written anywhere. */ +START_TEST(test_unaligned_write_spanning_two_words) +{ + uint8_t data[20]; + int i; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + + for (i = 0; i < 20; i++) + data[i] = (uint8_t)(i + 1); + + ck_assert_int_eq(hal_flash_write(base + 5, data, 20), 0); + + for (i = 0; i < 5; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + for (i = 0; i < 20; i++) + ck_assert_uint_eq(mock_flash[5 + i], data[i]); + for (i = 25; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +/* A write that fits entirely inside a single flash word must still work: + * the buggy and fixed forms agree here (the loop runs only once), so this + * guards against a fix that breaks the common case. */ +START_TEST(test_unaligned_write_single_word) +{ + uint8_t data[6]; + int i; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + + for (i = 0; i < 6; i++) + data[i] = (uint8_t)(0xA0 + i); + + ck_assert_int_eq(hal_flash_write(base + 3, data, 6), 0); + + for (i = 0; i < 3; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + for (i = 0; i < 6; i++) + ck_assert_uint_eq(mock_flash[3 + i], data[i]); + for (i = 9; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +Suite *flash_write_suite(void) +{ + Suite *s = suite_create("flash-write-mcxa"); + TCase *tc = tcase_create("flash-write-mcxa"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_unaligned_write_spanning_two_words); + tcase_add_test(tc, test_unaligned_write_single_word); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_write_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From 8df96895f6f4ac825674a61a6d414aef3fdd9756 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 14:56:48 +0200 Subject: [PATCH 07/23] F-6408: zeroize UDS from OTP keystore generator's heap and stack buffers otp-keystore-gen.c reads the device root UDS into the stack buffer `uds` and copies it into the heap buffer `otp_buf` at OTP_UDS_OFFSET, then on every exit path calls free(otp_buf) without wiping it first, and never clears `uds`. Both copies of the highest-value device secret remain in the host process's freed heap chunk and stack frame. Add a local secure_zero() helper (no wolfSSL dependency, matching this standalone host tool's existing bare-gcc build) and call it on the success path and on the write-failure/short-UDS-read error paths, before free()/exit(), mirroring the zeroize-before-release pattern already used elsewhere in this tree (src/x86/ata.c, src/x86/ahci.c). Paths that exit before `uds` is populated are left untouched since there is no secret to wipe yet. --- .gitignore | 1 + tools/keytools/otp/otp-keystore-gen.c | 18 ++ tools/unit-tests/Makefile | 10 ++ .../unit-otp-keystore-gen-zeroize.c | 157 ++++++++++++++++++ 4 files changed, 186 insertions(+) create mode 100644 tools/unit-tests/unit-otp-keystore-gen-zeroize.c diff --git a/.gitignore b/.gitignore index bca5feab91..683ca3d598 100644 --- a/.gitignore +++ b/.gitignore @@ -200,6 +200,7 @@ tools/unit-tests/unit-linux-loader-e820 tools/unit-tests/unit-linux-loader-syssize tools/unit-tests/unit-mpusize tools/unit-tests/unit-otp-keystore +tools/unit-tests/unit-otp-keystore-gen-zeroize tools/unit-tests/unit-tpm-api-names tools/unit-tests/unit-elf-bss-guard tools/unit-tests/unit-fit-fpga diff --git a/tools/keytools/otp/otp-keystore-gen.c b/tools/keytools/otp/otp-keystore-gen.c index f60945c147..43ddb19d3c 100644 --- a/tools/keytools/otp/otp-keystore-gen.c +++ b/tools/keytools/otp/otp-keystore-gen.c @@ -43,6 +43,19 @@ extern struct keystore_slot PubKeys[]; const char outfile[] = "otp.bin"; +/** + * @brief Wipe secret bytes from memory before the buffer holding them is + * freed or the process exits, so the root UDS does not linger in a freed + * heap chunk or a stale stack frame. + */ +static void secure_zero(void *buf, size_t len) +{ + volatile uint8_t *p = (volatile uint8_t *)buf; + while (len-- > 0U) { + *p++ = 0U; + } +} + int main(void) { int n_keys = keystore_num_pubkeys(); @@ -116,6 +129,7 @@ int main(void) close(rand_fd); if (rlen != (ssize_t)sizeof(uds)) { fprintf(stderr, "Error: failed to read random UDS (%zd)\n", rlen); + secure_zero(uds, sizeof(uds)); close(ofd); free(otp_buf); exit(5); @@ -125,11 +139,15 @@ int main(void) if (write(ofd, otp_buf, OTP_SIZE) != OTP_SIZE) { fprintf(stderr, "Error writing to %s: %s\n", outfile, strerror(errno)); + secure_zero(otp_buf + OTP_UDS_OFFSET, sizeof(uds)); + secure_zero(uds, sizeof(uds)); close(ofd); free(otp_buf); exit(3); } fprintf(stderr, "%s successfully created.\nGoodbye.\n", outfile); + secure_zero(otp_buf + OTP_UDS_OFFSET, sizeof(uds)); + secure_zero(uds, sizeof(uds)); close(ofd); free(otp_buf); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 9a1e0c3e9d..39f01ef344 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -74,6 +74,7 @@ TESTS+=unit-flash-erase-g0 TESTS+=unit-flash-erase-c0 TESTS+=unit-flash-erase-u3 TESTS+=unit-otp-keystore +TESTS+=unit-otp-keystore-gen-zeroize TESTS+=unit-x86-paging-oob TESTS+=unit-ahci-unlock-panic TESTS+=unit-ata-security-passphrase-zeroize @@ -362,6 +363,15 @@ unit-flash-erase-u3: unit-flash-erase-u3.c ../../hal/stm32u3.c ../../hal/stm32u3 unit-otp-keystore: unit-otp-keystore.c ../../src/flash_otp_keystore.c gcc -o $@ unit-otp-keystore.c $(CFLAGS) $(LDFLAGS) +# unit-otp-keystore-gen-zeroize includes the host tool +# tools/keytools/otp/otp-keystore-gen.c directly (via #define main) and +# interposes read()/malloc()/free() (dlsym RTLD_NEXT, hence -ldl) to observe +# the OTP buffer and UDS stack array at free() time. +unit-otp-keystore-gen-zeroize: unit-otp-keystore-gen-zeroize.c \ + ../keytools/otp/otp-keystore-gen.c ../../src/keystore.c + gcc -o $@ unit-otp-keystore-gen-zeroize.c ../../src/keystore.c \ + -DFLASH_OTP_KEYSTORE $(CFLAGS) $(LDFLAGS) -ldl + unit-update-flash-self-update: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c \ diff --git a/tools/unit-tests/unit-otp-keystore-gen-zeroize.c b/tools/unit-tests/unit-otp-keystore-gen-zeroize.c new file mode 100644 index 0000000000..83dce7acfa --- /dev/null +++ b/tools/unit-tests/unit-otp-keystore-gen-zeroize.c @@ -0,0 +1,157 @@ +/* unit-otp-keystore-gen-zeroize.c + * + * Regression test for the OTP keystore generator (host tool) leaving the + * device root UDS behind in a freed heap chunk and in the `uds` stack + * buffer instead of wiping both before the buffer is released / the + * process exits. + * + * Interposes read() (to learn where the `uds` stack array lives) and + * free() (called synchronously from inside the still-live gen_main() + * frame, right before the OTP heap buffer is released) so the snapshot + * of both buffers happens while they are still valid memory -- no + * use-after-free/return required. + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#define main gen_main +#include "../keytools/otp/otp-keystore-gen.c" +#undef main + +static uint8_t *captured_uds_ptr; +static size_t captured_uds_len; +static void *captured_otp_buf_ptr; +static int capture_next_malloc; + +static uint8_t otp_buf_snapshot[OTP_SIZE]; +static uint8_t uds_snapshot[OTP_UDS_LEN]; +static int free_call_count; + +ssize_t read(int fd, void *buf, size_t count) +{ + static ssize_t (*real_read)(int, void *, size_t) = NULL; + if (real_read == NULL) + real_read = dlsym(RTLD_NEXT, "read"); + + if (count == OTP_UDS_LEN && captured_uds_ptr == NULL) { + captured_uds_ptr = (uint8_t *)buf; + captured_uds_len = count; + } + return real_read(fd, buf, count); +} + +void *malloc(size_t size) +{ + static void *(*real_malloc)(size_t) = NULL; + void *ptr; + if (real_malloc == NULL) + real_malloc = dlsym(RTLD_NEXT, "malloc"); + + ptr = real_malloc(size); + /* gen_main() calls malloc() exactly once, for the OTP_SIZE image + * buffer; capture that single allocation by call order, not by + * size, so it cannot collide with an unrelated same-size alloc. */ + if (capture_next_malloc && captured_otp_buf_ptr == NULL) { + captured_otp_buf_ptr = ptr; + capture_next_malloc = 0; + } + return ptr; +} + +void free(void *ptr) +{ + static void (*real_free)(void *) = NULL; + if (real_free == NULL) + real_free = dlsym(RTLD_NEXT, "free"); + + /* Snapshot the OTP heap buffer and the (still live, since gen_main() + * hasn't returned yet) `uds` stack array before the real free() runs. */ + if (ptr != NULL && ptr == captured_otp_buf_ptr) { + free_call_count++; + memcpy(otp_buf_snapshot, ptr, OTP_SIZE); + if (captured_uds_ptr != NULL && captured_uds_len == OTP_UDS_LEN) + memcpy(uds_snapshot, captured_uds_ptr, OTP_UDS_LEN); + } + real_free(ptr); +} + +static void reset_capture(void) +{ + captured_uds_ptr = NULL; + captured_uds_len = 0; + captured_otp_buf_ptr = NULL; + capture_next_malloc = 1; + free_call_count = 0; + memset(otp_buf_snapshot, 0, sizeof(otp_buf_snapshot)); + memset(uds_snapshot, 0, sizeof(uds_snapshot)); +} + +START_TEST(test_success_path_zeroizes_uds_before_free) +{ + int r; + int i; + int all_zero; + char tempdir[] = "/tmp/wolfboot-otp-gen-XXXXXX"; + char *dir = mkdtemp(tempdir); + char cwd[4096]; + + ck_assert_ptr_nonnull(dir); + ck_assert_ptr_nonnull(getcwd(cwd, sizeof(cwd))); + ck_assert_int_eq(chdir(dir), 0); + + reset_capture(); + r = gen_main(); + ck_assert_int_eq(r, 0); + ck_assert_int_eq(free_call_count, 1); + ck_assert_ptr_nonnull(captured_uds_ptr); + + /* Root UDS must not remain in the freed heap chunk at the OTP UDS + * offset. */ + all_zero = 1; + for (i = 0; i < OTP_UDS_LEN; i++) { + if (otp_buf_snapshot[OTP_UDS_OFFSET + i] != 0) + all_zero = 0; + } + ck_assert_msg(all_zero, + "UDS bytes still present in freed otp_buf heap chunk at free() time"); + + /* Root UDS must not remain in the `uds` stack buffer either. */ + all_zero = 1; + for (i = 0; i < OTP_UDS_LEN; i++) { + if (uds_snapshot[i] != 0) + all_zero = 0; + } + ck_assert_msg(all_zero, + "UDS bytes still present on the stack (`uds`) at free() time"); + + ck_assert_int_eq(chdir(cwd), 0); + unlink("otp.bin"); + rmdir(dir); +} +END_TEST + +static Suite *otp_keystore_gen_zeroize_suite(void) +{ + Suite *s = suite_create("otp_keystore_gen_zeroize"); + TCase *tc = tcase_create("uds_zeroize"); + tcase_add_test(tc, test_success_path_zeroizes_uds_before_free); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + Suite *s = otp_keystore_gen_zeroize_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + int failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From f20a1e13a6923e44a331f0a574b90b7909d00f13 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:06:17 +0200 Subject: [PATCH 08/23] F-6403: add successful-boot test coverage for HW-swap wolfBoot_start test_hwswap_highversion_rollback_denied was the only test for update_flash_hwswap.c's wolfBoot_start and always ends in boot_panic, so the entire successful-boot path (IMG_STATE_UPDATING->TESTING transitions before and after the HW dualbank swap, and the same-version boundary of the anti-rollback check) was never exercised and mutations there would go undetected. Add three tests that reach do_boot() and pin each of those code paths. --- tools/unit-tests/unit-update-flash-hwswap.c | 108 ++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tools/unit-tests/unit-update-flash-hwswap.c b/tools/unit-tests/unit-update-flash-hwswap.c index 596dbe55ed..7fc108606c 100644 --- a/tools/unit-tests/unit-update-flash-hwswap.c +++ b/tools/unit-tests/unit-update-flash-hwswap.c @@ -95,6 +95,43 @@ static void reset_mock_stats(void) dualbank_swap_called = 0; } +static void assert_part_state(uint8_t part, uint8_t expected) +{ + uint8_t st = 0xBB; + ck_assert_int_eq(wolfBoot_get_partition_state(part, &st), 0); + ck_assert_uint_eq(st, expected); +} + +/* PART_BOOT_EXT/PART_UPDATE_EXT (set in this test's CFLAGS) route + * wolfBoot_set_partition_state() through ext_flash_write() instead of + * hal_flash_write(). Pre-seeding a partition trailer from the test body + * (outside wolfBoot_start()) therefore needs ext flash unlocked too. + */ +static void set_part_state(uint8_t part, uint8_t newst) +{ + hal_flash_unlock(); + ext_flash_unlock(); + ck_assert_int_eq(wolfBoot_set_partition_state(part, newst), 0); + ext_flash_lock(); + hal_flash_lock(); +} + +/* wolfBoot_start() only wraps its own IMG_STATE_TESTING writes with + * hal_flash_unlock()/hal_flash_lock(), so with PART_BOOT_EXT it never + * unlocks the ext flash that ext_flash_write() actually requires here. + * No shipped HW-assisted-swap target defines PART_BOOT_EXT (dualbank + * swap is an internal-flash-only mechanism), so this is a limitation of + * this test's build config rather than of wolfBoot_start() itself; + * unlock ext flash around the call so the state-transition code under + * test can run. + */ +static void run_wolfboot_start(void) +{ + ext_flash_unlock(); + wolfBoot_start(); + ext_flash_lock(); +} + static void prepare_flash(void) { int ret; @@ -226,15 +263,86 @@ START_TEST (test_hwswap_highversion_rollback_denied) { } END_TEST +/* Successful first boot of a freshly-flashed BOOT image awaiting + * confirmation: the IMG_STATE_UPDATING -> IMG_STATE_TESTING transition + * (update_flash_hwswap.c:91-97) must run and do_boot() must be reached. + * With no UPDATE candidate available, wolfBoot_dualboot_candidate() + * selects PART_BOOT directly, so this pins the transition without + * exercising hal_flash_dualbank_swap(). + */ +START_TEST (test_hwswap_first_boot_success) { + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + set_part_state(PART_BOOT, IMG_STATE_UPDATING); + + run_wolfboot_start(); + + ck_assert_int_eq(do_boot_called, 1); + ck_assert_int_eq(dualbank_swap_called, 0); + assert_part_state(PART_BOOT, IMG_STATE_TESTING); + cleanup_flash(); +} +END_TEST + +/* Successful boot of an UPDATE candidate via HW-assisted swap: pins the + * post-swap IMG_STATE_UPDATING -> IMG_STATE_TESTING transition + * (update_flash_hwswap.c:107-113). The mock hal_flash_dualbank_swap() + * does not physically relocate the partitions, so the post-swap state + * checked by wolfBoot_start() is the one written directly to PART_BOOT + * here, simulating what a real swap would expose at that address. + */ +START_TEST (test_hwswap_postswap_success) { + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + set_part_state(PART_BOOT, IMG_STATE_UPDATING); + + run_wolfboot_start(); + + ck_assert_int_eq(do_boot_called, 1); + ck_assert_int_eq(dualbank_swap_called, 1); + assert_part_state(PART_BOOT, IMG_STATE_TESTING); + cleanup_flash(); +} +END_TEST + +/* Booting the highest version with BOOT and UPDATE at the same version + * must succeed: pins the "<" in "(max_v > 0U) && (active_v < max_v)" + * (update_flash_hwswap.c:62). A mutation to "<=" would spuriously panic + * here (no test above exercises a successful boot at max_v). + */ +START_TEST (test_hwswap_sameversion_success) { + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 2, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + + wolfBoot_start(); + + ck_assert_int_eq(do_boot_called, 1); + cleanup_flash(); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfboot-hwswap"); TCase *rollback_denied = tcase_create("HW-swap high-version rollback denied"); + TCase *successful_boot = + tcase_create("HW-swap successful boot"); tcase_add_test(rollback_denied, test_hwswap_highversion_rollback_denied); suite_add_tcase(s, rollback_denied); tcase_set_timeout(rollback_denied, 5); + + tcase_add_test(successful_boot, test_hwswap_first_boot_success); + tcase_add_test(successful_boot, test_hwswap_postswap_success); + tcase_add_test(successful_boot, test_hwswap_sameversion_success); + suite_add_tcase(s, successful_boot); + tcase_set_timeout(successful_boot, 5); return s; } From e09074a0c62bad49c73c1150de34a23eb6948701 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:07:05 +0200 Subject: [PATCH 09/23] F-6401: emit build-time warning for FPGA_NONFATAL=1 FPGA_NONFATAL downgrades a failed PL bitstream load from fatal (panic) to a logged non-fatal warning, letting boot continue without the programmable logic. Every other fail-safe-weakening option in this file (ALLOW_DOWNGRADE, DISABLE_BACKUP, WOLFBOOT_UDS_UID_FALLBACK_FORTEST) emits a $(warning ...) so the tradeoff is visible at build time; FPGA_NONFATAL was missing one. --- options.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/options.mk b/options.mk index ffb0ef99c2..e6772d499d 100644 --- a/options.mk +++ b/options.mk @@ -986,6 +986,7 @@ WOLFBOOT_LOAD_FPGA_ADDRESS ?= 0 # logged warning that continues the boot. FPGA_NONFATAL ?= 0 ifeq ($(FPGA_NONFATAL),1) + $(warning FPGA_NONFATAL=1 continues booting when the FPGA bitstream fails to load; the device may run without security-relevant programmable logic) CFLAGS+=-DWOLFBOOT_FPGA_NONFATAL endif # FIT_CONFIG_SELECT=1 lets the target pick a per-board FIT configuration From 32e41f0f40182e6cd0daad4bad4e46b04aea7ff2 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:13:28 +0200 Subject: [PATCH 10/23] F-6399: fix imx_rt DCACHE invalidation to include down-alignment offset hal_flash_write() and hal_flash_erase() aligned the invalidation start address down to a 32-byte cache line but rounded the length up from "len" alone, omitting the (address - aligned_address) offset. Whenever (address % 32) + (len % 32) > 32, the invalidated range fell short of address + len, leaving the last cache line stale after a write/erase. Extract the range computation into hal_flash_cache_align_range() (hal/imx_rt.h, dependency-free so it's unit-testable without the NXP SDK) and include the down-alignment offset before rounding the length up, so the invalidated range always covers [address, address+len). --- hal/imx_rt.c | 9 +- hal/imx_rt.h | 46 +++++++ tools/unit-tests/Makefile | 7 ++ tools/unit-tests/unit-imx-rt-cache-align.c | 133 +++++++++++++++++++++ 4 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 hal/imx_rt.h create mode 100644 tools/unit-tests/unit-imx-rt-cache-align.c diff --git a/hal/imx_rt.c b/hal/imx_rt.c index dc5a62fd61..15b6e1bb5f 100644 --- a/hal/imx_rt.c +++ b/hal/imx_rt.c @@ -26,6 +26,7 @@ #include #include "image.h" #include "printf.h" +#include "imx_rt.h" #include "fsl_cache.h" #include "fsl_common.h" #include "fsl_iomuxc.h" @@ -966,8 +967,8 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) * (see definition of DCACHE_InvalidateByRange). * To ensure all data is included we align the address downwards, and the length upwards. */ - uint32_t aligned_address = address - (address % 32); - uint32_t aligned_len = len + (32 - (len % 32)); + uint32_t aligned_address, aligned_len; + hal_flash_cache_align_range(address, (uint32_t)len, &aligned_address, &aligned_len); DCACHE_InvalidateByRange(aligned_address, aligned_len); /* Re-enable interrupts */ asm volatile("cpsie i"); @@ -1010,8 +1011,8 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) * (see definition of DCACHE_InvalidateByRange). * To ensure all data is included we align the address downwards, and the length upwards. */ - uint32_t aligned_address = address - (address % 32); - uint32_t aligned_len = len + (32 - (len % 32)); + uint32_t aligned_address, aligned_len; + hal_flash_cache_align_range(address, (uint32_t)len, &aligned_address, &aligned_len); DCACHE_InvalidateByRange(aligned_address, aligned_len); /* Re-enable interrupts */ asm volatile("cpsie i"); diff --git a/hal/imx_rt.h b/hal/imx_rt.h new file mode 100644 index 0000000000..a1da88a662 --- /dev/null +++ b/hal/imx_rt.h @@ -0,0 +1,46 @@ +/* imx_rt.h + * + * Support routines for the i.MX RT HAL, kept free of NXP MCUXpresso SDK + * dependencies so they can be exercised directly in the host unit tests. + * + * 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 + */ +#ifndef IMX_RT_H +#define IMX_RT_H + +#include + +/* Flash is memory mapped, so after a program/erase, the affected range must + * be invalidated in the data cache to ensure coherency. The cache line size + * is 32 bytes, so both the address and length passed to + * DCACHE_InvalidateByRange() must be 32-byte aligned: the address is rounded + * down, and the length is rounded up by the same down-alignment offset plus + * "len", so that the invalidated range always fully covers + * [address, address + len). */ +static inline void hal_flash_cache_align_range(uint32_t address, uint32_t len, + uint32_t *aligned_address, uint32_t *aligned_len) +{ + uint32_t start = address - (address % 32); + uint32_t unaligned_len = len + (address - start); + + *aligned_address = start; + *aligned_len = unaligned_len + ((32 - (unaligned_len % 32)) % 32); +} + +#endif /* IMX_RT_H */ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 39f01ef344..6db5f5c29e 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -83,6 +83,7 @@ TESTS+=unit-elf-bss-guard TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-va416x0-fram TESTS+=unit-flash-write-mcxa +TESTS+=unit-imx-rt-cache-align # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -590,6 +591,12 @@ unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-p unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c gcc -o $@ unit-flash-write-mcxa.c -Imcxa_fsl_stub $(CFLAGS) $(LDFLAGS) +# unit-imx-rt-cache-align only pulls in hal/imx_rt.h, which is dependency-free +# by design, avoiding the (not vendored) NXP MCUXpresso SDK headers that +# hal/imx_rt.c itself requires. +unit-imx-rt-cache-align: unit-imx-rt-cache-align.c ../../hal/imx_rt.h + gcc -o $@ unit-imx-rt-cache-align.c $(CFLAGS) $(LDFLAGS) + unit-elf-bss-guard: unit-elf-bss-guard.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) diff --git a/tools/unit-tests/unit-imx-rt-cache-align.c b/tools/unit-tests/unit-imx-rt-cache-align.c new file mode 100644 index 0000000000..3a6311a679 --- /dev/null +++ b/tools/unit-tests/unit-imx-rt-cache-align.c @@ -0,0 +1,133 @@ +/* unit-imx-rt-cache-align.c + * + * Regression test for F-6399: in hal/imx_rt.c, hal_flash_write() and + * hal_flash_erase() invalidate the data cache over a range computed by + * rounding "address" down and "len" up to 32-byte cache-line boundaries. + * The length was rounded up from "len" alone, omitting the down-alignment + * offset (address - aligned_address). Whenever + * (address % 32) + (len % 32) > 32, the resulting range's end fell short of + * the real end of the write/erase (address + len), leaving the last cache + * line stale after the flash operation. + * + * hal_flash_cache_align_range() (hal/imx_rt.h) is the exact routine used by + * both hal_flash_write() and hal_flash_erase() to compute that range; this + * test drives it directly and checks that [aligned_address, aligned_address + * + aligned_len) always fully covers [address, address + len). + * + * 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 "../../hal/imx_rt.h" + +static void check_covers(uint32_t address, uint32_t len) +{ + uint32_t aligned_address, aligned_len; + + hal_flash_cache_align_range(address, len, &aligned_address, &aligned_len); + + /* Both must be 32-byte aligned, as required by DCACHE_InvalidateByRange. */ + ck_assert_uint_eq(aligned_address % 32, 0); + ck_assert_uint_eq(aligned_len % 32, 0); + + /* The invalidated range must start at or before the write/erase, and end + * at or after it: [aligned_address, aligned_address + aligned_len) + * must contain [address, address + len). */ + ck_assert(aligned_address <= address); + ck_assert(aligned_address + aligned_len >= address + len); +} + +/* address % 32 = 4, len % 32 = 29: 4 + 29 = 33 > 32, the case the buggy + * "len + (32 - (len % 32))" formula under-covered by 8 bytes. */ +START_TEST(test_straddles_cache_line) +{ + check_covers(0x60000000u + 4u, 29u); +} +END_TEST + +/* Same straddling condition, at the boundary: address % 32 = 31, + * len % 32 = 1 (31 + 1 = 32, not > 32) must still be covered exactly. */ +START_TEST(test_boundary_not_straddling) +{ + check_covers(0x60000000u + 31u, 1u); +} +END_TEST + +/* len an exact multiple of 32: must not require a spurious extra line. */ +START_TEST(test_len_multiple_of_32) +{ + check_covers(0x60000000u, 64u); + check_covers(0x60000000u + 32u, 32u); +} +END_TEST + +/* Sector-aligned erase (address % 32 == 0): always covered regardless of + * len % 32. */ +START_TEST(test_aligned_address) +{ + check_covers(0x60000000u, 1u); + check_covers(0x60000000u, 4096u); + check_covers(0x60000000u, 33u); +} +END_TEST + +/* Sweep every (address % 32, len) combination for a representative address + * base and a range of lengths, to catch any other under-coverage case. */ +START_TEST(test_sweep) +{ + uint32_t off; + uint32_t len; + + for (off = 0; off < 32; off++) { + for (len = 1; len <= 96; len++) { + check_covers(0x60000000u + off, len); + } + } +} +END_TEST + +Suite *imx_rt_cache_align_suite(void) +{ + Suite *s = suite_create("imx-rt-cache-align"); + TCase *tc = tcase_create("imx-rt-cache-align"); + + tcase_add_test(tc, test_straddles_cache_line); + tcase_add_test(tc, test_boundary_not_straddling); + tcase_add_test(tc, test_len_multiple_of_32); + tcase_add_test(tc, test_aligned_address); + tcase_add_test(tc, test_sweep); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = imx_rt_cache_align_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From e38296c984aef23fc47a02338f72c471d242c833 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:18:21 +0200 Subject: [PATCH 11/23] F-6398: fix OOB write in bmpatch when reconstructed image grows past source size bmpatch mmaps `base` sized to the source file (len1) then memcpy's the reconstructed output into base+len3, which runs past the mapping once the destination image is larger than the source (a common case for delta updates). This causes writes into the zero-fill slack of the last mapped page to be silently dropped (never synced to the file even after the trailing ftruncate) or, for larger overruns, a SIGBUS. Reproduced with the existing `delta-test` fixtures: patching 1.txt back to the (larger) 0.txt already corrupts the tail of the output today. Write reconstructed blocks with pwrite(fd1, ...) instead of memcpy into the undersized mapping; regular file writes grow the file naturally instead of faulting/silently discarding data past the mapped region. The mmap of `base` is only used for reads, so this is a minimal, in-place-preserving fix. Verified `make -C tools/delta delta-test` now passes both directions (shrinking 0->1 and growing 1->0). --- tools/delta/bmdiff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/delta/bmdiff.c b/tools/delta/bmdiff.c index da515e1961..099dac60ff 100644 --- a/tools/delta/bmdiff.c +++ b/tools/delta/bmdiff.c @@ -149,7 +149,7 @@ int main(int argc, char *argv[]) if (r < 0) exit(5); if (r > 0) { - memcpy(base + len3, dest, r); + pwrite(fd1, dest, r, len3); len3 += r; } } while (r > 0); From dd0712ec5275f96b26989f98b6dfd8b32d5e0293 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:23:43 +0200 Subject: [PATCH 12/23] F-6130: clear disk_encrypt_key/nonce before panic paths that skip the final cleanup update_disk.c wolfBoot_start() already zeroizes disk_encrypt_key/nonce and the decrypted header before most wolfBoot_panic() halts, but four paths were missed: anti-rollback rejection, FIT FPGA load failure, FIT kernel load failure, and flash-protect failure. Since wolfBoot_panic() halts forever on real targets, these paths left the key live in BSS on a halted device. Add the same disk_decrypted_header_clear()/disk_crypto_clear() pair used on the other panic paths immediately before each. --- src/update_disk.c | 16 ++++++++++++++++ tools/unit-tests/unit-update-disk.c | 8 ++++++++ 2 files changed, 24 insertions(+) diff --git a/src/update_disk.c b/src/update_disk.c index 0607e7a71c..767df7c266 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -397,6 +397,10 @@ void RAMFUNCTION wolfBoot_start(void) uint32_t cur_ver = selected ? pB_ver_u : pA_ver_u; if ((max_ver > 0U) && (cur_ver < max_ver)) { wolfBoot_printf("Rollback to lower version not allowed\r\n"); +#ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); + disk_crypto_clear(); +#endif wolfBoot_panic(); return; } @@ -573,6 +577,10 @@ void RAMFUNCTION wolfBoot_start(void) if (fpga != NULL) { if (fit_load_fpga(fit, fpga) != 0) { wolfBoot_printf("FIT: FPGA load failed\r\n"); +#ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); + disk_crypto_clear(); +#endif wolfBoot_panic(); } } @@ -584,6 +592,10 @@ void RAMFUNCTION wolfBoot_start(void) if (new_load == NULL) { wolfBoot_printf("FIT: failed to load kernel '%s'\r\n", kernel); +#ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); + disk_crypto_clear(); +#endif wolfBoot_panic(); } load_address = new_load; @@ -627,6 +639,10 @@ void RAMFUNCTION wolfBoot_start(void) #ifndef TZEN if (hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) < 0) { wolfBoot_printf("Error protecting bootloader flash region\r\n"); +#ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); + disk_crypto_clear(); +#endif wolfBoot_panic(); } #endif diff --git a/tools/unit-tests/unit-update-disk.c b/tools/unit-tests/unit-update-disk.c index d92ec43fff..072f239e6a 100644 --- a/tools/unit-tests/unit-update-disk.c +++ b/tools/unit-tests/unit-update-disk.c @@ -361,6 +361,8 @@ END_TEST START_TEST(test_update_disk_rejects_rollback_after_higher_image_failure) { + size_t i; + reset_mocks(); build_image(part_a_image, 7, 0xA1); build_image(part_b_image, 5, 0xB2); @@ -370,6 +372,12 @@ START_TEST(test_update_disk_rejects_rollback_after_higher_image_failure) ck_assert_int_gt(wolfBoot_panicked, 0); ck_assert_int_eq(mock_do_boot_called, 0); + for (i = 0; i < ENCRYPT_KEY_SIZE; i++) { + ck_assert_uint_eq(disk_encrypt_key[i], 0); + } + for (i = 0; i < ENCRYPT_NONCE_SIZE; i++) { + ck_assert_uint_eq(disk_encrypt_nonce[i], 0); + } } END_TEST From 9a25fae89d48902bd661b898ee644be5ec36583c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:28:09 +0200 Subject: [PATCH 13/23] F-6127: fix custom-TLV 8-byte value saturation in arg2num arg2num() parsed --custom-tlv values with signed strtoll(), which saturates to LLONG_MAX (0x7FFFFFFFFFFFFFFF) on positive overflow. For LEN==8 no masking is applied afterwards (unlike LEN 1/2/4), so any value >= 2^63 silently encoded as 0x7FFFFFFFFFFFFFFF instead of the value the user supplied, breaking the TLV encode/decode roundtrip. Switch to strtoull() and reject (exit 16) when it reports ERANGE for an 8-byte value, mirroring the existing fw_version range check. --- tools/keytools/sign.c | 11 ++- tools/unit-tests/unit-sign-custom-tlv-le.py | 77 ++++++++++++--------- 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index a399633097..0c38fa2f1d 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -2651,11 +2651,16 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in uint64_t arg2num(const char *arg, size_t len) { - uint64_t ret = (uint64_t) -1; + uint64_t ret; + errno = 0; if (strncmp(arg, "0x", 2) == 0) { - ret = strtoll(arg + 2, NULL, 16); + ret = strtoull(arg + 2, NULL, 16); } else { - ret = strtoll(arg, NULL, 10); + ret = strtoull(arg, NULL, 10); + } + if ((len == 8) && (errno == ERANGE)) { + fprintf(stderr, "Custom TLV value out of range: %s\n", arg); + exit(16); } switch (len) { case 1: diff --git a/tools/unit-tests/unit-sign-custom-tlv-le.py b/tools/unit-tests/unit-sign-custom-tlv-le.py index b30161c408..0870d13408 100644 --- a/tools/unit-tests/unit-sign-custom-tlv-le.py +++ b/tools/unit-tests/unit-sign-custom-tlv-le.py @@ -111,6 +111,50 @@ def make_ed25519_key(path): 0x04, 0x03, 0x02, 0x01])), ] +# Regression cases for arg2num() saturating 8-byte values >= 2^63 through +# the signed strtoll() (fixed to use strtoull()). 0x0102030405060708 above +# is below LLONG_MAX and never exercises the saturation path, so these two +# values (high bit set, and the all-ones max) are needed to catch it. +SATURATE_CASES = [ + (0x0034, 8, 0x8000000000000001, bytes([0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x80])), + (0x0035, 8, 0xFFFFFFFFFFFFFFFF, bytes([0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF])), +] + + +def run_sign_and_check(cases, work, image, key): + cmd = [SIGN, "--ed25519", "--sha256"] + for tag, length, val, _ in cases: + cmd += ["--custom-tlv", hex(tag), str(length), hex(val)] + cmd += [image, key, "1"] + + r = subprocess.run(cmd, cwd=ROOT, capture_output=True, text=True) + if r.returncode != 0: + skip("sign failed: " + r.stderr.strip()) + + signed = image.replace(".bin", "_v1_signed.bin") + if not os.path.exists(signed): + skip("sign did not produce a signed image") + + with open(signed, "rb") as f: + data = f.read(512) + + failures = [] + for tag, length, val, expected in cases: + got = find_tlv_bytes(data, tag, len(data)) + if got is None: + failures.append( + "tag 0x%04x (len=%d val=%s): TLV not found in header" % + (tag, length, hex(val))) + elif got != expected: + failures.append( + "tag 0x%04x (len=%d val=%s): got [%s], want [%s] (LE)" % + (tag, length, hex(val), + " ".join("%02x" % b for b in got), + " ".join("%02x" % b for b in expected))) + return failures + def main(): if not ensure_sign(): @@ -125,37 +169,8 @@ def main(): with open(image, "wb") as f: f.write(bytes(range(256)) * 8) # 2 KiB dummy payload - cmd = [SIGN, "--ed25519", "--sha256"] - for tag, length, val, _ in TEST_CASES: - cmd += ["--custom-tlv", hex(tag), str(length), hex(val)] - cmd += [image, key, "1"] - - r = subprocess.run(cmd, cwd=ROOT, capture_output=True, text=True) - if r.returncode != 0: - skip("sign failed: " + r.stderr.strip()) - - signed = image.replace(".bin", "_v1_signed.bin") - if not os.path.exists(signed): - skip("sign did not produce a signed image") - - with open(signed, "rb") as f: - data = f.read(512) - - failures = [] - for tag, length, val, expected in TEST_CASES: - got = find_tlv_bytes(data, tag, len(data)) - if got is None: - failures.append( - "tag 0x%04x (len=%d val=%s): TLV not found in header" % - (tag, length, hex(val))) - elif got != expected: - failures.append( - "tag 0x%04x (len=%d val=%s): got [%s], want [%s] (LE); " - "raw memcpy from host-endian uint64_t produces wrong bytes " - "on big-endian build hosts" % - (tag, length, hex(val), - " ".join("%02x" % b for b in got), - " ".join("%02x" % b for b in expected))) + failures = run_sign_and_check(TEST_CASES, work, image, key) + failures += run_sign_and_check(SATURATE_CASES, work, image, key) if failures: for msg in failures: From 72186fa8d34cb2c6e6cbb117fe81bad92461a5fd Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:46:24 +0200 Subject: [PATCH 14/23] F-6126: add mutation-pinning test coverage for wolfBoot_check_flash_image_elf The scattered-ELF flash integrity check's final image_CT_compare() rejection branch had zero unit test coverage, so a weakened comparison (e.g. != 0 -> == 0, or a dropped return -2) would silently pass. Add a positive case (correctly-hashed scattered image verifies OK) and a negative case (single corrupted byte in a scattered segment's flash-resident payload is rejected via the digest-mismatch branch). --- tools/unit-tests/Makefile | 11 + tools/unit-tests/unit-image-elf-scatter.c | 353 ++++++++++++++++++++++ 2 files changed, 364 insertions(+) create mode 100644 tools/unit-tests/unit-image-elf-scatter.c diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6db5f5c29e..81ca7bb168 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -80,6 +80,7 @@ TESTS+=unit-ahci-unlock-panic TESTS+=unit-ata-security-passphrase-zeroize TESTS+=unit-fwtpm-nv-oob TESTS+=unit-elf-bss-guard +TESTS+=unit-image-elf-scatter TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-va416x0-fram TESTS+=unit-flash-write-mcxa @@ -206,6 +207,12 @@ unit-update-disk:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED -DWOLFBOOT_RAMBOOT_M unit-update-disk-oob:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED \ -DWOLFBOOT_RAMBOOT_MAX_SIZE=0x1000 \ -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +# Regression coverage for wolfBoot_check_flash_image_elf() (scattered-ELF +# integrity check). WOLFBOOT_NO_SIGN keeps this to the hashing path only (no +# signature verification is exercised by that function). +unit-image-elf-scatter:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ + -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DWOLFBOOT_ELF_FLASH_SCATTER -DWOLFBOOT_ELF \ + -DIMAGE_HEADER_SIZE=256 unit-string:CFLAGS+=-fno-builtin @@ -601,6 +608,10 @@ unit-elf-bss-guard: unit-elf-bss-guard.c gcc -o $@ $< -I../../include -DWOLFBOOT_ELF -DARCH_FLASH_OFFSET=0 \ -DWOLFBOOT_NO_PRINTF -g $(LDFLAGS) +unit-image-elf-scatter: ../../include/target.h unit-image-elf-scatter.c + gcc -o $@ unit-image-elf-scatter.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c \ + $(CFLAGS) $(LDFLAGS) + %.o:%.c gcc -c -o $@ $^ $(CFLAGS) diff --git a/tools/unit-tests/unit-image-elf-scatter.c b/tools/unit-tests/unit-image-elf-scatter.c new file mode 100644 index 0000000000..ad3d1c23dc --- /dev/null +++ b/tools/unit-tests/unit-image-elf-scatter.c @@ -0,0 +1,353 @@ +/* unit-image-elf-scatter.c + * + * Regression/mutation-pinning test for wolfBoot_check_flash_image_elf() + * (src/image.c), the WOLFBOOT_ELF_FLASH_SCATTER integrity check that + * update_flash.c relies on (and wolfBoot_panic()s on failure of) before + * booting/staging a "scattered" ELF image whose PT_LOAD segments already + * live at their final flash addresses (paddr + BASE_OFF), separate from the + * manifest partition that holds the ELF header/program header table. + * + * The function re-hashes the scattered image and compares the digest + * against the HDR_HASH TLV stored in the manifest header via + * image_CT_compare(). A mutation of that "!= 0" to "== 0", or a dropped + * "return -2", would silently accept a corrupted scattered image. These + * tests pin: (1) a correctly-hashed scattered image verifies OK, and (2) a + * single corrupted byte in a scattered segment's flash-resident payload + * (not the manifest/ELF headers) is rejected via the digest-mismatch branch. + * + * 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 +#include +#include + +#include "user_settings.h" +#include "wolfboot/wolfboot.h" +#include "elf.h" + +/* Pull in elf.c and image.c directly (in-tree compilation, mirroring + * unit-image.c / unit-elf-bss-guard.c). This gives the test direct access + * to the *static* hashing helpers used internally by + * wolfBoot_check_flash_image_elf() (header_hash, update_hash_flash_fwimg, + * update_hash_flash_addr, final_hash), so the expected digest can be + * computed independently by replaying the same primitives on the same + * bytes, rather than tautologically trusting the function's own output. + * + * libwolfboot.c is intentionally *not* pulled in here (it would clash with + * static helpers of the same name already defined in image.c, e.g. im2n()); + * the handful of libwolfboot.c symbols image.c still needs at link time + * (wolfBoot_find_header/wolfBoot_get_blob_version/wolfBoot_get_blob_type) + * are provided below, with wolfBoot_find_header reusing the real TLV-parsing + * logic from libwolfboot.c verbatim, since HDR_HASH lookup correctness is + * central to this test. */ +#include "elf.c" +#include "image.c" + +#include "unit-mock-flash.c" + +#define MOCK_ADDRESS_BOOT 0xCD000000 + +uint32_t wolfBoot_get_blob_version(uint8_t *blob) +{ + (void)blob; + return 1; +} + +uint16_t wolfBoot_get_blob_type(uint8_t *blob) +{ + (void)blob; + return HDR_IMG_TYPE_APP; +} + +/* Verbatim copy of wolfBoot_find_header() from src/libwolfboot.c (not + * pulled in as a whole to avoid the im2n() redefinition clash noted above). + */ +uint16_t wolfBoot_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr) +{ + uint8_t *p; + uint16_t len, htype; + uintptr_t p_addr, max_addr; + + *ptr = NULL; + + if (haystack == NULL) { + return 0; + } + + p_addr = (uintptr_t)haystack; + if (p_addr < IMAGE_HEADER_OFFSET) { + return 0; + } + + max_addr = p_addr - IMAGE_HEADER_OFFSET; + if (max_addr > (UINTPTR_MAX - IMAGE_HEADER_SIZE)) { + return 0; + } + max_addr += IMAGE_HEADER_SIZE; + + if (p_addr > max_addr) { + return 0; + } + + while (p_addr < max_addr) { + if ((max_addr - p_addr) < 4U) { + break; + } + p = (uint8_t *)p_addr; + htype = (uint16_t)(p[0] | (p[1] << 8)); + if (htype == 0) { + break; + } + if ((p[0] == HDR_PADDING) || ((p_addr & 0x01U) != 0U)) { + p_addr++; + continue; + } + + len = (uint16_t)(p[2] | (p[3] << 8)); + if ((4U + len) > (uint16_t)(IMAGE_HEADER_SIZE - IMAGE_HEADER_OFFSET)) { + break; + } + if ((max_addr - p_addr) < (uintptr_t)(4U + len)) { + break; + } + + if (htype == type) { + *ptr = (uint8_t *)(p_addr + 4U); + return len; + } + p_addr += (uintptr_t)(4U + len); + } + return 0; +} + +/* --- Scattered ELF layout (ELF64, 1 PT_LOAD segment) --- + * + * The manifest partition (PART_BOOT) holds: image header (IMAGE_HEADER_SIZE + * bytes, with the HDR_HASH TLV) immediately followed by the ELF64 header and + * its single program header (tightly packed, no gaps). The PT_LOAD segment + * itself is *not* stored in the manifest: its bytes live at a separate flash + * address (ph.paddr), exactly as wolfBoot_check_flash_image_elf expects for + * a scattered image (BASE_OFF is 0 here since ARCH_SIM is not defined). + * + * The program header's offset/file_size are chosen so that: + * - ph.offset == ELF_HDR_SZ (no padding before the segment) + * - ph.offset + ph.file_size == fw_size (no trailing bytes to hash) + * so the only bytes fed to the hash are: the manifest header (up to + * HDR_HASH), the ELF header + program header table, and the segment + * payload at its scattered flash address. + */ +#define SEG_SIZE 64U +#define PH_COUNT 1U +#define ELF_HDR_SZ (sizeof(elf64_header) + PH_COUNT * sizeof(elf64_program_header)) +#define IMG_FW_SIZE (ELF_HDR_SZ + SEG_SIZE) + +/* The scattered PT_LOAD segment's flash-resident payload. Its address is + * used directly as the program header's paddr (BASE_OFF == 0), standing in + * for "flash at address paddr" the way a real target would. */ +static uint8_t segment_flash[SEG_SIZE]; + +static void write_le16(uint8_t *p, uint16_t v) +{ + p[0] = (uint8_t)(v & 0xFFU); + p[1] = (uint8_t)((v >> 8) & 0xFFU); +} + +/* Builds the manifest header + ELF header/PHT at MOCK_ADDRESS_BOOT, and the + * scattered segment payload in segment_flash[], with the HDR_HASH TLV left + * zeroed (caller must patch it in via patch_expected_digest()). */ +static void build_scattered_image(void) +{ + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + uint32_t magic = WOLFBOOT_MAGIC; + uint32_t fw_size = IMG_FW_SIZE; + elf64_header *eh; + elf64_program_header *ph; + unsigned int i; + + memset(manifest, 0, IMAGE_HEADER_SIZE + ELF_HDR_SZ); + + /* --- manifest header --- */ + memcpy(manifest + 0, &magic, sizeof(magic)); + memcpy(manifest + 4, &fw_size, sizeof(fw_size)); + /* HDR_HASH TLV at offset 8: type(2) + len(2) + digest(WOLFBOOT_SHA_DIGEST_SIZE) */ + write_le16(manifest + 8, HDR_HASH); + write_le16(manifest + 10, WOLFBOOT_SHA_DIGEST_SIZE); + /* digest bytes at manifest+12 are left zeroed; patched in later */ + + /* --- ELF64 header, immediately following the manifest header --- */ + eh = (elf64_header *)(manifest + IMAGE_HEADER_SIZE); + memcpy(eh->ident, ELF_IDENT_STR, 4); + eh->ident[ELF_CLASS_OFF] = ELF_CLASS_64; + eh->ident[5] = ELF_ENDIAN_LITTLE; + eh->type = ELF_HET_EXEC; + eh->machine = 0; + eh->version = 1; + eh->entry = 0x2000; + eh->ph_offset = sizeof(elf64_header); + eh->sh_offset = 0; + eh->flags = 0; + eh->header_size = sizeof(elf64_header); + eh->ph_entry_size = sizeof(elf64_program_header); + eh->ph_entry_count = PH_COUNT; + eh->sh_entry_size = 0; + eh->sh_entry_count = 0; + eh->sh_str_index = 0; + + /* --- single PT_LOAD program header, immediately after the ELF header --- */ + ph = (elf64_program_header *)((uint8_t *)eh + sizeof(elf64_header)); + ph->type = ELF_PT_LOAD; + ph->flags = 5; /* R+X, not inspected by the function under test */ + ph->offset = ELF_HDR_SZ; + ph->vaddr = 0; /* not read by wolfBoot_check_flash_image_elf */ + ph->paddr = (uint64_t)(uintptr_t)segment_flash; + ph->file_size = SEG_SIZE; + ph->mem_size = SEG_SIZE; + ph->align = 1; + + /* --- scattered segment payload, deterministic non-trivial pattern --- */ + for (i = 0; i < SEG_SIZE; i++) { + segment_flash[i] = (uint8_t)(0xA0U + i); + } +} + +/* Independently recomputes the expected HDR_HASH digest by replaying the + * exact same hashing primitives wolfBoot_check_flash_image_elf() uses + * internally (header_hash / update_hash_flash_fwimg / update_hash_flash_addr + * / final_hash), rather than deriving the "expected" value from the + * function under test itself. */ +static void compute_expected_digest(uint8_t *out) +{ + struct wolfBoot_image boot; + wolfBoot_hash_t ctx; + + ck_assert_int_eq(wolfBoot_open_image(&boot, PART_BOOT), 0); + ck_assert_int_eq(header_hash(&ctx, &boot), 0); + ck_assert_int_eq(update_hash_flash_fwimg(&ctx, &boot, 0, (uint32_t)ELF_HDR_SZ), 0); + ck_assert_int_eq( + update_hash_flash_addr(&ctx, (uintptr_t)segment_flash, SEG_SIZE, + PART_IS_EXT(&boot)), + 0); + ck_assert_int_eq(final_hash(&ctx, out), 0); +} + +static void patch_expected_digest(const uint8_t *digest) +{ + uint8_t *manifest = (uint8_t *)(uintptr_t)MOCK_ADDRESS_BOOT; + memcpy(manifest + 12, digest, WOLFBOOT_SHA_DIGEST_SIZE); +} + +static void map_boot_partition(void) +{ + int ret = mmap_file("/tmp/wolfboot-unit-elf-scatter-boot.bin", + (void *)MOCK_ADDRESS_BOOT, WOLFBOOT_PARTITION_SIZE, + NULL); + ck_assert_int_ge(ret, 0); +} + +static void unmap_boot_partition(void) +{ + munmap((void *)MOCK_ADDRESS_BOOT, WOLFBOOT_PARTITION_SIZE); +} + +START_TEST(test_elf_scatter_valid_image_verifies_ok) +{ + uint8_t expected_digest[WOLFBOOT_SHA_DIGEST_SIZE]; + unsigned long entry = 0; + int ret; + + map_boot_partition(); + + build_scattered_image(); + compute_expected_digest(expected_digest); + patch_expected_digest(expected_digest); + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq((uint32_t)entry, 0x2000U); + + unmap_boot_partition(); +} +END_TEST + +START_TEST(test_elf_scatter_corrupted_segment_rejected) +{ + uint8_t expected_digest[WOLFBOOT_SHA_DIGEST_SIZE]; + unsigned long entry = 0; + int ret; + + map_boot_partition(); + + build_scattered_image(); + compute_expected_digest(expected_digest); + patch_expected_digest(expected_digest); + + /* Sanity check: with the digest matching, verification must succeed + * first, so the corruption below is the only thing that changes. */ + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + ck_assert_int_eq(ret, 0); + + /* Corrupt a single byte of the scattered segment's *flash-resident* + * payload (at its paddr location), strictly after the expected digest + * was computed and stored. The ELF/program headers are untouched, so + * every structural check (magic, elf_open, scatter-format, ph parsing, + * bounds) still passes -- only the final digest comparison should + * fail. */ + segment_flash[SEG_SIZE / 2] ^= 0xFFU; + + ret = wolfBoot_check_flash_image_elf(PART_BOOT, &entry); + + /* -2 is the dedicated "digest mismatch" return value from + * image_CT_compare() failing in wolfBoot_check_flash_image_elf(); any + * other value (0, or -1 from an earlier structural check) means either + * the corruption was not detected, or it was detected for the wrong + * reason. */ + ck_assert_int_eq(ret, -2); + + unmap_boot_partition(); +} +END_TEST + +Suite *elf_scatter_suite(void) +{ + Suite *s = suite_create("ELF flash-scatter image check"); + TCase *tc = tcase_create("wolfBoot_check_flash_image_elf"); + tcase_add_test(tc, test_elf_scatter_valid_image_verifies_ok); + tcase_add_test(tc, test_elf_scatter_corrupted_segment_rejected); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = elf_scatter_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From eb5574f19e4f00ac606868b0ee322e0d6606bc5c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:47:17 +0200 Subject: [PATCH 15/23] F-6124: emit build-time warning for SIGN=NONE / WOLFBOOT_NO_SIGN SIGN=NONE disables firmware signature authenticity verification entirely (wolfBoot_verify_authenticity() becomes a stub that always confirms), leaving only a hash check an attacker can satisfy. Every other fail-safe-weakening option in this file (ALLOW_DOWNGRADE, DISABLE_BACKUP, WOLFBOOT_UDS_UID_FALLBACK_FORTEST, FPGA_NONFATAL) emits a $(warning ...) so the tradeoff is visible at build time; SIGN=NONE was missing one despite being the most security-critical. --- options.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/options.mk b/options.mk index e6772d499d..b833c6a116 100644 --- a/options.mk +++ b/options.mk @@ -164,6 +164,7 @@ endif ## DSA Settings ifeq ($(SIGN),NONE) + $(warning SIGN=NONE / WOLFBOOT_NO_SIGN=1 disables firmware signature verification; images are NOT authenticated. Do not use in production.) SIGN_OPTIONS+=--no-sign ifeq ($(HASH),SHA384) STACK_USAGE=3760 From 102ddc38fda41b21f75be1ef27add8c0416b24ec Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 15:57:54 +0200 Subject: [PATCH 16/23] F-5964: fix stale in-word offset in hal_flash_write byte-wise path (samr21/same51) In the else branch of hal_flash_write, off was computed once per call from the original (call-time) "address" instead of the current position "address + i". The word index dst_idx advanced with i, but the fill loop kept starting at the stale off, so once destination address and source buffer had different alignment mod 4, every word after the first was filled at the wrong byte offset, dropping and misplacing data. Derive off and dst from address + i so each word's offset tracks the current position, mirroring the fix already applied to mcxa.c for the same bug class (F-5963). --- hal/same51.c | 11 +- hal/samr21.c | 11 +- tools/unit-tests/Makefile | 8 + tools/unit-tests/unit-flash-write-same51.c | 168 +++++++++++++++++++++ tools/unit-tests/unit-flash-write-samr21.c | 168 +++++++++++++++++++++ 5 files changed, 356 insertions(+), 10 deletions(-) create mode 100644 tools/unit-tests/unit-flash-write-same51.c create mode 100644 tools/unit-tests/unit-flash-write-samr21.c diff --git a/hal/same51.c b/hal/same51.c index ebd9aa13eb..a1950df99c 100644 --- a/hal/same51.c +++ b/hal/same51.c @@ -280,7 +280,9 @@ void hal_init(void) /* Turn off watchdog */ WDT_CTRL &= (~WDT_EN); /* Run the bootloader with interrupts off */ +#ifndef WOLFBOOT_UNIT_TEST_FLASH_WRITE __asm__ volatile ("cpsid i"); +#endif /* Initialize clock */ clock_init(); @@ -357,17 +359,16 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } else { uint32_t val; uint8_t *vbytes = (uint8_t *)(&val); - uint32_t off = (address % 4); - dst = (uint32_t *)(address - off); - uint32_t dst_idx = (i + off) >> 2; - val = dst[dst_idx]; + uint32_t off = ((address + i) % 4); + dst = (uint32_t *)(address + i - off); + val = *dst; while (off < 4) { if (i < len) vbytes[off++] = data[i++]; else off++; } - dst[dst_idx] = val; + *dst = val; } if ((i == len) || ((i % 16)== 0)) NVMCTRLB = (NVMCMD_WQW | NVMCMD_KEY); diff --git a/hal/samr21.c b/hal/samr21.c index 31dbb51c35..d9c4f19538 100644 --- a/hal/samr21.c +++ b/hal/samr21.c @@ -106,7 +106,9 @@ void hal_init(void) { WDT_CTRL &= (~WDT_EN); +#ifndef WOLFBOOT_UNIT_TEST_FLASH_WRITE __asm__ volatile ("cpsid i"); +#endif uint32_t i, reg; /* enable clocks for the power, sysctrl and gclk modules */ APBAMASK_REG = APBAMASK_PM_EN | APBAMASK_SYSCTRL_EN | APBAMASK_GCLK_EN; @@ -176,17 +178,16 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } else { uint32_t val; uint8_t *vbytes = (uint8_t *)(&val); - uint32_t off = (address % 4); - dst = (uint32_t *)(address - off); - uint32_t dst_idx = (i + off) >> 2; - val = dst[dst_idx]; + uint32_t off = ((address + i) % 4); + dst = (uint32_t *)(address + i - off); + val = *dst; while (off < 4) { if (i < len) vbytes[off++] = data[i++]; else off++; } - dst[dst_idx] = val; + *dst = val; } } /* Enable write protection */ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 81ca7bb168..c4bc4c0035 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -84,6 +84,8 @@ TESTS+=unit-image-elf-scatter TESTS+=unit-arm-tee-psa-ipc TESTS+=unit-va416x0-fram TESTS+=unit-flash-write-mcxa +TESTS+=unit-flash-write-samr21 +TESTS+=unit-flash-write-same51 TESTS+=unit-imx-rt-cache-align # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit @@ -598,6 +600,12 @@ unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-p unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c gcc -o $@ unit-flash-write-mcxa.c -Imcxa_fsl_stub $(CFLAGS) $(LDFLAGS) +unit-flash-write-samr21: unit-flash-write-samr21.c ../../hal/samr21.c + gcc -o $@ unit-flash-write-samr21.c $(CFLAGS) $(LDFLAGS) + +unit-flash-write-same51: unit-flash-write-same51.c ../../hal/same51.c + gcc -o $@ unit-flash-write-same51.c $(CFLAGS) $(LDFLAGS) + # unit-imx-rt-cache-align only pulls in hal/imx_rt.h, which is dependency-free # by design, avoiding the (not vendored) NXP MCUXpresso SDK headers that # hal/imx_rt.c itself requires. diff --git a/tools/unit-tests/unit-flash-write-same51.c b/tools/unit-tests/unit-flash-write-same51.c new file mode 100644 index 0000000000..3c750163a4 --- /dev/null +++ b/tools/unit-tests/unit-flash-write-same51.c @@ -0,0 +1,168 @@ +/* unit-flash-write-same51.c + * + * Regression test for F-5964: in the byte-wise (unaligned) path of + * hal_flash_write() (hal/samr21.c, hal/same51.c), the in-word offset was + * computed every loop iteration as + * uint32_t off = (address % 4); + * using the original (call-time) "address", never the current position + * "address + i". The word index "dst_idx = (i + off) >> 2" does advance + * with "i", but the byte-fill loop + * while (off < 4) { ... vbytes[off++] = data[i++]; ... } + * starts filling at the stale "off" instead of "(address + i) % 4". Once + * the first partial word has been written, if the destination address and + * the source buffer have different alignment mod 4 (so the fast 32-bit + * path never re-syncs), every subsequent word is filled starting at the + * wrong byte position: some destination bytes are left unwritten and + * others get the wrong source byte. + * + * 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 + +/* hal/same51.c's hal_init() contains an ARMv7E-M-only "cpsid i" instruction + * that the host assembler rejects; this test never calls hal_init(), so + * exclude just that instruction (not otherwise used by hal_flash_write()). */ +#define WOLFBOOT_UNIT_TEST_FLASH_WRITE + +#include "image.h" + +#include "../../hal/same51.c" + +/* hal_flash_write() pokes the NVMCTRL command register (NVMCTRLB, fixed at + * NVMCTRL_BASE == 0x41004000) directly; map that page so those writes don't + * fault. Its contents are otherwise irrelevant here. */ +static void map_nvmctrl(void) +{ + int flags = MAP_PRIVATE | MAP_ANONYMOUS; +#ifdef MAP_FIXED_NOREPLACE + flags |= MAP_FIXED_NOREPLACE; +#else + flags |= MAP_FIXED; +#endif + void *p = mmap((void *)(uintptr_t)NVMCTRL_BASE, 4096, + PROT_READ | PROT_WRITE, flags, -1, 0); + ck_assert_ptr_eq(p, (void *)(uintptr_t)NVMCTRL_BASE); +} + +static void unmap_nvmctrl(void) +{ + munmap((void *)(uintptr_t)NVMCTRL_BASE, 4096); +} + +/* "address" is treated as a real pointer into memory-mapped flash. Keep the + * mock flash buffer inside the 32-bit range, matching how "address" (a + * uint32_t) is used by the real target. */ +#define MOCK_FLASH_SIZE 64 +static uint8_t *mock_flash; + +static void setup(void) +{ + map_nvmctrl(); + mock_flash = mmap(NULL, MOCK_FLASH_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(mock_flash, MAP_FAILED); + memset(mock_flash, 0xFF, MOCK_FLASH_SIZE); +} + +static void teardown(void) +{ + munmap(mock_flash, MOCK_FLASH_SIZE); + unmap_nvmctrl(); +} + +/* Destination misaligned by 1 (mod 4), source buffer misaligned by 2 (mod + * 4): the two never share the same alignment, so after the first partial + * word the fast 32-bit path can never re-sync and every remaining word + * goes through the buggy byte-wise path. Before the fix, this drops and + * misplaces bytes past the first word. */ +START_TEST(test_unaligned_write_mismatched_alignment) +{ + uint8_t rawbuf[64]; + uint8_t *data = rawbuf; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + while (((uintptr_t)data % 4) != 2) + data++; + for (i = 0; i < 12; i++) + data[i] = (uint8_t)(0xA0 + i); + + ck_assert_int_eq(hal_flash_write(base + 5, data, 8), 0); + + for (i = 0; i < 5; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + for (i = 0; i < 8; i++) + ck_assert_uint_eq(mock_flash[5 + i], data[i]); + for (i = 13; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +/* A write that fits entirely inside a single flash word must still work: + * buggy and fixed forms agree here (the fill loop runs to completion on + * the first iteration), guarding against a fix that breaks the common + * case. */ +START_TEST(test_unaligned_write_single_word) +{ + uint8_t data[3]; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + for (i = 0; i < 3; i++) + data[i] = (uint8_t)(0xB0 + i); + + ck_assert_int_eq(hal_flash_write(base + 1, data, 3), 0); + + ck_assert_uint_eq(mock_flash[0], 0xFF); + for (i = 0; i < 3; i++) + ck_assert_uint_eq(mock_flash[1 + i], data[i]); + for (i = 4; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +Suite *flash_write_suite(void) +{ + Suite *s = suite_create("flash-write-same51"); + TCase *tc = tcase_create("flash-write-same51"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_unaligned_write_mismatched_alignment); + tcase_add_test(tc, test_unaligned_write_single_word); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_write_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} diff --git a/tools/unit-tests/unit-flash-write-samr21.c b/tools/unit-tests/unit-flash-write-samr21.c new file mode 100644 index 0000000000..e974b4fc51 --- /dev/null +++ b/tools/unit-tests/unit-flash-write-samr21.c @@ -0,0 +1,168 @@ +/* unit-flash-write-samr21.c + * + * Regression test for F-5964: in the byte-wise (unaligned) path of + * hal_flash_write() (hal/samr21.c, hal/same51.c), the in-word offset was + * computed every loop iteration as + * uint32_t off = (address % 4); + * using the original (call-time) "address", never the current position + * "address + i". The word index "dst_idx = (i + off) >> 2" does advance + * with "i", but the byte-fill loop + * while (off < 4) { ... vbytes[off++] = data[i++]; ... } + * starts filling at the stale "off" instead of "(address + i) % 4". Once + * the first partial word has been written, if the destination address and + * the source buffer have different alignment mod 4 (so the fast 32-bit + * path never re-syncs), every subsequent word is filled starting at the + * wrong byte position: some destination bytes are left unwritten and + * others get the wrong source byte. + * + * 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 + +/* hal/samr21.c's hal_init() contains an ARMv6-M-only "cpsid i" instruction + * that the host assembler rejects; this test never calls hal_init(), so + * exclude just that instruction (not otherwise used by hal_flash_write()). */ +#define WOLFBOOT_UNIT_TEST_FLASH_WRITE + +#include "image.h" + +#include "../../hal/samr21.c" + +/* hal_flash_write() pokes the NVMCTRL command register (NVMCTRLA_REG, fixed + * at NVMCTRL_BASE == 0x41004000) directly at entry/exit. Map that page so + * those writes don't fault; its contents are otherwise irrelevant here. */ +static void map_nvmctrl(void) +{ + int flags = MAP_PRIVATE | MAP_ANONYMOUS; +#ifdef MAP_FIXED_NOREPLACE + flags |= MAP_FIXED_NOREPLACE; +#else + flags |= MAP_FIXED; +#endif + void *p = mmap((void *)(uintptr_t)NVMCTRL_BASE, 4096, + PROT_READ | PROT_WRITE, flags, -1, 0); + ck_assert_ptr_eq(p, (void *)(uintptr_t)NVMCTRL_BASE); +} + +static void unmap_nvmctrl(void) +{ + munmap((void *)(uintptr_t)NVMCTRL_BASE, 4096); +} + +/* "address" is treated as a real pointer into memory-mapped flash. Keep the + * mock flash buffer inside the 32-bit range, matching how "address" (a + * uint32_t) is used by the real target. */ +#define MOCK_FLASH_SIZE 64 +static uint8_t *mock_flash; + +static void setup(void) +{ + map_nvmctrl(); + mock_flash = mmap(NULL, MOCK_FLASH_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(mock_flash, MAP_FAILED); + memset(mock_flash, 0xFF, MOCK_FLASH_SIZE); +} + +static void teardown(void) +{ + munmap(mock_flash, MOCK_FLASH_SIZE); + unmap_nvmctrl(); +} + +/* Destination misaligned by 1 (mod 4), source buffer misaligned by 2 (mod + * 4): the two never share the same alignment, so after the first partial + * word the fast 32-bit path can never re-sync and every remaining word + * goes through the buggy byte-wise path. Before the fix, this drops and + * misplaces bytes past the first word. */ +START_TEST(test_unaligned_write_mismatched_alignment) +{ + uint8_t rawbuf[64]; + uint8_t *data = rawbuf; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + while (((uintptr_t)data % 4) != 2) + data++; + for (i = 0; i < 12; i++) + data[i] = (uint8_t)(0xA0 + i); + + ck_assert_int_eq(hal_flash_write(base + 5, data, 8), 0); + + for (i = 0; i < 5; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + for (i = 0; i < 8; i++) + ck_assert_uint_eq(mock_flash[5 + i], data[i]); + for (i = 13; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +/* A write that fits entirely inside a single flash word must still work: + * buggy and fixed forms agree here (the fill loop runs to completion on + * the first iteration), guarding against a fix that breaks the common + * case. */ +START_TEST(test_unaligned_write_single_word) +{ + uint8_t data[3]; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + for (i = 0; i < 3; i++) + data[i] = (uint8_t)(0xB0 + i); + + ck_assert_int_eq(hal_flash_write(base + 1, data, 3), 0); + + ck_assert_uint_eq(mock_flash[0], 0xFF); + for (i = 0; i < 3; i++) + ck_assert_uint_eq(mock_flash[1 + i], data[i]); + for (i = 4; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +Suite *flash_write_suite(void) +{ + Suite *s = suite_create("flash-write-samr21"); + TCase *tc = tcase_create("flash-write-samr21"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_unaligned_write_mismatched_alignment); + tcase_add_test(tc, test_unaligned_write_single_word); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_write_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From dd16ccb590587c9dfdc14c08ae74f4e3dfe33332 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 16:05:38 +0200 Subject: [PATCH 17/23] F-6405: pin partition-fit accept boundary in wolfBoot_open_image_address/open_self_address The fw_size > (WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE) guard was only tested on the reject side (oversize + 1); no test asserted that a firmware payload exactly filling the partition payload budget is accepted. A >/>= mutation at image.c:1402 or image.c:1618 would silently reject a maximally-sized valid image and survive the existing suite. Add positive boundary assertions (via wolfBoot_open_image()/wolfBoot_open_image_address() and wolfBoot_open_self_address()) paired with the existing reject-side checks in test_open_image. --- tools/unit-tests/unit-image.c | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tools/unit-tests/unit-image.c b/tools/unit-tests/unit-image.c index 84e3276048..be99438880 100644 --- a/tools/unit-tests/unit-image.c +++ b/tools/unit-tests/unit-image.c @@ -1028,6 +1028,29 @@ START_TEST(test_open_image) ck_assert_int_eq(ret, -1); ck_assert_uint_eq(img.hdr_ok, 0); + /* A firmware payload that exactly fills the partition payload budget + * (fw_size == WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE) must be + * accepted by the partition-fit check in wolfBoot_open_image_address(), + * with fw_base/trailer positioned right after the header / at the end + * of the partition. */ + memset(self_hdr, 0xFF, sizeof(self_hdr)); + ((uint32_t *)self_hdr)[0] = WOLFBOOT_MAGIC; + ((uint32_t *)self_hdr)[1] = WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE; + + ext_flash_erase(0, WOLFBOOT_SECTOR_SIZE); + ext_flash_write(0, self_hdr, IMAGE_HEADER_SIZE); + + memset(&img, 0, sizeof(img)); + hdr_cpy_done = 0; + ret = wolfBoot_open_image(&img, PART_UPDATE); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(img.hdr_ok, 1); + ck_assert_uint_eq(img.fw_size, WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE); + ck_assert_ptr_eq(img.fw_base, (uint8_t *)WOLFBOOT_PARTITION_UPDATE_ADDRESS + + IMAGE_HEADER_SIZE); + ck_assert_ptr_eq(img.trailer, (uint8_t *)WOLFBOOT_PARTITION_UPDATE_ADDRESS + + WOLFBOOT_PARTITION_SIZE); + /* Self header must reject bad magic and leave hdr_ok cleared */ memset(self_hdr, 0xFF, sizeof(self_hdr)); ((uint32_t *)self_hdr)[0] = ~WOLFBOOT_MAGIC; @@ -1038,6 +1061,19 @@ START_TEST(test_open_image) ck_assert_int_eq(ret, -1); ck_assert_uint_eq(img.hdr_ok, 0); + /* Self header must accept sizes that exactly fill the partition + * payload budget (accept side of the same boundary checked below) */ + memset(self_hdr, 0xFF, sizeof(self_hdr)); + ((uint32_t *)self_hdr)[0] = WOLFBOOT_MAGIC; + ((uint32_t *)self_hdr)[1] = WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE; + + memset(&img, 0, sizeof(img)); + ret = wolfBoot_open_self_address(&img, self_hdr, + (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(img.hdr_ok, 1); + ck_assert_uint_eq(img.fw_size, WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE); + /* Self header must reject sizes beyond the partition payload budget */ memset(self_hdr, 0xFF, sizeof(self_hdr)); ((uint32_t *)self_hdr)[0] = WOLFBOOT_MAGIC; From c4e853ea4b13cc3bcbf701f4f648afeb0fbe095f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 16:17:51 +0200 Subject: [PATCH 18/23] F-6131: enforce full-transfer bounds check in spi_flash_read/spi_flash_write (QSPI) spi_flash_read only rejected address > FLASH_DEVICE_SIZE (off-by-one, admits address == FLASH_DEVICE_SIZE) and never validated address+len against the device size, so an in-bounds start with an out-of-range extent was not rejected. spi_flash_write had no bounds check at all. Add an inclusive/full-extent check to both. --- src/qspi_flash.c | 16 +++++++++-- tools/unit-tests/unit-qspi-flash.c | 46 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/qspi_flash.c b/src/qspi_flash.c index 6a446af263..bc6095b7ea 100644 --- a/src/qspi_flash.c +++ b/src/qspi_flash.c @@ -389,10 +389,11 @@ int spi_flash_read(uint32_t address, void *data, int len) uint32_t altMode = QSPI_DATA_MODE_NONE; #endif - if (address > FLASH_DEVICE_SIZE) { + if ((len < 0) || (address >= FLASH_DEVICE_SIZE) || + ((uint64_t)address + (uint32_t)len > FLASH_DEVICE_SIZE)) { #ifdef DEBUG_QSPI - wolfBoot_printf("QSPI Flash Read: Invalid address (0x%x > 0x%x max)\n", - address, FLASH_DEVICE_SIZE); + wolfBoot_printf("QSPI Flash Read: Invalid address (0x%x, len %d, max 0x%x)\n", + address, len, FLASH_DEVICE_SIZE); #endif return -1; } @@ -427,6 +428,15 @@ int spi_flash_write(uint32_t address, const void *data, int len) len, data, address); #endif + if ((len < 0) || (address >= FLASH_DEVICE_SIZE) || + ((uint64_t)address + (uint32_t)len > FLASH_DEVICE_SIZE)) { +#ifdef DEBUG_QSPI + wolfBoot_printf("QSPI Flash Write: Invalid address (0x%x, len %d, max 0x%x)\n", + address, len, FLASH_DEVICE_SIZE); +#endif + return -1; + } + /* write by page */ pages = ((len + (FLASH_PAGE_SIZE-1)) / FLASH_PAGE_SIZE); for (page = 0; page < pages; page++) { diff --git a/tools/unit-tests/unit-qspi-flash.c b/tools/unit-tests/unit-qspi-flash.c index 17e1bf1648..344c7272a5 100644 --- a/tools/unit-tests/unit-qspi-flash.c +++ b/tools/unit-tests/unit-qspi-flash.c @@ -17,6 +17,7 @@ static uint32_t program_addrs[8]; static int write_enable_call_count; static int write_enable_status_seq[8]; static int current_write_enable_call; +static int read_call_count; void spi_init(int polarity, int phase) { @@ -72,6 +73,11 @@ int qspi_transfer(uint8_t fmode, const uint8_t cmd, return 0; } + if (cmd == FLASH_READ_CMD) { + read_call_count++; + return 0; + } + return 0; } @@ -84,6 +90,7 @@ static void setup(void) write_enable_call_count = 0; current_write_enable_call = 0; memset(write_enable_status_seq, 0, sizeof(write_enable_status_seq)); + read_call_count = 0; } START_TEST(test_qspi_write_splits_last_page_to_remaining_bytes) @@ -124,6 +131,42 @@ START_TEST(test_qspi_write_stops_after_midloop_write_enable_failure) } END_TEST +START_TEST(test_qspi_read_rejects_address_at_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_read(FLASH_DEVICE_SIZE, buf, sizeof(buf)); + + ck_assert_int_eq(ret, -1); + ck_assert_int_eq(read_call_count, 0); +} +END_TEST + +START_TEST(test_qspi_read_rejects_transfer_extending_past_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_read(FLASH_DEVICE_SIZE - 4, buf, sizeof(buf)); + + ck_assert_int_eq(ret, -1); + ck_assert_int_eq(read_call_count, 0); +} +END_TEST + +START_TEST(test_qspi_write_rejects_transfer_extending_past_device_size) +{ + uint8_t buf[16]; + int ret; + + ret = spi_flash_write(FLASH_DEVICE_SIZE - 4, buf, sizeof(buf)); + + ck_assert_int_ne(ret, 0); + ck_assert_int_eq(program_call_count, 0); +} +END_TEST + static Suite *qspi_flash_suite(void) { Suite *s; @@ -134,6 +177,9 @@ static Suite *qspi_flash_suite(void) tcase_add_checked_fixture(tc, setup, NULL); tcase_add_test(tc, test_qspi_write_splits_last_page_to_remaining_bytes); tcase_add_test(tc, test_qspi_write_stops_after_midloop_write_enable_failure); + tcase_add_test(tc, test_qspi_read_rejects_address_at_device_size); + tcase_add_test(tc, test_qspi_read_rejects_transfer_extending_past_device_size); + tcase_add_test(tc, test_qspi_write_rejects_transfer_extending_past_device_size); suite_add_tcase(s, tc); return s; } From 0955a424210ff9e139f598ed8e663e715c79df2c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 16:45:10 +0200 Subject: [PATCH 19/23] Added new tests to .gitignore --- .gitignore | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.gitignore b/.gitignore index 683ca3d598..b1bc7fb6ad 100644 --- a/.gitignore +++ b/.gitignore @@ -213,6 +213,18 @@ tools/unit-tests/unit-fwtpm-nv-oob tools/unit-tests/unit-x86-paging-oob tools/unit-tests/unit-ahci-unlock-panic tools/unit-tests/unit-ata-security-passphrase-zeroize +tools/unit-tests/unit-arm-tee-psa-ipc +tools/unit-tests/unit-flash-write-mcxa +tools/unit-tests/unit-flash-write-same51 +tools/unit-tests/unit-flash-write-samr21 +tools/unit-tests/unit-image-elf-scatter +tools/unit-tests/unit-image-hybrid +tools/unit-tests/unit-imx-rt-cache-align +tools/unit-tests/unit-update-disk-oob +tools/unit-tests/unit-update-ram-enc +tools/unit-tests/unit-update-ram-enc-nopart +tools/unit-tests/unit-va416x0-fram +tools/unit-tests/unit-wolfhsm_flash_hal From aecf9049782835fc28dff1341cc0910a7305816e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 17:01:08 +0200 Subject: [PATCH 20/23] hal: advance source index in aligned flash-write fast path The bulk FLASH_Program path read data+w but never advanced w, so a partial-word tail after an aligned run re-read the input from offset 0. Affects kinetis, mcxa, mcxw. Pin with an mcxa bulk+tail test case. --- hal/kinetis.c | 1 + hal/mcxa.c | 1 + hal/mcxw.c | 1 + tools/unit-tests/unit-flash-write-mcxa.c | 26 ++++++++++++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/hal/kinetis.c b/hal/kinetis.c index c0f86c5b38..91d83d4757 100644 --- a/hal/kinetis.c +++ b/hal/kinetis.c @@ -337,6 +337,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) ret = FLASH_Program(&pflash, address, (uint8_t*)data + w, len_align); if (ret != kStatus_FTFx_Success) return -1; + w += len_align; len -= len_align; address += len_align; } diff --git a/hal/mcxa.c b/hal/mcxa.c index f1dd73c904..1f819103a3 100644 --- a/hal/mcxa.c +++ b/hal/mcxa.c @@ -101,6 +101,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) ret = FLASH_ProgramPhrase(&pflash, address, (uint8_t*)data + w, len_align); if (ret != kStatus_Success) return -1; + w += len_align; len -= len_align; address += len_align; } diff --git a/hal/mcxw.c b/hal/mcxw.c index 0d3d352a9b..776c0ecf09 100644 --- a/hal/mcxw.c +++ b/hal/mcxw.c @@ -183,6 +183,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) write_flash_qword((uint32_t *)(address + i), (const uint32_t *)(data + w + i)); } + w += len_align; len -= len_align; address += len_align; } diff --git a/tools/unit-tests/unit-flash-write-mcxa.c b/tools/unit-tests/unit-flash-write-mcxa.c index 29eaa2f86b..b98c2606bd 100644 --- a/tools/unit-tests/unit-flash-write-mcxa.c +++ b/tools/unit-tests/unit-flash-write-mcxa.c @@ -136,6 +136,31 @@ START_TEST(test_unaligned_write_single_word) } END_TEST +/* An aligned write longer than one flash word takes the bulk fast path + * (FLASH_ProgramPhrase over data + w) for the first 16 bytes, then a + * partial-word tail for the rest. The fast path must advance the data source + * index "w" by the bulk length; if it does not, the tail re-reads the input + * from the start and programs the wrong bytes. Write 24 bytes at an aligned + * address: mock_flash[0..23] must equal the input. Before the fix, + * mock_flash[16..23] held data[0..7] instead of data[16..23]. */ +START_TEST(test_aligned_write_bulk_then_tail) +{ + uint8_t data[24]; + int i; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + + for (i = 0; i < 24; i++) + data[i] = (uint8_t)(i + 1); + + ck_assert_int_eq(hal_flash_write(base, data, 24), 0); + + for (i = 0; i < 24; i++) + ck_assert_uint_eq(mock_flash[i], data[i]); + for (i = 24; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + Suite *flash_write_suite(void) { Suite *s = suite_create("flash-write-mcxa"); @@ -144,6 +169,7 @@ Suite *flash_write_suite(void) tcase_add_checked_fixture(tc, setup, teardown); tcase_add_test(tc, test_unaligned_write_spanning_two_words); tcase_add_test(tc, test_unaligned_write_single_word); + tcase_add_test(tc, test_aligned_write_bulk_then_tail); suite_add_tcase(s, tc); return s; From d977baa60ef4645de7fbb4177a3da239126afde0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 17:01:22 +0200 Subject: [PATCH 21/23] hal/mcxw: fix truncated empty-word sentinel empty_qword[0] was 0xFFFFFFF (7 nibbles), so a fully-erased 16-byte word never compared equal and was always reprogrammed. --- hal/mcxw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal/mcxw.c b/hal/mcxw.c index 776c0ecf09..6a06019c51 100644 --- a/hal/mcxw.c +++ b/hal/mcxw.c @@ -155,7 +155,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) int w = 0; const uint32_t flash_word_size = 16; const uint32_t empty_qword[4] = { - 0xFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF + 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF }; while (len > 0) { From 103eec95fe25ce89bb682cf39dd0afcb559f0e88 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 17:01:28 +0200 Subject: [PATCH 22/23] tools/delta: check pwrite result in bmpatch Detect short writes and errors instead of advancing the output offset as if all bytes were written, which could silently corrupt the image. --- tools/delta/bmdiff.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/delta/bmdiff.c b/tools/delta/bmdiff.c index 099dac60ff..8e203de330 100644 --- a/tools/delta/bmdiff.c +++ b/tools/delta/bmdiff.c @@ -149,7 +149,15 @@ int main(int argc, char *argv[]) if (r < 0) exit(5); if (r > 0) { - pwrite(fd1, dest, r, len3); + int off = 0; + while (off < r) { + ssize_t wr = pwrite(fd1, dest + off, r - off, len3 + off); + if (wr <= 0) { + perror("pwrite"); + exit(5); + } + off += (int)wr; + } len3 += r; } } while (r > 0); From d383fc1009640097d760faff678abb5f5b9b207a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 17:01:28 +0200 Subject: [PATCH 23/23] unit-tests: build otp-keystore-gen test without generated keystore.c src/keystore.c is generated and gitignored, so the test failed to build on a clean tree. Use the checked-in unit-keystore.c stub instead. --- tools/unit-tests/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index c4bc4c0035..e03275400d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -378,8 +378,8 @@ unit-otp-keystore: unit-otp-keystore.c ../../src/flash_otp_keystore.c # interposes read()/malloc()/free() (dlsym RTLD_NEXT, hence -ldl) to observe # the OTP buffer and UDS stack array at free() time. unit-otp-keystore-gen-zeroize: unit-otp-keystore-gen-zeroize.c \ - ../keytools/otp/otp-keystore-gen.c ../../src/keystore.c - gcc -o $@ unit-otp-keystore-gen-zeroize.c ../../src/keystore.c \ + ../keytools/otp/otp-keystore-gen.c unit-keystore.c + gcc -o $@ unit-otp-keystore-gen-zeroize.c unit-keystore.c \ -DFLASH_OTP_KEYSTORE $(CFLAGS) $(LDFLAGS) -ldl unit-update-flash-self-update: ../../include/target.h unit-update-flash.c