From a76882aa8ca3bf97a9da10d60a4f09abfa67322b Mon Sep 17 00:00:00 2001 From: John Bland Date: Fri, 6 Dec 2024 20:15:34 -0500 Subject: [PATCH 1/8] properly unlock flash on re-entry of wolfBoot_swap_and_final_erase and move it out of the lock logic of update and delta update --- src/update_flash.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index 20cc67cc0..b0c9368b2 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -245,6 +245,11 @@ static int wolfBoot_swap_and_final_erase(int resume) if ((resume == 1) && (swapDone == 0) && (st != IMG_STATE_FINAL_FLAGS)) { return -1; } + hal_flash_unlock(); +#ifdef EXT_FLASH + ext_flash_unlock(); +#endif + if (swapDone == 0) { /* IMG_STATE_FINAL_FLAGS allows re-entry without blowing away swap */ if (st != IMG_STATE_FINAL_FLAGS) { @@ -282,6 +287,12 @@ static int wolfBoot_swap_and_final_erase(int resume) wolfBoot_set_partition_state(PART_BOOT, IMG_STATE_TESTING); /* erase the last sector(s) of update */ wb_flash_erase(update, WOLFBOOT_PARTITION_SIZE - eraseLen, eraseLen); + +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); + return 0; } #endif @@ -480,15 +491,17 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, sector++; } #ifndef DISABLE_BACKUP - /* start re-entrant final erase, return code is only for resumption in - * wolfBoot_start */ - wolfBoot_swap_and_final_erase(0); #endif out: #ifdef EXT_FLASH ext_flash_lock(); #endif hal_flash_lock(); + /* start re-entrant final erase, return code is only for resumption in + * wolfBoot_start */ + if (ret == 0) { + wolfBoot_swap_and_final_erase(0); + } /* encryption key was not erased, will be erased by success */ return ret; } @@ -741,14 +754,14 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) } #endif /* WOLFBOOT_FLASH_MULTI_SECTOR_ERASE */ - /* start re-entrant final erase, return code is only for resumption in - * wolfBoot_start*/ - wolfBoot_swap_and_final_erase(0); /* encryption key was not erased, will be erased by success */ #ifdef EXT_FLASH ext_flash_lock(); #endif hal_flash_lock(); + /* start re-entrant final erase, return code is only for resumption in + * wolfBoot_start*/ + wolfBoot_swap_and_final_erase(0); #else /* DISABLE_BACKUP */ /* Direct Swap without power fail safety */ From bbdf14e127a82b991f212dcd6bf6aa1fa963bad2 Mon Sep 17 00:00:00 2001 From: John Bland Date: Fri, 6 Dec 2024 20:22:50 -0500 Subject: [PATCH 2/8] update footprint --- tools/test.mk | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tools/test.mk b/tools/test.mk index 2a02afbfc..f3b304c4d 100644 --- a/tools/test.mk +++ b/tools/test.mk @@ -986,40 +986,40 @@ test-all: clean test-size-all: - make test-size SIGN=NONE LIMIT=4840 NO_ARM_ASM=1 + make test-size SIGN=NONE LIMIT=4860 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED25519 LIMIT=11428 NO_ARM_ASM=1 + make test-size SIGN=ED25519 LIMIT=11448 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC256 LIMIT=17948 NO_ARM_ASM=1 + make test-size SIGN=ECC256 LIMIT=17968 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13492 NO_ARM_ASM=1 + make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13512 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA2048 LIMIT=11212 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 LIMIT=11232 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=11788 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=11808 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA4096 LIMIT=11500 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 LIMIT=11520 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12076 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12096 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC384 LIMIT=17516 NO_ARM_ASM=1 + make test-size SIGN=ECC384 LIMIT=17536 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC384 NO_ASM=1 LIMIT=14884 NO_ARM_ASM=1 + make test-size SIGN=ECC384 NO_ASM=1 LIMIT=14904 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED448 LIMIT=13440 NO_ARM_ASM=1 + make test-size SIGN=ED448 LIMIT=13464 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA3072 LIMIT=11352 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 LIMIT=11372 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=11892 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=11912 NO_ARM_ASM=1 make keysclean make test-size SIGN=LMS LMS_LEVELS=2 LMS_HEIGHT=5 LMS_WINTERNITZ=8 \ WOLFBOOT_SMALL_STACK=0 IMAGE_SIGNATURE_SIZE=2644 \ - IMAGE_HEADER_SIZE?=5288 LIMIT=7516 NO_ARM_ASM=1 + IMAGE_HEADER_SIZE?=5288 LIMIT=7536 NO_ARM_ASM=1 make keysclean make test-size SIGN=XMSS XMSS_PARAMS='XMSS-SHA2_10_256' \ IMAGE_SIGNATURE_SIZE=2500 IMAGE_HEADER_SIZE?=4096 \ - LIMIT=8232 NO_ARM_ASM=1 + LIMIT=8252 NO_ARM_ASM=1 make keysclean make clean - make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=20148 \ + make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=20168 \ IMAGE_SIGNATURE_SIZE=2420 IMAGE_HEADER_SIZE?=8192 From e70dca82b5fa8ff04af40e095c2ae4d46623719f Mon Sep 17 00:00:00 2001 From: John Bland Date: Fri, 6 Dec 2024 20:55:50 -0500 Subject: [PATCH 3/8] update tests to properly simulate flash locks add hal_flash_unlock after setting the key since setting the key locks flash --- hal/sim.c | 34 ++++++++++++++++++++++++++++++---- src/update_flash.c | 24 +++++++++++++----------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/hal/sim.c b/hal/sim.c index 629c904e7..1816374d2 100644 --- a/hal/sim.c +++ b/hal/sim.c @@ -54,6 +54,8 @@ static uint8_t *flash_base; int forceEmergency = 0; uint32_t erasefail_address = 0xFFFFFFFF; +int flashLocked = 1; +int extFlashLocked = 1; #define INTERNAL_FLASH_FILE "./internal_flash.dd" #define EXTERNAL_FLASH_FILE "./external_flash.dd" @@ -134,12 +136,12 @@ static int mmap_file(const char *path, uint8_t *address, uint8_t** ret_address) void hal_flash_unlock(void) { - /* no op */ + flashLocked = 0; } void hal_flash_lock(void) { - /* no op */ + flashLocked = 1; } void hal_prepare_boot(void) @@ -150,6 +152,10 @@ void hal_prepare_boot(void) int hal_flash_write(uintptr_t address, const uint8_t *data, int len) { int i; + if (flashLocked == 1) { + wolfBoot_printf("FLASH IS BEING WRITTEN TO WHILE LOCKED\n"); + return -1; + } if (forceEmergency == 1 && address == WOLFBOOT_PARTITION_BOOT_ADDRESS) { /* implicit cast abide compiler warning */ memset((void*)address, 0, len); @@ -179,6 +185,10 @@ int hal_flash_write(uintptr_t address, const uint8_t *data, int len) int hal_flash_erase(uintptr_t address, int len) { + if (flashLocked == 1) { + wolfBoot_printf("FLASH IS BEING ERASED WHILE LOCKED\n"); + return -1; + } /* implicit cast abide compiler warning */ wolfBoot_printf( "hal_flash_erase addr %p len %d\n", (void*)address, len); if (address == erasefail_address + WOLFBOOT_PARTITION_BOOT_ADDRESS) { @@ -227,16 +237,20 @@ void hal_init(void) void ext_flash_lock(void) { - /* no op */ + extFlashLocked = 1; } void ext_flash_unlock(void) { - /* no op */ + extFlashLocked = 0; } int ext_flash_write(uintptr_t address, const uint8_t *data, int len) { + if (extFlashLocked == 1) { + wolfBoot_printf("EXT FLASH IS BEING WRITTEN TO WHILE LOCKED\n"); + return -1; + } memcpy(flash_base + address, data, len); return 0; } @@ -249,6 +263,10 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) int ext_flash_erase(uintptr_t address, int len) { + if (extFlashLocked == 1) { + wolfBoot_printf("EXT FLASH IS BEING ERASED WHILE LOCKED\n"); + return -1; + } memset(flash_base + address, FLASH_BYTE_ERASED, len); return 0; } @@ -287,6 +305,14 @@ void do_boot(const uint32_t *app_offset) int ret; size_t app_size = WOLFBOOT_PARTITION_SIZE - IMAGE_HEADER_SIZE; + if (flashLocked == 0) { + wolfBoot_printf("WARNING FLASH IS UNLOCKED AT BOOT"); + } + + if (extFlashLocked == 0) { + wolfBoot_printf("WARNING EXT FLASH IS UNLOCKED AT BOOT"); + } + #ifdef __APPLE__ typedef int (*main_entry)(int, char**, char**, char**); NSObjectFileImage fileImage = NULL; diff --git a/src/update_flash.c b/src/update_flash.c index b0c9368b2..823f9726b 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -245,36 +245,38 @@ static int wolfBoot_swap_and_final_erase(int resume) if ((resume == 1) && (swapDone == 0) && (st != IMG_STATE_FINAL_FLAGS)) { return -1; } + hal_flash_unlock(); -#ifdef EXT_FLASH + + /* IMG_STATE_FINAL_FLAGS allows re-entry without blowing away swap */ + if (st != IMG_STATE_FINAL_FLAGS) { + /* store the sector at tmpBootPos into swap */ + wolfBoot_copy_sector(boot, swap, tmpBootPos / WOLFBOOT_SECTOR_SIZE); + /* set FINAL_SWAP for re-entry */ + wolfBoot_set_partition_state(PART_UPDATE, IMG_STATE_FINAL_FLAGS); + } +#ifdef EXT_ENCRYPTED ext_flash_unlock(); -#endif if (swapDone == 0) { - /* IMG_STATE_FINAL_FLAGS allows re-entry without blowing away swap */ - if (st != IMG_STATE_FINAL_FLAGS) { - /* store the sector at tmpBootPos into swap */ - wolfBoot_copy_sector(boot, swap, tmpBootPos / WOLFBOOT_SECTOR_SIZE); - /* set FINAL_SWAP for re-entry */ - wolfBoot_set_partition_state(PART_UPDATE, IMG_STATE_FINAL_FLAGS); - } -#ifdef EXT_ENCRYPTED /* get encryption key and iv if encryption is enabled */ wolfBoot_get_encrypt_key((uint8_t*)tmpBuffer, (uint8_t*)&tmpBuffer[ENCRYPT_KEY_SIZE/sizeof(uint32_t)]); -#endif /* write TRAIL, encryption key and iv if enabled to tmpBootPos*/ tmpBuffer[TRAILER_OFFSET_WORDS] = WOLFBOOT_MAGIC_TRAIL; wb_flash_erase(boot, tmpBootPos, WOLFBOOT_SECTOR_SIZE); wb_flash_write(boot, tmpBootPos, (void*)tmpBuffer, sizeof(tmpBuffer)); } +#endif /* erase the last boot sector(s) */ wb_flash_erase(boot, WOLFBOOT_PARTITION_SIZE - eraseLen, eraseLen); /* set the encryption key */ #ifdef EXT_ENCRYPTED wolfBoot_set_encrypt_key((uint8_t*)tmpBuffer, (uint8_t*)&tmpBuffer[ENCRYPT_KEY_SIZE/sizeof(uint32_t)]); + /* wolfBoot_set_encrypt_key calls hal_flash_unlock, need to unlock again */ + hal_flash_unlock(); #endif /* write the original contents of tmpBootPos back */ if (tmpBootPos < boot->fw_size + IMAGE_HEADER_SIZE) { From c06eda03b44bb88cf14d5c9506c8fc578d9dcda8 Mon Sep 17 00:00:00 2001 From: John Bland Date: Mon, 9 Dec 2024 12:06:46 -0500 Subject: [PATCH 4/8] fix all cases where flash was written or erased while locked --- src/update_flash.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index 823f9726b..8e22c8cd5 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -247,6 +247,9 @@ static int wolfBoot_swap_and_final_erase(int resume) } hal_flash_unlock(); +#ifdef EXT_ENCRYPTED + ext_flash_unlock(); +#endif /* IMG_STATE_FINAL_FLAGS allows re-entry without blowing away swap */ if (st != IMG_STATE_FINAL_FLAGS) { @@ -256,8 +259,6 @@ static int wolfBoot_swap_and_final_erase(int resume) wolfBoot_set_partition_state(PART_UPDATE, IMG_STATE_FINAL_FLAGS); } #ifdef EXT_ENCRYPTED - ext_flash_unlock(); - if (swapDone == 0) { /* get encryption key and iv if encryption is enabled */ wolfBoot_get_encrypt_key((uint8_t*)tmpBuffer, @@ -937,9 +938,25 @@ void RAMFUNCTION wolfBoot_start(void) wolfBoot_check_self_update(); #endif +#ifdef NVM_FLASH_WRITEONCE + /* nvm_select_fresh_sector needs unlocked flash in cases where */ + hal_flash_unlock(); +#ifdef EXT_FLASH + ext_flash_unlock(); +#endif +#endif + bootRet = wolfBoot_get_partition_state(PART_BOOT, &bootState); updateRet = wolfBoot_get_partition_state(PART_UPDATE, &updateState); +#ifdef NVM_FLASH_WRITEONCE + /* nvm_select_fresh_sector needs unlocked flash in cases where */ + hal_flash_lock(); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif +#endif + #if !defined(DISABLE_BACKUP) /* resume the final erase in case the power failed before it finished */ resumedFinalErase = wolfBoot_swap_and_final_erase(1); From 3608cacec53d51504463a733ee1002791f44de1f Mon Sep 17 00:00:00 2001 From: John Bland Date: Mon, 9 Dec 2024 13:27:09 -0500 Subject: [PATCH 5/8] unlock flash when updating partition state --- src/update_flash.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/update_flash.c b/src/update_flash.c index 8e22c8cd5..5049ca378 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -650,7 +650,15 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) * we need to set to UPDATING, otherwise there's no way to tell the * original direction of the update once interrupted */ else if ((inverse == 0) && (fallback_allowed == 1)) { + hal_flash_unlock(); +#ifdef EXT_FLASH + ext_flash_unlock(); +#endif wolfBoot_set_partition_state(PART_UPDATE, IMG_STATE_UPDATING); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); } return wolfBoot_delta_update(&boot, &update, &swap, inverse, resume); From ef5ef95b4e313ab6231dcf79b13ea4e3c306a6b4 Mon Sep 17 00:00:00 2001 From: John Bland Date: Mon, 9 Dec 2024 13:47:54 -0500 Subject: [PATCH 6/8] fix comments --- src/libwolfboot.c | 2 +- src/update_flash.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index b22c9d6cf..17b4e0777 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -268,7 +268,7 @@ static int RAMFUNCTION nvm_select_fresh_sector(int part) } } finish: - /* Erase the non-selected partition */ + /* Erase the non-selected partition, requires unlocked flash */ addrErase -= WOLFBOOT_SECTOR_SIZE * (!sel); if (*((uint32_t*)(addrErase + WOLFBOOT_SECTOR_SIZE - sizeof(uint32_t))) != FLASH_WORD_ERASED) { diff --git a/src/update_flash.c b/src/update_flash.c index 5049ca378..332a053fe 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -947,7 +947,8 @@ void RAMFUNCTION wolfBoot_start(void) #endif #ifdef NVM_FLASH_WRITEONCE - /* nvm_select_fresh_sector needs unlocked flash in cases where */ + /* nvm_select_fresh_sector needs unlocked flash in cases where the unused + * sector needs to be erased */ hal_flash_unlock(); #ifdef EXT_FLASH ext_flash_unlock(); @@ -958,7 +959,6 @@ void RAMFUNCTION wolfBoot_start(void) updateRet = wolfBoot_get_partition_state(PART_UPDATE, &updateState); #ifdef NVM_FLASH_WRITEONCE - /* nvm_select_fresh_sector needs unlocked flash in cases where */ hal_flash_lock(); #ifdef EXT_FLASH ext_flash_lock(); From 0572d34ae5029809297e1a482e29c8db8d144a06 Mon Sep 17 00:00:00 2001 From: John Bland Date: Tue, 17 Dec 2024 07:51:56 -0500 Subject: [PATCH 7/8] fix wrong conditional compile flag --- src/update_flash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/update_flash.c b/src/update_flash.c index 332a053fe..ca2cf4f52 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -247,7 +247,7 @@ static int wolfBoot_swap_and_final_erase(int resume) } hal_flash_unlock(); -#ifdef EXT_ENCRYPTED +#ifdef EXT_FLASH ext_flash_unlock(); #endif From bd9b64df9f77dff33cd14f09766bff6245c6068b Mon Sep 17 00:00:00 2001 From: John Bland Date: Tue, 17 Dec 2024 08:19:40 -0500 Subject: [PATCH 8/8] move disable backup flag to correct spot --- src/update_flash.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index ca2cf4f52..914b0b483 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -493,8 +493,6 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, wb_flash_erase(boot, sector * WOLFBOOT_SECTOR_SIZE, WOLFBOOT_SECTOR_SIZE); sector++; } -#ifndef DISABLE_BACKUP -#endif out: #ifdef EXT_FLASH ext_flash_lock(); @@ -502,9 +500,11 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, hal_flash_lock(); /* start re-entrant final erase, return code is only for resumption in * wolfBoot_start */ +#ifndef DISABLE_BACKUP if (ret == 0) { wolfBoot_swap_and_final_erase(0); } +#endif /* encryption key was not erased, will be erased by success */ return ret; }