Cap the library fw_size clamp at UINT32_MAX (a > 4 GiB file would
otherwise truncate to a small value), say 'restore' not 'store' in the
scatter-restore error message, and print stdout as well as stderr in
the compile-check scripts on failure.
paddr/filesz/offset were unsigned long, which is 32-bit on the
ELF-scatter targets (aurix-tc375, sim32). ELF64 program headers
truncate before the segment guards run, so filesz > UINT32_MAX is
never true and an out-of-range paddr wraps in-range. Use uint64_t to
match wolfBoot_check_flash_image_elf so the guards see untruncated
values.
panic() returns under UNIT_TEST, so the LoadImage-failure site fell
through to StartImage on a failed load. Add the return to match the
zero-size guard; on target panic() never returns, so behavior is
unchanged.
The ELF scatter destination is the exec region, which sits outside the
boot partition that stores the signed ELF. Bounding it to the boot
partition rejected every legitimate segment (aurix exec is below boot,
sim scatter is above it) and bricked corruption recovery. The paddr is
covered by the image signature verified before this restore path and the
overflow check keeps it from wrapping, so no destination bound is needed;
this also matches the check function, which bounds no destination.
If the snap allocation fails but the work allocation succeeds, the
initialisation loop never runs and nsc_tmpl_free() would release
indeterminate work[].pValue pointers. Zero work[] right after the
allocation (before the NULL check) so every path that reaches the
free hands out initialised, NULL pValue entries.
The H7 OTP memory is one-time programmable, so the keystore and UDS
are permanent once written; the H7 has no OTP block-lock register
(unlike the H5), so there is no write-protection step to perform.
Replace the misleading TODO with the reason the no-op is correct.
keystore_get_size() returns -1 on invalid or oversized OTP slot
data; storing it in uint16_t made -1 become 65535, which passed the
hdrSz <= 0 check and was fed to the ECC/RSA parser as a 65535-byte
read from the keystore buffer. Keep it as int and reject values
<= 0 or above KEYSTORE_PUBKEY_SIZE.
wolfPKCS11/PSA_Store_Read/Write added int len to unsigned
in_buffer_offset before validation: a sufficiently negative len
wrapped, hit the truncation branch, and was replaced by the remaining
object or capacity bytes, bypassing Write's later len < 0 guard.
Reject len < 0 first in all four functions (post-clamp guard kept).
get_sha_block() accepted offset == fw_size and always read
WOLFBOOT_SHA_BLOCK_SIZE bytes; wolfBoot_peek_image() always reported
the full block size. Both could hand callers a window past the end
of the image. Reject offset >= fw_size, clamp the external read to
the bytes remaining, and report the clamped size (0 when no bytes
remain). Hash callers already clamp their input, so this is API
hardening; pinned by test_peek_image_bounds (internal + ext cases).
wolfBoot_load_flash_image_elf() ignored read_flash_fwimage failures
and used the program-header fields unchecked, so a failed read
consumed indeterminate stack data and a malformed (but signed) ELF
could drive an out-of-bounds source read and erase/write at an
unintended destination.
Check the ELF header and program header reads, then validate each
PT_LOAD segment before copying: file_size fits a 32-bit length, the
source stays inside the manifest image, the paddr range fits the
destination address width, and the destination stays inside the boot
partition (mirrors the sibling check-function validation). Check the
copy result.
New unit tests in unit-image-elf-scatter.c cover the load path: a
valid restore (positive control) and rejections for source past
fw_size, paddr range overflow, destination outside the boot
partition, and a program header that cannot be read. The first three
fail pre-fix (the mock flash layer also catches the two
out-of-range destinations with its own address check); the phdr
read-failure case consumed uninitialized stack data pre-fix
(valgrind: conditional jump on uninitialised value at the
is_loadable check).
wc_ecc_rs_raw_to_sig() takes a word32* outlen, but the wolfHSM
client/server path in wolfBoot_verify_signature_ecc() declared the
buffer length as size_t and cast the pointer. On a 64-bit
big-endian target the API reads the high (zero) half, so the DER
conversion sees outlen 0 and the write-back lands in the wrong
half of the size_t.
Declare tmpSigSz as word32 and pass &tmpSigSz directly; this also
matches the word32 sigLen of wc_ecc_verify_hash() below.
Add a host compile check for the WOLFBOOT_ENABLE_WOLFHSM_CLIENT
build of image.c, which unit CI did not cover (only the PIC32CZ
cross build).
The PCR-extension block in wolfBoot_unlock_disk() is guarded with
!defined(ARCH_SIM) while the function itself only builds for
ARCH_SIM, so it can never compile. The exclusion is deliberate
(eb2978ab: do not extend the unseal PCR on the simulator, or the
secret becomes un-unsealable), not an oversight.
Clarify the comment with the intended build scope instead of
removing the block: the code and the WOLFBOOT_NO_UNSEAL_PCR_EXTEND
option exist for the day the unlock-disk path is ported to a
non-sim target.
boot_x86_64.c declared x86_64_efi_do_boot(uint8_t *) while the HAL
defines (uint32_t *, uint8_t *): the linker connected the incompatible
pair and the call was undefined behavior, mostly latent because the
second parameter was discarded. Match the AArch64 sibling: single
const uint32_t *boot_addr in the declaration, definition and call,
and remove the unused dts_address parameter.
The UEFI MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid
byte), but x86_64_efi_do_boot() set it to boot_addr + size, describing
every image as one byte longer than it is; the AArch64 sibling already
uses size - 1. A zero-size image would underflow that computation and
hand an empty range to LoadImage, so reject it up front, as the
AArch64 sibling does. The local panic() returns to the unit test under
UNIT_TEST so the zero-size path is observable.
The WOLFBOOT_FSP low-memory size check was the only per-slot rejection
in the boot retry loop that used a bare break, so an image whose
header-declared fw_size exceeded the tolum window aborted the boot
instead of trying the other slot. Every other rejection in the same
loop switches partitions and retries; match that.
main() loaded a file of any size and parsed it as a manifest: header
fields (size, TLVs) were read past the end of the heap allocation, and
a header claiming a larger fw_size drove the image hash over an
unbounded range. Reject files smaller than IMAGE_HEADER_SIZE before
parsing and clamp fw_size to the bytes actually loaded.
STAGE1_AUTH only authenticates the stage2 wolfBoot payload; the FSP-M and
FSP-S blobs are executed unverified. Remove the dead fsp_m/ret
declarations in start(), the orphaned .sig_fsp_s placeholder section
(no stage1 linker script places it), and correct the comment that
claimed the FSPs were authenticated. Add a compile check for the
STAGE1_AUTH variant (unit-x86-fsp-stage1auth-build.py), which the unit
test CI never builds for lack of an i686 toolchain.
Verification: full unit suite 1096 checks, 0 failures; both
STAGE1_AUTH variants of boot_x86_fsp.c compile clean; no references to
sig_fsp_s remain.
In the DISABLE_BACKUP branch of wolfBoot_update() the ELF-scatter restore
block passed the boot struct by value to the pointer-taking PART_IS_EXT
macro, so DISABLE_BACKUP + WOLFBOOT_ELF_FLASH_SCATTER + EXT_FLASH did not
even compile, and the load result was discarded. Mirror the
wolfBoot_start() pattern (PART_IS_EXT(&boot), panic on load failure), drop
the dead base local, and add a compile check for the combination
(unit-elf-scatter-db-build.py) guarding the one test that needs a
DISABLE_BACKUP-excluded symbol.
Verification: full unit suite 1096 checks, 0 failures; new test fails
pre-fix (struct vs pointer compile error), passes post-fix.
The per-sector fill loop in wolfBoot_delta_update() advances in
DELTA_BLOCK_SIZE steps, so a WOLFBOOT_SECTOR_SIZE that is not a multiple
of DELTA_BLOCK_SIZE writes past the one-sector SWAP partition and
misaligns the resume path. Enforce the invariant with a #error and add a
negative build test (unit-delta-sector-align.py).
Verification: full unit suite 1096 checks, 0 failures; new test fails
pre-fix (misaligned config built), passes post-fix (build rejected).
PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that
test_noramboot_ext_flash_short_read_rejected set the shared mock
globals mock_ext_flash_short_len/mock_ext_flash_short_bytes and only
cleared them after the ck_assert. The suite runs CK_NOFORK, so a
failing assertion longjmps out of the test and leaves every full-size
ext_flash_read truncated for test_noramboot_highversion_rollback_denied,
which then fails for an unrelated reason.
Clear the mock globals immediately after wolfBoot_start() returns,
before the assertion: the assert only checks wolfBoot_staged_ok, which
is set during wolfBoot_start(), so the ordering changes nothing about
what is tested and the mock state can no longer leak into the next
test on a failure path.
Verification: full unit suite in wolfboot-ci-sim (make -C
tools/unit-tests; make run) exit 0; unit-update-ram-noramboot 3/3.
The F-12065 fix compared the int return of ext_flash_read() directly
against the uint32_t os_image.fw_size. That int-vs-unsigned comparison
triggers -Wsign-compare, which is a hard error under the default
-Werror -Wextra for every EXT_FLASH+NO_XIP update_ram target (e.g.
zynqmp) at -O0 and on host x86_64 gcc.
Check the error range explicitly and cast for the size comparison,
matching the established pattern in src/disk_fs.c (ret < 0 check
followed by (uint32_t)ret != len). Semantics are unchanged: negative
returns and positive short reads are both rejected.
Verified: host gcc -Werror -Wextra -fsyntax-only warns on the old
line and is clean on this one; unit-update-ram-noramboot 3/3.
When the bad-block marker is found on the block's second page, the
first page has already been copied to the caller's buffer and counted
in pos; the skip advanced only the source address, so the bad block's
page stayed in the output and the read returned len with the bad
block's content mixed into the image.
Record the output position at the start of each erase block and, on a
bad block, rewind both pos and the data pointer to it before advancing
the source address. This preserves the data = original + pos
invariant (no out-of-bounds write) and discards the bad block's
delivered pages.
test_bad_marker_second_page_dropped now asserts the bad block is fully
skipped (output starts at the next block's first page); it fails
against the previous code.
95227f82 (fdt: rewrite device tree parser with capacity bound and full
validation) changed wolfBoot_get_dts_size() to take a capacity argument
but left the mock in unit-update-disk-fs.c with the old 1-arg
signature, so the test no longer compiles (conflicting types). Update
the mock; behavior is unchanged (always -1, no DTS in this test).
PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that
test_aligned_page_multiple_write_xip filled its 512-byte source at
offset 3 * FLASH_PAGE_SIZE (768) of the 1024-byte flash model, so
both the fixture and the HAL's XIP staging read 256 bytes past the
modeled region; it only passed because mmap rounds the mapping up to
a full page.
Move the source to pages 2-3 (offset 2 * FLASH_PAGE_SIZE) with the
destination at pages 0-1: non-overlapping, fully inside the model.
Verification: unit-rp2350-flash-write 6/6, full unit suite green.
PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that the
register offsets introduced by the F-12104 fix are wrong, and the
Linux kernel's Intel PCH SPI driver (drivers/spi/spi-intel.c)
confirms it:
FDATA(n) = 0x10 + 4n -> 0x48 is FDATA14, a scratch data register
FRACC = 0x50 -> not FREG0
FREG(n) = 0x54 + 4n -> FREG0 = 0x54, FREG1 = 0x58
FPR0-4 = 0x84-0x9C (BXT/CNL protection-range base)
Two consequences. First, the BIOS range source: Intel flash region
numbering is region 0 = flash descriptor, region 1 = BIOS, and the
kernel driver's partition code reflects that ("start from the
mandatory descriptor region", then iterate FREG(1..)). The original
pre-F-12104 code read FREG1 (0x58) for the BIOS range and was right
on that point; the F-12104 fix regressed it to FREG0 (0x50), which
is the FRACC register. Restore FREG1.
Second, FPR0: the original 0x48 came from the buggy PCI-config-space
write path and is a FDATA scratch register in the SPIBAR map, so the
readback check passed on a register that never programs protection.
FPR0 is 0x84 (BXT/CNL PR base; JSL is not in the kernel's platform
table but follows the same-generation layout). HSFSTS_CTL at 0x04
and the RPE (bit 15) / WPE (bit 31) / base / limit fields match the
kernel's PR_ definitions and are unchanged.
unit-kontron-tgl-spi.c: mirror the corrected offsets (FREG0 decoy at
0x54, FREG1 BIOS source at 0x58, FPR0 at 0x84) and assert FPR0
carries the FREG1 range; the test runs the real extracted
tgl_lock_bios_region(), so it fails if the HAL offsets drift.
Verification: unit-kontron-tgl-spi 4/4, full unit suite green, sim
build green, cstyle clean on changed hunks.
The Kontron TGL SPI regression test kept the MMIO shadow in a
uint8_t array and reached into it through uint32_t * casts. The
accesses are all 32-bit at 4-aligned register offsets (0x04, 0x48,
0x50, 0x58), so the model is now a uint32_t array indexed by
offset/4: no casts, no alignment or strict-aliasing doubt, and the
redundant byte-clear before the 32-bit FREG0 store is gone.
Verification: unit-kontron-tgl-spi 4/4, full unit suite green,
cstyle clean.
pkcs11_pin is pre-populated from the compile-time credential
(ENCRYPT_PKCS11_PIN), so the RAM copy exists from image load, not
from a successful C_Login. pkcs11_crypto_deinit() only wiped it
inside the encrypt_initialized branch, so on a target where init
never completed the credential stayed in retained memory after the
pre-handoff path ran.
Move pkcs11_pin_wipe() out of the branch: the token interaction
(C_CloseSession) stays conditional on an established session, the
credential wipe is unconditional. deinit only runs on the terminal
pre-handoff paths, so this cannot break the init retry in
wolfBoot_initialize_encryption, which runs at decryption time,
well before handoff.
test_pkcs11_deinit_no_session now re-populates the pin and asserts
every byte is zero after deinit without init (plus no C_CloseSession
and repeat-call safety). Pre-fix it failed with "pkcs11_pin byte 0
not wiped" (1/2); post-fix 2/2.
Verification: unit-pkcs11-pin-zeroize 2/2, full unit suite green,
sim build green, cstyle clean on changed hunks.
pkcs11_crypto_deinit() - the only caller of pkcs11_pin_wipe() - was
invoked from the update_flash path alone (src/update_flash.c:1715).
On the RAMBOOT, hwswap and disk pre-handoff paths the PKCS#11 login
credential stayed in retained bootloader memory after handoff.
Add the same #ifdef ENCRYPT_PKCS11 deinit block after the WOLFHSM
cleanup in src/update_ram.c, src/update_flash_hwswap.c and
src/update_disk.c, in the same position as the existing update_flash
call (before hal_flash_protect/hal_prepare_boot). update_ram.c and
update_flash_hwswap.c did not include encrypt.h, where
pkcs11_crypto_deinit() is declared - add the include (update_flash.c
and update_disk.c already had it). The deinit is a no-op when crypto
was never initialized, so the calls are safe on every build.
Verification: full build with PKCS11 enabled (sim config +
CFLAGS_EXTRA: ENCRYPT_PKCS11, EXT_ENCRYPTED, EXT_FLASH,
WOLFCRYPT_SECURE_MODE, SECURE_PKCS11, WOLFPKCS11_USER_SETTINGS +
mechanism/sizes/PIN) compiles all sources cleanly; the link stops on
pre-existing externals (token library + secure-mode wolfssl objects
that a real target's link config supplies) - a control build of the
unpatched tree fails identically with the same undefined-symbol set.
sim and kontron_vx3060_s2 builds green (PKCS11 disabled, hunks
inactive). cstyle clean on the changed hunks.
pkcs11_pin is a file-scope copy of the compile-time credential
passed to C_Login() for the token holding the firmware-decryption
key. pkcs11_crypto_deinit() runs on the pre-handoff path but only
closed the session, leaving the credential in retained bootloader
memory where a post-handoff attacker could recover it and
authenticate to the token.
Wipe the copy (volatile zeroize) after the final C_CloseSession().
No re-init path exists after deinit in the product flow (init is
only called from the verification paths), so wiping inside the
deinit is safe.
Add unit-pkcs11-pin-zeroize: a full init/deinit cycle with a
stubbed PKCS#11 backend that asserts the pin copy is all zero
after deinit and the session was closed, plus a no-session deinit
safety case.
Verification: unit-pkcs11-pin-zeroize 1/2 pre-fix (pin byte 0 not
wiped), 2/2 post-fix; full unit suite green; sim build green;
kontron_vx3060_s2 CI build green.
pci_program_bridge() used orig_cmd both as the saved COMMAND
register value and as the accumulator for the decode bits enabled
while programming. Error paths after a window was programmed
(post-enum MMIO or IO alignment failures) restored that mutated
value and left the programmed bridge windows active, so the bridge
kept decoding address ranges the allocator rollback had just
returned.
Keep the two values separate: saved_cmd holds the original
register content for the error path, new_cmd accumulates the
decode bits and is written on success (seeded from saved_cmd, so
the success path preserves the bits it did not manage, exactly as
before). The error path now disables every bridge window
(prefetch, MMIO, IO) before restoring saved_cmd.
Test: test_program_bridge_oom_late_restore programs a prefetch
window behind the bridge, then exhausts the MMIO pool so the
post-enum MMIO alignment fails. Pre-fix the restored COMMAND was
0x0006 (original 0x0004 plus the MEM_SPACE bit for the discarded
window) and the prefetch window stayed programmed
(0x9000-0x900F); post-fix the original COMMAND is restored and
all windows are disabled.
mb2_build_boot_info_header() emitted the requested tags and set
total_size without appending the Multiboot2 end tag (type 0,
size 8). A strict consumer walking the output section sees no
terminator inside total_size (or a zero-sized pseudo-terminator
only if the destination buffer happens to be zero-filled), so
the handoff is structurally invalid.
Reserve eight bytes for the end tag after the requested tags,
write {type 0, flags 0, size 8}, and include it in total_size.
The write is bounds-checked against the caller's max_size like
the other tag builders, and idx is already 8-byte aligned since
every tag size is a multiple of 8.
Tests: the existing layout assertions now expect the end tag
inside total_size (basic mem info: 24 -> 32, mem map with one
entry: 48 -> 56), and a new consumer-style test walks the
generated output section the way a strict Multiboot2 consumer
would: both requested tags found, well-formed end tag (type 0,
size 8), and the end tag is the last structure in total_size.
Pre-fix: 3 failures (total_size short by 8, walk finds no end
tag); post-fix: 36/36.
The non-RAMBOOT copy-to-RAM path only rejected negative
ext_flash_read() results. Backends return the number of bytes
read on success (filesystem.c forwards XFREAD's count), so a
positive short read was accepted and boot continued with a
truncated RAM image.
Require the read to return exactly os_image.fw_size; any other
result aborts the boot, matching the header-copy check already
in the same file (ret != IMAGE_HEADER_SIZE).
Note: the RAMBOOT image-load path (WOLFBOOT_USE_RAMBOOT) has the
same `ret < 0` pattern on its img_size read; outside this
finding's scope, left as-is.
Test: unit-update-ram-noramboot gains a short-read case (mock
ext_flash_read withholds 1 byte from the full-image copy only).
Pre-fix the truncated image staged for boot (staged_ok == 1);
post-fix the boot is aborted with "Error loading image ...
(ret 5299)".
tgl_lock_bios_region() wrote the protected range and the FLOCKDN
value through PCI configuration space (offsets 0x48 and 0x04, the
status/command dword) instead of the SPI controller's
memory-mapped registers at the BAR0 base, and took the range from
FREG1 (non-BIOS) instead of FREG0 (BIOS). The lock now writes
FPR0 and BIOS/H SFSTS/CTL through mmio_write32(), verifies both
by readback, and returns an error if the bits do not stick.
The helper had no callers: no hal_flash_protect() override
existed, so the weak no-op default ran before handoff and the
BIOS region stayed writable. Add the override routing to
tgl_lock_bios_region().
Including <hal.h> for the hook signature also exposes the
hal_flash_write/hal_flash_erase stubs as mismatching the HAL
contract; fix their address parameter to haladdr_t.
Add unit-kontron-tgl-spi: runs the extracted
tgl_lock_bios_region() and hal_flash_protect() against mocked
PCI config space and an MMIO array at the BAR address (build
fails pre-fix - hal_flash_protect undefined - 4/4 pass
post-fix).
ext_flash_read() initialized its bad-block page counter once per
request, so the marker was inspected only on the first two pages
read and a bad erase block later in the request was delivered as
valid data. Restart the counter at the start of each erase block.
The skip path also rewound the logical position to a block
boundary without rewinding the output pointer, so a marker found
after some pages had been delivered continued the read past the
end of the caller's buffer. pos and data already agree
(data = original + pos) after any delivered pages, so the skip
only advances the source address.
Add unit-p1021-read-badblock: runs the extracted
ext_flash_read() against a mocked ELBC on a simulated NAND with
three 16 KiB blocks (2/5 checks fail pre-fix, 5/5 pass
post-fix).
The double-word fast path was selected on 'len - i > 3' but always
reads and programs two 32-bit words, so an aligned 4-7 byte tail
read up to four bytes past the caller's buffer and programmed them
into flash. Require at least eight remaining bytes; shorter tails
fall to the RMW branch, which rewrites the unit with the
out-of-range bytes read back from flash.
Add unit-stm32wb-write: runs the extracted hal_flash_write()
against a host register file with stale destination flash and a
source canary (3/5 checks fail pre-fix, 5/5 pass post-fix).
The double-word fast path of hal_flash_write() was selected on
"len - i > 3" but always reads and programs two 32-bit words, so
an aligned 4-7 byte tail read up to four bytes past the caller's
buffer and programmed them into flash. Require at least eight
remaining bytes before taking the fast path; shorter tails fall to
the RMW branch, which rewrites the unit with the out-of-range bytes
read back from flash. Same fix as the STM32G4 twin (F-11023).
Add unit-stm32l4-write: runs the extracted hal_flash_write()
against a host register/flash model with a canary after the source
(3/5 checks fail pre-fix, 5/5 pass post-fix).
flash_range_program() requires a page-aligned address and a
page-multiple length (pico-sdk ROM, invalid_params_if on both). The
partition-state path without NVM_FLASH_WRITEONCE issues 1-byte
(trailer) and 4-byte (magic) writes that violated the contract on
every state transition.
Keep the direct-program fast path for page-aligned page-multiple
writes; otherwise read the page back from XIP, merge the write, and
program the full page. The AND program keeps the trailer flag
accumulation intact.
Add unit-rp2350-flash-write: runs the extracted hal_flash_write
against a mock flash_range_program() that enforces the ROM contract
(4/6 checks fail pre-fix, 6/6 pass post-fix).
Daniele Lacamera
David Garske
Brett Nicholas
Marco Oliverio