Fix STM32U5 fallback erase and rollback handling

src/libwolfboot.c

@@ -767,26 +767,29 @@ int wolfBoot_get_update_sector_flag(uint16_t sector, uint8_t *flag)
 /**
  * @brief Erase a partition.
  *
- * This function erases a partition.
+ * This function erases a partition. It manages the flash lock internally:
+ * the target flash (internal or external) is unlocked before the erase and
+ * locked again before returning. Callers do not need to unlock the flash
+ * beforehand, and on return the flash is left locked.
  *
  * @param[in] part Partition number.
  */
 void RAMFUNCTION wolfBoot_erase_partition(uint8_t part)
 {
-    uint32_t address = 0;
+    uintptr_t address = 0;
     int size = 0;
 
     switch (part) {
         case PART_BOOT:
-            address = (uint32_t)WOLFBOOT_PARTITION_BOOT_ADDRESS;
+            address = (uintptr_t)WOLFBOOT_PARTITION_BOOT_ADDRESS;
             size = WOLFBOOT_PARTITION_SIZE;
             break;
         case PART_UPDATE:
-            address = (uint32_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS;
+            address = (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS;
             size = WOLFBOOT_PARTITION_SIZE;
             break;
         case PART_SWAP:
-            address = (uint32_t)WOLFBOOT_PARTITION_SWAP_ADDRESS;
+            address = (uintptr_t)WOLFBOOT_PARTITION_SWAP_ADDRESS;
             size = WOLFBOOT_SECTOR_SIZE;
             break;
         default:
@@ -799,7 +802,9 @@ void RAMFUNCTION wolfBoot_erase_partition(uint8_t part)
             ext_flash_erase(address, size);
             ext_flash_lock();
         } else {
+            hal_flash_unlock();
             hal_flash_erase(address, size);
+            hal_flash_lock();
         }
     }
 }

src/update_flash.c

@@ -233,9 +233,7 @@ void RAMFUNCTION wolfBoot_check_self_update(void)
             wolfBoot_get_image_type(PART_UPDATE) == (HDR_IMG_TYPE_WOLFBOOT | HDR_IMG_TYPE_AUTH)) {
         uint32_t update_version = wolfBoot_update_firmware_version();
         if (update_version <= wolfboot_version) {
-            hal_flash_unlock();
             wolfBoot_erase_partition(PART_UPDATE);
-            hal_flash_lock();
             return;
         }
         if (wolfBoot_verify_integrity(&update) < 0) {

src/update_flash_hwswap.c

@@ -46,19 +46,24 @@ void RAMFUNCTION wolfBoot_start(void)
     int active;
     struct wolfBoot_image fw_image;
     uint8_t p_state;
-#ifndef ALLOW_DOWNGRADE
-    uint32_t boot_v = wolfBoot_current_firmware_version();
-    uint32_t update_v = wolfBoot_update_firmware_version();
-    uint32_t max_v = (boot_v > update_v) ? boot_v : update_v;
-#endif
     active = wolfBoot_dualboot_candidate();
 
     if (active < 0) /* panic if no images available */
         boot_panic();
 
     for (;;) {
 #ifndef ALLOW_DOWNGRADE
+        uint32_t boot_v = wolfBoot_current_firmware_version();
+        uint32_t update_v = wolfBoot_update_firmware_version();
+        uint32_t max_v = (boot_v > update_v) ? boot_v : update_v;
         uint32_t active_v = (active == PART_UPDATE) ? update_v : boot_v;
+        /* Anti-rollback defense-in-depth: wolfBoot_dualboot_candidate() always
+         * selects the max-version partition, and after a failed image is erased
+         * its version reads 0, so the remaining valid partition becomes the new
+         * max. This guard therefore should not normally trigger, but it is kept
+         * as a safety net in case a lower version is ever selected while a
+         * higher one is still present.
+         */
         if ((max_v > 0U) && (active_v < max_v)) {
             wolfBoot_printf("Rollback to lower version not allowed\n");
             boot_panic();

tools/scripts/sim-dualbank-rollback-denied.sh

@@ -1,4 +1,11 @@
 #!/bin/bash
+#
+# NOTE: Despite the "rollback-denied" filename (kept to avoid churning the CI
+# workflow references), this script now verifies the corrupted-update *fallback*
+# path: after the UPDATE image is invalidated, wolfBoot must fall back and boot
+# the remaining valid image instead of denying the boot. The denial message must
+# NOT appear and the application handoff must be reached.
+#
 set -euo pipefail
 
 if [ ! -f ".config" ]; then
@@ -45,21 +52,22 @@ rollback_output="$(timeout 3s ./wolfboot.elf get_version 2>&1)"
 rollback_rc=$?
 set -e
 
-if [ "$rollback_rc" -eq 0 ]; then
-    echo "Expected rollback denial, but boot continued normally." >&2
+if [ "$rollback_rc" -ne 0 ]; then
+    echo "Expected fallback boot after invalidating the corrupted update, got exit code: $rollback_rc" >&2
+    echo "$rollback_output" >&2
     exit 1
 fi
 
-if [ "$rollback_rc" -ne 124 ] && [ "$rollback_rc" -ne 80 ]; then
-    echo "Unexpected exit code while checking rollback denial: $rollback_rc" >&2
+if printf '%s\n' "$rollback_output" | grep -q "Rollback to lower version not allowed"; then
+    echo "Rollback denial message should not be present after fallback." >&2
     echo "$rollback_output" >&2
     exit 1
 fi
 
-if ! printf '%s\n' "$rollback_output" | grep -q "Rollback to lower version not allowed"; then
-    echo "Rollback denial message not found in output." >&2
+if ! printf '%s\n' "$rollback_output" | grep -q "Simulator do_boot app_offset"; then
+    echo "Fallback boot did not reach the application handoff." >&2
     echo "$rollback_output" >&2
     exit 1
 fi
 
-echo "Dualbank rollback-to-older-version denial verified."
+echo "Dualbank corrupted-update fallback verified."

tools/unit-tests/unit-enc-nvm.c

@@ -74,9 +74,6 @@ START_TEST (test_nvm_update_with_encryption)
     /* Sanity */
     ck_assert(home_off <= WOLFBOOT_SECTOR_SIZE);
 
-    /* unlock the flash to allow operations */
-    hal_flash_unlock();
-
     /* Check swap erase */
     wolfBoot_erase_partition(PART_SWAP);
     ck_assert(erased_swap == 1);
@@ -87,6 +84,7 @@ START_TEST (test_nvm_update_with_encryption)
 
     erased_update = 0;
     wolfBoot_erase_partition(part);
+    hal_flash_unlock();
 #ifndef FLAGS_HOME
     ck_assert(erased_update == 1);
 #else
@@ -206,7 +204,9 @@ START_TEST (test_nvm_update_with_encryption)
     ck_assert_msg(ret == 1, "Failed to select most recent sector after deleting flags\n");
 
     /* Start over, update some sector flags */
+    hal_flash_lock();
     wolfBoot_erase_partition(PART_UPDATE);
+    hal_flash_unlock();
     wolfBoot_set_update_sector_flag(0, SECT_FLAG_UPDATED);
     wolfBoot_set_update_sector_flag(1, SECT_FLAG_UPDATED);
     wolfBoot_set_update_sector_flag(2, SECT_FLAG_UPDATED);
@@ -259,7 +259,9 @@ START_TEST (test_nvm_update_with_encryption)
     /* Erase partition and start over */
     erased_update = 0;
     erased_boot = 0;
+    hal_flash_lock();
     wolfBoot_erase_partition(part);
+    hal_flash_unlock();
 #ifndef FLAGS_HOME
     ck_assert(erased_update == 1);
 #else
@@ -269,9 +271,9 @@ START_TEST (test_nvm_update_with_encryption)
     ret = nvm_select_fresh_sector(PART_UPDATE);
     ck_assert_msg(ret == 0, "Failed to select right sector after reading sector state\n");
 
-    /* re-lock the flash: update_trigger implies unlocking/locking */
     hal_flash_lock();
 
+    /* re-lock the flash: update_trigger implies unlocking/locking */
     /* Triggering update to set flags */
     wolfBoot_update_trigger();
 
@@ -307,9 +309,7 @@ START_TEST(test_set_encrypt_key_propagates_flash_write_error)
             WOLFBOOT_SECTOR_SIZE, NULL);
     ck_assert(ret >= 0);
 
-    hal_flash_unlock();
     wolfBoot_erase_partition(PART_BOOT);
-    hal_flash_lock();
 
     hal_flash_write_fail = 1;
     ret = wolfBoot_set_encrypt_key(key, nonce);
@@ -335,9 +335,7 @@ START_TEST(test_erase_encrypt_key_propagates_flash_write_error)
             (void *)MOCK_ADDRESS_SWAP, WOLFBOOT_SECTOR_SIZE, NULL);
     ck_assert(ret >= 0);
 
-    hal_flash_unlock();
     wolfBoot_erase_partition(PART_BOOT);
-    hal_flash_lock();
 
     ret = wolfBoot_set_encrypt_key(key, nonce);
     ck_assert_int_eq(ret, 0);

tools/unit-tests/unit-nvm.c

@@ -70,9 +70,6 @@ START_TEST (test_nvm_select_fresh_sector)
     /* Sanity */
     ck_assert(home_off <= WOLFBOOT_SECTOR_SIZE);
 
-    /* unlock the flash to allow operations */
-    hal_flash_unlock();
-
     /* Check swap erase */
     wolfBoot_erase_partition(PART_SWAP);
     ck_assert(erased_swap == 1);
@@ -83,6 +80,7 @@ START_TEST (test_nvm_select_fresh_sector)
 
     erased_update = 0;
     wolfBoot_erase_partition(part);
+    hal_flash_unlock();
 #ifndef FLAGS_HOME
     ck_assert(erased_update == 1);
 #else
@@ -201,7 +199,9 @@ START_TEST (test_nvm_select_fresh_sector)
     ck_assert_msg(ret == 1, "Failed to select most recent sector after deleting flags\n");
 
     /* Start over, update some sector flags */
+    hal_flash_lock();
     wolfBoot_erase_partition(PART_UPDATE);
+    hal_flash_unlock();
     wolfBoot_set_update_sector_flag(0, SECT_FLAG_UPDATED);
     wolfBoot_set_update_sector_flag(1, SECT_FLAG_UPDATED);
     wolfBoot_set_update_sector_flag(2, SECT_FLAG_UPDATED);
@@ -253,7 +253,9 @@ START_TEST (test_nvm_select_fresh_sector)
     /* Erase partition and start over */
     erased_update = 0;
     erased_boot = 0;
+    hal_flash_lock();
     wolfBoot_erase_partition(part);
+    hal_flash_unlock();
 #ifndef FLAGS_HOME
     ck_assert(erased_update == 1);
 #else
@@ -262,10 +264,10 @@ START_TEST (test_nvm_select_fresh_sector)
 
     ret = nvm_select_fresh_sector(PART_UPDATE);
     ck_assert_msg(ret == 0, "Failed to select right sector after reading sector state\n");
-
-    /* re-lock the flash: update_trigger implies unlocking/locking */
+
     hal_flash_lock();
 
+    /* re-lock the flash: update_trigger implies unlocking/locking */
     /* Triggering update to set flags */
     wolfBoot_update_trigger();
 
@@ -302,8 +304,8 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error)
             WOLFBOOT_SECTOR_SIZE, NULL);
     ck_assert(ret >= 0);
 
-    hal_flash_unlock();
     wolfBoot_erase_partition(PART_UPDATE);
+    hal_flash_unlock();
 
     magic = get_partition_magic(PART_UPDATE);
     *magic = wolfboot_magic_trail;
@@ -313,8 +315,11 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error)
     erased_nvm_bank1 = 0;
     hal_flash_write_fail = 1;
 
+    /* Flash was left unlocked by the hal_flash_unlock() above, so
+     * partition_magic_write() runs against unlocked flash and re-locks after. */
     ret = partition_magic_write(PART_UPDATE,
             PART_UPDATE_ENDFLAGS - sizeof(uint32_t));
+    hal_flash_lock();
     ck_assert_int_eq(ret, -1);
     ck_assert_int_eq(erased_update, 0);
 #ifdef FLAGS_HOME
@@ -325,8 +330,6 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error)
     ck_assert_int_eq(erased_nvm_bank0, 0);
     ck_assert_int_eq(erased_nvm_bank1, 0);
     ck_assert_uint_eq(*magic, wolfboot_magic_trail);
-
-    hal_flash_lock();
 }
 END_TEST
 
@@ -348,8 +351,8 @@ START_TEST(test_get_update_sector_flag_rejects_invalid_magic)
             (void *)MOCK_ADDRESS_SWAP, WOLFBOOT_SECTOR_SIZE, NULL);
     ck_assert(ret >= 0);
 
-    hal_flash_unlock();
     wolfBoot_erase_partition(PART_UPDATE);
+    hal_flash_unlock();
 
     magic = get_partition_magic(PART_UPDATE);
     *magic = 0xFFFFFFFF;
@@ -379,8 +382,8 @@ START_TEST(test_update_sector_flag_high_index_does_not_alias_low_index)
             (void *)MOCK_ADDRESS_SWAP, WOLFBOOT_SECTOR_SIZE, NULL);
     ck_assert(ret >= 0);
 
-    hal_flash_unlock();
     wolfBoot_erase_partition(PART_UPDATE);
+    hal_flash_unlock();
 
     ret = wolfBoot_set_update_sector_flag(1, SECT_FLAG_SWAPPING);
     ck_assert_int_eq(ret, 0);

tools/unit-tests/unit-update-flash-hwswap.c

@@ -226,18 +226,15 @@ static int add_payload(uint8_t part, uint32_t version, uint32_t size)
     return 0;
 }
 
-/* Regression test for the uint32_t->int cast in wolfBoot_start (hwswap).
+/* Regression test for corrupted-image fallback in wolfBoot_start (hwswap).
  *
  * BOOT carries a higher version than UPDATE, but its image is unbootable
  * (oversize header), so wolfBoot_start falls back to the UPDATE partition,
- * which holds a LOWER version. Anti-rollback must deny booting the lower
- * version (boot_panic, no do_boot).
- *
- * Both versions are above INT_MAX: with the buggy (int)cast they were clamped
- * to 0, max_v became 0, the "(max_v > 0U)" guard was skipped, and the
- * rolled-back UPDATE image was staged for boot.
+ * which holds a lower version. After invalidating the failing BOOT image,
+ * anti-rollback must re-evaluate versions and allow booting the only
+ * remaining valid image.
  */
-START_TEST (test_hwswap_highversion_rollback_denied) {
+START_TEST (test_hwswap_highversion_fallback_boots_lower_version) {
     uint32_t oversize = WOLFBOOT_PARTITION_SIZE;
 
     reset_mock_stats();
@@ -257,8 +254,8 @@ START_TEST (test_hwswap_highversion_rollback_denied) {
 
     wolfBoot_start();
 
-    /* Rollback to the lower UPDATE version must be denied */
-    ck_assert_int_eq(do_boot_called, 0);
+    ck_assert_int_eq(dualbank_swap_called, 1);
+    ck_assert_int_eq(do_boot_called, 1);
     cleanup_flash();
 }
 END_TEST
@@ -329,14 +326,15 @@ END_TEST
 Suite *wolfboot_suite(void)
 {
     Suite *s = suite_create("wolfboot-hwswap");
-    TCase *rollback_denied =
-        tcase_create("HW-swap high-version rollback denied");
+    TCase *fallback_boots =
+        tcase_create("HW-swap high-version fallback boots lower version");
     TCase *successful_boot =
         tcase_create("HW-swap successful boot");
 
-    tcase_add_test(rollback_denied, test_hwswap_highversion_rollback_denied);
-    suite_add_tcase(s, rollback_denied);
-    tcase_set_timeout(rollback_denied, 5);
+    tcase_add_test(fallback_boots,
+        test_hwswap_highversion_fallback_boots_lower_version);
+    suite_add_tcase(s, fallback_boots);
+    tcase_set_timeout(fallback_boots, 5);
 
     tcase_add_test(successful_boot, test_hwswap_first_boot_success);
     tcase_add_test(successful_boot, test_hwswap_postswap_success);