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.
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.
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.
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.
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.
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.
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 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.
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.
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).
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).
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).
In the DMA RX path of qspi_transfer(), an unaligned destination or a
length not divisible by four ran the read through the 4096-byte
dma_tmpbuf. When the requested rxLen exceeded the temp buffer, dmaLen
was truncated to sizeof(dma_tmpbuf) (so only that much was DMA'd), but
the code then copied the full rxLen out with
memcpy(rxData, dmaPtr, rxLen) - reading past the end of dma_tmpbuf and
reporting the whole read as successful.
Process the RX in passes: through the temp buffer (at most
sizeof(dma_tmpbuf) per pass, rounded up to a cache line and capped)
when the destination is unaligned, directly into rxData otherwise.
Only the bytes actually DMA'd in a pass are copied out, and the
destination advances with each pass; the aligned single-pass behavior
is unchanged.
Added tools/unit-tests/unit-versal-qspi-dma: it compiles the real
hal/versal.c (generated host copy with ARM asm blanked, VERSAL_QSPI_BASE
rebased onto host memory, and the real qspi_dma_wait() renamed so the
test can provide one that performs the DMA move the GQSPIDMA_*
registers describe). It covers the aligned large read (one direct DMA
move, behavior unchanged), a small unaligned read (one temp-buffer
pass, only the requested bytes copied), and an unaligned read larger
than the temp buffer (3 passes of 4096/4096/808, all bytes correct);
the oversized case fails pre-fix (single 4096-byte move, full-length
out-of-bounds copy).
Verified: unit-versal-qspi-dma 3/3 green post-fix (oversized case
fails pre-fix), tools/unit-tests suite green (109 binaries).
ext_flash_erase() initialized idx once before the multi-sector
while (len > 0) loop and never reset it, while cmd was memset to zero
each iteration. From the second sector on the erase command was written
at cmd[idx] instead of cmd[0]: in three-byte address mode the flash
received leading zero bytes before the opcode (malformed command, the
erase silently failed); in four-byte address mode the writes ran past
the end of the 8-byte cmd buffer. The loop also kept erasing after a
failed write-enable, transfer, or ready wait, so the final return
value could report success despite failed sectors.
Reset idx = 0 at the start of each iteration and stop immediately on
any write-enable, transfer, or ready-wait failure, returning the error.
Added tools/unit-tests/unit-zynq-erase-loop: zynq.c needs the Xilinx
SDK headers (board build tree) and cannot be compiled on the host, but
the bug is entirely in this one function, so the Makefile extracts
ext_flash_erase() verbatim from hal/zynq.c and the test runs it
against emulated qspi_* functions that record every operation. It
checks the per-sector command bytes and that failures stop the loop
and propagate; it fails against the pre-fix code (malformed command
from sector two, failures masked as success).
Verified: unit-zynq-erase-loop 4/4 green post-fix (4 fail pre-fix),
tools/unit-tests suite green (108 binaries).
hal_flash_write() and ext_flash_write() issued xspi_write_en() once
before xspi_flash_write(), but that function emits one 256-byte Page
Program command per chunk when len > XSPI_IP_BUF_SIZE. NOR flash
clears its write-enable latch after each program operation, so a
write larger than 256 bytes programmed the first page and was
rejected by the flash for every remaining page, while the wrapper
still returned success.
Move the Write Enable into the page loop in xspi_flash_write(),
after the prior program has completed and before each Page Program
command, and drop it from the two wrappers.
Added tools/unit-tests/unit-ls1028a-xspi-write: the XSPI driver is
plain C register access (a few bare asm barrier statements are
blanked out in the Makefile-generated host copy), so the test
includes the real HAL file, rebases XSPI_BASE onto host memory, and
emulates the FlexSPI controller at the XSPI_IPCMD_START() boundary.
The emulated NOR keeps the write-enable latch of real NOR (set by
WEN, consumed by each Page Program, program rejected without it) and
captures the TX FIFO byte stream in the order the driver hands it
over, since the controller drains the FIFO while the command runs.
The test checks the WEN/PP command sequence and the resulting flash
contents; it fails against the pre-fix single-WEN code.
Verified: unit-ls1028a-xspi-write 4/4 green post-fix (3 fail
pre-fix), tools/unit-tests suite green (107 binaries).
The ext_flash_write()/ext_flash_read() page loops sized every
iteration from the original request length instead of len - pos,
did not cap it to the current page, and advanced by page_size - col.
A 600-byte request with 512-byte pages therefore processed 512 bytes
twice, reading/writing 424 bytes past the request on the second
iteration.
Worse, ELBC_FBCR was programmed with the column offset. Per P1021RM
12.3.30 the byte count lives in bits 31:20 and bits 0-19 are reserved,
so the byte count never moved off its reset value 0: every transfer
was a full page + spare transfer with FPAR[CI] ignored. Partial-page
programs therefore wrote the whole page from the FCM buffer,
including stale columns outside the request, and partial reads
discarded the programmed start column.
Each iteration now uses chunk = min(len - pos, page_size - col),
advances by chunk, and programs FBCR with the chunk in the BC field -
except for a full page transfer from column 0, which keeps BC = 0,
the only setting that generates/checks ECC.
The unit test (tools/unit-tests/unit-p1021-fcm-bytes) now also covers
the page loops: the host harness backs FLASH_BASE_ADDR with an 8-KB
FCM array, intercepts register writes, and emulates the FCM<->NAND
data move the hardware performs on an LSOR write, decoding
FBAR/FPAR/FBCR exactly as the RM specifies (BC = FBCR >> 20, BC = 0
means the whole page with CI ignored). It checks the programmed FBCR
values and the resulting NAND contents, and fails against the pre-fix
loops (FBCR holds the column in the reserved bits, second page BC = 0).
Verified: unit-p1021-fcm-bytes 13/13 green post-fix (4 fail pre-fix),
tools/unit-tests suite green (106 binaries).
hal_flash_set_addr() initializes flash_idx to the NAND page column
(flash_idx = col), but hal_flash_read_bytes() and
hal_flash_write_bytes() loop on `while (flash_idx < len)`, comparing the
absolute FCM buffer index against the caller's relative byte count.
With col != 0:
- if len <= col the loop runs zero times: the page program proceeds
with stale FCM buffer contents (write), or the caller's buffer is
left unchanged (read);
- if len > col only len - col bytes are copied;
- a final one-to-three byte remainder is copied as a full word,
reading or writing outside the requested range.
Compute an absolute end (flash_idx + len) and copy exactly len bytes,
handling the tail byte-wise. The confirmed call sites in
ext_flash_write()/ext_flash_read() pass col = address % page_size, so
any unaligned NAND access was affected.
Added tools/unit-tests/unit-p1021-fcm-bytes: the P1021 is PowerPC, but
the FCM helpers are plain byte copies, so the test includes the real
HAL file compiled on the host (PowerPC-asm accessors from nxp_ppc.h
shadowed with host versions; the generated nxp_p1021_host.c differs from
hal/nxp_p1021.c only in that three bare __asm__ cache-barrier
statements are blanked out). It drives the static helpers with flash_buf
backed by a host array and fails against the pre-fix loop.
Verified: unit-p1021-fcm-bytes 7/7 green post-fix (6 fail pre-fix),
tools/unit-tests suite green (106 binaries).
open_kernel_image() passed the caller's uint32_t *sz straight through to
file->Read() as the BufferSize argument. On x86-64 that parameter is a
UINTN (64-bit), and per the UEFI spec the firmware reads it as 64-bit
and writes back the number of bytes actually read. Two consequences on
every ordinary image load:
- The 64-bit read of *BufferSize reaches the four bytes adjacent to
kernel_size/update_size on the efi_main stack, so the requested size
is whatever sits next to it (with non-zero neighbors the read is
reported as a short one, EFI_END_OF_FILE, and the load fails).
- The 8-byte write of the completed byte count stores past the 4-byte
object and corrupts the stack.
Use a local UINTN readsz, pass &readsz and a properly cast buffer
pointer, and copy the completed byte count into *sz -- the same pattern
the AArch64 sibling (hal/aarch64_efi.c) already uses.
Added tools/unit-tests/unit-efi-x86-open-image as host build coverage
for hal/x86_64_efi.c (which previously had none on the host): it
includes the HAL directly and stands in for the gnu-efi runtime with a
mock that follows the spec'd BufferSize contract, so it fails against
the pre-fix code. It only builds where the gnu-efi development headers
are installed (same dependency as the make/CMake x86_64_efi target) and
uses -fshort-wchar like the real target build.
Verified: unit-efi-x86-open-image 3/3 green post-fix (fails pre-fix),
tools/unit-tests suite green (105 binaries), x86_64_efi make build
(config/examples/x86_64_efi.config) compiles and links wolfboot.elf.
unit-flash-write-cc26x2 includes hal/cc26x2.c directly with stubbed TI
SDK headers and injects Fapi status codes, pinning the new -1 returns
from hal_flash_write()/hal_flash_erase(). All four error-injection
cases fail against the pre-fix code.
This is also the only build coverage hal/cc26x2.c has; it immediately
flagged the const-discarding FlashProgram() call, now cast.
A failed WREN leaves the write-enable latch clear, so the following
page program or sector erase is silently ignored by the device: the
command itself completes at the controller, the flash never goes BUSY,
and octospi_wait_ready() reports idle on its first poll. Same
false-success shape the previous commit closed for the status read.
FlashProgram() and FlashSectorErase() return a Fapi_Status_t
(FAPI_STATUS_FSM_ERROR, FAPI_STATUS_INCORRECT_DATABUFFER_LENGTH, ...)
that both functions discarded. The subsequent
FlashCheckFsmForReady() loop only waits for the flash controller FSM
to go idle again and does not report whether the program or erase
actually succeeded, so a failed operation (e.g. locked/faulty sector)
was reported as success to the callers in src/update_flash.c and
src/libwolfboot.c that key their control flow on the return value.
Check both return values and return -1 on failure, matching the
error-handling convention of the sibling HALs (mcxa, mcxn, same51,
lpc55s69, psoc6, renesas-rx). A silently corrupted image would still
be caught later by wolfBoot_verify_integrity/authenticity, so severity
remains low; this makes the HAL contract honest.
Verified: hal/cc26x2.c has no in-repo build target (TI SDK board),
so the file was object-compiled with arm-none-eabi-gcc against
FAPI/driverlib prototypes matching the TI public API (-Wall -Wextra,
clean). tools/unit-tests suite green.
octospi_wait_ready() pre-zeros sr and ignored the return value of
octospi_cmd(). A failed status-register transfer therefore left sr == 0
and the do/while loop exited as if the flash were no longer BUSY, so a
page program or sector erase that could not be confirmed completed was
reported as successful by nor_flash_write()/nor_flash_erase() and, with
it, by hal_flash_write/erase and ext_flash_write/erase.
Make octospi_wait_ready() return -1 when the status read transfer
fails, and abort both NOR mutation loops with -1 in that case, matching
the existing error handling for the program/erase command transfers
themselves.
Note: the wait loop remains unbounded if the flash genuinely stays
BUSY (stuck operation) — a separate concern from the false-success
path fixed here; a bounded timeout would need a timing reference this
HAL does not have and no sibling HAL currently uses one.
Verified: make TARGET=stm32n6 (full build, wolfboot.bin + signed
test-app image), tools/unit-tests suite green. Hardware fault
injection required to reproduce, so no host unit test.
Discovered while working on custom board, but generic to MAX32666. All
three reproduce behavior the MSDK adopts by default.
- Set VTOR to wolfBoot's vector table. The boot ROM leaves VTOR pointing
to 0, and flash is not aliased at address 0, so any fault while
wolfBoot runs was handled by the ROM's handlers instead of wolfBoot's.
- Wait for the ICC invalidation to complete before setting the enable
bit. The cache was previously enabled while the invalidation was still
in progress (taken from MSDK).
- Disable the ICC read buffer through the TME registers.
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.
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.
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.
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.
Daniele Lacamera
Marco Oliverio
Mattia Moffa
David Garske