boot: bootutil: skip redundant primary validation on boot with MCUBOOT_BOOTSTRAP - #2782
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes MCUBOOT_BOOTSTRAP boot-time behavior by avoiding an expensive primary-slot boot_validate_slot() call on every normal boot when no swap is pending, while keeping the stronger validation behavior for overwrite-only mode where an interrupted copy can leave a misleading header.
Changes:
- Split the
MCUBOOT_BOOTSTRAP/BOOT_SWAP_TYPE_NONEpath to keep full primary validation forMCUBOOT_OVERWRITE_ONLY. - For swap-based modes, gate bootstrap on a cheap primary-header check instead of always validating the primary slot’s signature.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * still present (e.g. the swapped-out image after an update), | ||
| * that verify can dominate boot time. | ||
| */ | ||
| if (boot_check_header_erased(state, BOOT_SLOT_PRIMARY)) |
There was a problem hiding this comment.
Good catch — adopted. The swap-mode gate now uses !boot_check_header_valid() instead of boot_check_header_erased(), so a primary that is erased or has a corrupt header is bootstrapped from the secondary, restoring the recovery behavior the original boot_validate_slot() failure path provided for a present-but-invalid primary. The secondary is still fully validated before any copy, so this recovers a corrupt primary only from a known-good image, and it keeps the performance win — no primary-slot signature verify on a normal boot with a structurally valid image.
The MCUBOOT_OVERWRITE_ONLY branch keeps the full boot_validate_slot(): there a valid header can sit over a truncated body from an interrupted offset-0 copy, which the cheap header check cannot detect.
Updated in 100af01 (commit message adjusted to match).
…T_BOOTSTRAP With MCUBOOT_BOOTSTRAP, boot_prepare_image_for_update() runs a full boot_validate_slot() on the primary slot on every boot (when the swap type is NONE) to decide whether to bootstrap an image from the secondary slot. For a signed image that is a public-key signature verification, and it runs on every normal boot even when the primary already holds a valid, confirmed image, and even when MCUBOOT_VALIDATE_PRIMARY_SLOT is disabled. On a Cortex-M33 with ECDSA-P384/SHA-384 and FIH_PROFILE_HIGH this added ~14 s to every boot. The full validation guards against a primary that is present but invalid. For MCUBOOT_OVERWRITE_ONLY detecting that requires a signature verify: overwrite-only copies start at offset 0, so an interrupted copy can leave a valid header magic over an incomplete image body, which only full validation catches. Swap-based modes do not need a signature verify here: this block runs only in the no-partial-swap branch (boot_status_is_reset()), after boot_complete_partial_swap() has finished any interrupted swap, so a primary with a structurally valid header holds a good image. Keep the full validation for MCUBOOT_OVERWRITE_ONLY. For swap-based modes gate bootstrap on the cheap boot_check_header_valid() check instead: a primary that is erased or whose header is corrupt is (re-)bootstrapped from the secondary -- which is still validated before any copy -- while a structurally valid primary boots as-is. This keeps the original recovery behavior for a missing or corrupt primary and drops only the redundant per-boot public-key verification of a valid one. The per-mode #ifdef keeps the FIH_NOT_EQ() comparison inline for overwrite-only rather than laundering a fault-injection result through a bool. Signed-off-by: Jay Beavers <jay@tolttechnologies.com>
be26b3b to
100af01
Compare
…CUBOOT_BOOTSTRAP Widen the MCUBOOT_BOOTSTRAP swap-mode bootstrap gate from boot_check_header_erased() to !boot_check_header_valid(), so a primary that is erased OR has a corrupt header is (re-)bootstrapped from the secondary -- which is still validated before any copy -- while a structurally valid primary boots as-is with no signature verify. This restores the recovery behavior the original boot_validate_slot() failure path provided for a present-but-invalid primary, at the same near-zero cost, keeping the boot-time win. Reconciles the downstream fix with upstream mcuboot PR mcu-tools#2782, which adopted this behavior after Copilot review. Also bump boot/zephyr/VERSION to 26.7.5. Signed-off-by: Jay Beavers <jay@tolttechnologies.com>
Don't believe that to be true, bootstrap has multiple uses and one of them is flash corruption of the main image which can happen irrespective of what mode is used |
Sure, there are scenarios for this. However if the mcuboot consumer doesn't want per boot validation to detect a theoretical in place corruption, they can turn it off using MCUBOOT_VALIDATE_PRIMARY_SLOT = N. However it still runs when this is set. As I was working through my mcuboot, I was seeing up to three image validations in a row in some circumstances. This fix helps reduce unwanted/unconfigured extra validations when it is turned off. |
d3zd3z
left a comment
There was a problem hiding this comment.
I'll wait for any feedback you might have about the suggestions from Claude.
| * offset 0, if interrupted, it might leave a valid header | ||
| * magic, so also run validation on the primary slot to be | ||
| * sure it's not OK. | ||
| #ifdef MCUBOOT_OVERWRITE_ONLY |
There was a problem hiding this comment.
Claude has this suggestion. Although it kind of ends up suggesting not actually fixing what is the point of this change. There is some in here about a case this seem to break, though.
The performance problem here is real and the diagnosis is convincing — a per-boot
ECDSA-P384 verify under FIH_PROFILE_HIGH is a serious cost, and it genuinely is
gratuitous when the primary already holds a good confirmed image.
My hesitation is with the discriminator. Splitting on MCUBOOT_OVERWRITE_ONLY is
splitting on how a bad primary could have gotten there, and I don't think that
argument closes. I'd suggest splitting on MCUBOOT_VALIDATE_PRIMARY_SLOT instead —
i.e. on whether anything downstream will catch a bad primary.
Why the current discriminator doesn't quite hold
boot_check_header_valid() (bootutil_loader.c:69) is purely structural — ih_magic,
size arithmetic against the slot size, encryption/compression flag sanity. No hash, no
signature, no security counter. So the states the new swap arm stops catching are:
valid header + bad signature, valid header + corrupted image body, and valid header +
insufficient rollback counter.
The commit message argues those can't occur in swap modes, because an interrupted swap
would already have been resumed by boot_complete_partial_swap() in the sibling
!boot_status_is_reset() branch. I traced that and I agree as far as it goes — in
swap_move.c:353, swap_status_init() writes the primary trailer magic before erasing
the first primary data sector, so an interruption leaves recoverable state.
But that reasoning only rules out the interrupted swap route into a bad primary. It
says nothing about the others, none of which involve a swap and all of which leave
boot_status_is_reset() true:
- a primary flashed by external tooling or JTAG, signed with the wrong key or truncated
- flash bit-rot in the image body, with the header sector intact
- a security counter below the stored value, under
MCUBOOT_HW_ROLLBACK_PROT
In every one of those cases the old code would have re-bootstrapped from the validated
secondary. The new code boots (or refuses to boot) the bad primary.
What actually happens downstream, per config
This is where I think the interesting decision lives, because the two configs behave
very differently after this patch:
MCUBOOT_VALIDATE_PRIMARY_SLOT enabled. loader.c:1925 fully validates the primary
before the jump regardless, so there is no security weakening at all. What's lost is
autonomous recovery: the device now refuses to boot where it previously self-healed from
the secondary. Also worth noting — in this config the primary is validated twice per
boot today, once at 1561 and once at 1925. That double verify is what the commit subject
describes as redundant, though as I argue below I don't think the second one can safely be
removed.
MCUBOOT_VALIDATE_PRIMARY_SLOT disabled. loader.c:1942 only checks ih_magic. The
bootstrap-time validate was the only signature verification of the primary on a normal
boot. After this patch a structurally-valid-but-tampered primary boots unverified.
In fairness, that second case leaves bootstrap builds behaving the same as a plain
non-bootstrap no-validate build, which already trusts the primary — so it's a reduction
in that config's posture rather than a hole beyond mcuboot's existing threat model. But
it's a real change and it should be a deliberate choice rather than a side effect.
The suggestion
Given the ~15 s → sub-second number, I'm guessing your build has
MCUBOOT_VALIDATE_PRIMARY_SLOT off — if it were on you'd still be paying one full
P-384 verify at loader.c:1925 and would have landed around 14 s, not sub-second. Can
you confirm? If so, the config you actually need the fix for is exactly the one where the
current patch is doing the most.
That points at inverting the split:
#if defined(MCUBOOT_OVERWRITE_ONLY) || defined(MCUBOOT_VALIDATE_PRIMARY_SLOT)
/* Full validate. Overwrite-only needs it because an interrupted copy can
* leave a valid header over an incomplete image. Validate-primary-slot
* builds need it because this is what lets a bad primary be recovered from
* the secondary rather than merely rejected at 1925.
*/
FIH_CALL(boot_validate_slot, fih_rc, state, BOOT_SLOT_PRIMARY, bs, 0);
if (boot_check_header_erased(state, BOOT_SLOT_PRIMARY) ||
FIH_NOT_EQ(fih_rc, FIH_SUCCESS))
#else
/* No downstream verify of the primary exists in this configuration, so a
* signature check here would be verifying an image the build has already
* declared it does not verify. Structural check is sufficient to decide
* whether to bootstrap.
*/
if (!boot_check_header_valid(state, BOOT_SLOT_PRIMARY))
#endifThis gives you the full win in your configuration, and it's principled rather than
incidental: a build that has said "don't verify the primary on boot" shouldn't be doing a
public-key verify of the primary on boot. Meanwhile VALIDATE_PRIMARY_SLOT builds keep
both their signature check and their recovery behavior, and overwrite-only is untouched.
The remaining redundancy — the double verify in VALIDATE_PRIMARY_SLOT builds — looks
tempting to fix by caching the fih_rc from 1561 and reusing it at 1925, but I don't
think that survives scrutiny, so I'd leave it alone:
- The primary's contents are not stable between the two call sites. The update loop and
boot_perform_update()sit in between, and 1904 explicitly reloads the headers when
BOOT_SWAP_TYPE != NONEbecause the slot was just rewritten. Worse, the bootstrap block
at 1590 can itself setBOOT_SWAP_TYPE_REVERT, so the very path that would populate the
cache is a path that then overwrites the primary. Reusing the cached value there means
validating the old image and booting the new one. It would only be sound under a guard of
"swap type isNONEfor this image and no update ran" — which is the fast path, so it
would still pay off, but it's a conditional reuse, not a free one. - Independently: the comment at 1927 says the all-possible-values re-check is a deliberate
FI countermeasure, and 1925 is the last gate before jumping to the image. Caching leaves
afih_retsitting in RAM across a long window that includes flash writes, and then has
the final gate trust it. That's a materially better glitch target than a verify performed
immediately before the jump — especially underFIH_PROFILE_HIGH.
So the double verify in VALIDATE_PRIMARY_SLOT builds is probably just the price of the FI
hardening, and not something to optimize away. That doesn't affect the suggestion above,
which doesn't depend on caching at all.
Smaller points
- The PR body says the swap arm gates on
boot_check_header_erased(), but the code uses
boot_check_header_valid(). They differ — the latter also checks sizes and flags. Worth
fixing before merge since the commit message is the permanent record of the rationale. - Similarly, "the primary here is settled — a valid image or an erased slot" reads as an
unconditional invariant. It holds for interrupted-swap resumption specifically; I'd
narrow the wording.
On the testing question
You asked which sim configs to cover. Worth knowing that the existing bootstrap sim tests
won't tell you anything here: make_bootstrap_image() (sim/src/image.rs:426) installs the
primary with install_no_image(), i.e. an erased primary — exactly the case your patch
still handles. And bootstrap doesn't appear in the CI feature matrix at all, so I don't
think those tests are running in CI. The green checks on this PR aren't evidence either way.
The test that would actually exercise the change is a bootstrap × swap case that installs
a structurally valid but signature-invalid primary alongside a valid secondary. Under the
old code that recovers; under this patch it doesn't. That single test would settle the
design question empirically — and adding bootstrap to the CI matrix would keep it settled.
nordicjm
left a comment
There was a problem hiding this comment.
breaks bootstrap, if you build zephyr's smp_svr and load it with bootstrap enabled, then load the same image as a firmware update on nrf5340 and do nrfjprog -f nrf53 --erasepage 0x14000 && nrfjprog -f nrf53 --reset then you get this:
kernel reboot cold
kernel reboot cold�*** Using Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
I: Starting bootloader
I: Image index: 0, Swap type: none
I: Primary image: magic=good, swap_type=0x1, copy_done=0x3, image_ok=0x1
I: Secondary image: magic=unset, swap_type=0x1, copy_done=0x3, image_ok=0x3
I: Boot source: primary slot
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Bootloader chainload address offset: 0x10000
I: Image version: v0.0.0
I: Jumping to the first image slot
uart:~$ *** Booting Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
uart:~$ [00:00:00.001,922] <inf> smp_sample: build time: Jul 15 2026 07:27:39
uart:~$ uart:~$ *** Using Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
I: Starting bootloader
I: Image index: 0, Swap type: none
I: Primary image: magic=good, swap_type=0x1, copy_done=0x3, image_ok=0x1
I: Secondary image: magic=unset, swap_type=0x1, copy_done=0x3, image_ok=0x3
I: Boot source: primary slot
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
E: Image in the primary slot is not valid!
E: Image in the primary slot is not valid!
I: Image 0 upgrade secondary slot -> primary slot
I: Erasing the primary slot
I: Image 0 copying the secondary slot to the primary slot: 0x16db4 bytes
I: Image index: 0, Swap type: none
I: Bootloader chainload address offset: 0x10000
I: Image version: v0.0.0
I: Jumping to the first image slot
uart:~$ *** Booting Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
uart:~$ [00:00:00.001,922] <inf> smp_sample: build time: Jul 15 2026 07:27:39
if you apply this and do the same process you get this:
*** Using Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
I: Starting bootloader
I: Image index: 0, Swap type: none
I: Primary image: magic=good, swap_type=0x1, copy_done=0x3, image_ok=0x1
I: Secondary image: magic=unset, swap_type=0x1, copy_done=0x3, image_ok=0x3
I: Boot source: primary slot
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Bootloader chainload address offset: 0x10000
I: Image version: v0.0.0
I: Jumping to the first image slot
uart:~$ *** Booting Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
uart:~$ [00:00:00.001,922] <inf> smp_sample: build time: Jul 15 2026 07:27:39
uart:~$ uart:~$ *** Using Zephyr OS build v4.4.0-8295-g0bd807ceeb75 ***
I: Starting bootloader
I: Image index: 0, Swap type: none
I: Primary image: magic=good, swap_type=0x1, copy_done=0x3, image_ok=0x1
I: Secondary image: magic=unset, swap_type=0x1, copy_done=0x3, image_ok=0x3
I: Boot source: primary slot
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
I: Image index: 0, Swap type: none
E: Image in the primary slot is not valid!
E: Unable to find bootable image
essentially breaking the whole bootstrap feature
|
@nordicjm Can you give me a bit more detail on the combination of flags you are using to build your failing case with? I'm trying to deal with the specific combination of: CONFIG_BOOT_BOOTSTRAP=y which in my case I am setting due to the use of a costlier crypto hash and the lack of a crypto accelerator. Unfortunately, these various interdependent configuration flags lead to a substantial set of permutations. Give me an idea of which combination you are using that breaks so I can better analyze the failure and see if there is a change that can be made that will allow the above combiation to work as well. |
|
it would be the default nrf5340 configuration then enabling bootstrap, so: |
With MCUBOOT_BOOTSTRAP, boot_prepare_image_for_update() runs a full
boot_validate_slot() on the primary slot on every boot (when the swap type is
NONE) to decide whether to bootstrap an image from the secondary slot. For a
signed image that is a public-key signature verification, and it runs on every
normal boot even when the primary already holds a valid, confirmed image, and
even when MCUBOOT_VALIDATE_PRIMARY_SLOT is disabled. On a Cortex-M33 with
ECDSA-P384/SHA-384 and FIH_PROFILE_HIGH this added ~14 s to every boot.
The validation guards against a primary that is present but invalid. That can
only arise here for MCUBOOT_OVERWRITE_ONLY: overwrite-only copies start at
offset 0, so an interrupted copy can leave a valid header magic over an
incomplete image. Swap-based modes cannot reach this point in that state: this
block runs only in the no-partial-swap branch (boot_status_is_reset()), and any
interrupted swap has already been finished by boot_complete_partial_swap() in
the sibling branch, so the primary here is settled -- a valid image or an
erased slot.
Keep the full validation for MCUBOOT_OVERWRITE_ONLY. For swap-based modes gate
bootstrap on the cheap boot_check_header_erased() check instead: a present
primary is bootable and needs no re-validation here, and an erased primary is
still bootstrapped from a validated secondary as before. The per-mode #ifdef
keeps the FIH_NOT_EQ() comparison inline rather than laundering a
fault-injection result through a bool.
Fixes #2781
Testing
MCUBOOT_OVERWRITE_ONLYpath is unchanged.