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 */ 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/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 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/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/hal/stm32wb.c b/hal/stm32wb.c index 6e41a6760f..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 @@ -254,11 +257,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_SWS_MASK) != RCC_CFGR_SWS_MSI) + ; /* Turn off PLL */ RCC_CR &= ~RCC_CR_PLLON; DMB(); 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/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"); 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/Makefile b/tools/unit-tests/Makefile index 6efd8e0b86..8b844f46ea 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 @@ -1425,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 @@ -1621,8 +1639,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-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; } 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; +} diff --git a/tools/unit-tests/unit-stm32l5-write.c b/tools/unit-tests/unit-stm32l5-write.c index 36fb79b031..f6fccc3500 100644 --- a/tools/unit-tests/unit-stm32l5-write.c +++ b/tools/unit-tests/unit-stm32l5-write.c @@ -3,13 +3,23 @@ * 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 - * 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 both words are stored through + * 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. @@ -39,7 +49,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 @@ -67,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 @@ -74,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" @@ -84,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 @@ -110,8 +150,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 +216,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 +263,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; diff --git a/tools/unit-tests/unit-stm32u5-write.c b/tools/unit-tests/unit-stm32u5-write.c index 630c2883e4..053e6564ec 100644 --- a/tools/unit-tests/unit-stm32u5-write.c +++ b/tools/unit-tests/unit-stm32u5-write.c @@ -3,18 +3,26 @@ * 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 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. * - * 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 +50,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). */ @@ -70,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 @@ -79,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" @@ -89,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++) @@ -199,6 +237,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 +285,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; 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);