Improve ARMORED code + minor fixes #665

danielinux · 2026-01-12T08:47:08Z

ARMORED=1

Improved fault mitigation: more redundancy, hardened
Added redundancy to ML_DSA verification

STM32_TZ SAU:

Set UPDATE partition to secure. It is only accessed via NSC when the application is running
flash_top_secure in hal_tz_sau_init to the same range

Fixes:

Bug preventing rollback in case of ARMORED=1.
rolled-back image state handling: force update flags on rollback. Set success after complete rollback.

Copilot

Pull request overview

This PR enhances fault mitigation in ARMORED mode with improved redundancy checks, adds ARMORED support for ML_DSA signature verification, fixes rollback handling bugs, and reconfigures STM32 TrustZone SAU regions to set the UPDATE partition as secure.

Changes:

Enhanced ARMORED mode with additional redundancy in signature verification and version checking
Added redundant verification checks for ML_DSA signatures when ARMORED=1
Fixed rollback logic to properly handle version checks and state transitions with ARMORED enabled
Reconfigured STM32_TZ SAU to mark UPDATE partition as secure and adjusted flash region mappings

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tools/test.mk	Updated size limits for test targets to accommodate increased code size from ARMORED enhancements
src/update_flash.c	Added rollback detection logic, refactored version check to work with ARMORED, and improved rollback state handling
src/image.c	Added ARMORED redundancy checks to ML_DSA verification and updated signature verification macros
include/image.h	Refactored VERIFY_VERSION_ALLOWED macro for both GCC and IAR with improved redundancy and changed signed to unsigned comparisons
hal/stm32_tz.c	Reconfigured SAU regions to mark UPDATE partition as secure with dynamic flash_top_secure calculation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/image.c

Copilot · 2026-01-12T11:23:34Z

include/image.h

+            "b .\n" \
            "2:\n" \
            "pop {r4, r5, r6, r7}\n" \
            : /* No output operands */ \


The clobbered registers list for the IAR version is missing 'lr', 'cc', and 'memory' compared to the GCC version on line 620. Since this inline assembly calls functions with 'bl' instructions (which modifies 'lr') and performs comparisons (which modifies condition codes), these should be included in the clobber list to ensure the compiler preserves these registers correctly.

Copilot · 2026-01-12T11:23:34Z

include/image.h

+        "mov r0, %0\n" \
+        "mov r4, %0\n" \
+        "cmp r0, #1\n" \
+        "bne do_check\n" \
+        "cmp r4, #1\n" \
+        "bne do_check\n" \
+        "cmp r0, r4\n" \
+        "bne do_check\n" \
+        "cmp r0, #1\n" \
+        "bne do_check\n" \
+        "b end_check\n" \


The IAR version includes additional validation checks (lines 641-651) that trigger breakpoints if fb_ok is neither 0 nor 1, while the GCC version (lines 546-556) does not have equivalent validation. This inconsistency means the two compiler paths have different security hardening levels. Consider adding similar validation to the GCC version for consistency.

tools/test.mk

hal/stm32_tz.c

Copilot · 2026-01-12T11:23:35Z

hal/stm32_tz.c

+    /* Non-secure flash alias (entire NS flash window) */
+    sau_init_region(1, 0x08000000, FLASH_TOP, 0);
+
+    /* Secure: update partition in secure alias (use matching FLASH_TOP base) */
+    uint32_t flash_top_secure = FLASH_TOP;
+    if ((WOLFBOOT_PARTITION_UPDATE_ADDRESS & 0xFF000000u) !=
+        (FLASH_TOP & 0xFF000000u)) {
+        flash_top_secure =
+            (WOLFBOOT_PARTITION_UPDATE_ADDRESS & 0xFF000000u) |
+            (FLASH_TOP & 0x00FFFFFFu);
+    }
+    sau_init_region(2, WOLFBOOT_PARTITION_UPDATE_ADDRESS, flash_top_secure, 1);


The SAU configuration has been significantly changed. Region 1 now covers the entire flash window (0x08000000 to FLASH_TOP) as non-secure, while Region 2 marks only the UPDATE partition as secure. This means the BOOT partition is now accessible as non-secure via Region 1. Verify this is the intended behavior and that the BOOT partition doesn't need secure access protection during runtime. The description states the UPDATE partition is only accessed via NSC, but this configuration change may have broader security implications.

Co-authored-by: Copilot <[email protected]>

rizlik · 2026-01-12T11:12:07Z

hal/stm32_tz.c

-    /* Secure: application flash area (second bank) */
-    sau_init_region(2, WOLFBOOT_PARTITION_UPDATE_ADDRESS, FLASH_TOP, 0);
+    /* Non-secure flash alias (entire NS flash window) */
+    sau_init_region(1, 0x08000000, FLASH_TOP, 0);


NIT: can you use a constant instead of 0x08000000?

rizlik · 2026-01-12T11:45:39Z

hal/stm32_tz.c

+    uint32_t flash_top_secure = FLASH_TOP;
+    if ((WOLFBOOT_PARTITION_UPDATE_ADDRESS & 0xFF000000u) !=
+        (FLASH_TOP & 0xFF000000u)) {
+        flash_top_secure =
+            (WOLFBOOT_PARTITION_UPDATE_ADDRESS & 0xFF000000u) |
+            (FLASH_TOP & 0x00FFFFFFu);
+    }


I"m slightly confused here, if the FLASH_TOP is also the top of WOLFBOOT_PARTITION_UPDATE_ADDRESS then flash_top_secure stores the top bit only, otherwise it stores the full address. Is it on purpose?

rizlik · 2026-01-12T11:46:07Z

hal/stm32_tz.c

+            (WOLFBOOT_PARTITION_UPDATE_ADDRESS & 0xFF000000u) |
+            (FLASH_TOP & 0x00FFFFFFu);
+    }
+    sau_init_region(2, WOLFBOOT_PARTITION_UPDATE_ADDRESS, flash_top_secure, 1);


This makes the region NSC, not secure as mentioned by the comment on line 307

rizlik · 2026-01-12T11:50:14Z

include/image.h

+        "bhs ver_panic\n" \
+        "b end_check\n" \
+        "ver_panic:\n" \
+        "b .\n" \


do we need multiple b. here

rizlik · 2026-01-12T11:50:23Z

include/image.h

+            "bhs 3f\n" \
+            "b 2f\n" \
+            "3:\n" \
+            "b .\n" \


rizlik · 2026-01-12T11:50:53Z

include/image.h

+            "beq 4f\n" \
+            "bkpt 0xE1\n" \
+            "4:\n" \
+            "cmp r4, #0\n" \


do you think cmp cmp cmp cmp beq beq beq beq is a more robust pattern?

rizlik · 2026-01-12T12:00:43Z

src/update_flash.c

 #endif
    int fallback_image = 0;
+#ifndef DISABLE_BACKUP
+    int rollback = 0;


NIT: do you think it make sense to call it fallback or in_fallback to match fallback_allowed?

rizlik · 2026-01-12T12:03:05Z

src/update_flash.c

+        {
+            uint32_t fb_ok = (fallback_allowed == 1);
+            VERIFY_VERSION_ALLOWED(fb_ok);
+            (void)fb_ok;


can you elaborate the reason of the fb_ok aliasing here?

rizlik · 2026-01-12T12:05:52Z

src/update_flash.c

+            if (updateRet != 0) {
+                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();
+                updateRet = 0;
+                updateState = IMG_STATE_UPDATING;
+            }


what's the reason of this?

danielinux added 5 commits January 12, 2026 09:44

Fix handling rollback when ARMORED=1 and FLASH_TOP

08692b1

[ARMORED] Improved redundancy of verification

50d8aed

Silencing compiler warning

12f99a8

Update size limits

22d2024

Adjusted size limits for gcc 13.2 (CI)

7efb613

rizlik requested a review from Copilot January 12, 2026 11:18

Copilot started reviewing on behalf of rizlik January 12, 2026 11:19 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

Update src/image.c: remove duplicated code

d4e66e5

Co-authored-by: Copilot <[email protected]>

danielinux requested a review from rizlik January 12, 2026 11:42

rizlik requested changes Jan 12, 2026

View reviewed changes

Improve ARMORED code + minor fixes #665

Are you sure you want to change the base?

Improve ARMORED code + minor fixes #665

Uh oh!

Conversation

danielinux commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danielinux commented Jan 12, 2026 •

edited

Loading