Skip to content

Fenrir fixes 2026-08 -11 + build regressions fixes - #851

Merged
dgarske merged 25 commits into
wolfSSL:masterfrom
danielinux:fenrir-fixes-2026-08-11
Aug 12, 2026
Merged

Fenrir fixes 2026-08 -11 + build regressions fixes#851
dgarske merged 25 commits into
wolfSSL:masterfrom
danielinux:fenrir-fixes-2026-08-11

Conversation

@danielinux

@danielinux danielinux commented Aug 11, 2026

Copy link
Copy Markdown
Member

cfaf145 libwolfboot: propagate the head-block write failure
fca6bf0 Address PR review on the encrypted write and sector copy paths
31a9adc test: bump footprint limits
e24fff0 libwolfboot: declare ForceZero() in the test-app build
db8e768 aarch64: link the ARM ChaCha port when ChaCha is selected
4726dce update_disk: use wc_ForceZero() in the DISK_ENCRYPT helpers
f9fe138 F-6130: clear disk_encrypt_key/nonce on the FIT DTS load failure path
f755125 F-6757: fix partial-word hal_flash_write on nrf52, nrf5340 and stm32l0
40021b8 F-7069: clear the EH authValue from the stack in wolfBoot_tpm2_get_timestamp
a799a98 F-7382: clip QSPI page program transfers at the device page boundary
ab7b79c F-7383: use one consistent sector size in mcxw hal_flash_erase
bc743ad F-7987: abort the swap when a sector copy fails
f446a4a F-8007: wipe TPM advanced-IO staging buffers in TPM2_IoCb()
93edc29 F-7985: enter legacy uImage at ih_ep when it differs from ih_load
5dfdec3 F-7992: bound staged ciphertext in ext_flash_encrypt_write()
f9957da F-8003: separate decoded key objects for hybrid signers
748fa8a F-8006: return errors from delta base-hash validation in sign tool
e200579 F-7969: validate boot-side digest before delta base hash compare
7608e33 F-7988: clamp unaligned head size in ext_flash_encrypt_write()
e6655e6 F-7989: clamp unaligned head size in ext_flash_decrypt_read()

When a read starts in the middle of an encryption block, the head copy
size was computed as ENCRYPT_BLOCK_SIZE - row_offset without regard for
the requested length. A read shorter than the remainder of the block
(e.g. 1 byte at offset 1) copied up to 15 decrypted bytes into a buffer
sized for fewer, and left read_remaining negative, so the subsequent
flash_read_size = read_remaining & ~(ENCRYPT_BLOCK_SIZE - 1) was passed
to ext_flash_read() as a negative length.

Clamp the head size to the bytes actually requested.

Add a unit test covering short unaligned reads (offsets 1/4/8/15) that
checks the return value, the decrypted contents and that no byte past
the requested length is written; the ext_flash_read() mock now also
rejects negative lengths.
When a write starts in the middle of an encryption block, or is shorter
than a full block, the head copy size was computed as
ENCRYPT_BLOCK_SIZE - row_offset without regard for the requested length.
A write shorter than the remainder of the block (e.g. 1 byte at offset 1)
copied up to ENCRYPT_BLOCK_SIZE-1 bytes out of the caller's buffer into
the read-modify-write block, and left sz negative, so the subsequent
step = sz & ~(ENCRYPT_BLOCK_SIZE - 1) was passed to ext_flash_write() as
a negative length. This is reachable from wb_flash_write_verify_word()
(4-byte writes), from wolfBoot_nsc_write_update() and from the delta
patch writer.

Clamp the head size to the bytes actually requested, and return the
result of the head block write when the request fits within that block.

Add a unit test covering short unaligned writes (offsets 1/0/8/blk-1)
that checks the return value, the data read back and that the bytes past
the requested length were not taken from the caller's buffer; the
ext_flash_write() mock now also rejects negative lengths.
wolfBoot_delta_update() compared the boot partition digest against the
delta base hash using base_hash_sz, the length returned by
wolfBoot_find_header() for the boot header's hash TLV, without ever
checking it. When the tag is absent, find_header() sets base_hash to
NULL and returns 0, so wolfBoot_hardened_CT_compare(NULL, ..., 0)
compared zero bytes and reported a match: the base image digest gate
silently succeeded instead of rejecting the patch. A short or oversized
TLV length would likewise truncate the comparison or read past the
delta base hash in the update header.

The gate is reachable because wolfBoot_update() runs before the boot
partition is verified, so the boot header contents are not guaranteed
to carry a well-formed digest TLV at that point.

Reject the patch when the base image has no usable digest, and compare
a fixed WOLFBOOT_SHA_DIGEST_SIZE. The inverse and resume paths are
unaffected, as they do not use this gate.

Add unit-update-flash-delta coverage for a boot header without a
digest TLV.
make_header_ex() validated the delta base image digest with direct exit(1)
calls. Those are reachable in normal use: base_diff() looks up the base
digest for the selected hash algorithm, and when the base image was signed
with a different algorithm the lookup yields NULL, yet make_header_delta()
is still called. Aborting there skips base_diff()'s cleanup (the temporary
patch file is left in /tmp) and, more importantly, main()'s
zero_and_free(kbuf, key_buffer_sz) and algorithm-specific key free, so the
raw and decoded private signing key are never scrubbed.

Use the function's existing 'failure:' path instead, which returns -1 and
propagates through base_diff() to main()'s unified cleanup.

Reaching 'failure:' from there uncovered a latent double fclose(): the
image-size probe closes 'f' without clearing it, so the cleanup block
closed the same stream again. Clear the pointer after the fclose().

Add unit-sign-delta-basehash-cleanup.py, which signs a SHA256 base image,
requests a SHA384 delta against it, and asserts the run fails with the
temporary patch file removed.
The sign tool kept a single file-static struct for the decoded private
key, so a hybrid run that picks two algorithms sharing one member (e.g.
ECC521 primary + ECC256 secondary, or RSA2048 + RSAPSS2048) had the
secondary load_key() re-init and overwrite the still-live primary key
before either signature was produced. The primary signature was then
made with the secondary key, and the final cleanup in main() dispatched
only on CMD.sign, so the secondary key never reached its algorithm
specific zeroizing free.

Give the primary and the secondary signer their own storage, select it
with key_obj(secondary) in load_key()/load_key_ecc()/load_key_rsa()/
sign_digest()/set_signature_sizes(), and free both keys at exit through
the new free_key() helper.
ext_flash_encrypt_write() encrypted the whole caller-supplied buffer into
ENCRYPT_CACHE, which is only NVM_CACHE_SIZE bytes, without any check that
the request fits. A request longer than the cache (reachable from the
non-secure world through wolfBoot_nsc_write_update(), which only bounds
len against the partition size) overran the staging buffer and made
ext_flash_write() read past its end.

Stage and flush the ciphertext in NVM_CACHE_SIZE chunks instead. The
encryption stream is not restarted between chunks, so the resulting flash
content is unchanged for requests that already fitted.
wolfBoot_start() parsed both ih_load and ih_ep from the U-Boot legacy
uImage header, relocated the payload to ih_load, then discarded ih_ep and
passed the load address to do_boot(). An image built with the entry point
ahead of the load address (a preamble before the entry, as U-Boot bootm
handles by copying to ih_load and jumping to ih_ep) was staged correctly
but entered at the wrong address.

Keep ih_load as the relocation destination and remember ih_ep as the
entry point when the two differ, then override load_address just before
do_boot(). The override is skipped when a later stage (ELF/FIT) re-derived
the load address, since that stage supplies its own entry point.

Extend unit-update-ram-uboot with a case where ih_ep = ih_load + 0x40:
it asserts the payload lands at ih_load and do_boot() is entered at
ih_ep. Fails before this change (jumps to ih_load).
With WOLFTPM_ADV_IO the TIS layer hands the raw command payload to the
HAL callback, so TPM2_IoCb() stages it in stack-local txBuf/rxBuf.  Both
were left intact on the normal return and on the wait-state error return,
keeping a TPM command's plaintext authValue (and the response bytes) in
bootloader stack SRAM.  Wipe them like TPM2_TIS_Read()/TPM2_TIS_Write()
already do for their own staging buffers in the non-advanced-IO path.

Adds unit-tpm-advio-zeroize, which drives TPM2_IoCb() through the write,
read, payload-error and wait-state-timeout paths with a mock SPI slave
and inspects the staging buffers afterwards.
wolfBoot_copy_sector() discarded the return value of every flash
operation it performed and unconditionally returned the number of bytes
processed. Callers therefore treated a partially written sector as a
completed one and advanced the persistent sector flags, which are the
only record used to resume an interrupted swap. A write error while
copying BOOT into UPDATE (the backup step) could leave both the running
image and its backup corrupted with no way to redo the sector.

Check the result of every erase/read/write in wolfBoot_copy_sector() and
return -1 on the first failure. In the interruptible swap loop, the
delta loop and the DISABLE_BACKUP direct copy, stop on a negative return
without advancing the sector flag or confirming the boot partition, so
the swap is retried from the last completed step on the next boot.
hal_flash_erase() in hal/mcxw.c rounded the start address down with the
runtime pflash_sector_size (queried from FLASH_GetProperty() in hal_init())
but stepped address and len by the compile-time WOLFBOOT_SECTOR_SIZE. When
the two differ, a larger WOLFBOOT_SECTOR_SIZE steps over hardware sectors
inside the requested range and leaves them unerased, while a smaller one
issues erase commands at non-sector-aligned addresses. A zero size reported
by the driver would also divide by zero.

Take a local sector_size, fall back to WOLFBOOT_SECTOR_SIZE when the driver
reports zero and use it for the alignment and both loop steps, as
hal/mcxn.c already does.

Add unit-flash-erase-mcxw, using the existing WOLFBOOT_UNIT_TEST_FLASH_ERASE
guard convention to compile hal_flash_erase() in isolation without the NXP
MCUXpresso SDK headers.
spi_flash_write() chunked purely by length, issuing up to a full
FLASH_PAGE_SIZE page program at address + page*FLASH_PAGE_SIZE. NOR
flash page program wraps within the device's own page, so a transfer
starting mid-page (e.g. 0x10F0 with 256 bytes) programmed the tail of
the page and then wrapped the rest back over the start of the same
page, corrupting already-programmed data and leaving the intended
range unwritten.

Drive the loop from the running address and clip each transfer to the
bytes remaining in the current page, matching src/spi_flash.c.
…mestamp

wolfBoot_tpm2_get_timestamp() derives (or copies) the endorsement-hierarchy
authValue into the stack-local eh_handle before issuing TPM2_GetTime. The
wolfTPM2_UnsetAuth() calls on the way out only clear the copies wolfTPM keeps
in the device session slots, and the existing TPM2_ForceZero() only clears the
reel master secret, so the derived per-device authValue was left resident in
the Secure stack frame after the non-secure entry veneer returned.

Wipe eh_handle before returning, matching the scrubbing already done for the
master secret. Add unit-tpm-mfgid-eh-zeroize, which captures the handle passed
to wolfTPM2_SetIdentityAuth() and snapshots the dead frame on both the success
and TPM2_GetTime-error paths.
The byte-wise branch of hal_flash_write() derived the containing word from
the call-time "address" instead of the current position "address + i":

    int off = (address + i) - (((address + i) >> 2) << 2);
    dst = (uint32_t *)(address - off);
    val = dst[i >> 2];

so "dst[i >> 2]" addressed physical byte "address - off + (i & ~3)". Any
iteration with "i" not a multiple of 4 modified the wrong byte, and with
off != 0 it did so through a misaligned 32-bit flash access (a HardFault on
the Cortex-M0+ of stm32l0). A word-aligned 6-byte write, for instance, put
data[5] at "address + 4" and left "address + 5" erased.

Use the form already applied to hal/samr21.c and hal/same51.c: base the
word on "address + i - off", read it with a single aligned access, and fill
it byte by byte up to the next word boundary.

Add unit-flash-write-nrf52, covering the aligned-with-tail, mismatched
alignment and single-word cases against hal/nrf52.c.
dd0712e added the disk_decrypted_header_clear()/disk_crypto_clear() pair
to the wolfBoot_start() panic paths that were missing it, but the FIT
flat-device-tree load failure was one more: when wolfBoot_fit_memcpy()
fails to relocate the DTS, wolfBoot_panic() is entered with
disk_encrypt_key/disk_encrypt_nonce still live in BSS, and that call never
returns on a real target.

Add unit-update-disk-fit, which drives wolfBoot_start() through the FIT
branch with DISK_ENCRYPT enabled and snapshots the module statics from the
WOLFBOOT_HOOK_PANIC hook.
ForceZero() is only visible in libwolfboot.c, which pulls in misc.c
inline; update_disk.c called it without a declaration, so any config with
disk encryption failed to build. Use the exported wc_ForceZero() from
memory.o instead, which is always linked.

Add a zynqmp_sdcard ENCRYPT build job to CI, the only one that compiles
these paths.
Under WOLFSSL_ARMASM, chacha.c calls wc_chacha_crypt_bytes(), which
arch.mk never adds for AArch64 -- it only pulls in the aes/sha ports.
Any AArch64 build using ChaCha failed to link. Add the object in
options.mk, where ChaCha is selected.

Also add a ChaCha variant of the zynqmp_sdcard ENCRYPT build to CI.
Copilot AI lite review requested due to automatic review settings August 11, 2026 15:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR bundles a set of boot/update security and correctness regressions fixes across wolfBoot’s update flows (RAM/flash/disk), TPM handling, signing tooling, and related HAL drivers, and backs them with expanded unit-test coverage and CI build matrix tweaks.

Changes:

  • Fix/extend boot and update correctness (uImage ih_ep handling in RAM boot, abort swap on sector copy failures, QSPI page-boundary clipping, MCXW erase stride consistency, partial-word flash-write fixes).
  • Harden key/material handling (disk decrypt key/nonce zeroization on FIT failure paths, TPM stack buffer/handle wiping, signing tool cleanup paths & hybrid key separation).
  • Add regression tests and CI coverage for the above behaviors (new unit tests + Makefile/workflow updates).

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/unit-tests/unit-update-ram-uboot.c Adds uImage ih_ep≠ih_load test coverage.
tools/unit-tests/unit-update-flash.c Adds tests for aborting swap on copy failure and delta base-hash rejection when boot lacks digest TLV.
tools/unit-tests/unit-update-disk.c Updates test stub to wc_ForceZero naming.
tools/unit-tests/unit-update-disk-fit.c New tests ensuring disk decrypt key/nonce scrub on FIT DTS load failure paths.
tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c New regression test ensuring EH authValue doesn’t persist on stack.
tools/unit-tests/unit-tpm-advio-zeroize.c New regression test ensuring TPM ADV_IO staging buffers are wiped on all exits.
tools/unit-tests/unit-sign-hybrid-keyload.c Adds test ensuring secondary hybrid key load doesn’t clobber primary decoded key.
tools/unit-tests/unit-sign-delta-basehash-cleanup.py New integration-style test ensuring sign tool unwinds/cleans up on base-hash validation failure.
tools/unit-tests/unit-qspi-flash.c Adds test for clipping first page program at boundary.
tools/unit-tests/unit-flash-write-nrf52.c New regression tests for partial-word flash writes (nrf52).
tools/unit-tests/unit-flash-erase-mcxw.c New unit tests for consistent sector-size stride in mcxw erase.
tools/unit-tests/unit-extflash.c Adds negative-length assertions and new unaligned/oversized encrypted extflash read/write tests.
tools/unit-tests/Makefile Wires new unit tests and python test into build/run targets.
tools/keytools/sign.c Separates primary/secondary decoded key storage; ensures base-hash failures unwind; centralizes key free/zeroize paths.
src/update_ram.c Enters legacy uImage at ih_ep when it differs from ih_load (when applicable).
src/update_flash.c Propagates flash read/write/erase failures up to abort swaps and avoid advancing sector flags incorrectly.
src/update_disk.c Uses wc_ForceZero and scrubs disk key/nonce on FIT DTS load failure before panic.
src/tpm.c Wipes ADV_IO staging buffers and clears derived EH authValue from stack on return.
src/qspi_flash.c Clips page program transfers at device page boundaries.
src/libwolfboot.c Bounds/stages encrypted extflash writes and clamps unaligned head copies; clamps unaligned decrypt head size.
options.mk Links AArch64 ARM ChaCha port object when ChaCha is selected.
hal/stm32l0.c Fixes partial-word flash write addressing/word assembly.
hal/nrf5340.c Fixes partial-word flash write addressing/word assembly.
hal/nrf52.c Fixes partial-word flash write addressing/word assembly.
hal/mcxw.c Uses a consistent runtime sector size (with zero fallback) for erase alignment/stride.
.gitignore Ignores new unit-test binaries.
.github/workflows/test-configs.yml Adds encrypted disk-loader build coverage (AES/ChaCha) for zynqmp_sdcard config.
Suppressed comments (1)

src/update_flash.c:336

  • ext_flash_check_read() (ext_flash_read/ext_flash_decrypt_read) also returns a byte count on success. Only checking for <0 can accept short reads and continue the swap with incomplete data. Require the requested size here so a partial read aborts the sector copy.
                  if (ext_flash_check_read((uintptr_t)(src->hdr) +
                                         src_sector_offset + pos,
                                     (void *)buffer, FLASHBUFFER_SIZE) < 0) {
                      ret = -1;
                      goto out;
                  }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/libwolfboot.c
Comment thread src/libwolfboot.c
Comment thread src/update_flash.c
The encrypted key handling calls ForceZero(), but misc.c is only included
inline under __WOLFBOOT or UNIT_TEST. The test-app build of libwolfboot.c
defines neither, so on MMU targets with EXT_ENCRYPTED the call had no
declaration. GCC 14 rejects that; older compilers only warned.

Add EXT_ENCRYPTED to the guard, which keeps ForceZero() static and adds no
link dependency.
The error handling added by the fixes in this branch costs 40 bytes of
common code, so every stm32f407-discovery configuration grew by that
amount. Raise each limit by 40, keeping the previous headroom.
ext_flash_encrypt_write() writes whole ENCRYPT_BLOCK_SIZE blocks:

  - A length that is not a multiple of the block size dropped the trailing
    bytes, since the remainder loop rounds down. Merge them into the block
    that already backs them, as the unaligned head is handled.
  - len == 0 fell through to a read-modify-write of the containing block,
    re-encrypting it in place. Return early instead.

wolfBoot_copy_sector() checked the reads added in F-7987 for a negative
return, but ext_flash_read() and ext_flash_check_read() return the number
of bytes read (docs/HAL.md), so a short read was accepted and a partially
filled buffer copied on. Require the full FLASHBUFFER_SIZE.

Add unit-update-flash-enc coverage for the two write cases.
@danielinux
danielinux force-pushed the fenrir-fixes-2026-08-11 branch from 42a428b to fca6bf0 Compare August 11, 2026 16:46
@danielinux danielinux assigned danielinux and dgarske and unassigned danielinux Aug 11, 2026

@dgarske dgarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skoll Code Review

Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 19 total — 12 posted, 7 skipped
10 finding(s) posted as inline comments (see file-level comments below)
2 finding(s) not tied to a diff line (full detail below)

Posted findings

  • [High] ext_flash_encrypt_write() discards the head-block write failure, defeating the F-7987 swap-abort it is paired withsrc/libwolfboot.c:2638-2648
  • [Medium] *The load_address == uboot_load identity guard is defeated by elf_load_image_mmu() publishing pentry before it validatessrc/update_ram.c:665-670
  • [Medium] disk_crypto_clear() leaves the expanded key schedule and nonce live, so the new scrub does not actually destroy the keysrc/update_disk.c:237-241
  • [Medium] main() still exit(1)s on secondary-key load failure, skipping the very key scrubbing F-8006 restorestools/keytools/sign.c:3825-3828
  • [Medium] nrf52/nrf5340/stm32l0: the sibling 32-bit fast path still indexes off the original address and is left broken by the F-6757 fixhal/nrf52.c:75-82
  • [Low] ext_flash_encrypt_write() chunk size is not rounded down to an ENCRYPT_BLOCK_SIZE multiplesrc/libwolfboot.c:2650-2665
  • [Low] mcxw hal_flash_erase() does not extend len by the round-down amount, leaving the last sector unerasedhal/mcxw.c:232-247
  • [Low] ih_ep is still discarded when ih_load == 0, and the TODO that documented that gap was removedsrc/update_ram.c:513-527
  • [Low] free_key() can run on a never-initialized key2 objecttools/keytools/sign.c:3859-3862
  • [Low] Two new unit-test binaries are missing from .gitignore.gitignore:207-227

Findings not tied to a diff line

F-8006's double-fclose fix is incomplete: three more fclose(f) sites in make_header_ex still leave f dangling

File: tools/keytools/sign.c:1779,1855,1929
Function: make_header_ex
Severity: Medium

The f = NULL; added at line 1515 is correct and necessary - without it the four new goto failure sites in the delta base-hash block would double-fclose at the cleanup label. But the identical pattern remains at three other sites in the same function: line 1779 (SHA256 image-hash loop), line 1855 (SHA384) and line 1929 (SHA3-384) all call fclose(f) without clearing f. Between those points and the next reassignment of f (line 1951 or 1988) several goto failure sites are reachable: line 1968 (signature malloc failure), line 1982 (primary sign_digest failure), lines 2008/2015 (secondary sign_digest failure). The cleanup block at line 2347 then does if (f) fclose(f); on the stale pointer. Reproduced on the PR build under valgrind: ./sign --rsapss2048 --sha3 image.bin rsa2048.der 7 (sign_digest returns -1 for RSA-PSS + SHA3, then goto failure) gives Invalid read of size 4 ... fclose ... Block was alloc'd at ... fopen ... make_header_ex. This is pre-existing on master, but it is exactly the defect class F-8006 set out to remove, a few lines away in the same function.

Recommendation: Add f = NULL; after the fclose(f) calls at lines 1779, 1855 and 1929. That makes if (f) fclose(f); in the cleanup block correct for every goto failure path in the function, matching what the rest of the function already does.

Referenced code: tools/keytools/sign.c:1779,1855,1929 (8 lines)


PART_SANITY_CHECK() panics three lines above the DISK_ENCRYPT scrub, leaving the last terminal exit uncovered

File: src/update_disk.c:687-692
Function: wolfBoot_start
Severity: Low

Auditing the terminal exits reached by the F-6130 change: every wolfBoot_panic() after the key is populated is now covered (lines 300, 307, 318, 332, 371, 415, 513, 563, 598, 613, 638 (new) and 675), and the FSP "doesn't fit in low memory" break falls into the if (failures) scrub. The one remaining hole is PART_SANITY_CHECK(&os_image) at line 687, which expands to if (hdr_ok != 1 || sha_ok != 1 || signature_ok != 1) wolfBoot_panic(); (include/image.h:1692) or to the ARMORED bne . spin (include/image.h:278). It sits immediately above the success-path scrub at lines 690-692, so a sanity-check failure hangs forever with disk_encrypt_key, disk_encrypt_nonce and dec_hdr still live in RAM - the exact exposure this PR closes on the DTS path. wolfBoot_hook_boot(&os_image) at line 684 has the same property if a project hook does not return. (dec_hdr is correctly in scope and populated at the new cleanup site.)

Recommendation: Move the #ifdef DISK_ENCRYPT scrub block above wolfBoot_hook_boot() and PART_SANITY_CHECK(). The key material is no longer needed once the payload is decrypted, so scrubbing earlier costs nothing and closes the last terminal exit.

Referenced code: src/update_disk.c:687-693 (7 lines)


Skipped findings

  • [Low] New trailing partial-block RMW programs up to ENCRYPT_BLOCK_SIZE-1 bytes past the requested range
  • [Low] F-7987's abort-on-failure is a no-op for internal-flash HALs, including three the same PR touches
  • [Low] The delta loop's non-encrypted wb_flash_write() return is still ignored while everything around it now checks
  • [Low] unit-flash-write-nrf52 build rule omits -Wno-int-to-pointer-cast, unlike its sibling rules
  • [Info] Delta base-hash validation failure now exits with status 255 instead of 1
  • [Info] TPM2_IoCb wipes the full staging buffers on every TIS transaction instead of just the transferred bytes
  • [Info] Blanket +40 byte size-limit bump across every algorithm with no explanation

Review generated by Skoll

Comment thread src/libwolfboot.c
Comment thread src/update_ram.c
Comment thread src/update_disk.c
Comment thread tools/keytools/sign.c
Comment thread src/libwolfboot.c
Comment thread hal/mcxw.c
Comment thread src/update_ram.c
Comment thread tools/keytools/sign.c
Comment thread .gitignore
Comment thread hal/nrf52.c
@dgarske dgarske removed their assignment Aug 11, 2026
ext_flash_encrypt_write() captured the unaligned head block's write result
but only returned it when the request fit inside that block; otherwise the
remainder loop overwrote it with its own status. On an external encrypted
partition this function is wb_flash_write(), so a failed head program was
reported as success and defeated the swap abort added in F-7987.

Also round the staging size down to a whole number of encryption blocks.
NVM_CACHE_SIZE defaults to WOLFBOOT_SECTOR_SIZE and is always a multiple
today, but an override would write stale cache bytes and desynchronise the
keystream. Assert the invariant at compile time.
F-6757 fixed the byte-wise path but left the fast path above it indexing
dst[i >> 2]/src[i >> 2] off the call-time base. The guard only proves that
"address + i" and "data + i" are word aligned, so when the destination and
source share a non-zero misalignment the byte path advances i to the next
word boundary and the fast path then copies the wrong word, through an
unaligned 32-bit access that faults on the Cortex-M0+ of stm32l0.

Index both pointers by i directly, and cover the case the existing tests
deliberately avoided.
hal_flash_erase() rounded an unaligned address down to the sector boundary
but left len at the caller's value, so a request ending in a later sector
erased only the first one. Grow len by the same amount.

test_erase_zero_runtime_sector_falls_back covered a request that ends
0x10 into the second sector, so its one-erase expectation encoded the
under-erase; it now expects both.
The ih_ep override was skipped when load_address no longer equalled the
recorded ih_load, as a proxy for "a later stage supplied its own entry
point". elf_load_image_mmu() publishes *pentry before validating the
program headers, so a rejected ELF also rewrites load_address and silently
suppressed the override; conversely a stage landing on ih_load would let it
through. Use a flag set in the ELF and FIT success paths instead, and
publish *pentry only after validation.

Also note in the ih_load == 0 branch that ih_ep is ignored there, which the
TODO removed by F-7985 used to record.
main() exit(1)'d on a key load failure. For the hybrid secondary key that
happens with the primary raw buffer live and the primary key object
initialized, so neither zero_and_free(kbuf) nor free_key() ran -- the case
F-8006 set out to fix. Jump to the tail cleanup instead; the exit status is
unchanged.

Also document why free_key() tolerates an uninitialized or already-freed
object, since load_key() has paths that produce both.
unit-flash-write-nrf52 and unit-tpm-mfgid-eh-zeroize were added to the
unit-tests Makefile without the matching ignore entries.
@dgarske
dgarske merged commit b1c2db1 into wolfSSL:master Aug 12, 2026
409 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants