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.
test-configs.yml already builds this config; what nothing covers is the
keytools half of the documented procedure -- signing and encrypting an
AArch64 payload and assembling the boot image. That is where the wrong
key size in the docs hid, so build it in CI.
Builds with DEBUG_UART=1, the mode the docs tell users to run and one
that did not compile until now. It stops short of booting: the target
emits nothing on the console under QEMU, so there is nothing to assert.
DEBUG_UART=1 did not compile: uart_init() declared an unused local and
-Werror rejected it, so the target's console output could never be
enabled. Drop the local.
The documented key file is 44 bytes, but AES256-CTR wants a 32-byte key
plus a 16-byte IV; sign fails with "Error reading IV" as written. Use
48 bytes, matching the key the HAL used to carry.
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.
The wait loop increments timeout past FLASH_TIMEOUT_TRIES on its way
out, so "timeout == FLASH_TIMEOUT_TRIES" never matched and a hung
command returned success. Test the completion flag instead.
That made the failure path reachable for the first time, and it did not
exit: the break left the inner block loop only, so the outer loop
respun the same failing command forever with pos unchanged.
Removing the hardcoded test key is right, but nothing on this target
calls wolfBoot_set_encrypt_key() any more, so the shipped example and
the docs walk-through (whose example key was that same test key) no
longer boot an encrypted image. Say so in both places.
NXP's qoriq-fman3 dtsi numbers the 10G memacs 0x8/0x9, and phydevs
holds the 10G port at FM1_10GEC1. Bounding the raw cell-index against
the table traded an out-of-bounds read for a silently dropped MAC
fixup on exactly the DTBs that motivated the guard. Translate instead,
and skip only indexes with no slot.
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 NOR-extent test ran after the microcode table had already been
dereferenced, so a container near the top of the bank read past the
window the guard exists to enforce -- a machine check on hardware. Bound
the fixed part before the first dereference and the table before the
walk. The accept/reject outcome is unchanged; the out-of-bank read is
what goes away.
Use the file's own QE_MAX_RISC for the count bound instead of a
hand-written 2, matching the t10xx and p1021 siblings; the shared length
formula already handles count > 1.
The versal host copy deleted every asm statement, including the three
with output operands, leaving reads of uninitialised automatics -- at
-O0 an infinite loop in hal_delay_us(). Rewrite those three into
assignments from a host counter and only delete asm with an empty
output field; a new rule guard fails the build if any asm survives,
so a future one cannot be silently dropped.
clean now removes the 17 generated sources and the conditionally-built
tests, and .gitignore covers all 125 test binaries plus the generated
sources -- a full build left 30 untracked files before. The gnu-efi
skip announces itself like the adjacent multilib probe.
open_kernel_image() left *sz set on the read-failure path though it
documents both outputs as 0 on failure. The p1021 FCM helpers used
32-bit accesses from flash_idx, which starts at the page column, so an
odd column made every access misaligned -- an alignment interrupt on
the guarded eLBC window on e500.
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 extract rules append with >> across several commands. Without
.DELETE_ON_ERROR: a failure partway through leaves a truncated header
with a fresh mtime that the next make treats as up to date.
tfm.c never tests WC_NO_HARDEN, so dropping it enabled nothing; it only
un-silenced the advisory #warning in settings.h, which -Werror turns
into a build failure for any USE_FAST_MATH + WOLFCRYPT_TZ_PSA build.
Define TFM_TIMING_RESISTANT, which is what makes tfm.c constant time.
XSPI_IPCMDDONE only says the controller finished driving the sequence
on the bus. The device keeps WIP set for the ~ms the cycle takes and
ignores Write Enable while it is set, so the per-page WREN this PR
added was dropped for pages 2..N and their Page Programs with it.
The erase loops polled the same bit with inverted polarity: they
exited as soon as WIP read *set*, i.e. once the erase had started, and
would spin forever if they missed the busy window. FLASH_READY_MSK is
renamed FLASH_SR_WIP_MSK so the sense is not guessable, and both paths
now share xspi_wait_ready().
The test device models WIP, so a missing wait shows up as a rejected
Write Enable.
The block-crossing short write issued two programming commands
back-to-back. The F021 FSM rejects a command while busy, and the
read-modify-write that stages the next block reads the array while the
first program is in flight, so it stages undefined data and programs
it. The multi-block branch already waits after every command.
The test FSM now stays busy for a few polls and holds the programmed
data back until it drains, so the missing wait is observable.
The bad-block check reads flash_buf[bad_marker], which sits in the
spare region. A BC != 0 transfer loads only the requested main-area
bytes, so the marker byte is leftover FCM RAM: a stale non-0xFF value
makes ext_flash_read() declare the block bad, skip to the next erase
block and return data from the wrong address while still returning
len. BC = 0 is also the only setting that checks ECC.
Latent until the FBCR shift fix made BC take effect. read_size still
bounds the copy out of the buffer, so the F-7976 sizing fix stands.
P1021RM 12.3.30 numbers BC as bits 20-31 MSB-first, i.e. unshifted --
same convention every FPAR field macro in this file already follows
(FPAR_SP_PI at RM 17-21 is << 10, CI at RM 23-31 is unshifted). The
<< 20 put the count in reserved bits 0-19 and left BC at 0, which the
RM defines as "whole page + spare, FPAR[MS]/[CI] treated as zero": a
partial program filled the FCM buffer at offset col but then wrote the
entire page back, stale bytes included.
The test emulator decoded FBCR the same wrong way, so it could not see
this; it now decodes BC as the hardware does, and fails on the shifted
macro while still catching the original F-7976 bug.
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 partial-tail handling skipped the words that hold no requested
byte. Neither part can do that: U5 only starts the 128-bit program on
the fourth word (a partial quad-word leaves FLASH_SR_WDW set and
hal_flash_wait_complete() never returns), and L5 has no 32-bit program
mode, so splitting the double word into two PG cycles programs
nothing and faults the second on alignment.
Build the full unit first, taking bytes outside [i, len) from the
current flash content as hal/stm32h5.c does, then store every word.
That drops the over-read F-9763/F-9764 targeted without ever issuing
a partial unit. L5 TZ claim widened to the 8-byte boundary to match.
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.
The extraction rules added for the fenrir-fixes tests used awk one-shot
scripts. They are all plain 'print from line A to line B' jobs, which
is sed -n '/A/,/B/p' territory; replace every awk rule with sed (and
grep for the single WRITE_BLOCK_SIZE define the TI extract carries).
Two ranges cannot stop at the 4th '} QE_PACKED;' (sed ranges cannot
count), so those blocks are anchored on the unique line that follows
them: the qe_check_firmware() comment for the T10xx extract and
hal_fman_init() for the T2080 one (the signature trimmed with
$$d). The regenerated extracts are byte-identical to the awk output
except trailing blank lines/comments.
Full suite re-run: 123/123 green.
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.
hal_flash_write() looped while (i < len) but programmed whole 16-byte
units: each iteration read all four 32-bit source words unconditionally
and wrote the full quadword to flash, so a write whose length is not a
multiple of 16 read up to 12 bytes past the caller's buffer - which
the NSC update path validates for exactly len - and wrote those bytes
to flash.
Program the unit word by word: each word is written only when at least
one of its bytes is requested, and a partial final word is padded with
the erased value (0xFF) in its upper bytes so no source bytes past len
are read. The single PG/wait/clear sequence per unit is unchanged.
Test: tools/unit-tests/unit-stm32u5-write.c is the 16-byte twin of the
STM32L5 test (F-9763): it extracts the real hal_flash_write() and its
wait/clear helpers, maps the destination flash at a 32-bit host address
(the parameter is uint32_t on this 32-bit target) pre-filled with stale
data, and runs short writes; pre-fix the bytes past len landed in the
destination flash in all four partial-unit cases.
hal_flash_write() looped while (i < len) but programmed whole 8-byte
units: each iteration read both 32-bit source words (src[i>>2] and
src[(i>>2)+1]) unconditionally, so a write whose length is not a
multiple of 8 read up to 4 bytes past the caller's buffer - which the
NSC update path validates for exactly len - and wrote those bytes to
flash.
Program the unit word by word: the first word is always programmed
(it contains byte i), the second only when at least one of its bytes
is requested, and a partial final word is padded with the erased
value (0xFF) in its upper bytes so no source bytes past len are read.
The TZ non-secure claim is extended to the flash span the program
actually writes (len rounded up to the word boundary).
Test: tools/unit-tests/unit-stm32l5-write.c extracts the real
hal_flash_write() and its wait/clear helpers, maps the destination
flash at a 32-bit host address (the parameter is uint32_t on this
32-bit target) pre-filled with stale data, and runs short writes;
pre-fix the bytes past len (including un-padded upper word bytes)
landed in the destination flash.
hal_fman_init() checked only the NOR window guard and the QEF magic,
then copied fw->microcode[i].count words from fw + code_offset into
FMan instruction RAM and armed the engine. No version, count, length
self-consistency, or code-offset validation: a count of zero was
accepted as a successful upload of nothing, and a structurally
consistent blob with an out-of-image offset made the loader copy
arbitrary memory into FMan IRAM.
Add the same structural checks the T10xx qe_check_firmware() path
applies: version, count (1..2, the FMan RISC count), self-consistent
length, every code range inside the declared image, and the whole
image inside the remaining NOR bank (offset arithmetic that cannot
overflow uintptr_t, matching the existing window guard). All 64-bit,
fail closed - FMan stays unconfigured on any mismatch.
Test: tools/unit-tests/unit-t2080-fman-loader.c extracts the real
structures and hal_fman_init() and runs them against a test-owned NOR
image (FMAN slot at a nonzero bank offset, guard slack after the bank)
and an emulated FMan IRAM; pre-fix the loader accepted all four
invalid-container cases.
A file larger than 4 GiB used to be truncated when FileSize() (UINT64)
was assigned straight into the uint32_t *sz: only a tiny fragment was
allocated and read, and the loader then operated on an incomplete
image. The guard that rejects zero or > UINT32_MAX sizes (before any
allocation) was introduced by the F-9738 cleanup of the same function
(b2fc3eb2); this commit adds the regression coverage for it.
The oversized case needs no real 4 GiB file or allocation: the guard
runs before AllocatePages, so the mock reports a 4 GiB + 4 KiB size
and the test asserts rejection with nothing allocated and the file
closed. Pre-fix the code skipped the guard, reached the Read with the
truncated size, and published a nonzero address with a truncated
EFI_STATUS return.
open_kernel_image() wrote the allocated address into *_addr before the
read, and returned from a failed Read() or undersized-image check
without clearing it; efi_main() ignored both return values and treated
any nonzero kernel_addr/update_addr as a loaded image. A kernel.img
shorter than the header (or a failing Read) with no update.img left a
nonzero, rejected buffer that wolfBoot_start() would boot from. The
file handle was also never closed and the allocated pages were leaked
on the failure paths.
Mirror the AArch64 sibling: initialize *_addr/*sz to zero, publish the
address only on full success, FreePages on read/size failure, close
the file on every path, reject non-fitting file sizes, return -1
instead of a truncated EFI_STATUS, and have efi_main() acknowledge the
reliable zero-on-failure contract.
Test: the existing F-9734 harness in
tools/unit-tests/unit-efi-x86-open-image.c now tracks FreePages/Close
and injects Read and allocation failures; pre-fix the rejected loads
published nonzero addresses, returned truncated EFI_STATUS values,
and never closed the file.
cacheSector() and programBytesToErasedFlash() filled uint32_t page
buffers with the byte-valued FLASH_BYTE_ERASED, so in the default
(non-inverted) build each 32-bit word became 0x000000FF instead of
0xFFFFFFFF. A one-byte write into an erased page then programmed
zeroes into the unrelated bytes of the same words, and the sector
cache carried the corrupted pattern into every read-modify-write.
Assign FLASH_WORD_ERASED to each word, which tracks the flag polarity
(0xFFFFFFFF / 0x00000000UL). The bytewise fills elsewhere in the file
were already correct.
Test: tools/unit-tests/unit-aurix-erased-fill.c extracts the real
functions and runs them against an emulated tc3_flash_* API; it is
built for both flag polarities. Pre-fix the default-polarity build
shows the 0x00 high bytes in all three fill cases (the inverted build
passes, masking the bug as the finding notes).
The comment justified WC_NO_HARDEN on the grounds that 'wolfBoot only
does public asymmetric operations'. That is not true for software
DICE: with WOLFCRYPT_TZ_PSA and no WOLFBOOT_DICE_HW, src/dice/dice.c
retrieves the private IAK (wolfboot_attest_get_private_key) and signs
the attestation claims with wc_CoseSign1_Sign_ex(), and arch.mk still
selects USE_FAST_MATH for those builds. A configuration maintainer
trusting the comment would have left the private-key signing path
without timing hardening.
Gate the definition to verify-only configurations: WC_NO_HARDEN is
now defined only when WOLFCRYPT_TZ_PSA is absent or WOLFBOOT_DICE_HW
is set (hardware DICE keeps signing in the crypto engine), and the
comment states the actual rationale.
Verified with a preprocessor probe: plain fast-math and hardware DICE
still get WC_NO_HARDEN; software DICE no longer does.
The comment labeled IMG_STATE_TESTING as an 'ELF loading state', but
the constant is wolfBoot's general dual-bank confirm/rollback marker:
it is set on the BOOT partition after every swap (update_flash.c,
update_flash_hwswap.c, update_ram.c - none ELF-specific) and cleared
by wolfBoot_success(); if still present at the next boot it triggers
a rollback. A maintainer trusting the old comment could have treated
the state as inert on non-ELF targets and disabled the anti-brick
path.
Reword to the accurate description already used in
docs/flash_partitions.md, keeping the note that it only lives on the
boot partition so it cannot conflict with IMAGE_STATE_UPDATING.
Comment-only change; no code or behavior changes.
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.
qe_check_firmware() validated the QE/FMAN microcode header for
self-consistency (magic, version, count, length == computed size,
optional CRC) but never bounded the per-microcode code_offset against
the declared image length. qe_upload_firmware() reads code_offset +
4*count bytes from the firmware start and copies them into QE IRAM
(and programs the microcode's trap table), so a structurally valid
blob with an out-of-image offset made wolfBoot copy arbitrary memory
into the engine ahead of image authentication.
Reject the firmware when any microcode's words extend past the
declared end (64-bit sum, no wrap). This protects both the QE and the
FMAN firmware paths, which share the check.
Test: tools/unit-tests/unit-t10xx-qe-firmware.c extracts the real
qe_* structures and qe_check_firmware() from hal/nxp_t10xx.c and
builds the blobs in memory; pre-fix both out-of-image variants were
accepted (one of them segfaulting the test harness while building the
blob's claimed code).
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.
ext_flash_write() chunked the request into page-indexed FLASH_PAGE_SIZE
transfers (page 0, page 1, ...) without accounting for the start
address's offset inside the physical page. A write starting mid-page
sent a full-page Page Program across the boundary; NOR wraps the write
pointer at the page start, so the excess bytes clobbered the start of
the page.
Rewrite the loop to advance from the current address and cap each
transfer at the end of the physical page holding it, mirroring the
LS1028A (F-7978) and Zynq (F-7979) fixes. Failure-stop semantics are
unchanged.
Test: tools/unit-tests/unit-versal-ext-write.c extracts the real
QspiDev_t and ext_flash_write() from hal/versal.c (the qspi_* helpers
are file-static, so the test emulates them) and programs an emulated
NOR that models the wrap, so the pre-fix corruption is observable.
ext_flash_write() chunked the request by page index (address +
page * FLASH_PAGE_SIZE, xferSz = min(len, FLASH_PAGE_SIZE)) without
accounting for the start address's offset inside the device page. A
write starting mid-page sent a full-page Page Program across the
boundary - and NOR wraps the write pointer at the page start, so the
excess bytes clobbered the beginning of the page. The page-index math
also made the second transfer read up to FLASH_PAGE_SIZE bytes past
the end of the caller's buffer for unaligned requests.
Rewrite the loop position-based: each transfer is capped to the bytes
remaining in the physical page holding the current address, and
address/data/len advance by the actual chunk. Page-aligned requests
behave as before; the failure-stop semantics (break on failed
write-enable/transfer/wait, error propagated) are unchanged.
Added tools/unit-tests/unit-zynq-ext-write: it awk-extracts the real
ext_flash_write() from hal/zynq.c (same approach as the F-7980 erase
test) and runs it against emulated qspi_* functions whose NOR models
the real wrap (a crossing program overwrites the page start it wraps
into). Tests: page-aligned multi-page write; a 300 byte write at
offset 200 crossing two boundaries (pre-fix: a 200+256 program wraps
and clobbers the page start; post-fix: 56 bytes to the edge, then the
rest from the next page start); failed write-enable and failed
transfer both stop the loop with -1.
Verified: unit-zynq-ext-write 4/4 green post-fix (1/4 fails pre-fix),
tools/unit-tests suite green (114 binaries).
xspi_flash_write() capped each chunk to XSPI_IP_BUF_SIZE (256, the NOR
page size) but not to the bytes remaining in the current physical
page, so a write starting mid-page sent one Page Program across the
page boundary. NOR wraps the write pointer to the page start, so the
excess bytes clobbered the beginning of the next page (and the
programmed data was corrupted to match).
Cap each chunk to min(len, page room left in the current page).
Aligned or page-start writes are unchanged; a mid-page start now
sends a short first program plus the remaining full pages.
The unit-ls1028a-xspi-write harness now models the NOR wrap (a
crossing program clobbers the page start it wraps into), and gains
test_write_crossing_page_no_wrap: a 256 byte write starting 16 bytes
before a boundary must not touch the first 16 bytes of the next page
(pre-fix the emulator shows the clobber and a 240+256 program).
test_write_unaligned's expectations are updated to the correct
156/256/188 chunking.
Verified: unit-ls1028a-xspi-write 5/5 green post-fix (2/5 fail
pre-fix), tools/unit-tests suite green (113 binaries).
The fsl,fman-memac loop in hal_dts_fixup() read the node's cell-index
straight from the device tree and used it to index the file-static
phydevs[5] with no bounds check - the qman-portal loop immediately
above it does guard (if (i >= QMAN_NUM_PORTALS) break). Standard NXP
FMan device trees assign the 10G MACs cell-index 8/9, so any such DTB
read past the array and fdt_setprop() wrote the result into the kernel
device tree as a MAC address.
Skip nodes whose cell-index falls outside the table (i < 0 || i >=
sizeof(phydevs)/sizeof(phydevs[0])), mirroring the qman-portal guard
but continuing to the next node so a stray index does not drop the
remaining valid MACs.
Added tools/unit-tests/unit-t10xx-dts-memac: it compiles the real
src/fdt.c and the real hal_dts_fixup() (awk-extracted with its
liodn/qman static tables) and feeds it hand-built DTBs in wolfBoot's
FDT dialect (struct-relative offsets, big-endian, compatible values
inline): an out-of-bounds cell-index (8, as in real 10G DTBs) must not
gain a local-mac-address, a valid index (1) must get phydevs[1]'s
MAC, and an out-of-bounds node followed by a valid one (8 then 2) must
skip the first and still fix the second. Pre-fix the first two tests
fail (the OOB MAC is written into the DTB).
Verified: unit-t10xx-dts-memac 3/3 green post-fix (2/3 fail pre-fix),
tools/unit-tests suite green (113 binaries).
FRAM_Erase() already negated its failure status (return -(int)status),
and its only caller ext_flash_erase() applied the same conversion a
second time (return -(int)status). The two negations cancelled, so a
failed FRAM erase made ext_flash_erase() return +1/+2 - a
success-looking value to callers that test ret < 0, e.g. diag_erase()
in src/libwolfboot.c which maps ret < 0 to failure.
FRAM_Erase() now returns the hal_status_t unmodified (and
hal_status_ok on success); ext_flash_erase() is the single negation
point, matching the already-correct ext_flash_write()/ext_flash_read()
pair (FRAM_Write/FRAM_Read return un-negated statuses).
Added to unit-va416x0-fram: a successful 64 byte erase (two 32 byte
FRAM chunks, six SPI transfers) returning 0 with the shadow IRAM
filled 0xFF, and a failed erase returning a negative code with the
shadow IRAM untouched. Pre-fix the failure case returned +2.
Verified: unit-va416x0-fram 5/5 green post-fix (failure case returned
+2 pre-fix), tools/unit-tests suite green (112 binaries).
iram_write() and iram_fill() compute the enclosing 32-bit word with
(uintptr_t)dst & ~3u. ~3u is a 32-bit constant, so on 64-bit
platforms the AND zeroes the high word of the destination pointer and
the subsequent word read/write faults (verified: both helpers segfault
on x86_64 for any 64-bit destination). On the 32-bit va416x0 target
the mask happens to cover the whole pointer, so this only bites where
the code runs with 64-bit addresses.
Use ~(uintptr_t)3 so the mask is always pointer-width. No behavior
change on 32-bit targets.
Also make the EXT_FLASH section of hal/va416x0.c host-testable (split
the WOLFBOOT_UNIT_TEST_VA416X0_FRAM guard around it, mock the
SYSCONFIG register block the lock/unlock pair touches) and add
unaligned iram_write/iram_fill regression tests that would segfault
pre-fix on the host.
Verified: unit-va416x0-fram 3/3 green post-fix (both new tests
segfault pre-fix), tools/unit-tests suite green (112 binaries).
hal/raspi3.c unconditionally defined TEST_ENCRYPT, so every build
compiled with EXT_ENCRYPTED (the shipped config/examples/raspi3-
encrypted.config sets ENCRYPT=1) ran the hal_init() branch that passed
a fixed 32-byte AES key and fixed 32-byte nonce literal to
wolfBoot_set_encrypt_key(). Anyone with the source or the bootloader
binary knew the complete encryption secret and could decrypt captured
encrypted firmware images.
Remove the TEST_ENCRYPT define and the hardcoded key/nonce block. The
unit-test suites and tools/test.mk install their own test keys and do
not depend on this hook. Without a provisioned key the encryption
globals stay zeroed, so encrypted updates fail to decrypt and are
rejected (fail closed); per-device key provisioning (OTP, secure
element, or an authenticated provisioning flow, with zeroization of
temporary RAM copies) is a product decision tracked on the finding.
Verified: no TEST_ENCRYPT or key-literal references remain in
hal/raspi3.c, no other file references the hook (grep), raspi3.c
syntax-checks clean with ARCH_AARCH64.
qe_upload_firmware() validated only the self-declared consistency of
the QE microcode blob (QEF magic, version, count range, length equal to
the computed size, optional CRC32). The per-microcode code_offset and
count fields and the total length were never bounded to the 64 KiB
buffer actually read from NAND - which, on the shipped p1021 config,
lives in the update partition (QE_FW_NAND 0x01F00000 inside the
0x1200000-0x21FFFFF update range). A structurally consistent blob with
a large code_offset or word count made the upload loop read arbitrarily
far past the buffer and copy it into QE instruction RAM before the
bootloader authenticated anything.
Reject a blob whose declared length exceeds QE_FW_LENGTH, and reject
any microcode whose code_offset + count words extends past the declared
length (checked in 64-bit so the offset/count sum cannot wrap). This
bounds every read the upload performs - the microcode words and the
trailing CRC32 - to the validated region, and fails closed.
The finding's other recommendation - cryptographic authentication of
the microcode before activation - is a design decision (keys, signed
manifest, build flow; QE is also needed for NAND access, so it runs in
hal_init() ahead of image authentication) and is tracked as a note on
the finding rather than forced in here.
Added tools/unit-tests/unit-p1021-qe-firmware: it compiles the real
hal/nxp_p1021.c (generated host copy, PowerPC accessors shadowed) and
drives qe_upload_firmware() with crafted blobs: a valid blob (accepted,
words counted in QE instruction RAM), a microcode sitting past the end
of the declared length (rejected; pre-fix accepted), a self-consistent
blob longer than the 64 KiB buffer (rejected; pre-fix accepted), and a
bad magic (rejected).
Verified: unit-p1021-qe-firmware 4/4 green post-fix (both out-of-bounds
cases accepted pre-fix), tools/unit-tests suite green (112 binaries).
The short-write path of hal_flash_write() checked only len <
WRITE_BLOCK_SIZE, not whether (address % WRITE_BLOCK_SIZE) + len stayed
inside the block. A short write starting near the end of a block copied
past the end of the WRITE_BLOCK_SIZE stack staging buffer via
memcpy(temp + off, data, len) and programmed only the first block,
losing the bytes intended for the next block.
Clamp the first chunk to the bytes remaining in the start block,
read-modify-write that block, and - when the write crosses the block
boundary - continue with the remaining bytes (always less than one
block, starting aligned) in the next block with its own
read-modify-write. The len >= WRITE_BLOCK_SIZE path is unchanged.
Added tools/unit-tests/unit-ti-hercules-write: ti_hercules.c needs the
TI FAPI vendor headers (board build tree) and cannot be compiled on the
host, so the Makefile extracts hal_flash_unlock_helper() and
hal_flash_write() verbatim and the test emulates the FAPI calls, with
Fapi_issueProgrammingCommand() performing the program into a host flash
array mapped at a fixed low address (Hercules flash is memory-mapped,
so the read-back is a plain pointer read). It covers a short write
inside one block, a short write crossing a block boundary (segfaults
pre-fix on the staging-buffer overrun), a one-byte write at the last
byte of a block, and a multi-block write (direct programming,
unchanged).
Verified: unit-ti-hercules-write 4/4 green post-fix (crossing case
segfaults pre-fix), tools/unit-tests suite green (111 binaries).
disk_read() called sdhci_uhs_recover() after any nonzero status from a
data transfer. The recovery is a guess - no platform or retained state
in wolfBoot identifies a warm-reset UHS condition, so on a plain 3.3V
cold boot any first transfer failure (CRC, DMA, media, controller)
switched the host to 1.8V signaling (SDHCI_SRS15_V18SE) without a CMD11
voltage negotiation. When the retry also failed, the voltage setting
was never rolled back and g_uhs_recovered prevented further recovery:
a transient error left the host - for the rest of the boot and the
next stage - in a signaling state the card is not using.
Make the guess self-correcting: if the retry of the failed chunk also
fails, restore the previous signaling (clear V18SE, re-enable the SD
clock) via sdhci_uhs_recover_rollback() before reporting the error. If
the retry succeeds the switch stays - that is the warm-reset UHS case
the recovery exists for. A later failure in a subsequent chunk no
longer rolls back a configuration that is actually working. The
unconditional break-on-error for eMMC builds is unchanged.
Added tools/unit-tests/unit-sdhci-uhs-recover: it compiles the real
src/sdhci.c (generated host copy with the three asm statements blanked
and the real sdhci_read() renamed so the test can provide a scripted
one) with register access backed by a host array, and asserts on the
SDHCI_SRS15/SDHCI_SRS11 state for: a clean read (no switch), a failed
first attempt with a successful retry (switch kept), a failed retry
(roll back), and a later failure after a working recovery (switch
kept). The rollback case fails pre-fix.
Verified: unit-sdhci-uhs-recover 4/4 green post-fix (rollback case
fails pre-fix), src/sdhci.c syntax-clean under DISK_EMMC,
tools/unit-tests suite green (110 binaries).
Brett Nicholas
David Garske
Daniele Lacamera