From 194322ec7324c13b079b3395cde28b9fab37a472 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 14:36:29 +0200 Subject: [PATCH 01/11] F-12921: erase keyvault payload on object removal The PKCS#11 and PSA store Remove paths invalidated the metadata and freed the bitmap slot but left the payload in flash, so removed keys stayed recoverable by a physical reader. Both Remove paths now call erase_object_payload() before invalidating the metadata; the existing sector read-modify-write preserves neighboring slots. Raw-flash deletion tests added to both unit suites. --- src/pkcs11_store.c | 3 + src/psa_store.c | 3 + tools/unit-tests/unit-pkcs11_store.c | 86 +++++++++++++++++++++++++++ tools/unit-tests/unit-psa_store.c | 87 ++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index c7135d5d9f..8351d6cde3 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -571,6 +571,9 @@ int wolfPKCS11_Store_Remove(int type, CK_ULONG id1, CK_ULONG id2) if (buf == NULL) return NOT_AVAILABLE_E; + /* Erase the payload before invalidating the metadata, so key + * material does not remain recoverable in flash after removal. */ + erase_object_payload(buf); delete_object((int32_t)type, (uint32_t)id1, (uint32_t)id2); return 0; } diff --git a/src/psa_store.c b/src/psa_store.c index f540deb883..8f95e965aa 100644 --- a/src/psa_store.c +++ b/src/psa_store.c @@ -581,6 +581,9 @@ int wolfPSA_Store_Remove(int type, unsigned long id1, unsigned long id2) if (buf == NULL) return NOT_AVAILABLE_E; + /* Erase the payload before invalidating the metadata, so key + * material does not remain recoverable in flash after removal. */ + erase_object_payload(buf); delete_object((int32_t)type, (uint32_t)id1, (uint32_t)id2); return 0; } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index f556eecc71..bf69221e79 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -550,6 +550,89 @@ START_TEST(test_store_rejects_negative_len) } END_TEST +/* Removing an object must erase the payload from flash, not just + * invalidate the metadata: key material must not remain recoverable + * after the API reports a successful deletion. */ +START_TEST(test_remove_erases_payload_from_flash) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok_a = 10; + const CK_ULONG id_obj_a = 20; + const CK_ULONG id_tok_b = 30; + const CK_ULONG id_obj_b = 40; + void *store = NULL; + unsigned char key_a[256]; + unsigned char key_b[128]; + uint8_t *buf_a; + uint8_t *buf_b; + uint32_t i; + int ret; + + memset(key_a, 0xAB, sizeof(key_a)); + memset(key_b, 0xCD, sizeof(key_b)); + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Two live objects: A gets slot 0, B gets the adjacent slot 1 */ + ret = wolfPKCS11_Store_Open(type, id_tok_a, id_obj_a, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store, key_a, sizeof(key_a)); + ck_assert_int_eq(ret, (int)sizeof(key_a)); + wolfPKCS11_Store_Close(store); + + ret = wolfPKCS11_Store_Open(type, id_tok_b, id_obj_b, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPKCS11_Store_Close(store); + + buf_a = find_object_buffer(type, id_tok_a, id_obj_a); + buf_b = find_object_buffer(type, id_tok_b, id_obj_b); + ck_assert_ptr_nonnull(buf_a); + ck_assert_ptr_nonnull(buf_b); + ck_assert_ptr_eq(buf_a, vault_base + 2 * WOLFBOOT_SECTOR_SIZE); + ck_assert_ptr_eq(buf_b, vault_base + 2 * WOLFBOOT_SECTOR_SIZE + + KEYVAULT_OBJ_SIZE); + + /* Remove A: the payload must be erased from the raw flash */ + ret = wolfPKCS11_Store_Remove(type, id_tok_a, id_obj_a); + ck_assert_int_eq(ret, 0); + + /* The 8-byte id prefix is preserved by the erase (it keeps the slot + * identity used by the backup-recovery check); the payload region + * itself must be erased to 0xFF across the whole slot. */ + ck_assert_uint_eq(((uint32_t *)buf_a)[0], (uint32_t)id_tok_a); + ck_assert_uint_eq(((uint32_t *)buf_a)[1], (uint32_t)id_obj_a); + for (i = 2 * sizeof(uint32_t); i < KEYVAULT_OBJ_SIZE; i++) { + ck_assert_msg(buf_a[i] == 0xFF, + "Payload survives removal at slot offset %u: 0x%02x", + i, buf_a[i]); + } + + /* The sector read-modify-write must leave the neighboring object B + * intact in flash */ + for (i = 2 * sizeof(uint32_t); + i < 2 * sizeof(uint32_t) + sizeof(key_b); i++) { + ck_assert_msg(buf_b[i] == 0xCD, + "Neighbor object clobbered at slot offset %u: 0x%02x", + i, buf_b[i]); + } + ret = wolfPKCS11_Store_Open(type, id_tok_b, id_obj_b, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Read(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPKCS11_Store_Close(store); + + /* A is no longer addressable */ + ret = wolfPKCS11_Store_Open(type, id_tok_a, id_obj_a, 1, &store); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); + ret = wolfPKCS11_Store_Remove(type, id_tok_a, id_obj_a); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); +} +END_TEST + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -563,6 +646,7 @@ Suite *wolfboot_suite(void) TCase* tcase_find_bounds = tcase_create("find_bounds"); TCase* tcase_remanence = tcase_create("shorter_overwrite_erases_residual"); TCase* tcase_neg_len = tcase_create("rejects_negative_len"); + TCase* tcase_remove_erase = tcase_create("remove_erases_payload"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -571,6 +655,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); tcase_add_test(tcase_remanence, test_shorter_overwrite_erases_residual_key_material); tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); + tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); @@ -579,6 +664,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_remanence); suite_add_tcase(s, tcase_neg_len); + suite_add_tcase(s, tcase_remove_erase); return s; } diff --git a/tools/unit-tests/unit-psa_store.c b/tools/unit-tests/unit-psa_store.c index 7c04b37ed0..0823568212 100644 --- a/tools/unit-tests/unit-psa_store.c +++ b/tools/unit-tests/unit-psa_store.c @@ -337,6 +337,90 @@ START_TEST(test_store_rejects_negative_len) } END_TEST +/* Removing an object must erase the payload from flash, not just + * invalidate the metadata: key material must not remain recoverable + * after the API reports a successful deletion. */ +START_TEST(test_remove_erases_payload_from_flash) +{ + const int type = WOLFPSA_STORE_KEY; + const unsigned long id1_a = 10; + const unsigned long id2_a = 20; + const unsigned long id1_b = 30; + const unsigned long id2_b = 40; + void *store = NULL; + unsigned char key_a[256]; + unsigned char key_b[128]; + uint8_t *buf_a; + uint8_t *buf_b; + uint32_t i; + int ret; + + memset(key_a, 0xAB, sizeof(key_a)); + memset(key_b, 0xCD, sizeof(key_b)); + + ret = mmap_file("/tmp/wolfboot-unit-psa-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Two live objects: A gets slot 0, B gets the adjacent slot 1 */ + ret = wolfPSA_Store_Open(type, id1_a, id2_a, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, key_a, sizeof(key_a)); + ck_assert_int_eq(ret, (int)sizeof(key_a)); + wolfPSA_Store_Close(store); + + ret = wolfPSA_Store_Open(type, id1_b, id2_b, 0, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Write(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPSA_Store_Close(store); + + buf_a = find_object_buffer(type, id1_a, id2_a); + buf_b = find_object_buffer(type, id1_b, id2_b); + ck_assert_ptr_nonnull(buf_a); + ck_assert_ptr_nonnull(buf_b); + ck_assert_ptr_eq(buf_a, vault_base + 2 * WOLFBOOT_SECTOR_SIZE); + ck_assert_ptr_eq(buf_b, vault_base + 2 * WOLFBOOT_SECTOR_SIZE + + KEYVAULT_OBJ_SIZE); + + /* Remove A: the payload must be erased from the raw flash */ + ret = wolfPSA_Store_Remove(type, id1_a, id2_a); + ck_assert_int_eq(ret, 0); + + /* The 8-byte id prefix is preserved by the erase (it keeps the slot + * identity used by the backup-recovery check); the payload region + * itself must be erased to 0xFF across the whole slot. */ + ck_assert_uint_eq(((uint32_t *)buf_a)[0], (uint32_t)id1_a); + ck_assert_uint_eq(((uint32_t *)buf_a)[1], (uint32_t)id2_a); + for (i = 2 * sizeof(uint32_t); i < KEYVAULT_OBJ_SIZE; i++) { + ck_assert_msg(buf_a[i] == 0xFF, + "Payload survives removal at slot offset %u: 0x%02x", + i, buf_a[i]); + } + + /* The sector read-modify-write must leave the neighboring object B + * intact in flash */ + for (i = 2 * sizeof(uint32_t); + i < 2 * sizeof(uint32_t) + sizeof(key_b); i++) { + ck_assert_msg(buf_b[i] == 0xCD, + "Neighbor object clobbered at slot offset %u: 0x%02x", + i, buf_b[i]); + } + ret = wolfPSA_Store_Open(type, id1_b, id2_b, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPSA_Store_Read(store, key_b, sizeof(key_b)); + ck_assert_int_eq(ret, (int)sizeof(key_b)); + wolfPSA_Store_Close(store); + + /* A is no longer addressable */ + ret = wolfPSA_Store_Open(type, id1_a, id2_a, 1, &store); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); + ret = wolfPSA_Store_Remove(type, id1_a, id2_a); + ck_assert_int_eq(ret, NOT_AVAILABLE_E); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot-psa-store"); @@ -348,6 +432,7 @@ Suite *wolfboot_suite(void) TCase *tcase_tail = tcase_create("shorter_overwrite_clears_tail"); TCase *tcase_zeroize = tcase_create("cache_commit_zeroizes_cached_sector"); TCase *tcase_neg_len = tcase_create("rejects_negative_len"); + TCase *tcase_remove_erase = tcase_create("remove_erases_payload"); tcase_add_test(tcase_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -357,6 +442,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_tail, test_shorter_overwrite_clears_tail); tcase_add_test(tcase_zeroize, test_cache_commit_zeroizes_cached_sector); tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); + tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); suite_add_tcase(s, tcase_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete); @@ -365,6 +451,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_tail); suite_add_tcase(s, tcase_zeroize); suite_add_tcase(s, tcase_neg_len); + suite_add_tcase(s, tcase_remove_erase); return s; } From 394f160a83aba53bfceb1c4f3d224a5c2a90ff89 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 14:40:35 +0200 Subject: [PATCH 02/11] F-12873: update_disk: FSP low-mem check reuses validated slot_max The final image-size check re-derived the low-memory limit with a uint32 subtraction and no ordering check, so an inverted tolum wrapped into a near-2^32 limit and accepted any image. Compare the tolum/load_address ordering in 32-bit (low-memory) form when computing slot_max, and reuse that validated value in the check. Add a unit test for the inverted-tolum case (fails closed, both slots rejected). --- src/update_disk.c | 21 +++++++++++++-------- tools/unit-tests/unit-update-disk-fsp.c | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/update_disk.c b/src/update_disk.c index 66ada95228..0d77858c6d 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -547,11 +547,15 @@ void RAMFUNCTION wolfBoot_start(void) * The header sits ahead of the payload in the same file, hence the * IMAGE_HEADER_SIZE. */ #if defined(WOLFBOOT_FSP) - /* Fail closed on an inverted tolum: the subtraction would otherwise wrap - * to a near-2^64 bound, which is the opposite of a cap. */ - if ((uintptr_t)(stage2_params->tolum) > (uintptr_t)load_address) { - slot_max = (uint64_t)(uintptr_t)(stage2_params->tolum) - - (uint64_t)(uintptr_t)load_address; + /* Fail closed on an inverted tolum: with tolum at or below the load + * address there is no low-memory window, so the cap is zero. Both + * are low-memory addresses, so compare them in their 32-bit form. + * The subtraction would otherwise wrap into a near-2^32 bound, which + * is the opposite of a cap. */ + if ((uint32_t)(uintptr_t)(stage2_params->tolum) > + (uint32_t)(uintptr_t)load_address) { + slot_max = (uint64_t)(uint32_t)(uintptr_t)(stage2_params->tolum) - + (uint64_t)(uint32_t)(uintptr_t)load_address; } else { slot_max = 0; @@ -697,9 +701,10 @@ void RAMFUNCTION wolfBoot_start(void) #endif #ifdef WOLFBOOT_FSP - /* Verify image size fits in low memory */ - if (os_image.fw_size > ((uint32_t)(stage2_params->tolum) - - (uint32_t)(uintptr_t)load_address)) { + /* Verify image size fits in low memory. Reuse the validated + * slot_max: it is zero when tolum is inverted, where the raw + * subtraction would wrap into a near-2^32 limit. */ + if (os_image.fw_size > slot_max) { wolfBoot_printf("Image size %u doesn't fit in low memory\r\n", os_image.fw_size); selected ^= 1; diff --git a/tools/unit-tests/unit-update-disk-fsp.c b/tools/unit-tests/unit-update-disk-fsp.c index 8a775afb0f..08694902b3 100644 --- a/tools/unit-tests/unit-update-disk-fsp.c +++ b/tools/unit-tests/unit-update-disk-fsp.c @@ -236,6 +236,22 @@ START_TEST(test_fsp_both_slots_oversized_panics) } END_TEST +START_TEST(test_fsp_inverted_tolum_rejects_both_slots) +{ + /* tolum below the load address inverts the low-memory limit. The + * limit must fail closed (no slot may load), not wrap into a + * near-2^32 limit that accepts any image. */ + reset_mocks(); + mock_stage2_params.tolum = + (uint32_t)((uintptr_t)load_buffer - 1); + + wolfBoot_start(); + + ck_assert_int_gt(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 0); +} +END_TEST + START_TEST(test_fsp_fitting_slot_boots) { /* Both slots fit and versions are equal: primary (A) boots. */ @@ -258,6 +274,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc, test_fsp_oversized_slot_falls_back_to_other_slot); tcase_add_test(tc, test_fsp_both_slots_oversized_panics); + tcase_add_test(tc, test_fsp_inverted_tolum_rejects_both_slots); tcase_add_test(tc, test_fsp_fitting_slot_boots); suite_add_tcase(s, tc); From 22356e6d02d0f40e7c3d33bb8de8470767a950b6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 14:43:15 +0200 Subject: [PATCH 03/11] F-12874: stm32wb switch SYSCLK to MSI and confirm SWS before disabling PLL The MSIRDY wait read RCC_CFGR (bit 1 is SW status) instead of RCC_CR, so it never gated, and the MSI selection was cleared only in a local variable, never written back to RCC_CFGR. The PLL was disabled while still the SYSCLK source, dropping the system clock. Now: wait MSIRDY, commit the MSI selection, wait for SWS to confirm, then turn off PLL. --- hal/stm32wb.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hal/stm32wb.c b/hal/stm32wb.c index 6e41a6760f..8951dcbca8 100644 --- a/hal/stm32wb.c +++ b/hal/stm32wb.c @@ -254,11 +254,17 @@ static void clock_pll_off(void) /* Enable internal high-speed oscillator. */ RCC_CR |= RCC_CR_MSION; DMB(); - while ((RCC_CFGR & RCC_CR_MSIRDY) == 0) {}; + /* Wait for MSI to be ready. */ + while ((RCC_CR & RCC_CR_MSIRDY) == 0) + ; /* Select MSI as SYSCLK source. */ reg32 = RCC_CFGR; reg32 &= ~(RCC_CFGR_SW_MASK); + RCC_CFGR = reg32; DMB(); + /* Wait for the switch to be confirmed (SWS, bits 3:2). */ + while (((RCC_CFGR >> 2) & RCC_CFGR_SW_MASK) != RCC_CFGR_SW_MSI) + ; /* Turn off PLL */ RCC_CR &= ~RCC_CR_PLLON; DMB(); From 9475169bdcf819994fddb0b1330c8106c19fd1c6 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 14:43:15 +0200 Subject: [PATCH 04/11] F-12875: stm32f7 fix non-dual-bank sector 11 start address Sector 11 was at 0x818C000, inside sector 10's 256 KB range (0x8180000-0x81C0000), so hal_flash_erase mapped the upper part of sector 10 to sector 11 and erased the wrong sector. Sector 11 is 0x81C0000, contiguous after sector 10 and ending at FLASH_TOP. --- hal/stm32f7.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal/stm32f7.c b/hal/stm32f7.c index 07d22fd54e..38b60bb682 100644 --- a/hal/stm32f7.c +++ b/hal/stm32f7.c @@ -177,7 +177,7 @@ void fork_bootloader(void); # define FLASH_SECTOR_8 0x8100000 /* 256 Kb */ # define FLASH_SECTOR_9 0x8140000 /* 256 Kb */ # define FLASH_SECTOR_10 0x8180000 /* 256 Kb */ -# define FLASH_SECTOR_11 0x818C000 /* 256 Kb */ +# define FLASH_SECTOR_11 0x81C0000 /* 256 Kb */ #endif # define FLASH_TOP 0x8200000 From 49c1fff18609a015741864c555334b75db3c7361 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 14:54:57 +0200 Subject: [PATCH 05/11] F-12876: riscv_sbi: report remote-fence timeout as SBI error sbi_wait_ipi_done() returned void, so a target hart that never completed its fence within the bounded wait was invisible and both the standard and legacy remote-fence interfaces reported success while the caller kept relying on a fence that may not have run. Return SBI_ERR_FAILED when any target does not complete within the bound, and propagate it through both SBI interfaces (standard RFENCE via err, legacy v0.1 remote fence via a0). --- src/riscv_sbi.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/riscv_sbi.c b/src/riscv_sbi.c index 55420b3395..734f475937 100644 --- a/src/riscv_sbi.c +++ b/src/riscv_sbi.c @@ -536,13 +536,16 @@ static void sbi_post_ipi(unsigned long mask, unsigned long base, * the targets increment ipi_done after executing the fences, so this * completes only when every target has finished its fence. The SBI * remote-fence calls are synchronous; the bound guards against a wedged - * target turning into a wedged caller. */ -static void sbi_wait_ipi_done(unsigned long mask, unsigned long base, + * target turning into a wedged caller. Returns SBI_SUCCESS when every + * target completed, SBI_ERR_FAILED when any target did not complete + * within the bound. */ +static long sbi_wait_ipi_done(unsigned long mask, unsigned long base, unsigned long self) { unsigned long i; unsigned long h; uint32_t spin; + long err = SBI_SUCCESS; /* SBI v0.2: hart_mask_base == -1 selects all harts (see sbi_post_ipi). */ if (base == (unsigned long)-1) { base = 0; @@ -561,7 +564,11 @@ static void sbi_wait_ipi_done(unsigned long mask, unsigned long base, while (sbi_ipi_done[h] <= sbi_ipi_wait_gen[h] && spin > 0U) { spin--; } + if (sbi_ipi_done[h] <= sbi_ipi_wait_gen[h]) { + err = SBI_ERR_FAILED; + } } + return err; } /* Returns the (possibly advanced) PC to resume at. For ecall we skip the @@ -658,7 +665,7 @@ unsigned long sbi_handle_ecall(unsigned long *regs, unsigned long epc) break; } sbi_post_ipi(regs[A0], regs[A1], op, hartid); - sbi_wait_ipi_done(regs[A0], regs[A1], hartid); + err = sbi_wait_ipi_done(regs[A0], regs[A1], hartid); break; } @@ -786,9 +793,9 @@ unsigned long sbi_handle_ecall(unsigned long *regs, unsigned long epc) sbi_post_ipi(fmask, 0, (eid == SBI_EXT_0_1_REMOTE_FENCE_I) ? SBI_IPI_OP_FENCE_I : SBI_IPI_OP_SFENCE, hartid); - sbi_wait_ipi_done(fmask, 0, hartid); + err = sbi_wait_ipi_done(fmask, 0, hartid); } - regs[A0] = 0; + regs[A0] = (unsigned long)err; return epc + 4; case SBI_EXT_0_1_SHUTDOWN: wolfBoot_printf("[SBI] legacy SHUTDOWN requested\n"); From 376f9e124f6530feb856e0cfd1017289175791e1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 15:49:31 +0200 Subject: [PATCH 06/11] F-12920: zero firmware-DTB initrd pointers in hal_get_boot_dts The CM4 firmware DTB is unverified (unsigned FAT partition) and this path never attaches an authenticated ramdisk, so a non-zero linux,initrd-start/end in it would direct the signed kernel to an unauthenticated initramfs in RAM. Zero both properties on the relocated DTB (fail closed on fixup error) and update the SECURITY comment to state the new behavior. --- hal/cm4.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/hal/cm4.c b/hal/cm4.c index fddcc3560b..566dc2e003 100644 --- a/hal/cm4.c +++ b/hal/cm4.c @@ -379,8 +379,11 @@ void* hal_get_boot_dts(void) sz = fdt_size(&ctx); /* SECURITY: the firmware DTB lives on the unsigned FAT partition (unless * the RPi EEPROM secure-boot is enabled), so it is NOT covered by wolfBoot's - * signature. Only /chosen/bootargs is overwritten below; /memory, - * /reserved-memory, per-device reg windows and initrd remain firmware / + * signature. Only /chosen/bootargs is overwritten below, and the + * /chosen/linux,initrd-{start,end} pointers are zeroed: this path never + * loads an authenticated ramdisk, so a non-zero firmware value would + * direct the signed kernel to an unauthenticated initramfs in RAM. + * /memory, /reserved-memory and per-device reg windows remain firmware / * attacker controlled. * This is NOT effectively optional: CM4_FIRMWARE_DTB is enabled by default * in both shipped Linux configurations (cm4_emmc_linux.config and @@ -442,6 +445,19 @@ void* hal_get_boot_dts(void) return NULL; } #endif + /* Zero the initrd pointers the firmware DTB may carry. This path never + * attaches an authenticated ramdisk (a FIT ramdisk subimage is only + * fixup'd into an FIT-embedded DTB, which bypasses this fallback), so + * any non-zero value here would point the signed kernel at an + * unauthenticated initramfs in RAM. The kernel loads no initrd from a + * zero range, and the properties are zeroed, not deleted, so the + * existing fdt_fixup_val64() covers both the present and absent case. + * Fail closed, like the bootargs fixup above. */ + if (fdt_fixup_val64(&ctx, off, "chosen", "linux,initrd-start", 0) != 0 || + fdt_fixup_val64(&ctx, off, "chosen", "linux,initrd-end", 0) != 0) { + wolfBoot_printf("cm4: DTB initrd fixup failed; refusing firmware DTB\n"); + return NULL; + } wolfBoot_printf("cm4: DTB relocated to %p, bootargs set\n", fdt); return fdt; #endif /* CM4_FIRMWARE_DTB */ From 8e86847c1f187b167bd56e9a4f8db95cf1b61d77 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 15:43:28 +0200 Subject: [PATCH 07/11] F-12870: STM32C0: program flash writes through absolute addresses The HAL contract is absolute (0x08000000-based) addresses, as the erase path and every NVM caller use, but the double-word fast path added the flash base on top and targeted an address space past the flash. The read-modify-write path located its unit from the request base indexed by i/4, landing in the wrong 8-byte unit once a request started inside one. Both paths now program the 8-byte unit at (address + i), and the now-dead FLASHMEM_ADDRESS_SPACE define is gone. Add unit-stm32c0-write: runs the extracted hal_flash_write() against a host register/flash model with a second mapping standing in for the wrong address space (4/5 checks fail pre-fix, 5/5 pass post-fix). --- hal/stm32c0.c | 26 ++- tools/unit-tests/Makefile | 19 +- tools/unit-tests/unit-stm32c0-write.c | 286 ++++++++++++++++++++++++++ 3 files changed, 315 insertions(+), 16 deletions(-) create mode 100644 tools/unit-tests/unit-stm32c0-write.c diff --git a/hal/stm32c0.c b/hal/stm32c0.c index d8605f67a3..b9a701218b 100644 --- a/hal/stm32c0.c +++ b/hal/stm32c0.c @@ -80,7 +80,6 @@ #define FLASH_SECR (*(volatile uint32_t *)(FLASH_BASE + 0x80)) /* RM0490 - 3.7.13 - FLASH_SECR */ #endif /* !WOLFBOOT_UNIT_TEST_FLASH_ERASE */ -#define FLASHMEM_ADDRESS_SPACE (0x08000000) #define FLASH_PAGE_SIZE (0x800) /* 2KB */ #define FLASH_PAGE_SIZE_SHIFT 11 /* (1 << FLASH_PAGE_SIZE_SHIFT) == FLASH_PAGE_SIZE*/ @@ -152,26 +151,25 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) flash_clear_errors(); if ((len - i >= 8) && ((((address + i) & 0x07) == 0) && ((((uint32_t)data) + i) & 0x07) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE); + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i); flash_wait_complete(); - dst[i >> 2] = src[i >> 2]; - dst[(i >> 2) + 1] = src[(i >> 2) + 1]; + dst[0] = src[0]; + dst[1] = src[1]; flash_wait_complete(); - i+=8; + i += 8; } else { + uint32_t unit_addr = (address + i) & (~0x07); + int off = (address + i) - unit_addr; uint32_t val[2]; uint8_t *vbytes = (uint8_t *)(val); - int off = (address + i) - (((address + i) >> 3) << 3); - uint32_t base_addr = address & (~0x07); /* aligned to 64 bit */ - int u32_idx = (i >> 2); - dst = (uint32_t *)(base_addr); - val[0] = dst[u32_idx]; - val[1] = dst[u32_idx + 1]; + dst = (uint32_t *)unit_addr; + val[0] = dst[0]; + val[1] = dst[1]; while ((off < 8) && (i < len)) vbytes[off++] = data[i++]; - dst[u32_idx] = val[0]; - dst[u32_idx + 1] = val[1]; + dst[0] = val[0]; + dst[1] = val[1]; flash_wait_complete(); } } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6efd8e0b86..1d9bcbc53a 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -133,6 +133,7 @@ TESTS+=unit-stm32l4-write TESTS+=unit-stm32wb-write TESTS+=unit-stm32l5-write TESTS+=unit-stm32u5-write +TESTS+=unit-stm32c0-write TESTS+=unit-nvm-cache-scrub TESTS+=unit-update-trigger-scrub TESTS+=unit-sdhci-uhs-recover @@ -1254,6 +1255,20 @@ stm32wb_write_extract.h: ../../hal/stm32wb.c unit-stm32wb-write: unit-stm32wb-write.c stm32wb_write_extract.h gcc -o $@ unit-stm32wb-write.c $(CFLAGS) $(LDFLAGS) +# unit-stm32c0-write runs the real hal_flash_write() from hal/stm32c0.c +# (the double-word fast path added the flash base to an already +# absolute address, targeting an address space past the flash, and the +# read-modify-write path located its unit from the request base instead +# of from the destination). Same harness as the STM32L4/STM32WB twins; +# the c0 helpers are static and un-prefixed. +stm32c0_write_extract.h: ../../hal/stm32c0.c + sed -n '/^static RAMFUNCTION void flash_wait_complete/,/^}/p' $< > $@ + sed -n '/^static void RAMFUNCTION flash_clear_errors/,/^}/p' $< >> $@ + sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ + +unit-stm32c0-write: unit-stm32c0-write.c stm32c0_write_extract.h + gcc -o $@ unit-stm32c0-write.c $(CFLAGS) $(LDFLAGS) + # unit-t10xx-flash-status runs the real hal_flash_write()/hal_flash_erase() # and hal_flash_status_wait() from hal/nxp_t10xx.c against a mock QPI # status model (F-11033: a timed-out program/erase used to report @@ -1621,8 +1636,8 @@ GENERATED_SRC:=aurix_erased_extract.h \ kontron_spi_extract.h kontron_spi_fn_extract.h \ rp2350_flash_write_extract.h \ sdhci_host.c \ - stm32g4_write_extract.h stm32l4_write_extract.h stm32l5_write_extract.h \ - stm32u5_write_extract.h stm32wb_write_extract.h \ + stm32c0_write_extract.h stm32g4_write_extract.h stm32l4_write_extract.h \ + stm32l5_write_extract.h stm32u5_write_extract.h stm32wb_write_extract.h \ t10xx_flash_status_extract.h t10xx_qe_firmware_extract.h \ t2080_fman_extract.h \ ti_hercules_write_extract.h versal_ext_write_extract.h versal_host.c \ diff --git a/tools/unit-tests/unit-stm32c0-write.c b/tools/unit-tests/unit-stm32c0-write.c new file mode 100644 index 0000000000..ab7a0abb5f --- /dev/null +++ b/tools/unit-tests/unit-stm32c0-write.c @@ -0,0 +1,286 @@ +/* unit-stm32c0-write.c + * + * Regression test: hal_flash_write() in hal/stm32c0.c took the + * double-word fast path on aligned requests and added 0x08000000 to + * the destination, but the HAL contract is absolute addresses (the + * erase path and every NVM caller pass 0x08000000-based addresses), + * so fast-path writes targeted an address space past the flash while + * the read-modify-write path targeted the right location. The RMW + * path also located its unit from the request base (address & ~7) + * indexed by i/4, which lands in the wrong 8-byte unit once the + * request starts inside one. + * + * The fix programs through the absolute address: the fast path + * copies the two words at (data + i) to the 8-byte unit at + * (address + i), and the RMW path re-computes the unit from + * (address + i) and rewrites it whole. + * + * The real functions are extracted by the Makefile and run with the + * FLASH registers on a host register file, the destination flash + * pre-filled with stale data, a second mapping standing in for the + * wrong address space (flash base + 0x08000000) that the pre-fix + * fast path writes into, and a canary after the source buffer. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +/* Host stand-in for the ARM placement attribute. */ +#define RAMFUNCTION + +/* Host FLASH register file. */ +static uint32_t g_flash_regs[0x40 / sizeof(uint32_t)]; +#define FLASH_BASE ((uintptr_t)g_flash_regs) +#define FLASH_SR (*(volatile uint32_t *)(FLASH_BASE + 0x10)) +#define FLASH_CR (*(volatile uint32_t *)(FLASH_BASE + 0x14)) +#define FLASH_SR_EOP (1 << 0) +#define FLASH_SR_PROGERR (1 << 3) +#define FLASH_SR_WRPERR (1 << 4) +#define FLASH_SR_PGAERR (1 << 5) +#define FLASH_SR_SIZERR (1 << 6) +#define FLASH_SR_BSY1 (1 << 16) +#define FLASH_CR_PG (1 << 0) + +/* Flash base as defined in hal/stm32c0.c: the pre-fix fast path added + * it to an already absolute address, so the extracted code only + * compiles while that constant is visible here. */ +#define FLASHMEM_ADDRESS_SPACE (0x08000000) + +/* Destination flash: pre-filled with stale data (rewrite scenario). + * hal_flash_write() takes the address as uint32_t (32-bit MCU), so + * on the 64-bit host the flash must live at an address that fits in + * 32 bits: map it at a fixed low location. */ +#define FLASH_MEM_SZ 256 +#define FLASH_MEM_ADDR 0x10000000UL +static uint8_t *g_flash_mem; + +/* The address space the pre-fix fast path wrote into: flash base + + * 0x08000000. Mapped so a pre-fix run fails the checks below instead + * of faulting; it must stay untouched by a correct write. */ +#define POISON_MEM_SZ 256 +#define POISON_MEM_ADDR (FLASH_MEM_ADDR + 0x08000000UL) +static uint8_t *g_poison; + +/* Source buffer, page aligned so the pointer is 8-aligned (the fast + * path checks the data alignment); a canary follows the data: a + * pre-fix short write reads the canary and lands it in the + * destination flash. The canary range avoids the data bytes + * (0x30..0x6F), the stale flash fill (0x12), the poison fill (0x5A) + * and the erased value (0xFF). */ +#define DATA_MAP_SZ 128 +#define DATA_SZ 64 +#define CANARY_SZ 32 +static uint8_t *g_data_map; +static uint8_t *g_data; +#define g_canary (g_data + DATA_SZ) + +/* The real functions from hal/stm32c0.c (extracted by the Makefile). */ +#include "stm32c0_write_extract.h" + +static void setup(void) +{ + int i; + + memset(g_flash_regs, 0, sizeof(g_flash_regs)); + for (i = 0; i < FLASH_MEM_SZ; i++) + g_flash_mem[i] = 0x12; /* stale */ + for (i = 0; i < POISON_MEM_SZ; i++) + g_poison[i] = 0x5A; + g_data = g_data_map; + for (i = 0; i < DATA_SZ; i++) + g_data[i] = (uint8_t)(0x30 + i); + for (i = 0; i < CANARY_SZ; i++) + g_canary[i] = (uint8_t)(0x70 + i); +} + +static void teardown(void) +{ +} + +static int canary_in_flash(void) +{ + int i; + + for (i = 0; i < CANARY_SZ; i++) + if (memchr(g_flash_mem, g_canary[i], FLASH_MEM_SZ) != NULL) + return 1; + return 0; +} + +/* The wrong address space must never receive a byte. */ +static int poison_untouched(void) +{ + int i; + + for (i = 0; i < POISON_MEM_SZ; i++) + if (g_poison[i] != 0x5A) + return 0; + return 1; +} + +/* An 8-aligned write of 64 bytes: all fast path. Pre-fix, every + * double word lands in the wrong address space and the flash keeps + * its stale content. */ +START_TEST(test_write_64_aligned){ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 64), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 64), 0); + for (i = 64; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(poison_untouched(), 1); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* An 8-aligned write of 60 bytes: fast path for the first 56 bytes, + * read-modify-write for the 4-byte tail. */ +START_TEST(test_write_60_tail) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, + g_data, 60), 0); + + ck_assert_int_eq(memcmp(g_flash_mem, g_data, 60), 0); + for (i = 60; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(poison_untouched(), 1); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write starting 4 bytes into an 8-byte unit: the first unit is + * partially programmed, then the request runs through the rest of + * the flash. The bytes before the request keep their stale value and + * nothing lands in the wrong address space. */ +START_TEST(test_write_24_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 24), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 24), 0); + for (i = 28; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(poison_untouched(), 1); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A 3-byte write starting 4 bytes into an 8-byte unit: one partial + * unit, the rest of it rewritten unchanged. */ +START_TEST(test_write_3_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 3), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 3), 0); + for (i = 7; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(poison_untouched(), 1); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A write starting 5 bytes into an 8-byte unit with a source 5 bytes + * into its own unit: the fast path fires mid-request with an index + * that is not a multiple of 4, so the two copied words must be taken + * from (data + i) and stored at the 8-byte unit at (address + i). */ +START_TEST(test_write_24_unaligned5) +{ + int i; + + g_data = g_data_map + 5; + for (i = 0; i < DATA_SZ; i++) + g_data[i] = (uint8_t)(0x30 + i); + for (i = 0; i < CANARY_SZ; i++) + g_canary[i] = (uint8_t)(0x70 + i); + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 5), + g_data, 24), 0); + + for (i = 0; i < 5; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 5, g_data, 24), 0); + for (i = 29; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(poison_untouched(), 1); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +Suite *stm32c0_write_suite(void) +{ + Suite *s = suite_create("stm32c0-write"); + TCase *tc = tcase_create("stm32c0-write"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_write_64_aligned); + tcase_add_test(tc, test_write_60_tail); + tcase_add_test(tc, test_write_24_unaligned4); + tcase_add_test(tc, test_write_3_unaligned4); + tcase_add_test(tc, test_write_24_unaligned5); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = stm32c0_write_suite(); + SRunner *sr = srunner_create(s); + + g_flash_mem = mmap((void *)FLASH_MEM_ADDR, FLASH_MEM_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | + MAP_FIXED, + -1, 0); + if (g_flash_mem == MAP_FAILED) + return 99; + g_poison = mmap((void *)POISON_MEM_ADDR, POISON_MEM_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | + MAP_FIXED, + -1, 0); + if (g_poison == MAP_FAILED) + return 99; + g_data_map = mmap(NULL, DATA_MAP_SZ, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, + -1, 0); + if (g_data_map == MAP_FAILED) + return 99; + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From ca06782f42e942eefb62f6eab4588ae3959398e9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 16:07:11 +0200 Subject: [PATCH 08/11] F-12877: STM32L5: program the 8-byte unit through an aligned pointer hal_flash_write() stored both words of the 64-bit program unit relative to the caller's address, so a write starting inside a unit split the two stores across two units: the flash has no 32-bit program mode, so nothing is programmed and the second store faults on alignment. Align the destination down to the unit, take the bytes outside the requested span from the unit itself, and store through the aligned pointer, as hal/stm32h5.c does. The TrustZone claim is unchanged: it is page-granular and already covers every non-secure byte the aligned program touches. Extend unit-stm32l5-write with unaligned-start cases. The host data model cannot observe the program-unit split (pre-fix the bytes land identically), so these pin the fixed layout: bytes before the request preserved, nothing past it touched. --- hal/stm32l5.c | 20 ++++++--- tools/unit-tests/unit-stm32l5-write.c | 62 +++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/hal/stm32l5.c b/hal/stm32l5.c index 63add01cb2..48ebaa2039 100644 --- a/hal/stm32l5.c +++ b/hal/stm32l5.c @@ -99,26 +99,32 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { int j; + uintptr_t cur_addr = (uintptr_t)dst + i; + uint32_t *unit = (uint32_t *)(cur_addr & (~0x07UL)); + int off = (int)(cur_addr & 0x07UL); + int i_aligned = i - off; /* Read-modify-write the whole 64-bit unit (as stm32h5.c): * there is no 32-bit program mode, so both words must be - * stored inside one PG window or nothing is programmed. */ + * stored inside one PG window or nothing is programmed. The + * unit is aligned down from the next byte, so an unaligned + * start keeps the bytes before the request. */ for (j = 0; j < 8; j++) { - if (i + j < len) - dword_bytes[j] = data[i + j]; + if ((j >= off) && (i_aligned + j < len)) + dword_bytes[j] = data[i_aligned + j]; else - dword_bytes[j] = ((const uint8_t *)dst)[i + j]; + dword_bytes[j] = ((const uint8_t *)unit)[j]; } *cr |= FLASH_CR_PG; - dst[i >> 2] = dword[0]; + unit[0] = dword[0]; ISB(); - dst[(i >> 2) + 1] = dword[1]; + unit[1] = dword[1]; hal_flash_wait_complete(0); if ((*sr & FLASH_SR_EOP) != 0) *sr |= FLASH_SR_EOP; *cr &= ~FLASH_CR_PG; - i += 8; + i = i_aligned + 8; } #if TZ_SECURE() hal_tz_release_nonsecure_area(); diff --git a/tools/unit-tests/unit-stm32l5-write.c b/tools/unit-tests/unit-stm32l5-write.c index 36fb79b031..6178045101 100644 --- a/tools/unit-tests/unit-stm32l5-write.c +++ b/tools/unit-tests/unit-stm32l5-write.c @@ -3,12 +3,20 @@ * Regression test: hal_flash_write() in hal/stm32l5.c read both words * of the 8-byte program unit regardless of the remaining length, so a * write not a multiple of 8 read up to 4 bytes past the caller's - * buffer and programmed them. + * buffer and programmed them. It also programmed through the caller's + * address without aligning it down to the 8-byte program unit, so a + * write starting inside a unit issued its two word stores in two + * different units: the flash has no 32-bit program mode, so nothing + * is programmed and the second store faults on alignment. * - * The fix read-modify-writes the whole unit: bytes outside [i, len) - * come from flash and go back unchanged. Both words are always stored - * in one PG window -- the flash has no 32-bit program mode -- which - * this host model cannot observe, so that is asserted by construction + * The fix read-modify-writes the whole unit: the destination is + * aligned down to the unit, the bytes outside the requested span come + * from flash and go back unchanged, and both words are stored through + * the aligned pointer. That is asserted here for unaligned starts + * (the host data model cannot observe the program-unit split itself, + * only where the bytes end up). Both words are always stored in one + * PG window -- the flash has no 32-bit program mode -- which this + * host model cannot observe, so that is asserted by construction * in the HAL, not here. * * Same harness as the STM32U5 twin: extracted functions, registers on @@ -39,7 +47,7 @@ /* Host stand-ins for the ARM primitives and the TZ build selection. */ #define RAMFUNCTION -#define ISB() do {} while (0) +#define ISB() do { } while (0) #define TZ_SECURE() (0) /* Host FLASH register file (offsets as in hal/stm32l5.h, non-secure @@ -110,8 +118,7 @@ static int canary_in_flash(void) /* A write of 60 bytes (not a multiple of 8): the last complete word * lands, the bytes past len keep their stale value, and no canary * byte is read or written. */ -START_TEST(test_write_60_no_overread) -{ +START_TEST(test_write_60_no_overread){ int i; ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, @@ -177,6 +184,43 @@ START_TEST(test_write_64_full_units) } END_TEST +/* A write of 20 bytes starting 4 bytes into an 8-byte unit: the + * first unit is only half requested, the rest of it keeps its stale + * value, and the request runs on through the following units. */ +START_TEST(test_write_20_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 20), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 20), 0); + for (i = 24; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A 3-byte write starting 4 bytes into an 8-byte unit: one partial + * unit, the rest of it rewritten unchanged. */ +START_TEST(test_write_3_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 3), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 3), 0); + for (i = 7; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + Suite *stm32l5_write_suite(void) { Suite *s = suite_create("stm32l5-write"); @@ -187,6 +231,8 @@ Suite *stm32l5_write_suite(void) tcase_add_test(tc, test_write_58_partial_word_padded); tcase_add_test(tc, test_write_3_single_word_padded); tcase_add_test(tc, test_write_64_full_units); + tcase_add_test(tc, test_write_20_unaligned4); + tcase_add_test(tc, test_write_3_unaligned4); suite_add_tcase(s, tc); return s; From cfd44bc5c1cebf1682d313036470bd8f79850918 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 16:31:07 +0200 Subject: [PATCH 09/11] F-12878: STM32U5: program the 16-byte unit through an aligned pointer An unaligned starting address split the four word stores across two 16-byte program units, leaving partial quad-words that set FLASH_SR_WDW and hang the wait for completion. Align the destination down to the unit, read-modify-write the whole unit, and store through the aligned pointer. The unit test gains unaligned-start cases. --- hal/stm32u5.c | 18 ++++--- tools/unit-tests/unit-stm32u5-write.c | 70 ++++++++++++++++++++++----- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/hal/stm32u5.c b/hal/stm32u5.c index b24185aee7..ac5725d4f1 100644 --- a/hal/stm32u5.c +++ b/hal/stm32u5.c @@ -94,27 +94,33 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { int j; + uintptr_t cur_addr = (uintptr_t)dst + i; + uint32_t *unit = (uint32_t *)(cur_addr & (~0x0FUL)); + int off = (int)(cur_addr & 0x0FUL); + int i_aligned = i - off; /* Read-modify-write the whole 128-bit unit (as stm32h5.c): * the program only starts on the 4th word, and a partial - * quad-word leaves FLASH_SR_WDW set, hanging the wait. */ + * quad-word leaves FLASH_SR_WDW set, hanging the wait. The + * unit is aligned down from the next byte, so an unaligned + * start keeps the bytes before the request. */ for (j = 0; j < 16; j++) { - if (i + j < len) - qword_bytes[j] = data[i + j]; + if ((j >= off) && (i_aligned + j < len)) + qword_bytes[j] = data[i_aligned + j]; else - qword_bytes[j] = ((const uint8_t *)dst)[i + j]; + qword_bytes[j] = ((const uint8_t *)unit)[j]; } *cr |= FLASH_CR_PG; for (j = 0; j < 4; j++) { - dst[(i >> 2) + j] = qword[j]; + unit[j] = qword[j]; ISB(); } hal_flash_wait_complete(0); if ((*sr & FLASH_SR_EOP) != 0) *sr |= FLASH_SR_EOP; *cr &= ~FLASH_CR_PG; - i += 16; + i = i_aligned + 16; } return 0; diff --git a/tools/unit-tests/unit-stm32u5-write.c b/tools/unit-tests/unit-stm32u5-write.c index 630c2883e4..d07b7dd609 100644 --- a/tools/unit-tests/unit-stm32u5-write.c +++ b/tools/unit-tests/unit-stm32u5-write.c @@ -3,18 +3,24 @@ * Regression test: hal_flash_write() in hal/stm32u5.c read all four * words of the 16-byte program unit regardless of the remaining * length, so a write not a multiple of 16 read up to 12 bytes past the - * caller's buffer and programmed them. + * caller's buffer and programmed them. It also programmed through the + * caller's address without aligning it down to the 16-byte program + * unit, so a write starting inside a unit issued its four word stores + * across two different units, leaving partial quad-words that set + * FLASH_SR_WDW and hang the wait for completion. * - * The fix read-modify-writes the whole unit: bytes outside [i, len) - * come from flash and go back unchanged. All four words are always - * stored -- the controller only starts the program on the fourth -- - * which this host model cannot observe, so that part is asserted by - * construction in the HAL, not here. + * The fix read-modify-writes the whole unit: the destination is + * aligned down to the unit, the bytes outside the requested span come + * from flash and go back unchanged, and all four words are stored + * through the aligned pointer. That is asserted here for unaligned + * starts (the host data model cannot observe the program-unit split + * itself, only where the bytes end up). All four words are always + * stored in one program -- the controller only starts it on the + * fourth word -- which this host model cannot observe, so that is + * asserted by construction in the HAL, not here. * - * The real functions are extracted by the Makefile and run with the - * FLASH registers on a host register file and the destination flash - * pre-filled with stale data; a canary after the source buffer catches - * any read past len. + * Same harness as the STM32L5 twin: extracted functions, registers on + * a host file, stale destination flash, canary after the source. * Copyright (C) 2026 wolfSSL Inc. * * This file is part of wolfBoot. @@ -42,7 +48,7 @@ /* Host stand-ins for the ARM primitives and the TZ build selection * (non-secure path: FLASH_NS_CR / FLASH_NS_SR). */ #define RAMFUNCTION -#define ISB() do {} while (0) +#define ISB() do { } while (0) #define TZ_SECURE() (0) /* Host FLASH register file (offsets as in hal/stm32u5.h). */ @@ -112,8 +118,7 @@ static int canary_in_flash(void) /* A write of 60 bytes (not a multiple of 16): the requested bytes * land, the partial final word is padded to the erased value, and * nothing past len is read or written. */ -START_TEST(test_write_60_no_overread) -{ +START_TEST(test_write_60_no_overread){ int i; ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, @@ -199,6 +204,43 @@ START_TEST(test_write_64_full_units) } END_TEST +/* A write of 20 bytes starting 4 bytes into a 16-byte unit: the + * first unit is only partly requested, the rest of it keeps its stale + * value, and the request runs on through the following units. */ +START_TEST(test_write_20_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 20), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 20), 0); + for (i = 24; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + +/* A 3-byte write starting 4 bytes into a 16-byte unit: one partial + * unit, the rest of it rewritten unchanged. */ +START_TEST(test_write_3_unaligned4) +{ + int i; + + ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)(g_flash_mem + 4), + g_data, 3), 0); + + for (i = 0; i < 4; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(memcmp(g_flash_mem + 4, g_data, 3), 0); + for (i = 7; i < FLASH_MEM_SZ; i++) + ck_assert_uint_eq(g_flash_mem[i], 0x12); + ck_assert_int_eq(canary_in_flash(), 0); +} +END_TEST + Suite *stm32u5_write_suite(void) { Suite *s = suite_create("stm32u5-write"); @@ -210,6 +252,8 @@ Suite *stm32u5_write_suite(void) tcase_add_test(tc, test_write_18_second_word_padded); tcase_add_test(tc, test_write_3_single_word_padded); tcase_add_test(tc, test_write_64_full_units); + tcase_add_test(tc, test_write_20_unaligned4); + tcase_add_test(tc, test_write_3_unaligned4); suite_add_tcase(s, tc); return s; From fc688f18c1d378edfd59c838aac8b54593c3f47d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 17:43:22 +0200 Subject: [PATCH 10/11] Address PR review: SWS macros + test decl style hal/stm32wb.c: dedicated RCC_CFGR_SWS_{MSI,MASK} macros for the clock-switch confirmation wait; SW/SWS encodings verified identical in RM0434 6.4.3 and the STM32WB55 SVD. unit-stm32u5-write.c: START_TEST brace on the next line, matching the file and the unit-suite convention. --- hal/stm32wb.c | 5 ++++- tools/unit-tests/unit-stm32u5-write.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hal/stm32wb.c b/hal/stm32wb.c index 8951dcbca8..805f900482 100644 --- a/hal/stm32wb.c +++ b/hal/stm32wb.c @@ -63,6 +63,9 @@ PKA_HandleTypeDef hpka = { }; #define RCC_CFGR_SW_MSI 0x0 #define RCC_CFGR_SW_PLL 0x3 #define RCC_CFGR_SW_MASK 0x3 +/* SWS (bits 3:2, read-only) mirrors the SW encoding (RM0434 6.4.3): */ +#define RCC_CFGR_SWS_MSI 0x0 +#define RCC_CFGR_SWS_MASK 0x3 #define RCC_CFGR_HPRE_MASK 0x0F #define RCC_CFGR_PPRE1_MASK 0x07 @@ -263,7 +266,7 @@ static void clock_pll_off(void) RCC_CFGR = reg32; DMB(); /* Wait for the switch to be confirmed (SWS, bits 3:2). */ - while (((RCC_CFGR >> 2) & RCC_CFGR_SW_MASK) != RCC_CFGR_SW_MSI) + while (((RCC_CFGR >> 2) & RCC_CFGR_SWS_MASK) != RCC_CFGR_SWS_MSI) ; /* Turn off PLL */ RCC_CR &= ~RCC_CR_PLLON; diff --git a/tools/unit-tests/unit-stm32u5-write.c b/tools/unit-tests/unit-stm32u5-write.c index d07b7dd609..fed3fa5dca 100644 --- a/tools/unit-tests/unit-stm32u5-write.c +++ b/tools/unit-tests/unit-stm32u5-write.c @@ -118,7 +118,8 @@ static int canary_in_flash(void) /* A write of 60 bytes (not a multiple of 16): the requested bytes * land, the partial final word is padded to the erased value, and * nothing past len is read or written. */ -START_TEST(test_write_60_no_overread){ +START_TEST(test_write_60_no_overread) +{ int i; ck_assert_int_eq(hal_flash_write((uint32_t)(uintptr_t)g_flash_mem, From 609294789fa9f9a4b1698e2ddb2296305bb4e280 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 7 Sep 2026 19:19:22 +0200 Subject: [PATCH 11/11] Address Fenrir PR review: make program-window observable in l5/u5 write tests The new unaligned tests passed against the pre-fix HAL (identical final bytes), so the alignment fix had no regression coverage. Mock hal_flash_wait_complete now diffs the flash per program window and asserts the changed bytes fit in one aligned unit; the 20-byte unaligned test goes red on the pre-fix HAL (l5: bytes 4-11 across two 8-byte units, u5: bytes 4-19 across two 16-byte units). --- tools/unit-tests/Makefile | 17 ++++++----- tools/unit-tests/unit-stm32l5-write.c | 44 +++++++++++++++++++++++---- tools/unit-tests/unit-stm32u5-write.c | 42 ++++++++++++++++++++++--- 3 files changed, 85 insertions(+), 18 deletions(-) diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 1d9bcbc53a..8b844f46ea 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -1440,20 +1440,23 @@ unit-ecc-raw-der: unit-ecc-raw-der.c # destination flash is a pre-filled host array, so the over-read is # observable through a canary after the source buffer. stm32l5_write_extract.h: ../../hal/stm32l5.c - sed -n '/^void RAMFUNCTION hal_flash_wait_complete/,/^}/p' $< > $@ - sed -n '/^void RAMFUNCTION hal_flash_clear_errors/,/^}/p' $< >> $@ + # hal_flash_wait_complete is mocked in the test (program-window + # check), so only the real clear_errors + write are extracted. + sed -n '/^void RAMFUNCTION hal_flash_clear_errors/,/^}/p' $< > $@ sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ unit-stm32l5-write: unit-stm32l5-write.c stm32l5_write_extract.h gcc -o $@ unit-stm32l5-write.c $(CFLAGS) $(LDFLAGS) # unit-stm32u5-write is the 16-byte-unit twin of the STM32L5 test: -# the real hal_flash_write() and its wait/clear helpers from -# hal/stm32u5.c, FLASH_NS_SR/CR on a host register file, destination -# flash at a 32-bit host address, canary after the source buffer. +# the real hal_flash_write() and clear_errors from hal/stm32u5.c +# (hal_flash_wait_complete is mocked in the test for the +# program-window check), FLASH_NS_SR/CR on a host register file, +# destination flash at a 32-bit host address, canary after the source. stm32u5_write_extract.h: ../../hal/stm32u5.c - sed -n '/^void RAMFUNCTION hal_flash_wait_complete/,/^}/p' $< > $@ - sed -n '/^void RAMFUNCTION hal_flash_clear_errors/,/^}/p' $< >> $@ + # hal_flash_wait_complete is mocked in the test (program-window + # check), so only the real clear_errors + write are extracted. + sed -n '/^void RAMFUNCTION hal_flash_clear_errors/,/^}/p' $< > $@ sed -n '/^int RAMFUNCTION hal_flash_write/,/^}/p' $< >> $@ unit-stm32u5-write: unit-stm32u5-write.c stm32u5_write_extract.h diff --git a/tools/unit-tests/unit-stm32l5-write.c b/tools/unit-tests/unit-stm32l5-write.c index 6178045101..f6fccc3500 100644 --- a/tools/unit-tests/unit-stm32l5-write.c +++ b/tools/unit-tests/unit-stm32l5-write.c @@ -12,12 +12,14 @@ * The fix read-modify-writes the whole unit: the destination is * aligned down to the unit, the bytes outside the requested span come * from flash and go back unchanged, and both words are stored through - * the aligned pointer. That is asserted here for unaligned starts - * (the host data model cannot observe the program-unit split itself, - * only where the bytes end up). Both words are always stored in one - * PG window -- the flash has no 32-bit program mode -- which this - * host model cannot observe, so that is asserted by construction - * in the HAL, not here. + * the aligned pointer. That is asserted here for unaligned starts. + * The program-window invariant is enforced by the mock + * hal_flash_wait_complete below: the real one only spins on + * FLASH_SR_BSY (never set on the host register file), so the mock + * diffs the flash against the previous window and asserts that the + * changed bytes fit in one aligned 8-byte unit. A pre-fix HAL split + * the two word stores across two units for an unaligned start, and + * the 20-byte unaligned test goes red on it. * * Same harness as the STM32U5 twin: extracted functions, registers on * a host file, stale destination flash, canary after the source. @@ -75,6 +77,10 @@ static uint32_t g_flash_regs[0x40 / sizeof(uint32_t)]; #define FLASH_MEM_ADDR 0x10000000UL static uint8_t *g_flash_mem; +/* Snapshot of the flash at the last program-window boundary; the + * mock hal_flash_wait_complete() diffs against it. */ +static uint8_t g_flash_prev[FLASH_MEM_SZ]; + /* Source buffer followed by a canary: a pre-fix short write reads the * canary and lands it in the destination flash. */ #define DATA_SZ 64 @@ -82,6 +88,31 @@ static uint8_t *g_flash_mem; static uint8_t g_data[DATA_SZ + CANARY_SZ]; #define g_canary (g_data + DATA_SZ) +/* Mock hal_flash_wait_complete(): the real one (hal/stm32l5.c) only + * spins on FLASH_SR_BSY, which the host register file never sets. This + * one adds the program-window check the host model cannot see any + * other way: the bytes changed since the previous window must fit + * within one aligned 8-byte program unit. The pre-fix HAL issued its + * two word stores relative to the caller address, so an unaligned + * start split them across two units and this assertion goes red. */ +static void hal_flash_wait_complete(uint8_t bank) +{ + int i; + int first = -1; + int last = -1; + + for (i = 0; i < FLASH_MEM_SZ; i++) { + if (g_flash_mem[i] != g_flash_prev[i]) { + if (first < 0) + first = i; + last = i; + } + } + if (first >= 0) + ck_assert_int_le(last, (first & ~0x07) + 7); + memcpy(g_flash_prev, g_flash_mem, FLASH_MEM_SZ); +} + /* The real functions from hal/stm32l5.c (extracted by the Makefile). */ #include "stm32l5_write_extract.h" @@ -92,6 +123,7 @@ static void setup(void) memset(g_flash_regs, 0, sizeof(g_flash_regs)); for (i = 0; i < FLASH_MEM_SZ; i++) g_flash_mem[i] = 0x12; /* stale */ + memcpy(g_flash_prev, g_flash_mem, FLASH_MEM_SZ); for (i = 0; i < DATA_SZ; i++) g_data[i] = (uint8_t)(0x30 + i); /* 0x70..0x8F: distinct from the data bytes (0x30..0x6F), the stale diff --git a/tools/unit-tests/unit-stm32u5-write.c b/tools/unit-tests/unit-stm32u5-write.c index fed3fa5dca..053e6564ec 100644 --- a/tools/unit-tests/unit-stm32u5-write.c +++ b/tools/unit-tests/unit-stm32u5-write.c @@ -13,11 +13,13 @@ * aligned down to the unit, the bytes outside the requested span come * from flash and go back unchanged, and all four words are stored * through the aligned pointer. That is asserted here for unaligned - * starts (the host data model cannot observe the program-unit split - * itself, only where the bytes end up). All four words are always - * stored in one program -- the controller only starts it on the - * fourth word -- which this host model cannot observe, so that is - * asserted by construction in the HAL, not here. + * starts. The program-window invariant is enforced by the mock + * hal_flash_wait_complete below: the real one only spins on + * FLASH_SR_BSY (never set on the host register file), so the mock + * diffs the flash against the previous window and asserts that the + * changed bytes fit in one aligned 16-byte unit. A pre-fix HAL split + * the four word stores across two units for an unaligned start, and + * the 20-byte unaligned test goes red on it. * * Same harness as the STM32L5 twin: extracted functions, registers on * a host file, stale destination flash, canary after the source. @@ -76,6 +78,10 @@ static uint32_t g_flash_regs[0x40 / sizeof(uint32_t)]; #define FLASH_MEM_ADDR 0x11000000UL static uint8_t *g_flash_mem; +/* Snapshot of the flash at the last program-window boundary; the + * mock hal_flash_wait_complete() diffs against it. */ +static uint8_t g_flash_prev[FLASH_MEM_SZ]; + /* Source buffer followed by a canary: pre-fix, a short write reads * bytes past len and lands them in the destination flash. The canary * range avoids the data bytes (0x30..0x6F), the stale fill (0x12) and @@ -85,6 +91,31 @@ static uint8_t *g_flash_mem; static uint8_t g_data[DATA_SZ + CANARY_SZ]; #define g_canary (g_data + DATA_SZ) +/* Mock hal_flash_wait_complete(): the real one (hal/stm32u5.c) only + * spins on FLASH_SR_BSY, which the host register file never sets. This + * one adds the program-window check the host model cannot see any + * other way: the bytes changed since the previous window must fit + * within one aligned 16-byte program unit. The pre-fix HAL issued its + * four word stores relative to the caller address, so an unaligned + * start split them across two units and this assertion goes red. */ +static void hal_flash_wait_complete(uint8_t bank) +{ + int i; + int first = -1; + int last = -1; + + for (i = 0; i < FLASH_MEM_SZ; i++) { + if (g_flash_mem[i] != g_flash_prev[i]) { + if (first < 0) + first = i; + last = i; + } + } + if (first >= 0) + ck_assert_int_le(last, (first & ~0x0F) + 15); + memcpy(g_flash_prev, g_flash_mem, FLASH_MEM_SZ); +} + /* The real functions from hal/stm32u5.c (extracted by the Makefile). */ #include "stm32u5_write_extract.h" @@ -95,6 +126,7 @@ static void setup(void) memset(g_flash_regs, 0, sizeof(g_flash_regs)); for (i = 0; i < FLASH_MEM_SZ; i++) g_flash_mem[i] = 0x12; /* stale */ + memcpy(g_flash_prev, g_flash_mem, FLASH_MEM_SZ); for (i = 0; i < DATA_SZ; i++) g_data[i] = (uint8_t)(0x30 + i); for (i = 0; i < CANARY_SZ; i++)