sbi_ipi_irq() read-and-cleared the per-hart op word before executing
the requested fence.i/sfence.vma, and sbi_wait_ipi_done() treated a
zero op word as completion. The SBI remote-fence ecalls are
synchronous, so a requester could return while the target had not yet
run its fence (e.g. resume relying on a new page table before the
target flushed its TLB).
Split the protocol into pending work and completion state, per hart:
ipi_done[h] is incremented by the target only after it has executed
the fence ops it consumed; the requester snapshots it into
ipi_wait_gen[h] before posting (so a concurrent coalesced consume of
two requesters' ops still increments past both snapshots) and waits
until ipi_done[h] passes the snapshot. SSIP posts are fire-and-forget
and do not wait, as before.
Verification:
- Built: riscv-none-elf-gcc 15.2 -fsyntax-only -Wall with
WOLFBOOT_RISCV_MMODE + WOLFBOOT_MMODE_SMODE_BOOT: clean.
- Tested: none (race condition; the contract skips failing-first tests
for races, and the file is MPFS S-mode monitor code with no host or
CI build target).
- Pitfalls: the shared DTIM struct gains two per-hart arrays; the
struct is self-initialized under init_magic by this same code on
every hart, so no cross-version ABI is broken. A target that never
runs M-soft still hits the bounded spin timeout as before. The
completion increment covers only fence ops, matching the only
waiting call sites (both RFENCE paths).
- Style: cstyle-check.sh flag count unchanged from the pre-change
file (3 pre-existing).
- Message: F-11029: prefix, no co-author trailers.
- Unverified: no multi-hart runtime execution (MPFS board only).
The RAM boot loop switched to the other partition on a verification
failure (active ^= 1, continue) with a comment claiming the failing
image was invalidated, but nothing was invalidated: wolfBoot_fallback_is_possible()
only sees that both partitions carry nonzero versions. Two present
but invalid images therefore alternated indefinitely, re-verifying
forever. (The flash path does not have this hole because it erases
the failing partition, which zeroes its version and makes the second
fallback check fail.)
Track the candidates attempted this boot: once both partitions have
failed, panic with a clear message. Same-version images are used in
the regression test so the anti-rollback guard cannot mask the
alternation.
unit-update-ram gains test_both_images_corrupted_panics: both
partitions carry valid-version images with corrupted digests; the
boot must panic after exactly two attempts. Pre-fix the loop ran
unbounded (process had to be killed; ~60k partition switches in 30 s).
Verification:
- Built: unit-update-ram, unit-update-ram-enc, unit-update-ram-nofixed,
unit-update-ram-uboot all compile.
- Tested: unit-update-ram 20/20, unit-update-ram-nofixed 3/3,
unit-update-ram-uboot 5/5; the new test shows Boot fail, Update
fail, panic.
- Pitfalls: attempt tracking is per boot session (local), no new
persistent state; the flash path is untouched.
- Style: cstyle-check.sh flag count on both changed files is
unchanged from the pre-change versions (pre-existing FMT/R1).
- Message: F-11028: prefix, no co-author trailers.
- Note: unit-update-ram.c defines 21 tests but wires 20 into the
suite (test_forward_update_samesize_notrigger was never added);
pre-existing, left as-is.
elf_load_image_mmu() skipped a segment (continue) when its mmu_cb
mapping failed, then kept loading the rest, published the ELF entry
point and returned success for a partially loaded image. The x86 FSP
payload path (boot_x86_fsp_payload.c) passes a real mmu_cb and only
panics on a non-zero return, so the buggy continue booted a payload
with a missing segment.
Return -6 with a fail-loud message, matching the program-header
clobber guard that aborts for the same reason: never silently drop a
a PT_LOAD segment.
unit-elf-mmu-fail fails the first segment's mapping and checks the
load is rejected with no entry point published; a second test checks
successful mappings still load the segments and publish the entry.
Verification:
- Built: unit test compiles elf.c (WOLFBOOT_ELF config); elf.c
syntax-clean under the WOLFBOOT_FSP config (gcc -fsyntax-only).
- Tested: unit-elf-mmu-fail 2/2; pre-fix the failure-path test got
ret == 0 (entry published for a partially loaded image).
- Pitfalls: no caller switches on the exact code (all check != 0);
-6 is new and dedicated to the mapping failure.
- Style: cstyle-check.sh on src/elf.c flags pre-existing FMT/R1
issues also present on the pre-change file; the new test trips the
uncrustify pointer-alignment class the sibling unit tests trip and
matches their local style.
- Message: F-11027: prefix, no co-author trailers.
The wolfHSM verify path of wolfBoot_verify_signature_ecc() converts
the fixed-width raw R||S signature to DER with wc_ecc_rs_raw_to_sig().
It passed the minimal field sizes (mp_unsigned_bin_size) while leaving
the pointers at the start of each fixed-width field, so whenever R or
S had a leading zero byte the conversion encoded a zero-padded integer
with the low bytes truncated, and wc_ecc_verify_hash() rejected an
otherwise valid signature (each component has roughly a 1 in 256
chance of a leading zero).
Pass the full-width fields (point_sz for both): the raw signature is
fixed-width and left-zero-padded, and the conversion strips the
padding itself. The multiprecision sizing only existed to compute the
minimal lengths and is dropped with the fix.
unit-ecc-raw-der signs until a leading-zero signature shows up, then
checks that the minimal-size pattern rejects it while the full-width
pattern accepts it, and that both patterns agree for leading-zero-free
signatures.
Verification:
- Built: src/image.c syntax-clean with WOLFBOOT_ENABLE_WOLFHSM_CLIENT
(gcc -fsyntax-only, __WOLFBOOT, ECC256/SHA256 config, partition
stubs); normal library build via make test-lib.
- Tested: unit-ecc-raw-der 3/3: the minimal pattern rejects the
leading-zero signature the full-width pattern accepts; both agree
on leading-zero-free signatures.
- Pitfalls: no key material touched; the dropped mp values had no
matching mp_clear before the fix either (stack variables).
- Style: cstyle-check.sh on src/image.c flags two pre-existing
violations (L1869 anonymous union, L2079 C99 declaration) outside
this change; the new test trips the same uncrustify
pointer-alignment class the unit-stm32l5/u5-write twins trip and
matches their local style.
- Message: F-11024: prefix, no co-author trailers.
- Unverified: no wolfHSM target builds in CI; the HSM branch was
checked by syntax-only compile, not a full target build.
Since F-9750 the head and tail read-modify-write paths decrypt the
stored neighbour block into block/enc_block before splicing the
caller's bytes, so the two stack buffers transiently hold plaintext the
caller never supplied, and several exits returned without scrubbing
them (stale head plaintext also outlived into the tail path). Funnel
every exit after the partition switch through a single cleanup that
ForceZero()s both buffers, matching the zeroization posture of the rest
of the campaign (F-7396 header cache, F-7966/F-7971 keys, aes_set_iv
IV). Defense-in-depth: the buffers are stack-local, but this is the most
long-lived plaintext in the write path.
Skoll review finding 4, 2026-08-21 wolfboot review.
The WOLFBOOT_DTS_MAX_SIZE/WOLFBOOT_DTS_MIN_SIZE pair was defined
separately in update_disk.c (F-7066) and update_ram.c (pre-existing), so
the two copies could drift. Move it to include/fdt.h, the FDT dialect
header both translation units already pull in via image.h; the hal
override (nxp_ppc.h, included before fdt.h in boot_ppc.c) keeps its
precedence. Also replace the 'bounded by the staging region' comment,
which claimed more than the code guarantees: the copy is clamped to
WOLFBOOT_DTS_MAX_SIZE, so the staging window at
WOLFBOOT_LOAD_DTS_ADDRESS must be at least that large (or the bound
overridden for the target), and the header comment now says so.
Skoll review finding 3, 2026-08-21 wolfboot review.
The F-9756 validation rejected seg_start > UINT64_MAX - filesz, but the
very next line truncates: load_addr = (uintptr_t)seg_start. On 32-bit
targets a paddr that fits in 64 bits but not in the 32-bit address space
(e.g. 0x1_0000_0000) passed every check and the flash hash walk read the
wrapped (possibly unmapped) address - the same fault class the check was
written to prevent. Bound the range by UINTPTR_MAX, the width the cast
actually uses, and pin the 32-bit-only case with a guard test.
Skoll review finding 1, 2026-08-21 wolfboot review.
Add unit-update-flash-enc-full, the full end-to-end suite (forward
updates, rollback, empty boot, diffbase) against the encrypted
swap, plus a byte-for-byte fallback-IV roundtrip test.
The suite exposes two product defects:
- ext_flash_encrypt_write() partial-block re-syncs re-anchored the
keystream at the standard-IV position once the one-shot fallback
IV offset had been consumed by the initial set_iv, corrupting the
tail of fallback-IV images. Capture the IV offset in effect at
entry and re-apply it on every re-sync.
- wolfBoot_final_swap() called wolfBoot_set_encrypt_key() with the
internal flash unlocked, but the backend expects the flash locked
(it manages the unlock/lock around the key write itself) and ends
with the flash locked. Lock before the call and drop the now
redundant lock on the failure path.
Test plumbing for the encrypted target: update-partition writes in
the tests now go through the encryption-aware writer, as the update
tool does; the hand-rolled TLV headers use the sign tool's dense
layout (padding gaps are ciphertext in encrypted builds); and the
testing-flag sites anchor on the state trailer, which sits ahead of
the key/nonce region in encrypted builds.
Verified: unit-update-flash-enc-full 35/35, unit-update-flash-enc
8/8, unit-extflash + AES128/256/ChaCha20 variants 8/8 each, full
unit suite green, stm32wb + AES256 cross-build green.
Both partial-block read-modify-write paths in ext_flash_encrypt_write
read the stored block (ciphertext), spliced the new plaintext in, and
re-encrypted the whole block. The untouched bytes were therefore
XOR'd with the keystream a second time: stored ciphertext came back
as plaintext in flash, and the next read of those bytes returned raw
ciphertext instead of the original data. Any encrypted update whose
first or last block partially overlaps a block with previous content
- e.g. a retry over a previously written update image - silently
corrupted the neighbouring bytes.
Decrypt the stored block before splicing (into the scratch buffer, so
no backend has to handle in-place decrypt) and re-encrypt the merged
plaintext. Erased (0xFF) bytes round-trip unchanged because the
decrypt/encrypt pair is the identity on the stored value.
The re-encryption re-syncs the stream to the block index first: the
decrypt step consumes keystream, and on the ChaCha/PKCS#11 backends
encrypt and decrypt share a single stream state, while on the AES
backends the decrypt context had not advanced with the full-block
writes. The tail path also syncs the decrypt context, which on the
AES backends sits at the first block's index after the aligned
writes. Fallback-IV offset handling mirrors ext_flash_decrypt_read.
New unit-extflash tests (run under the plain, AES-128, AES-256 and
ChaCha20 variants): a mid-block patch must leave the untouched bytes
of a previously written block intact, a trailing partial block must
leave the rest of a previously written block intact, and a stream
written in small unaligned chunks must round-trip byte for byte. All
three fail on the pre-fix code with every cipher.
wolfBoot_find_header() and the sign tool's re-parser checked each
field's 4+len against (uint16_t)(header_size - IMAGE_HEADER_OFFSET).
For any header of 64 KiB or more the cast wraps (0x10000 -> 0), so the
guard rejects every field and an image the tool signs cannot be parsed
by the bootloader - a pack/parse roundtrip break, fail-safe but fatal
for large TLVs (post-quantum signatures, big cert chains).
Compare in the uint32_t domain in both walkers. No shipped config
reaches this size yet (largest example is 12288), so this pins the
roundtrip for future large-header configs.
unit-parser-large-header (new) builds the walker with
IMAGE_HEADER_SIZE = 0x10008 - exactly the wrap boundary - and asserts
a 300-byte TLV and a 4-byte version field are located (both were
rejected pre-fix, proven against the pre-fix walker in a scratch
build).
Under EXT_ENCRYPTED + MMU, decrypt_header() decrypts the firmware
manifest into the file-scope dec_hdr buffer, which the blob-field
lookups and wolfBoot_ram_decrypt() consume - but never clear, so a
plaintext manifest of an image whose confidentiality is the point of
EXT_ENCRYPTED sat in .bss through do_boot(). The disk-boot twin
(update_disk.c) wipes its equivalent on every exit.
Add dec_hdr_clear() and invoke it once the field of interest has been
extracted: in wolfBoot_get_blob_version/type/diffbase_version (the
tails now extract into a local and return it, identical values in all
builds) and in wolfBoot_ram_decrypt right after the length field is
taken - the only field read from the manifest, the copy loop that
follows uses its own block buffer.
aes_set_iv() and pkcs11_crypto_set_iv() added the block counter to the
nonce-derived counter block and propagated the carry with a loop that
only runs - and whose trip count depends - on the nonce words: the
overflow branch reveals that the high word was within one count of
wrapping, and the inner loop's exit point reveals how many of the low
words are 0xFFFFFFFF. crypto_set_iv() runs once per encrypted block,
so a timing attacker gets one measurement per block.
Replace both with an unconditional branch-free four-word carry
(standard carry-out flags, three iterations regardless of content).
Arithmetic is identical: verified old-vs-new expression equality over
5M random counter/nonce inputs plus the full-carry, zero-counter and
max-counter boundary cases; unit-aes128/unit-aes256 encrypted
roundtrips pass with the new code, and the ENCRYPT_PKCS11
CKM_AES_CTR path compiles clean.
wolfBoot_crypto_set_iv() copies the firmware encryption nonce onto the
stack (local_nonce) for the AES/PKCS#11 backends and aes_set_iv() derives
iv_buf from it, and both returned without scrubbing the copies. The
rest of the codebase pairs key scrubs with nonce scrubs (e.g.
update_disk.c); these helpers are called once per encrypted block,
leaving a nonce copy on the stack at the end of every encrypted I/O
sequence, including the one preceding do_boot().
ForceZero both buffers after the derived IV is consumed. The PKCS#11
set_iv helper is intentionally left as-is: it writes the counter into
the persistent pkcs11_params CTR state, which the token updates
in-place and which must survive the call.
wolfBoot_swap_and_final_erase reads the staging-sector trailer into
tmpBuffer (which also stages the firmware key/nonce under EXT_ENCRYPTED)
and scrubs it on every exit except the resume early-return, which
returned -1 with the buffer still holding the bytes just read from
flash. Add the zeroize there so all four exits share the same
invariant.
The Doxygen comment documented a 'forcedEnc' parameter that does not
exist (the function takes address, data, len) and named AES for a
routine whose encryption step is the configured cipher - ChaCha20,
AES-CTR, or a PKCS#11-backed cipher, per build configuration.
wolfBoot_check_flash_image_elf() fed every PT_LOAD entry's
paddr/BASE_OFF straight into update_hash_flash_addr() with the 64-bit
file_size truncated to the uint32_t the reader consumes, and never
bounded an intermediate segment's file layout against the manifest.
The read loop then memcpy's from (or drives the flash driver at)
whatever address the image declares - an unauthenticated partition
(e.g. WOLFBOOT_SKIP_BOOT_VERIFY builds) or a corrupt one could walk
the hash over unmapped memory.
Validate each loadable segment before hashing and fail the check
instead of continuing:
- file_size must fit the uint32_t hash length,
- offset + file_size must stay inside the manifest image
(overflow-safe comparison; previously only the last segment was
checked, after the loop),
- paddr + BASE_OFF + file_size must not overflow the address space.
The mismatch log no longer prints the first 8 digest bytes.
Note: a full flash-region bound for paddr needs a configured
scatter-region size; no such knob exists in the target configuration
today (scattered segments are deliberately placed outside the
boot/update/swap partitions), so the region check is left as a
follow-up.
unit-image-elf-scatter gains three cases with a multi-segment
fixture: a 2^32 file_size and a segment layout extending past
fw_size (both verified OK pre-fix because the stored digest matched
the truncated/out-of-layout walk) are now rejected with -1, and a
paddr whose range overflows the address space is rejected before any
flash read (pre-fix: read at 0xfffffffffffffffb, segfault on host).
The FIT boot path relocated the flat-dt sub-image with a copy whose
length came from the FIT-declared data property length, never bounded
against the WOLFBOOT_LOAD_DTS_ADDRESS staging region - unlike the
sibling DTB paths, which all validate the parsed size against
WOLFBOOT_DTS_MIN_SIZE/WOLFBOOT_DTS_MAX_SIZE first. The length was also
harvested through a (int*)&dts_size cast of a uint32_t.
Relocate the parsed DTB size instead: validate it against the same
MIN/MAX bounds as the other DTB sources and copy that many bytes. An
out-of-range DTB is rejected (dts_addr stays NULL and the existing
fallback chain applies) rather than partially or oversize copied.
Applied to both call sites of the pattern: update_ram.c (memcpy) and
update_disk.c (wolfBoot_fit_memcpy), which also gains the DTS bounds
macros it was missing.
unit-update-disk-fit (drives the real update_disk.c wolfBoot_start)
gains two cases: a parsed size above WOLFBOOT_DTS_MAX_SIZE and one
below WOLFBOOT_DTS_MIN_SIZE are both rejected without a copy, while
the existing success/failure-copy cases keep passing. The staging
stand-in is grown so the pre-fix unbounded copy is observable as a
copy instead of a crash.
fit_find_images() took the FIT configuration's image names
(kernel/fdt/ramdisk/fpga) and the configuration name (default)
straight from fdt_getprop() and passed them on to
fdt_find_node_offset(), which strlen()s them; fit_load_image_inner()
strcmp()'d the compression property after only checking it was
non-empty. A property value not NUL-terminated within its declared
length makes those calls scan past the property - and past the end
of the blob for a property at the tail.
Add fit_getprop_string(), which returns the property value only
when it is NUL-terminated within its declared length, and use it
for the five name properties (a malformed value is rejected and the
type-based search still applies). Compare compression within the
declared length: the value must be exactly "gzip" or "none";
any other shape fails closed with the existing
unsupported-compression path instead of being strcmp()'d past the
property.
unit-fdt gains a FIT whose configuration kernel property is
unterminated (the valid default is still honored, the image name
is rejected); unit-fit-gzip gains a truncated compression="none"
value, which used to pass the subimage through as raw and now
fails closed. Both build variants (gzip enabled/disabled) run it.
fdt_get_string() bounded stroffset against size_dt_strings but
formed the string-table pointer from off_dt_strings without ever
validating either header field against totalsize; a DTB declaring a
large off_dt_strings with a small size_dt_strings made every
property lookup (fdt_getprop -> fdt_get_string) scan far outside
the blob.
Validate the structural layout in fdt_check_header() for finalized
(FDT_MAGIC) blobs: the reservation map, structure block and string
table must sit inside the blob and not overlap, checked in 64-bit
so the size fields cannot wrap. fdt_get_string() now requires a
valid header before forming the pointer. The SW_MAGIC (in-progress
edit) state keeps its existing check, since its layout is different.
Test fixtures are adjusted to the validated layout: the two
pre-existing fdt_get_string fixtures now set the header fields the
lookup relies on, the compatible-test builder sets the magic word
and points the reservation map at the canonical empty list right
after the header (it pointed into the string table before).
fdt_node_offset_by_compatible() compared each entry of a
multi-string compatible value with memcmp(compatible, prop,
complen+1) before locating the entry's NUL terminator. When the
declared property length equals the search length (no room for a
NUL), the comparison reads one byte past the property data and
accepts the entry when that byte happens to be zero (e.g. the
4-byte alignment padding).
Locate each entry's NUL terminator within the declared length
first and only compare entries whose length equals the search
length: nothing is read past the property, and an unterminated
trailing entry can no longer match.
unit-fdt gains four cases with a minimal single-node FDT builder:
an unterminated exact-length entry does not match (it did before
the fix), a terminated exact-length entry matches, multi-string
lists still match on later entries, and a longer entry that starts
with the search string does not match.
libwolfboot.c defined the external-flash header-cache flag as
uint32_t while image.c declared it extern int - the same object with
incompatible types in two translation units is undefined behaviour. It
is benign on every supported target (both are 32-bit) but a latent
portability defect; the flag is written from both files. Make the
definition int, matching both declarations.
In the wolfHSM branch of wolfBoot_verify_signature_ecc(), both failure
paths after wc_ecc_init_ex() - the HSM key-ID setup and
wc_ecc_import_unsigned() - did a bare return that skipped the
wc_ecc_free(&ecc) at the end of the function, leaking the key (and its
heap-allocated mp_ints in non-SP-math builds). Free the key before
returning on both paths, matching the plain wolfCrypt path.
The wc_ecc_import_unsigned() path sits in the server-only, non-cert-chain
configuration, which does not compile today (it references pubkey/
point_sz, declared only for the software and client builds) - see the
F-7995 note; the free is added there for consistency so the path is
correct if that configuration is ever made buildable.
wolfBoot_verify_signature_lms() returned without calling wc_LmsKey_Free()
when wc_LmsKey_SetParameters() or wc_LmsKey_ImportPubRaw() failed after a
successful wc_LmsKey_Init(), leaking the key (and whatever heap state the
LMS backend allocated for it). Free the key on both error paths, matching
the success path.
The test file headers ran to 20-40 lines of prose before the licence,
restating the whole finding and the harness design. Cut them to a short
paragraph on what broke and a short one on how the test reaches the
code. Same for the long inline blocks in the HAL and libwolfboot
changes.
Comments only; no functional change.
Drop the internal report numbers from source, test and Makefile
comments: they mean nothing outside the tracker and do not belong in
the tree. Condense the long inline comment blocks the review flagged,
and note in the raspi3 encrypted example and its docs that CI builds
but does not boot it, so the end-to-end path has no automated
coverage.
Comments and docs only; no functional change.
Widening the misc.c include guard with NVM_FLASH_WRITEONCE made this
file include <wolfssl/wolfcrypt/types.h> and <wolfcrypt/src/misc.c> in
every such build, including the two that cannot resolve them:
tools/check_config has no wolfSSL include path, and the STM32Cube
test-app has no stm32wbxx_hal_conf.h. Both fail to compile.
Scrub with a local volatile byte loop instead and put the guard back.
That also suits the RAMFUNCTION callers better, since ForceZero() lives
in flash and must not be called while flash is being programmed.
sdhci_wait_busy() runs at the top of every transfer and can now spin
for the full 30 s budget. Neither loop petted the watchdog, so on a
platform with one a stuck card produced a reset instead of the clean
I/O error the timeout exists to give. The shared deadline between the
two loops is deliberate and now says so.
The weak sdhci_platform_wdt_pet() default moves out of the
SDHCI_BLOCK_VIA_PDMA guard, since the busy waits call it on every
build. The test drops its SDHCI_WAIT_BUSY_TIMEOUT_MS=50 override and
steps its timer 1 ms per read instead, so the shipped default is what
is tested, and it counts watchdog services.
The exit_lock scrub was guarded to the case where ENCRYPT_CACHE is a
stack local, skipping both static ones -- NVM_FLASH_WRITEONCE aliases
it to NVM_CACHE, WOLFBOOT_SMALL_STACK gives it its own array. Those
are exactly the buffers where the plaintext key and nonce would stay
resident for the rest of the boot.
sdhci_uhs_recover_rollback() restored 3.3V but left g_uhs_recovered
set, leaving the host in a state the base code could not reach: back
at 3.3V with the recovery spent, so a genuinely UHS-I card could never
be retried for the rest of the boot.
The poll loops divided a 64-bit elapsed count by 1000 on every
iteration, which is a libgcc helper call on 32-bit targets. Compute
the timeout in microseconds once instead.
The trailer scrubbing added for F-9765 calls ForceZero() from
trailer_write()/partition_magic_write(), which compile under
NVM_FLASH_WRITEONCE in the test-app build of libwolfboot.c too. That
build defines neither __WOLFBOOT nor EXT_ENCRYPTED, so the file's
wolfcrypt/src/misc.c inline include (the only place ForceZero becomes
visible, and the reason the test-app wolfcrypt link set carries no
misc.c) was compiled out and -Werror rejected the implicit
declaration.
List NVM_FLASH_WRITEONCE in the include gate, for the same reason
EXT_ENCRYPTED is already listed there.
Verified with the exact CI test-app compile commands for the two
failing presets (stm32h5, stm32u5) on arm-none-eabi-gcc: HEAD
reproduces the implicit-declaration error, the fix compiles clean.
Unit suite 123/123.
Under NVM_FLASH_WRITEONCE the partition-trailer helpers stage a full
flash sector into the file-scope NVM_CACHE before rewriting it. In
EXT_ENCRYPTED builds that same sector is where the firmware key/nonce
live - ENCRYPT_CACHE is a macro alias of NVM_CACHE in this
configuration, and hal_set_key() stages the key there - so after a
normal boot's wolfBoot_set_partition_state(PART_BOOT,
IMG_STATE_TESTING) the plaintext key/nonce sat in .bss at a fixed
address across the handoff to the application. The file's only
ForceZero of the buffer is compiled out precisely for
NVM_FLASH_WRITEONCE, and neither trailer_write() nor
partition_magic_write() scrubbed the buffer on return.
Scrub the whole staged sector (ForceZero, NVM_CACHE_SIZE) before each
return of both helpers, on success and on the failure path (the key
is already staged by the time the flash write fails).
Test: tools/unit-tests/unit-nvm-cache-scrub.c extracts the real
trailer_write() and partition_magic_write() and runs them over a
staged sector carrying a key/nonce pattern; pre-fix the pattern
remained in NVM_CACHE after all three scenarios.
The doc comment claimed the key/nonce were always reset to 0xFF, but
only the flash-backed path does that (FLASH_BYTE_ERASED via
hal_set_key). The MMU path zeroizes the in-RAM copy with ForceZero()
(all-zero bytes) and the TSIP path has nothing to erase. State the
actual per-path behavior so a verification routine or self-test does
not assert the wrong erased pattern on MMU targets.
Comment-only change; no code or behavior changes.
sdhci_wait_busy() had no timeout in either its DATA0 polling loop or
its repeated CMD13 loop (the in-code TODO acknowledged it). A removed
card, a controller fault, or a card stuck in the programming state
left wolfBoot spinning forever instead of returning an I/O error.
Give both loops a shared, finite deadline (30 s worst-case erase
programming time, configurable for tests via
SDHCI_WAIT_BUSY_TIMEOUT_MS) and return -1 when it expires. Callers
already propagate a nonzero status as a failure.
Test: tools/unit-tests/unit-sdhci-wait-busy.c compiles the real
src/sdhci.c and scripts the controller through the host register file
(DATA0 held low; CMD13 responses with READY_FOR_DATA clear), with the
emulated timer advancing 1us per read and the deadline built to 50 ms,
so both stuck cases hit the deadline in wall milliseconds. Pre-fix the
DATA0 case hung until the test framework's timeout killed it.
Daniele Lacamera
David Garske
Brett Nicholas