From 0a6eb658f5b888ec6a4a42122a4e4b00b83a98b1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 18:22:40 +0200 Subject: [PATCH 1/3] Fix STM32U5 fallback erase and rollback handling --- src/libwolfboot.c | 2 ++ src/update_flash_hwswap.c | 8 +++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index a5de6834a9..20e167767c 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -799,7 +799,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(); } } } diff --git a/src/update_flash_hwswap.c b/src/update_flash_hwswap.c index 22ba5fde49..54f05f265b 100644 --- a/src/update_flash_hwswap.c +++ b/src/update_flash_hwswap.c @@ -46,11 +46,6 @@ 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 */ @@ -58,6 +53,9 @@ void RAMFUNCTION wolfBoot_start(void) 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; if ((max_v > 0U) && (active_v < max_v)) { wolfBoot_printf("Rollback to lower version not allowed\n"); From 020d126b63e1881954f2cf996929b423ab61591d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 18:37:36 +0200 Subject: [PATCH 2/3] Fix hwswap fallback CI regressions --- src/libwolfboot.c | 8 +++--- src/update_flash.c | 2 -- tools/scripts/sim-dualbank-rollback-denied.sh | 15 +++++----- tools/unit-tests/unit-enc-nvm.c | 14 ++++------ tools/unit-tests/unit-nvm.c | 24 ++++++++++------ tools/unit-tests/unit-update-flash-hwswap.c | 28 +++++++++---------- 6 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 20e167767c..a768f028c2 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -773,20 +773,20 @@ int wolfBoot_get_update_sector_flag(uint16_t sector, uint8_t *flag) */ 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: diff --git a/src/update_flash.c b/src/update_flash.c index 9e433c260b..516d085365 100644 --- a/src/update_flash.c +++ b/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) { diff --git a/tools/scripts/sim-dualbank-rollback-denied.sh b/tools/scripts/sim-dualbank-rollback-denied.sh index 5cf18001e5..9fbabe3a0f 100755 --- a/tools/scripts/sim-dualbank-rollback-denied.sh +++ b/tools/scripts/sim-dualbank-rollback-denied.sh @@ -45,21 +45,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." diff --git a/tools/unit-tests/unit-enc-nvm.c b/tools/unit-tests/unit-enc-nvm.c index a5fad7d020..435cebe78a 100644 --- a/tools/unit-tests/unit-enc-nvm.c +++ b/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); diff --git a/tools/unit-tests/unit-nvm.c b/tools/unit-tests/unit-nvm.c index 7ae77bc482..ab5faed3f9 100644 --- a/tools/unit-tests/unit-nvm.c +++ b/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,12 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error) erased_nvm_bank1 = 0; hal_flash_write_fail = 1; + if (locked) + hal_flash_unlock(); ret = partition_magic_write(PART_UPDATE, PART_UPDATE_ENDFLAGS - sizeof(uint32_t)); + if (!locked) + hal_flash_lock(); ck_assert_int_eq(ret, -1); ck_assert_int_eq(erased_update, 0); #ifdef FLAGS_HOME @@ -326,7 +332,7 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error) ck_assert_int_eq(erased_nvm_bank1, 0); ck_assert_uint_eq(*magic, wolfboot_magic_trail); - hal_flash_lock(); + locked = 1; } END_TEST @@ -348,8 +354,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 +385,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); diff --git a/tools/unit-tests/unit-update-flash-hwswap.c b/tools/unit-tests/unit-update-flash-hwswap.c index 7fc108606c..c5a0e44953 100644 --- a/tools/unit-tests/unit-update-flash-hwswap.c +++ b/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); From 2c1b3f28da74722a2786a62b38d48253a99e2082 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 2 Jul 2026 19:20:34 +0200 Subject: [PATCH 3/3] Address PR #815 review comments - Document wolfBoot_erase_partition() lock postcondition in doxygen - Comment the hwswap anti-rollback guard as defense-in-depth - Clarify sim-dualbank-rollback-denied.sh purpose via header comment - Simplify lock guards in unit-nvm partition_magic_write test --- src/libwolfboot.c | 5 ++++- src/update_flash_hwswap.c | 7 +++++++ tools/scripts/sim-dualbank-rollback-denied.sh | 7 +++++++ tools/unit-tests/unit-nvm.c | 9 +++------ 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index a768f028c2..51a4577775 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -767,7 +767,10 @@ 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. */ diff --git a/src/update_flash_hwswap.c b/src/update_flash_hwswap.c index 54f05f265b..6e86a5a0be 100644 --- a/src/update_flash_hwswap.c +++ b/src/update_flash_hwswap.c @@ -57,6 +57,13 @@ void RAMFUNCTION wolfBoot_start(void) 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(); diff --git a/tools/scripts/sim-dualbank-rollback-denied.sh b/tools/scripts/sim-dualbank-rollback-denied.sh index 9fbabe3a0f..9096f1be86 100755 --- a/tools/scripts/sim-dualbank-rollback-denied.sh +++ b/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 diff --git a/tools/unit-tests/unit-nvm.c b/tools/unit-tests/unit-nvm.c index ab5faed3f9..5ab08eb9ff 100644 --- a/tools/unit-tests/unit-nvm.c +++ b/tools/unit-tests/unit-nvm.c @@ -315,12 +315,11 @@ START_TEST(test_partition_magic_write_stops_on_flash_write_error) erased_nvm_bank1 = 0; hal_flash_write_fail = 1; - if (locked) - hal_flash_unlock(); + /* 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)); - if (!locked) - hal_flash_lock(); + hal_flash_lock(); ck_assert_int_eq(ret, -1); ck_assert_int_eq(erased_update, 0); #ifdef FLAGS_HOME @@ -331,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); - - locked = 1; } END_TEST