From 21c82fa4d881f6392ac0c6c016dea81f40bafe71 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 04:27:28 +0200 Subject: [PATCH 01/20] F-12883: nRF5340 uart_write CRLF dropped multiline tail data After emitting CRLF the buffer advanced to the newline, not the byte after it, so multiline input reprocessed the newline (stray CRLFs) and lost the following text. Extract the CRLF conversion to hal/nrf5340_uart.c (no nrfx) for host unit testing; add a regression test. --- CMakeLists.txt | 9 +- hal/nrf5340.c | 25 ++--- hal/nrf5340_uart.c | 56 +++++++++++ tools/unit-tests/Makefile | 4 + tools/unit-tests/unit-nrf5340-uart-crlf.c | 108 ++++++++++++++++++++++ 5 files changed, 182 insertions(+), 20 deletions(-) create mode 100644 hal/nrf5340_uart.c create mode 100644 tools/unit-tests/unit-nrf5340-uart-crlf.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c753d0baa..74a68b7dd0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1156,8 +1156,15 @@ if(TZEN) endif() endif() +# nRF5340 debug-UART CRLF conversion (host-testable, no nrfx registers) +set(WOLFBOOT_NRF5340_UART_SRC "") +if(WOLFBOOT_TARGET MATCHES "^nrf5340") + set(WOLFBOOT_NRF5340_UART_SRC hal/nrf5340_uart.c) +endif() + target_sources(wolfboothal PRIVATE include/hal.h hal/hal.c hal/${WOLFBOOT_TARGET}.c ${WOLFBOOT_FLASH_SOURCES} - ${PARTITION_SOURCE} ${WOLFBOOT_TZ_HAL_SOURCES}) + ${PARTITION_SOURCE} ${WOLFBOOT_TZ_HAL_SOURCES} + ${WOLFBOOT_NRF5340_UART_SRC}) #--------------------------------------------------------------------------------------------- diff --git a/hal/nrf5340.c b/hal/nrf5340.c index 64ae0611de..132e6b79ef 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -282,27 +282,14 @@ void uart_write_sz(const char* c, unsigned int sz) } } +/* CRLF conversion lives in nrf5340_uart.c so it can be unit-tested on the + * host without the nrfx register access the rest of this HAL needs. */ +void nrf5340_uart_crlf(const char* buf, unsigned int sz, + void (*sink)(const char*, unsigned int)); + void uart_write(const char* buf, unsigned int sz) { - const char* line; - unsigned int lineSz; - do { - /* find `\n` */ - line = memchr(buf, '\n', sz); - if (line == NULL) { - uart_write_sz(buf, sz); - break; - } - lineSz = line - buf; - if (lineSz > sz-1) - lineSz = sz-1; - - uart_write_sz(buf, lineSz); - uart_write_sz("\r\n", 2); /* handle CRLF */ - - buf = line; - sz -= lineSz + 1; /* skip \n, already sent */ - } while ((int)sz > 0); + nrf5340_uart_crlf(buf, sz, uart_write_sz); } #endif /* DEBUG_UART */ diff --git a/hal/nrf5340_uart.c b/hal/nrf5340_uart.c new file mode 100644 index 0000000000..f0febf1043 --- /dev/null +++ b/hal/nrf5340_uart.c @@ -0,0 +1,56 @@ +/* nrf5340_uart.c + * + * CRLF line conversion for the nRF5340 debug UART, split out of + * hal/nrf5340.c so the newline handling can be unit-tested on the host + * without the nrfx register access the rest of that HAL needs. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfBoot; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1335, USA + */ + +#ifdef DEBUG_UART + +#include + +/* Emit buf[0..sz) via "sink" with every '\n' rendered as CRLF. */ +void nrf5340_uart_crlf(const char* buf, unsigned int sz, + void (*sink)(const char*, unsigned int)) +{ + const char* line; + unsigned int lineSz; + do { + /* find '\n' */ + line = memchr(buf, '\n', sz); + if (line == NULL) { + sink(buf, sz); + break; + } + lineSz = (unsigned int)(line - buf); + if (lineSz > sz - 1) + lineSz = sz - 1; + + sink(buf, lineSz); + sink("\r\n", 2); /* handle CRLF */ + + buf = line + 1; /* advance past the emitted newline */ + sz -= lineSz + 1; + } while ((int)sz > 0); +} + +#endif /* DEBUG_UART */ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 9306819684..001d1ef5f9 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -184,6 +184,7 @@ endif TESTS+=unit-flash-write-mcxa TESTS+=unit-flash-write-nrf52 +TESTS+=unit-nrf5340-uart-crlf TESTS+=unit-flash-write-samr21 TESTS+=unit-flash-write-same51 TESTS+=unit-imx-rt-cache-align @@ -1652,6 +1653,9 @@ unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c unit-flash-write-nrf52: unit-flash-write-nrf52.c ../../hal/nrf52.c gcc -o $@ unit-flash-write-nrf52.c -DTARGET_nrf52 -I../../hal $(CFLAGS) $(LDFLAGS) +unit-nrf5340-uart-crlf: unit-nrf5340-uart-crlf.c ../../hal/nrf5340_uart.c + gcc -o $@ unit-nrf5340-uart-crlf.c -DDEBUG_UART $(CFLAGS) $(LDFLAGS) + unit-flash-write-samr21: unit-flash-write-samr21.c ../../hal/samr21.c gcc -o $@ unit-flash-write-samr21.c $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-nrf5340-uart-crlf.c b/tools/unit-tests/unit-nrf5340-uart-crlf.c new file mode 100644 index 0000000000..e808663f03 --- /dev/null +++ b/tools/unit-tests/unit-nrf5340-uart-crlf.c @@ -0,0 +1,108 @@ +/* unit-nrf5340-uart-crlf.c + * + * Regression test for F-12883: nrf5340_uart_crlf() (hal/nrf5340_uart.c, + * extracted from hal/nrf5340.c uart_write) advanced the buffer pointer to + * the newline instead of the byte after it after emitting CRLF, while + * shrinking the size as if the newline had been consumed. On multiline input + * it reprocessed the newline (emitting extra CRLFs) and dropped the text + * that followed it: "abc\ndef\n" came out as "abc\r\n" plus four stray + * CRLFs, with "def" lost. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfBoot; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1335, USA + */ + +#include +#include +#include + +#include "../../hal/nrf5340_uart.c" + +static char cap[256]; +static int caplen; + +static void sink(const char* c, unsigned int sz) +{ + memcpy(cap + caplen, c, sz); + caplen += (int)sz; +} + +static void reset_cap(void) +{ + caplen = 0; + cap[0] = '\0'; +} + +START_TEST(test_crlf_multiline) +{ + reset_cap(); + nrf5340_uart_crlf("abc\ndef\n", 8, sink); + /* both lines preserved, each CRLF-terminated, nothing dropped */ + ck_assert_str_eq(cap, "abc\r\ndef\r\n"); +} + +START_TEST(test_crlf_single_line_no_nl) +{ + reset_cap(); + nrf5340_uart_crlf("hello", 5, sink); + ck_assert_str_eq(cap, "hello"); +} + +START_TEST(test_crlf_single_line_with_nl) +{ + reset_cap(); + nrf5340_uart_crlf("hello\n", 6, sink); + ck_assert_str_eq(cap, "hello\r\n"); +} + +START_TEST(test_crlf_leading_nl) +{ + reset_cap(); + nrf5340_uart_crlf("\nabc", 4, sink); + ck_assert_str_eq(cap, "\r\nabc"); +} + +START_TEST(test_crlf_consecutive_nl) +{ + reset_cap(); + nrf5340_uart_crlf("a\n\nb\n", 5, sink); + ck_assert_str_eq(cap, "a\r\n\r\nb\r\n"); +} + +int main(void) +{ + Suite* s; + TCase* tc; + SRunner* sr; + int failed; + + s = suite_create("nrf5340-uart-crlf"); + tc = tcase_create("crlf"); + tcase_add_test(tc, test_crlf_multiline); + tcase_add_test(tc, test_crlf_single_line_no_nl); + tcase_add_test(tc, test_crlf_single_line_with_nl); + tcase_add_test(tc, test_crlf_leading_nl); + tcase_add_test(tc, test_crlf_consecutive_nl); + suite_add_tcase(s, tc); + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + failed = srunner_ntests_failed(sr); + srunner_free(sr); + return (failed == 0) ? 0 : 1; +} From bad0fa3fed1c85d5f578f4632fe33ff5b5ca9f68 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 10:30:36 +0200 Subject: [PATCH 02/20] F-13635: isolate each diag_read_header content gate The one negative test corrupted byte 0, tripping the magic and CRC gates at once, so a deleted gate was still caught by its siblings. Add three tests (magic/version/crc), each recomputing the header CRC so only the corrupted field's gate can reject. --- tools/unit-tests/unit-diagnostics.c | 61 +++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tools/unit-tests/unit-diagnostics.c b/tools/unit-tests/unit-diagnostics.c index 84f57a044b..5cbdc36c8d 100644 --- a/tools/unit-tests/unit-diagnostics.c +++ b/tools/unit-tests/unit-diagnostics.c @@ -174,6 +174,64 @@ START_TEST(test_crc_rejection) } END_TEST +/* F-13635: diag_read_header() has three independent content gates after the + * read gate - magic, format_version, and CRC. The pre-existing negative test + * corrupts byte 0, which trips the magic and CRC gates at once, so it cannot + * isolate a single gate: deleting any one of them would still be caught by + * the others. These three tests isolate each gate. Each corrupts one field + * and then recomputes the header CRC over the 12-byte header, so the CRC gate + * stays satisfied and only the corrupted field's gate can reject. With that + * gate deleted the header would be accepted and the count would stay at 1. */ +START_TEST(test_diag_header_gate_magic) +{ + struct wolfBoot_diag_header *hdr; + + diag_mmap("/tmp/wolfboot-unit-diag-gate-magic.bin"); + ck_assert_int_eq(wolfBoot_clear_failures(), 0); + record_one(WOLFBOOT_FAILURE_PHASE_UPDATE, + WOLFBOOT_FAILURE_CAUSE_HASH, PART_UPDATE, 1); + ck_assert_int_eq(wolfBoot_get_failure_count(), 1); + + hdr = (struct wolfBoot_diag_header *)(uintptr_t)DIAG_SECTOR_ADDR(0); + hdr->magic = 0xDEADBEEFUL; + hdr->crc = diag_crc32(hdr, 12); + ck_assert_int_eq(wolfBoot_get_failure_count(), 0); +} +END_TEST + +START_TEST(test_diag_header_gate_version) +{ + struct wolfBoot_diag_header *hdr; + + diag_mmap("/tmp/wolfboot-unit-diag-gate-version.bin"); + ck_assert_int_eq(wolfBoot_clear_failures(), 0); + record_one(WOLFBOOT_FAILURE_PHASE_UPDATE, + WOLFBOOT_FAILURE_CAUSE_HASH, PART_UPDATE, 1); + ck_assert_int_eq(wolfBoot_get_failure_count(), 1); + + hdr = (struct wolfBoot_diag_header *)(uintptr_t)DIAG_SECTOR_ADDR(0); + hdr->format_version = 99U; + hdr->crc = diag_crc32(hdr, 12); + ck_assert_int_eq(wolfBoot_get_failure_count(), 0); +} +END_TEST + +START_TEST(test_diag_header_gate_crc) +{ + struct wolfBoot_diag_header *hdr; + + diag_mmap("/tmp/wolfboot-unit-diag-gate-crc.bin"); + ck_assert_int_eq(wolfBoot_clear_failures(), 0); + record_one(WOLFBOOT_FAILURE_PHASE_UPDATE, + WOLFBOOT_FAILURE_CAUSE_HASH, PART_UPDATE, 1); + ck_assert_int_eq(wolfBoot_get_failure_count(), 1); + + hdr = (struct wolfBoot_diag_header *)(uintptr_t)DIAG_SECTOR_ADDR(0); + hdr->crc = diag_crc32(hdr, 12) + 1U; + ck_assert_int_eq(wolfBoot_get_failure_count(), 0); +} +END_TEST + START_TEST(test_clear) { struct wolfBoot_failure_record rec; @@ -257,6 +315,9 @@ Suite *wolfboot_suite(void) tcase_add_test(diag, test_record_and_read_newest_first); tcase_add_test(diag, test_ring_wrap_and_ordering); tcase_add_test(diag, test_crc_rejection); + tcase_add_test(diag, test_diag_header_gate_magic); + tcase_add_test(diag, test_diag_header_gate_version); + tcase_add_test(diag, test_diag_header_gate_crc); tcase_add_test(diag, test_clear); tcase_add_test(diag, test_torn_write_recovery); suite_add_tcase(s, diag); From f1aa75afebee2a7ffae5d7da209a5189621aa532 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 10:34:16 +0200 Subject: [PATCH 03/20] F-13633: test encrypt_key_is_valid erased-key rejection encrypt_key_is_valid() rejects an erased key (all 0x00 or all 0xFF) but had no test, so deleting the check or flipping its && to || survived. Add three cases (all-0x00, all-0xFF, valid); both erased cases fail under ||, which pins the &&. --- tools/unit-tests/unit-enc-nvm.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tools/unit-tests/unit-enc-nvm.c b/tools/unit-tests/unit-enc-nvm.c index 435cebe78a..7cf90e7baf 100644 --- a/tools/unit-tests/unit-enc-nvm.c +++ b/tools/unit-tests/unit-enc-nvm.c @@ -348,6 +348,31 @@ START_TEST(test_erase_encrypt_key_propagates_flash_write_error) } END_TEST +/* F-13633: encrypt_key_is_valid() rejects an erased key (all 0x00 or all + * 0xFF) and accepts a key that is neither. It was untested, so deleting the + * check or flipping its && to || would survive. Under a ||, both the all-0x00 + * and all-0xFF cases have one true operand and would be (wrongly) accepted, + * which pins the &&. */ +START_TEST(test_encrypt_key_is_valid) +{ + uint8_t key[ENCRYPT_KEY_SIZE]; + uint32_t i; + + for (i = 0; i < ENCRYPT_KEY_SIZE; i++) + key[i] = 0x00; + ck_assert_int_eq(encrypt_key_is_valid(key, ENCRYPT_KEY_SIZE), 0); + + for (i = 0; i < ENCRYPT_KEY_SIZE; i++) + key[i] = 0xFF; + ck_assert_int_eq(encrypt_key_is_valid(key, ENCRYPT_KEY_SIZE), 0); + + for (i = 0; i < ENCRYPT_KEY_SIZE; i++) + key[i] = 0x00; + key[0] = 0x42; + ck_assert_int_eq(encrypt_key_is_valid(key, ENCRYPT_KEY_SIZE), 1); +} +END_TEST + Suite *wolfboot_suite(void) { @@ -361,6 +386,7 @@ Suite *wolfboot_suite(void) test_set_encrypt_key_propagates_flash_write_error); tcase_add_test(nvm_update_with_encryption, test_erase_encrypt_key_propagates_flash_write_error); + tcase_add_test(nvm_update_with_encryption, test_encrypt_key_is_valid); suite_add_tcase(s, nvm_update_with_encryption); return s; From bdfddc5fadd8acef92ffa9350364cf1ddf3417d1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 10:44:17 +0200 Subject: [PATCH 04/20] Add gpt_parse_header hdr_size bounds test (F-6759) The hdr_size < GPT_HDR_MIN_SIZE || > GPT_SECTOR_SIZE guard was untested; a crafted 0xFFFFFFFF hdr_size would drive the CRC read ~4GB past the 512-byte stack header. New test rejects both bounds; the lower case recomputes the CRC over the reduced size so a deleted lower clause would accept the header (return 0), pinning the clause. --- tools/unit-tests/unit-disk.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tools/unit-tests/unit-disk.c b/tools/unit-tests/unit-disk.c index c4d6bc4748..3e8d408894 100644 --- a/tools/unit-tests/unit-disk.c +++ b/tools/unit-tests/unit-disk.c @@ -314,6 +314,37 @@ START_TEST(test_gpt_parse_header) } END_TEST +/* F-6759: gpt_parse_header() must reject an out-of-range hdr_size before the + * header CRC pass. Without the upper bound (hdr_size > GPT_SECTOR_SIZE) a + * crafted 0xFFFFFFFF hdr_size would drive gpt_crc32_update() to read ~4GB + * past the 512-byte stack header; without the lower bound (< 0x5C) fields + * outside the CRC-protected region would be accepted. Both clauses were + * untested. The guard rejects before the CRC, so these assert -1 without + * triggering the OOB. */ +START_TEST(test_gpt_parse_header_hdr_size_bounds) +{ + struct guid_ptable hdr; + uint8_t *gpt_hdr; + + build_gpt_disk(); + gpt_hdr = (uint8_t *)(fake_disk + GPT_SECTOR_SIZE); + + /* hdr_size above GPT_SECTOR_SIZE: rejected before the CRC pass. Under an + * ASAN build this also flags the ~4GB OOB stack read the deletion would + * cause (a plain build still returns -1 via the CRC mismatch). */ + d_put32(gpt_hdr + D_HDR_SIZE, GPT_SECTOR_SIZE + 1); + ck_assert_int_eq(gpt_parse_header(gpt_hdr, &hdr), -1); + + /* hdr_size below 0x5C: rejected. The CRC is recomputed over the reduced + * size, so a valid-CRC header would be *accepted* if the lower clause + * were deleted - which pins the clause (the plain CRC mismatch would + * otherwise mask the deletion). */ + d_put32(gpt_hdr + D_HDR_SIZE, 0x5B); + finalize_gpt_header_crc(gpt_hdr); + ck_assert_int_eq(gpt_parse_header(gpt_hdr, &hdr), -1); +} +END_TEST + START_TEST(test_gpt_parse_partition) { struct gpt_part_info info; @@ -1125,6 +1156,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_gpt, test_gpt_check_mbr_protective); tcase_add_test(tc_gpt, test_gpt_parse_header); + tcase_add_test(tc_gpt, test_gpt_parse_header_hdr_size_bounds); tcase_add_test(tc_gpt, test_gpt_parse_partition); tcase_add_test(tc_gpt, test_gpt_part_name_eq); tcase_add_test(tc_gpt, test_gpt_part_name_eq_bom_boundary); From ba74601e91c9507247060683dc1c534a48fcb255 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 11:01:38 +0200 Subject: [PATCH 05/20] F-13646: test HDR_CMDLINE sign/decode roundtrip The --cmdline option stores the OS command line as a signature-covered TLV (tag HDR_CMDLINE) that the bootloader recovers via wolfBoot_find_header(); the roundtrip had zero coverage. Add test_make_header_ex_roundtrip_cmdline_tlv: for lengths 1, 2, 3, 69, 70, 71, 255 (odd/even + extremes + the ~70-byte default-capacity boundary) sign via make_header_ex and assert the decoder returns exactly the signed bytes; 255 also pins the 256->512 header auto-grow. --- tools/unit-tests/unit-sign-encrypted-output.c | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tools/unit-tests/unit-sign-encrypted-output.c b/tools/unit-tests/unit-sign-encrypted-output.c index c18e7b247a..62a51d7517 100644 --- a/tools/unit-tests/unit-sign-encrypted-output.c +++ b/tools/unit-tests/unit-sign-encrypted-output.c @@ -541,6 +541,84 @@ START_TEST(test_make_header_ex_roundtrip_custom_tlvs_via_wolfboot_parser) } END_TEST +/* F-13646: HDR_CMDLINE encode/decode roundtrip. The sign --cmdline option + * stores the OS command line as a signature-covered TLV with the reserved + * tag HDR_CMDLINE; the bootloader recovers it via wolfBoot_find_header() + * (wolfBoot_efi_get_cmdline). For every valid 1..255-byte command line the + * decoder must return exactly those bytes. Lengths 1, 2, 3, 69, 70, 71, 255 + * exercise the extremes and the odd/even boundary that forces the walker to + * byte-step over the ALIGN_8 padding between TLVs; 255 also forces the + * header to auto-grow past the 256-byte default (the ~70-byte default + * capacity is the common case, so a silent header/bootloader IMAGE_HEADER_ + * SIZE mismatch is the realistic failure). */ +START_TEST(test_make_header_ex_roundtrip_cmdline_tlv) +{ + char tempdir[] = "/tmp/wolfboot-sign-XXXXXX"; + char image_path[PATH_MAX]; + char output_path[PATH_MAX]; + uint8_t *output_buf = NULL; + uint8_t image_buf[] = { 0x01, 0x02, 0x03, 0x04 }; + uint8_t pubkey[] = { 0xA5 }; + uint8_t cmdline[255]; + size_t output_len; + uint16_t lens[] = { 1, 2, 3, 69, 70, 71, 255 }; + uint16_t i; + uint16_t j; + int ret; + + ck_assert_ptr_nonnull(mkdtemp(tempdir)); + + snprintf(image_path, sizeof(image_path), "%s/image.bin", tempdir); + snprintf(output_path, sizeof(output_path), "%s/output.bin", tempdir); + ck_assert_int_eq(write_file(image_path, image_buf, sizeof(image_buf)), + 0); + + /* Deterministic, non-zero pattern across the full 255 bytes. */ + for (j = 0; j < 255; j++) { + cmdline[j] = (uint8_t)(0x10 + (j % 240)); + } + + for (i = 0; i < sizeof(lens) / sizeof(lens[0]); i++) { + uint16_t len = lens[i]; + + reset_cmd_defaults(); + CMD.header_sz = 256; + CMD.custom_tlvs = 1; + CMD.custom_tlv[0].tag = HDR_CMDLINE; + CMD.custom_tlv[0].len = len; + CMD.custom_tlv[0].buffer = malloc(len); + memcpy(CMD.custom_tlv[0].buffer, cmdline, len); + + reset_mocks(NULL, 0); + ret = make_header_ex(0, pubkey, sizeof(pubkey), image_path, + output_path, 0, 0, 0, 0, NULL, 0, NULL, 0); + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(read_file(output_path, &output_buf, &output_len), 0); + ck_assert_uint_eq(output_len, CMD.header_sz + sizeof(image_buf)); + /* The decoder must recover exactly the signed bytes. */ + assert_header_bytes(output_buf, HDR_CMDLINE, cmdline, len); + + /* A 255-byte command line cannot fit the 256-byte default header + * (signature + TLV on top), so sign auto-grows it to 512. Short + * lines stay at the default. 69..71 are the borderline and are not + * asserted here. */ + if (len == 255) { + ck_assert_uint_eq(CMD.header_sz, 512); + } else if (len <= 3) { + ck_assert_uint_eq(CMD.header_sz, 256); + } + + free(output_buf); + output_buf = NULL; + free_custom_tlv_buffers(); + unlink(output_path); + } + + unlink(image_path); + rmdir(tempdir); +} +END_TEST + START_TEST(test_make_header_ex_roundtrip_finds_tlv_that_exactly_fills_header) { char tempdir[] = "/tmp/wolfboot-sign-XXXXXX"; @@ -758,6 +836,8 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase, test_header_append_helpers_emit_little_endian_bytes); tcase_add_test(tcase, test_make_header_ex_roundtrip_custom_tlvs_via_wolfboot_parser); + tcase_add_test(tcase, + test_make_header_ex_roundtrip_cmdline_tlv); tcase_add_test(tcase, test_make_header_ex_roundtrip_finds_tlv_that_exactly_fills_header); tcase_add_test(tcase, From ef4d73dea65c0239d7b058981b96150813abe6b8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 11:17:17 +0200 Subject: [PATCH 06/20] Add header_required_size cross-branch regression test (F-13644) The size model was only compared against make_header_ex for NO_SIGN, no-timestamp, non-delta. Drive the cross product of sign/policy/ timestamp/delta/dts/hash-algo and assert the writer's final content size stays <= the model, via a test hook that records header_idx before the 0xFF padding. Hybrid is excluded (its secondary signature needs a real key context). Mutation-verified: removing the model's timestamp term makes the test fail (idx > required). Also reject a custom TLV reusing the --dts digest tag (F-9754): it would serialize ahead of the real digest and shadow it. --- tools/keytools/sign.c | 22 +++ tools/unit-tests/unit-sign-encrypted-output.c | 171 ++++++++++++++++++ 2 files changed, 193 insertions(+) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 6a3d62a7f8..221f142e92 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1486,6 +1486,11 @@ static int dts_hash_file(const char *file, int hash_algo, uint8_t *out, return ret; } +/* Test hook: the content header_idx from the last successful make_header_ex() + * (recorded before the 0xFF padding), so unit tests can compare the writer + * against header_required_size() without the auto-grow exit(1) path. */ +static uint32_t test_last_header_idx; + static uint32_t header_required_size(int is_diff, uint32_t cert_chain_sz, uint32_t secondary_key_sz) { @@ -1766,6 +1771,21 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, /* Add custom TLVs */ if (CMD.custom_tlvs > 0) { uint32_t i; + /* A custom TLV reusing a built-in tag is serialized before the + * generated TLV and shadows it: wolfBoot_find_header() walks from the + * start and returns the first match. The device-tree digest (0x35) is + * the reserved tag reachable from --custom-tlv (tags >= 0x30); a + * custom 0x35 ahead of the --dts digest would make DTB verification + * use the operator-supplied value. Reject the collision. */ + for (i = 0; i < CMD.custom_tlvs; i++) { + if (CMD.dts_file != NULL && + CMD.custom_tlv[i].tag == HDR_DEVICE_TREE_DIGEST) { + fprintf(stderr, + "Error: custom TLV tag 0x%04x is reserved for --dts\n", + (unsigned)HDR_DEVICE_TREE_DIGEST); + goto failure; + } + } for (i = 0; i < CMD.custom_tlvs; i++) { /* require 8-byte alignment */ /* The offset '4' takes into account 2B Tag + 2B Len, so that the @@ -2316,6 +2336,8 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, } } /* end if(sign != NO_SIGN) */ + test_last_header_idx = header_idx; + /* Add padded header at end */ while (header_idx < CMD.header_sz) { header[header_idx++] = 0xFF; diff --git a/tools/unit-tests/unit-sign-encrypted-output.c b/tools/unit-tests/unit-sign-encrypted-output.c index 62a51d7517..8d8de172e8 100644 --- a/tools/unit-tests/unit-sign-encrypted-output.c +++ b/tools/unit-tests/unit-sign-encrypted-output.c @@ -824,6 +824,173 @@ START_TEST(test_make_header_ex_rejects_signature_tlv_length_overflow) } END_TEST +/* F-13644: header_required_size() is a hand-maintained shadow of the manifest + * layout make_header_ex() writes; the auto-grow block sizes the buffer from + * the model, so an under-counted branch makes header_append_tag() exit(1) + * after all the hashing/signing. The existing boundary tests only cover + * NO_SIGN/no-ts/non-delta. This drives the cross product of the untested + * branches (sign, policy, timestamp, delta, dts, hash algo) and asserts the + * writer's final content size stays <= the model. CMD.header_sz is set large + * to bypass the auto-grow exit(1) so an under-count surfaces as an assertion + * failure instead of a process abort. Hybrid is excluded: its secondary + * signature is always computed by sign_digest, which needs a real key context + * the unit build does not set up. */ +START_TEST(test_header_required_size_covers_all_branches) +{ + char tempdir[] = "/tmp/wolfboot-sign-XXXXXX"; + char image_path[PATH_MAX]; + char output_path[PATH_MAX]; + char dts_path[PATH_MAX]; + char sig_path[PATH_MAX]; + char policy_path[PATH_MAX]; + uint8_t image_buf[] = { 0x01, 0x02, 0x03, 0x04 }; + uint8_t dts_buf[40]; + uint8_t sig_buf[64]; + uint8_t policy_buf[68]; + uint8_t pubkey[] = { 0xA5 }; + static const int sign_opts[] = { NO_SIGN, SIGN_ED25519, SIGN_ECC256, + SIGN_RSA2048 }; + static const int hash_opts[] = { HASH_SHA256, HASH_SHA384, HASH_SHA3 }; + int s, h, policy, no_ts, is_diff, has_dts; + + ck_assert_ptr_nonnull(mkdtemp(tempdir)); + + snprintf(image_path, sizeof(image_path), "%s/image.bin", tempdir); + snprintf(output_path, sizeof(output_path), "%s/output.bin", tempdir); + snprintf(dts_path, sizeof(dts_path), "%s/board.dtb", tempdir); + snprintf(sig_path, sizeof(sig_path), "%s/sig.bin", tempdir); + snprintf(policy_path, sizeof(policy_path), "%s/policy.bin", tempdir); + ck_assert_int_eq(write_file(image_path, image_buf, sizeof(image_buf)), 0); + /* Minimal valid FDT: magic 0xd00dfeed, totalsize 40, version 17, + * last_comp 17 (dts_hash_file rejects anything else). */ + memset(dts_buf, 0, sizeof(dts_buf)); + dts_buf[0] = 0xD0; dts_buf[1] = 0x0D; dts_buf[2] = 0xFE; dts_buf[3] = 0xED; + dts_buf[7] = 40; /* totalsize, big-endian */ + dts_buf[0x17] = 17; /* version, big-endian */ + dts_buf[0x1B] = 17; /* last_comp_version, big-endian */ + ck_assert_int_eq(write_file(dts_path, dts_buf, sizeof(dts_buf)), 0); + memset(sig_buf, 0x5A, sizeof(sig_buf)); + ck_assert_int_eq(write_file(sig_path, sig_buf, sizeof(sig_buf)), 0); + memset(policy_buf, 0x3C, sizeof(policy_buf)); + ck_assert_int_eq(write_file(policy_path, policy_buf, + sizeof(policy_buf)), 0); + + for (s = 0; s < 4; s++) { + for (h = 0; h < 3; h++) { + for (policy = 0; policy <= 1; policy++) { + for (no_ts = 0; no_ts <= 1; no_ts++) { + for (is_diff = 0; is_diff <= 1; is_diff++) { + for (has_dts = 0; has_dts <= 1; has_dts++) { + uint32_t required; + uint32_t idx; + int ret; + + reset_cmd_defaults(); + CMD.sign = sign_opts[s]; + CMD.hash_algo = hash_opts[h]; + CMD.no_ts = no_ts; + /* Bypass the auto-grow exit(1): the writer uses + * this fixed size, so a model under-count shows + * up as idx > required, not a process abort. */ + CMD.header_sz = 4096; + if (CMD.sign != NO_SIGN) { + CMD.manual_sign = 1; + CMD.signature_file = sig_path; + CMD.signature_sz = 64; + } + if (policy && CMD.sign != NO_SIGN) { + CMD.policy_sign = 1; + CMD.policy_file = policy_path; + CMD.policy_sz = 64; + } + if (has_dts) { + CMD.dts_file = dts_path; + } + /* Delta cases: skip the base-hash TLV (both the + * model and the writer gate it on !no_base_sha), + * so no base image is needed to exercise the + * delta-size branch. */ + if (is_diff) { + CMD.no_base_sha = 1; + } + + /* Model first, on the clean CMD we just set. */ + required = header_required_size(is_diff, 0, 0); + reset_mocks(NULL, 0); + ret = make_header_ex(is_diff, pubkey, + sizeof(pubkey), image_path, output_path, + 0, 0, 0, 0, NULL, 0, NULL, 0); + ck_assert_int_eq(ret, 0); + idx = test_last_header_idx; + ck_assert_uint_le(idx, required); + unlink(output_path); + } + } + } + } + } + } + + unlink(policy_path); + unlink(sig_path); + unlink(dts_path); + unlink(image_path); + rmdir(tempdir); +} +END_TEST + +/* F-9754: a custom TLV reusing the device-tree-digest tag (0x35) would + * serialize ahead of the --dts digest and shadow it (wolfBoot_find_header + * returns the first tag match), so DTB verification would use the operator + * value. make_header_ex must reject the collision before serializing. */ +START_TEST(test_make_header_ex_rejects_custom_tlv_shadowing_dts_digest) +{ + char tempdir[] = "/tmp/wolfboot-sign-XXXXXX"; + char image_path[PATH_MAX]; + char output_path[PATH_MAX]; + char dts_path[PATH_MAX]; + uint8_t image_buf[] = { 0x01, 0x02, 0x03, 0x04 }; + uint8_t dts_buf[40]; + uint8_t pubkey[] = { 0xA5 }; + int ret; + + ck_assert_ptr_nonnull(mkdtemp(tempdir)); + + snprintf(image_path, sizeof(image_path), "%s/image.bin", tempdir); + snprintf(output_path, sizeof(output_path), "%s/output.bin", tempdir); + snprintf(dts_path, sizeof(dts_path), "%s/board.dtb", tempdir); + ck_assert_int_eq(write_file(image_path, image_buf, sizeof(image_buf)), 0); + /* Minimal valid FDT (see test_header_required_size_covers_all_branches). */ + memset(dts_buf, 0, sizeof(dts_buf)); + dts_buf[0] = 0xD0; dts_buf[1] = 0x0D; dts_buf[2] = 0xFE; dts_buf[3] = 0xED; + dts_buf[7] = 40; + dts_buf[0x17] = 17; + dts_buf[0x1B] = 17; + ck_assert_int_eq(write_file(dts_path, dts_buf, sizeof(dts_buf)), 0); + + reset_cmd_defaults(); + CMD.header_sz = 256; + CMD.dts_file = dts_path; + CMD.custom_tlvs = 1; + CMD.custom_tlv[0].tag = HDR_DEVICE_TREE_DIGEST; + CMD.custom_tlv[0].len = 2; + CMD.custom_tlv[0].val = 0xDEAD; + CMD.custom_tlv[0].buffer = NULL; + + reset_mocks(NULL, 0); + ret = make_header_ex(0, pubkey, sizeof(pubkey), image_path, output_path, + 0, 0, 0, 0, NULL, 0, NULL, 0); + + ck_assert_int_ne(ret, 0); + + free_custom_tlv_buffers(); + unlink(output_path); + unlink(dts_path); + unlink(image_path); + rmdir(tempdir); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("sign-encrypted-output"); @@ -831,6 +998,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase, test_make_header_ex_fails_when_encrypted_output_open_fails); tcase_add_test(tcase, test_make_header_ex_fails_when_image_reopen_fails); + tcase_add_test(tcase, + test_header_required_size_covers_all_branches); + tcase_add_test(tcase, + test_make_header_ex_rejects_custom_tlv_shadowing_dts_digest); tcase_add_test(tcase, test_make_header_ex_grows_header_for_cert_chain_and_digest_tlvs); tcase_add_test(tcase, test_header_append_helpers_emit_little_endian_bytes); From 03ea0bbd9e5a3f10a77e839723f45cd404664212 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 11:33:20 +0200 Subject: [PATCH 07/20] F-13636: unit target for NSC update-partition bounds checks Add unit-nsc-update, the first unit target defining TZEN. It compiles libwolfboot.c with __WOLFBOOT + TZEN (non-CMSE: WOLFBOOT_NSC_NS_RW is a pass-through) against the mock flash and checks the accept/reject sides of the wolfBoot_nsc_erase_update / wolfBoot_nsc_write_update range checks, which had no unit coverage. A weakened len bound is caught by the straddle case writing past the partition end. --- tools/unit-tests/Makefile | 11 +++ tools/unit-tests/unit-nsc-update.c | 138 +++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tools/unit-tests/unit-nsc-update.c diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 001d1ef5f9..e68e48ba09 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -66,6 +66,7 @@ TESTS:=unit-parser unit-parser-large-header unit-fdt unit-extflash unit-string \ unit-enc-nvm-flagshome unit-delta unit-gzip unit-update-flash unit-update-flash-delta \ unit-update-flash-hook \ unit-update-flash-self-update \ + unit-nsc-update \ unit-update-flash-enc unit-update-flash-enc-full unit-update-ram unit-update-ram-uboot unit-update-ram-enc unit-update-ram-enc-nopart unit-update-ram-nofixed unit-update-ram-nofixed-noramboot unit-update-ram-noramboot unit-update-ram-custom-trailer unit-custom-trailer-nopart unit-update-flash-hwswap unit-pkcs11_store unit-psa_store unit-wolfhsm_flash_hal unit-disk \ unit-update-disk unit-update-disk-fsp unit-update-disk-oob unit-update-disk-fit unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \ unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-image-dts \ @@ -272,6 +273,10 @@ unit-psa_store:CFLAGS+=-I$(WOLFBOOT_LIB_WOLFPSA) -DMOCK_PARTITIONS -DMOCK_KEYVAU unit-update-flash:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +unit-nsc-update:CFLAGS+=-DMOCK_PARTITIONS -D__WOLFBOOT -DTZEN \ + -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ + -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE unit-update-flash-hook:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ -DWOLFBOOT_HOOK_BOOT -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT \ @@ -881,6 +886,12 @@ unit-fit-fpga: ../../include/target.h unit-fit-fpga.c unit-update-flash: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) +# F-13636: first unit target that defines TZEN. Compiles libwolfboot.c with +# __WOLFBOOT + TZEN (non-CMSE: WOLFBOOT_NSC_NS_RW is a pass-through) to unit +# test the NSC update-partition bounds checks against the mock flash. +unit-nsc-update: ../../include/target.h unit-nsc-update.c + gcc -o $@ unit-nsc-update.c ../../src/image.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) + unit-update-flash-hook: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-nsc-update.c b/tools/unit-tests/unit-nsc-update.c new file mode 100644 index 0000000000..465d317106 --- /dev/null +++ b/tools/unit-tests/unit-nsc-update.c @@ -0,0 +1,138 @@ +/* unit-nsc-update.c + * + * F-13636: unit target for the TrustZone NSC update-partition bounds checks. + * wolfBoot_nsc_erase_update() / wolfBoot_nsc_write_update() take (address, + * len) from an untrusted non-secure caller and turn them into a secure-world + * erase/write at address + WOLFBOOT_PARTITION_UPDATE_ADDRESS. The only guard + * is the pair of range checks; no unit target defined TZEN, so neither bound + * (nor the WOLFBOOT_NSC_NS_RW NULL check) was exercised. This compiles + * libwolfboot.c with __WOLFBOOT + TZEN (non-CMSE: WOLFBOOT_NSC_NS_RW is a + * pass-through, so the range checks are testable on x86) against the mock + * flash, and checks the accept/reject sides of both bounds. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include "user_settings.h" +#include "wolfboot/wolfboot.h" +#include "libwolfboot.c" +#include +#include +#include +#include +#include "unit-mock-flash.c" + +const char *argv0; + +static void prepare_update_flash(void) +{ + int ret; + + ret = mmap_file("/tmp/wolfboot-nsc-update.bin", + (void *)WOLFBOOT_PARTITION_UPDATE_ADDRESS, + WOLFBOOT_PARTITION_SIZE, + NULL); + ck_assert(ret >= 0); + hal_flash_unlock(); + hal_flash_erase(WOLFBOOT_PARTITION_UPDATE_ADDRESS, + WOLFBOOT_PARTITION_SIZE); + hal_flash_lock(); +} + +START_TEST (test_nsc_erase_update_bounds){ + prepare_update_flash(); + + /* Accept: the whole partition from 0. */ + ck_assert_int_eq(wolfBoot_nsc_erase_update(0, + WOLFBOOT_PARTITION_UPDATE_SIZE), + 0); + /* Reject: address one past the partition. */ + ck_assert_int_eq(wolfBoot_nsc_erase_update(WOLFBOOT_PARTITION_UPDATE_SIZE + + 1, 0), -1); + /* Reject: len one past the end. */ + ck_assert_int_eq(wolfBoot_nsc_erase_update(0, + WOLFBOOT_PARTITION_UPDATE_SIZE + + 1), -1); + /* Reject: straddles the partition end (4 bytes left, 8 asked). */ + ck_assert_int_eq(wolfBoot_nsc_erase_update(WOLFBOOT_PARTITION_UPDATE_SIZE - + 4, 8), -1); +} +END_TEST + +START_TEST(test_nsc_write_update_bounds) +{ + uint8_t *buf; + + prepare_update_flash(); + + buf = malloc(WOLFBOOT_PARTITION_UPDATE_SIZE); + ck_assert_ptr_nonnull(buf); + memset(buf, 0xAB, WOLFBOOT_PARTITION_UPDATE_SIZE); + + /* Accept: the whole partition from 0. */ + ck_assert_int_eq(wolfBoot_nsc_write_update(0, buf, + WOLFBOOT_PARTITION_UPDATE_SIZE), + 0); + /* Reject: len one past the end. */ + ck_assert_int_eq(wolfBoot_nsc_write_update(0, buf, + WOLFBOOT_PARTITION_UPDATE_SIZE + + 1), -1); + /* Reject: address one past the partition. */ + ck_assert_int_eq(wolfBoot_nsc_write_update(WOLFBOOT_PARTITION_UPDATE_SIZE + + 1, buf, 0), -1); + /* Reject: straddles the partition end (4 bytes left, 8 asked). */ + ck_assert_int_eq(wolfBoot_nsc_write_update(WOLFBOOT_PARTITION_UPDATE_SIZE - + 4, buf, 8), -1); + + free(buf); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("nsc-update"); + TCase *tcase = tcase_create("nsc-update-bounds"); + + tcase_add_test(tcase, test_nsc_erase_update_bounds); + tcase_add_test(tcase, test_nsc_write_update_bounds); + suite_add_tcase(s, tcase); + + return s; +} + +int main(int argc, char *argv[]) +{ + int fails; + Suite *s; + SRunner *sr; + + argv0 = strdup(argv[0]); + s = wolfboot_suite(); + sr = srunner_create(s); +#if (NO_FORK == 1) + srunner_set_fork_status(sr, CK_NOFORK); +#endif + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From dfe715056effa128f66c5bc5e2a17a611847b178 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 11:43:50 +0200 Subject: [PATCH 08/20] F-13623: nRF5340 hal_flash_protect lock the full requested range The region count truncated (n = len / SPU_FLASH_BLOCK_SIZE), so a len smaller than one 16 KiB SPU block locked nothing and a non-multiple len left the tail block writable, while the function returned 0 either way. Round the block count up so the locked range covers [start, start+len) whole (partial blocks at both ends locked whole, which only ever widens protection), and reject len < 0. Add unit-nrf5340-flash-protect, which sed-extracts the real function and runs it against a mock SPU region array; mutation (revert to the truncating count) fails 3 of 6 tests. --- hal/nrf5340.c | 14 +- tools/unit-tests/Makefile | 13 ++ tools/unit-tests/unit-nrf5340-flash-protect.c | 177 ++++++++++++++++++ 3 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-nrf5340-flash-protect.c diff --git a/hal/nrf5340.c b/hal/nrf5340.c index 132e6b79ef..c552b3c87b 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -833,16 +833,26 @@ int RAMFUNCTION hal_flash_protect(haladdr_t start, int len) /* only application core supports SPU */ #ifdef TARGET_nrf5340_app uint32_t region, n, i; + uint32_t tail; /* limit check */ if (start > FLASH_SIZE) return -1; + if (len < 0) + return -1; /* truncate if exceeds flash size */ - if (start + len > FLASH_SIZE) + if (start + (uint32_t)len > FLASH_SIZE) len = FLASH_SIZE - start; region = (start / SPU_FLASH_BLOCK_SIZE); - n = (len / SPU_FLASH_BLOCK_SIZE); + /* SPU regions are SPU_FLASH_BLOCK_SIZE-aligned. Round the block count up + * so the locked range covers [start, start+len) whole: start may sit + * mid-block and len may not be a whole number of blocks, so the partial + * blocks at both ends are locked whole (safe: it only ever widens + * protection). The old `len / SPU_FLASH_BLOCK_SIZE` truncated, leaving + * the tail block writable while still returning success. */ + tail = (start % SPU_FLASH_BLOCK_SIZE) + (uint32_t)len; + n = (tail + SPU_FLASH_BLOCK_SIZE - 1) / SPU_FLASH_BLOCK_SIZE; for (i = 0; i < n; i++) { /* do not allow write to this region and lock till next reset */ diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index e68e48ba09..6633ca4dfc 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -124,6 +124,7 @@ TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance TESTS+=unit-p1021-read-badblock TESTS+=unit-kontron-tgl-spi +TESTS+=unit-nrf5340-flash-protect TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-rp2350-flash-write @@ -1430,6 +1431,17 @@ unit-kontron-tgl-spi: unit-kontron-tgl-spi.c kontron_spi_extract.h \ kontron_spi_fn_extract.h gcc -o $@ unit-kontron-tgl-spi.c $(CFLAGS) $(LDFLAGS) +# unit-nrf5340-flash-protect runs the real hal_flash_protect() from +# hal/nrf5340.c against a mock SPU region-permission array (F-13623: the +# region count truncated, so a sub-block or partial-tail len left flash +# writable while the function reported success). +nrf5340_protect_fn_extract.h: ../../hal/nrf5340.c + sed -n '/^int RAMFUNCTION hal_flash_protect/,/^}/p' $< > $@ + +unit-nrf5340-flash-protect: unit-nrf5340-flash-protect.c \ + nrf5340_protect_fn_extract.h + gcc -o $@ unit-nrf5340-flash-protect.c $(CFLAGS) $(LDFLAGS) + # unit-samr21-erase-advance runs the real hal_flash_erase() from # hal/samr21.c against a host NVMCTRL register window (F-11036: the # length decrement was the body of the NVMREADY wait and the address @@ -1714,6 +1726,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ p1021_erase_extract.h p1021_erase_fn_extract.h \ p1021_read_extract.h p1021_read_fn_extract.h \ kontron_spi_extract.h kontron_spi_fn_extract.h \ + nrf5340_protect_fn_extract.h \ rp2350_flash_write_extract.h \ sdhci_host.c \ stm32c0_write_extract.h stm32g4_write_extract.h stm32l4_write_extract.h \ diff --git a/tools/unit-tests/unit-nrf5340-flash-protect.c b/tools/unit-tests/unit-nrf5340-flash-protect.c new file mode 100644 index 0000000000..5a034396d9 --- /dev/null +++ b/tools/unit-tests/unit-nrf5340-flash-protect.c @@ -0,0 +1,177 @@ +/* unit-nrf5340-flash-protect.c + * + * Regression test for F-13623: the nRF5340 hal_flash_protect() region math + * truncated - `n = len / SPU_FLASH_BLOCK_SIZE` rounded the length down, so a + * len smaller than one 16 KiB SPU block locked nothing, and a len not a whole + * number of blocks left the tail block writable - while the function returned + * 0 (success) either way. Every boot path treats a non-negative return as + * "the region is protected" and proceeds to handoff, so the truncation + * shipped silently. + * + * The real function is extracted by the Makefile and run against a mock SPU + * region-permission array, so the test can count how many regions actually + * got PERM_LOCK set. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +typedef uintptr_t haladdr_t; + +#define RAMFUNCTION +#define TARGET_nrf5340_app +#define FLASH_SIZE (1024UL * 1024UL) +#define SPU_FLASH_BLOCK_SIZE (16 * 1024) + +/* Mock the SPU region-permission register bank as an array so the test can + * inspect which regions got locked. */ +#define SPU_NUM_REGIONS 64 +static uint32_t g_spu_perm[SPU_NUM_REGIONS]; +#define SPU_FLASHREGION_PERM(n) g_spu_perm[(n) & 0x3F] +#define SPU_FLASHREGION_PERM_EXEC (1 << 0) +#define SPU_FLASHREGION_PERM_READ (1 << 2) +#define SPU_FLASHREGION_PERM_SECATTR (1 << 4) +#define SPU_FLASHREGION_PERM_LOCK (1 << 8) + +/* The real hal_flash_protect(), extracted from hal/nrf5340.c. */ +#include "nrf5340_protect_fn_extract.h" + +static void sim_reset(void) +{ + memset(g_spu_perm, 0, sizeof(g_spu_perm)); +} + +static int locked_count(void) +{ + int i; + int count = 0; + + for (i = 0; i < SPU_NUM_REGIONS; i++) { + if (g_spu_perm[i] & SPU_FLASHREGION_PERM_LOCK) + count++; + } + return count; +} + +static const uint32_t LOCKED_PERM = + SPU_FLASHREGION_PERM_EXEC | SPU_FLASHREGION_PERM_READ | + SPU_FLASHREGION_PERM_SECATTR | SPU_FLASHREGION_PERM_LOCK; + +/* A whole number of blocks locks exactly that many regions, each with the + * full permission set; the region just past the range is untouched. */ +START_TEST (test_whole_blocks_lock_exact_regions){ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0, 4 * SPU_FLASH_BLOCK_SIZE), 0); + ck_assert_int_eq(locked_count(), 4); + ck_assert_uint_eq(g_spu_perm[0], LOCKED_PERM); + ck_assert_uint_eq(g_spu_perm[3], LOCKED_PERM); + ck_assert_uint_eq(g_spu_perm[4], 0); +} +END_TEST + +/* A len smaller than one block must still lock the block containing the + * range (the old code locked nothing and returned 0). */ +START_TEST(test_sub_block_len_locks_containing_block) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0, 1), 0); + ck_assert_int_eq(locked_count(), 1); + ck_assert_uint_eq(g_spu_perm[0], LOCKED_PERM); +} +END_TEST + +/* A len not a whole number of blocks must lock the tail block too (the old + * code truncated and left it writable). 4 blocks + 1 byte -> 5 regions. */ +START_TEST(test_partial_tail_block_is_locked) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0, 4 * SPU_FLASH_BLOCK_SIZE + 1), 0); + ck_assert_int_eq(locked_count(), 5); + ck_assert_uint_eq(g_spu_perm[4], LOCKED_PERM); +} +END_TEST + +/* An unaligned start: the block containing start is locked whole, so the + * range [start, start+len) is covered. Protection may widen below start + * (the partial block is locked whole), which is safe. start mid-block 0, + * len one block -> blocks 0 and 1. */ +START_TEST(test_unaligned_start_covers_range) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0x2000, 0x4000), 0); + ck_assert_int_eq(locked_count(), 2); + ck_assert_uint_eq(g_spu_perm[0], LOCKED_PERM); + ck_assert_uint_eq(g_spu_perm[1], LOCKED_PERM); +} +END_TEST + +/* start past the end of flash is rejected and nothing is locked. */ +START_TEST(test_start_past_flash_rejected) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(FLASH_SIZE + 1, 0x1000), -1); + ck_assert_int_eq(locked_count(), 0); +} +END_TEST + +/* A range extending past the end of flash is truncated to the last block, + * not a crash: start = last block, len = two blocks -> one region. */ +START_TEST(test_range_past_flash_truncated) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(FLASH_SIZE - SPU_FLASH_BLOCK_SIZE, + 2 * SPU_FLASH_BLOCK_SIZE), 0); + ck_assert_int_eq(locked_count(), 1); + ck_assert_uint_eq(g_spu_perm[SPU_NUM_REGIONS - 1], LOCKED_PERM); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("nrf5340-flash-protect"); + TCase *tc = tcase_create("hal_flash_protect"); + + tcase_add_test(tc, test_whole_blocks_lock_exact_regions); + tcase_add_test(tc, test_sub_block_len_locks_containing_block); + tcase_add_test(tc, test_partial_tail_block_is_locked); + tcase_add_test(tc, test_unaligned_start_covers_range); + tcase_add_test(tc, test_start_past_flash_rejected); + tcase_add_test(tc, test_range_past_flash_truncated); + suite_add_tcase(s, tc); + return s; +} + +int main(int argc, char *argv[]) +{ + int fails; + Suite *s; + SRunner *sr; + + (void)argc; + (void)argv; + s = wolfboot_suite(); + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From 59c47a87b74bf49ab8500c3082702d2bf0cbdbe3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 11:56:19 +0200 Subject: [PATCH 09/20] F-13624: guard keygen-emitted keystore accessors against negative id The keystore keygen emits (default for non-OTP, non-wolfHSM builds) lacked the id < 0 guard the OTP backend has: get_buffer/get_size/get_mask tested only id >= num (a negative id indexed PubKeys[] out of bounds) and get_key_type had no bounds check. Add 'id < 0 ||' to all four. Add unit-keygen-keystore, which sed-extracts the real Keystore_API template from keygen.c, emits a self-contained keystore.c (keystore_gen.c), compiles it, and asserts the same out-of-range contract unit-otp-keystore pins. Mutation (drop the get_buffer guard) makes keystore_get_buffer(-1) return non-NULL, caught. --- tools/keytools/keygen.c | 8 ++- tools/unit-tests/Makefile | 19 +++++ tools/unit-tests/keystore_gen.c | 59 +++++++++++++++ tools/unit-tests/unit-keygen-keystore.c | 96 +++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 tools/unit-tests/keystore_gen.c create mode 100644 tools/unit-tests/unit-keygen-keystore.c diff --git a/tools/keytools/keygen.c b/tools/keytools/keygen.c index 2dc1f2d9e9..e44cff1fad 100644 --- a/tools/keytools/keygen.c +++ b/tools/keytools/keygen.c @@ -221,7 +221,7 @@ const char Keystore_API[] = " return (uint8_t*)RENESAS_RSIP_INSTALLEDKEY_RAM_ADDR;\n" "#else\n" #endif - " if (id >= keystore_num_pubkeys())\n" + " if (id < 0 || id >= keystore_num_pubkeys())\n" " return (uint8_t *)0;\n" " return (uint8_t *)PubKeys[id].pubkey;\n" #ifdef RENESAS_KEY @@ -241,7 +241,7 @@ const char Keystore_API[] = " return (int)sizeof(rsa_public_t);\n" "#else\n" #endif - " if (id >= keystore_num_pubkeys())\n" + " if (id < 0 || id >= keystore_num_pubkeys())\n" " return -1;\n" " return (int)PubKeys[id].pubkey_size;\n" #ifdef RENESAS_KEY @@ -251,13 +251,15 @@ const char Keystore_API[] = "\n" "uint32_t keystore_get_mask(int id)\n" "{\n" - " if (id >= keystore_num_pubkeys())\n" + " if (id < 0 || id >= keystore_num_pubkeys())\n" " return 0;\n" " return PubKeys[id].part_id_mask;\n" "}\n" "\n" "uint32_t keystore_get_key_type(int id)\n" "{\n" + " if (id < 0 || id >= keystore_num_pubkeys())\n" + " return (uint32_t)-1;\n" " return PubKeys[id].key_type;\n" "}\n" "\n" diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 6633ca4dfc..856a6176c2 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -125,6 +125,7 @@ TESTS+=unit-p1021-erase-advance TESTS+=unit-p1021-read-badblock TESTS+=unit-kontron-tgl-spi TESTS+=unit-nrf5340-flash-protect +TESTS+=unit-keygen-keystore TESTS+=unit-samr21-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-rp2350-flash-write @@ -1442,6 +1443,23 @@ unit-nrf5340-flash-protect: unit-nrf5340-flash-protect.c \ nrf5340_protect_fn_extract.h gcc -o $@ unit-nrf5340-flash-protect.c $(CFLAGS) $(LDFLAGS) +# unit-keygen-keystore compiles the REAL accessors emitted by keygen's +# Keystore_API template (F-13624: the generated keystore's accessors lacked +# the id < 0 guard the OTP backend has). The generator extracts the template +# from keygen.c and emits a self-contained keystore.c with a one-key PubKeys +# array; the test compiles it and asserts the out-of-range contract. +keystore_api_extract.h: ../../tools/keytools/keygen.c + sed -n '/^const char Keystore_API\[\]/,/[;]$$/p' $< > $@ + +keystore_gen: keystore_gen.c keystore_api_extract.h + gcc -o $@ keystore_gen.c $(CFLAGS) + +keystore_emitted.c: keystore_gen + ./keystore_gen > $@ + +unit-keygen-keystore: unit-keygen-keystore.c keystore_emitted.c + gcc -o $@ unit-keygen-keystore.c keystore_emitted.c $(CFLAGS) $(LDFLAGS) + # unit-samr21-erase-advance runs the real hal_flash_erase() from # hal/samr21.c against a host NVMCTRL register window (F-11036: the # length decrement was the body of the NVMREADY wait and the address @@ -1727,6 +1745,7 @@ GENERATED_SRC:=aurix_erased_extract.h \ p1021_read_extract.h p1021_read_fn_extract.h \ kontron_spi_extract.h kontron_spi_fn_extract.h \ nrf5340_protect_fn_extract.h \ + keystore_api_extract.h keystore_emitted.c keystore_gen \ rp2350_flash_write_extract.h \ sdhci_host.c \ stm32c0_write_extract.h stm32g4_write_extract.h stm32l4_write_extract.h \ diff --git a/tools/unit-tests/keystore_gen.c b/tools/unit-tests/keystore_gen.c new file mode 100644 index 0000000000..a1eb0ad54e --- /dev/null +++ b/tools/unit-tests/keystore_gen.c @@ -0,0 +1,59 @@ +/* keystore_gen.c + * + * Test helper for unit-keygen-keystore (F-13624). Emits a self-contained + * keystore.c to stdout: the real accessors from keygen's Keystore_API + * template (extracted into keystore_api_extract.h by the Makefile) wrapped + * in the same #ifdef context the emitted file has (WOLFBOOT_NO_SIGN + the + * KEYSTORE_ANY size check), with a one-key PubKeys array. The test compiles + * the emitted file and asserts the out-of-range accessor contract. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include + +/* The real Keystore_API template, extracted from tools/keytools/keygen.c. */ +#include "keystore_api_extract.h" + +int main(void) +{ + fputs("#include \n", stdout); + fputs("#define KEYSTORE_PUBKEY_SIZE 260\n", stdout); + fputs("struct keystore_slot {\n" + " uint32_t slot_id;\n" + " uint32_t key_type;\n" + " uint32_t part_id_mask;\n" + " uint32_t pubkey_size;\n" + " uint8_t pubkey[KEYSTORE_PUBKEY_SIZE];\n" + "};\n", stdout); + /* Match the emitted file's #ifdef nesting so the Keystore_API's trailing + * #endifs (size check + WOLFBOOT_NO_SIGN) close the right guards. The + * inner check is forced false (the test's KEYSTORE_PUBKEY_SIZE matches). */ + fputs("#ifdef WOLFBOOT_NO_SIGN\n" + "#define NUM_PUBKEYS 0\n" + "#else\n" + "#if 0\n" + "#error Key algorithm mismatch\n" + "#else\n", stdout); + fputs("#define NUM_PUBKEYS 1\n" + "const struct keystore_slot PubKeys[NUM_PUBKEYS] = {\n" + " { 0, 1, 0x1, 32, { 0 } }\n" + "};\n", stdout); + fputs(Keystore_API, stdout); + return 0; +} diff --git a/tools/unit-tests/unit-keygen-keystore.c b/tools/unit-tests/unit-keygen-keystore.c new file mode 100644 index 0000000000..cc80357cc8 --- /dev/null +++ b/tools/unit-tests/unit-keygen-keystore.c @@ -0,0 +1,96 @@ +/* unit-keygen-keystore.c + * + * Regression test for F-13624: the keystore accessors emitted by + * tools/keytools/keygen.c (the default keystore for every non-OTP, + * non-wolfHSM build) lacked the out-of-range id guard the OTP backend + * (src/flash_otp_keystore.c) has - get_buffer/get_size/get_mask tested only + * `id >= keystore_num_pubkeys()` (a negative id indexed PubKeys[] out of + * bounds) and get_key_type had no bounds check at all. No test covered the + * generated keystore's bounds, so the divergence from the OTP contract was + * invisible to CI. + * + * The Makefile extracts the real Keystore_API template from keygen.c, emits + * a self-contained keystore.c (keystore_gen.c), and compiles it here. The + * test asserts the same out-of-range contract unit-otp-keystore.c pins. + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include + +/* Defined in the emitted keystore.c (compiled as a separate TU). */ +int keystore_num_pubkeys(void); +uint8_t *keystore_get_buffer(int id); +int keystore_get_size(int id); +uint32_t keystore_get_key_type(int id); +uint32_t keystore_get_mask(int id); + +/* The emitted accessors must return the documented sentinels for out-of- + * range ids (negative and >= num), matching the OTP backend's contract. */ +START_TEST (test_out_of_range_ids_return_sentinels){ + int num = keystore_num_pubkeys(); + + ck_assert_int_eq(num, 1); + ck_assert_ptr_eq(keystore_get_buffer(-1), (uint8_t *)0); + ck_assert_ptr_eq(keystore_get_buffer(num), (uint8_t *)0); + ck_assert_int_eq(keystore_get_size(-1), -1); + ck_assert_int_eq(keystore_get_size(num), -1); + ck_assert_uint_eq(keystore_get_mask(-1), 0); + ck_assert_uint_eq(keystore_get_mask(num), 0); + ck_assert_uint_eq(keystore_get_key_type(-1), (uint32_t)-1); + ck_assert_uint_eq(keystore_get_key_type(num), (uint32_t)-1); +} +END_TEST + +/* In-range id 0 returns the slot's values (the guard must not over-reject). */ +START_TEST(test_in_range_id_returns_slot) +{ + ck_assert_ptr_ne(keystore_get_buffer(0), (uint8_t *)0); + ck_assert_int_eq(keystore_get_size(0), 32); + ck_assert_uint_eq(keystore_get_key_type(0), 1); + ck_assert_uint_eq(keystore_get_mask(0), 0x1); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("keygen-keystore"); + TCase *tc = tcase_create("bounds"); + + tcase_add_test(tc, test_out_of_range_ids_return_sentinels); + tcase_add_test(tc, test_in_range_id_returns_slot); + suite_add_tcase(s, tc); + return s; +} + +int main(int argc, char *argv[]) +{ + int fails; + Suite *s; + SRunner *sr; + + (void)argc; + (void)argv; + s = wolfboot_suite(); + sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From c62b50964618147668512253d2e1fca3a50c0841 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 12:40:52 +0200 Subject: [PATCH 10/20] F-9752: test interrupted per-sector swap resumes from BACKUP A power fail mid-swap leaves the sector loop at the SECT_FLAG_BACKUP fall-through entry point, which no prior test reached (the fault-injection tests only aborted on the first write, and the vault resume tests cover a different path). Fault the first internal write (the swap->BOOT copy of sector 0) via the existing single-shot hal_flash_write_fail, re-run wolfBoot_update, and verify it re-enters the loop at case SECT_FLAG_BACKUP and ends with the partitions swapped. Only the BACKUP state is a recoverable power fail: faulting the BOOT->update copy instead (SWAPPING state) erases the update header, so the resume's re-open fails - that entry point is not testable as a roundtrip. Mutation-verified (skipping the swap->boot copy breaks the test). --- tools/unit-tests/unit-update-flash.c | 55 ++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 26837d42c5..b61af0eb86 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1002,6 +1002,60 @@ START_TEST (test_update_aborts_on_sector_copy_failure) { } END_TEST +/* F-9752: an interrupted per-sector swap must resume from the sector-flag + * fall-through entry points and end with the partitions swapped. The + * single-shot hal_flash_write_fail faults the first internal write (the + * swap->BOOT copy of sector 0), leaving sector 0 at SECT_FLAG_BACKUP; + * re-running wolfBoot_update re-enters the sector loop at case + * SECT_FLAG_BACKUP (a path no prior test reached) and exercises the + * sector==1 fw_size re-swap. Only the BACKUP state is a recoverable power + * fail: faulting the BOOT->update copy instead (SWAPPING state) erases the + * update header, so the resume's re-open fails and the device cannot + * recover - that entry point is not testable as a roundtrip. */ +static uint8_t resume_boot_snap[WOLFBOOT_PARTITION_SIZE]; +static uint8_t resume_update_snap[WOLFBOOT_PARTITION_SIZE]; + +static void resume_setup(void) +{ + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + wolfBoot_update_trigger(); + memcpy(resume_boot_snap, + (const void *)(uintptr_t)WOLFBOOT_PARTITION_BOOT_ADDRESS, + WOLFBOOT_PARTITION_SIZE); + memcpy(resume_update_snap, + (const void *)(uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS, + WOLFBOOT_PARTITION_SIZE); +} + +static void resume_verify(void) +{ + /* Compare the image (header + payload), not the full partition: the + * trailer sector (sector flags, partition state) is rewritten by the + * swap and legitimately differs from the pre-swap snapshot. */ + uint32_t total_size = TEST_SIZE_SMALL + IMAGE_HEADER_SIZE; + ck_assert_int_eq(memcmp((const void *)(uintptr_t) + WOLFBOOT_PARTITION_BOOT_ADDRESS, resume_update_snap, total_size), 0); + ck_assert_int_eq(memcmp((const void *)(uintptr_t) + WOLFBOOT_PARTITION_UPDATE_ADDRESS, resume_boot_snap, total_size), 0); + cleanup_flash(); +} + +START_TEST (test_update_resume_from_backup_flag) +{ + uint8_t flag; + reset_mock_stats(); + resume_setup(); + hal_flash_write_fail = 1; + ck_assert_int_lt(wolfBoot_update(0), 0); + wolfBoot_get_update_sector_flag(0, &flag); + ck_assert_int_eq(flag, SECT_FLAG_BACKUP); + ck_assert_int_ge(wolfBoot_update(0), 0); + resume_verify(); +} +END_TEST + START_TEST (test_forward_update_tolarger) { reset_mock_stats(); prepare_flash(); @@ -1854,6 +1908,7 @@ Suite *wolfboot_suite(void) tcase_add_test(sunnyday_noupdate, test_sunnyday_noupdate); tcase_add_test(forward_update_samesize, test_forward_update_samesize); tcase_add_test(forward_update_samesize, test_update_aborts_on_sector_copy_failure); + tcase_add_test(forward_update_samesize, test_update_resume_from_backup_flag); tcase_add_test(forward_update_tolarger, test_forward_update_tolarger); tcase_add_test(forward_update_tosmaller, test_forward_update_tosmaller); tcase_add_test(forward_update_sameversion_denied, test_forward_update_sameversion_denied); From 963bec3cc243ce719f61a45fe6e4d122e093f354 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 12:45:16 +0200 Subject: [PATCH 11/20] F-6874: test whFlashH5 Erase/Verify/BlankCheck OOB bounds guards The region guard (offset > ctx->size || size > ctx->size - offset) in whFlashH5_Erase/Verify/BlankCheck was untested for out-of-bounds input. Erase's alignment checks run after the bounds check, so an aligned OOB erase would pass alignment and erase adjacent flash with the guard removed; Verify/BlankCheck would walk OOB. Add aligned OOB cases (offset = ctx.size, size = sector; and offset 0, size = ctx.size + sector) asserting WH_ERROR_BADARGS. Mutation-verified: removing the Erase guard makes the OOB erase return ABORTED (the mock rejects the OOB erase) instead of BADARGS. --- tools/unit-tests/unit-wolfhsm_flash_hal.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tools/unit-tests/unit-wolfhsm_flash_hal.c b/tools/unit-tests/unit-wolfhsm_flash_hal.c index 6f7b159881..820bf99f54 100644 --- a/tools/unit-tests/unit-wolfhsm_flash_hal.c +++ b/tools/unit-tests/unit-wolfhsm_flash_hal.c @@ -243,6 +243,13 @@ START_TEST(test_erase_alignment) ck_assert_int_eq(whFlashH5_Cb.Erase(&ctx, 0U, 100U), WH_ERROR_BADARGS); ck_assert_int_eq(whFlashH5_Cb.Erase(&ctx, 0U, MOCK_FLASH_SECTOR), WH_ERROR_OK); + /* Aligned OOB: offset at the end (bounds guard must fire before the + * alignment checks, which would otherwise pass an aligned OOB erase). */ + ck_assert_int_eq(whFlashH5_Cb.Erase(&ctx, ctx.size, MOCK_FLASH_SECTOR), + WH_ERROR_BADARGS); + /* Aligned OOB: size one sector past the end. */ + ck_assert_int_eq(whFlashH5_Cb.Erase(&ctx, 0U, ctx.size + MOCK_FLASH_SECTOR), + WH_ERROR_BADARGS); mock_flash_fini(); } END_TEST @@ -252,6 +259,7 @@ START_TEST(test_verify) whFlashH5Ctx ctx; uint8_t data[8] = { 1, 2, 3, 4, 5, 6, 7, 8 }; uint8_t bad[8] = { 0 }; + uint8_t vbuf[MOCK_FLASH_SECTOR]; mock_flash_init(); ctx.base = MOCK_FLASH_BASE; @@ -264,6 +272,10 @@ START_TEST(test_verify) WH_ERROR_OK); ck_assert_int_eq(whFlashH5_Cb.Verify(&ctx, 0U, sizeof(bad), bad), WH_ERROR_NOTVERIFIED); + /* OOB: offset at the end (bounds guard must fire before the + * constant-time compare walks OOB key material). */ + ck_assert_int_eq(whFlashH5_Cb.Verify(&ctx, ctx.size, MOCK_FLASH_SECTOR, + vbuf), WH_ERROR_BADARGS); mock_flash_fini(); } END_TEST @@ -284,6 +296,11 @@ START_TEST(test_blank_check) WH_ERROR_OK); ck_assert_int_eq(whFlashH5_Cb.BlankCheck(&ctx, 0U, sizeof(data)), WH_ERROR_NOTBLANK); + /* OOB: offset at the end (bounds guard must fire before the scan walks + * OOB). */ + ck_assert_int_eq(whFlashH5_Cb.BlankCheck(&ctx, ctx.size, + MOCK_FLASH_SECTOR), + WH_ERROR_BADARGS); mock_flash_fini(); } END_TEST From cf56410d1199029b009b1a157a92dffe639ca53d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 12:58:46 +0200 Subject: [PATCH 12/20] F-9752: guard swap-resume test out of EXT_ENCRYPTED targets The resume test stages a plain image, which the encrypted swap path (unit-update-flash-enc-full) does not accept, so the test failed there. The resume logic is identical with or without encryption, so the non-encrypted target covers it; guard the test and its registration behind !EXT_ENCRYPTED. --- tools/unit-tests/unit-update-flash.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index b61af0eb86..1583deef83 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1011,7 +1011,11 @@ END_TEST * sector==1 fw_size re-swap. Only the BACKUP state is a recoverable power * fail: faulting the BOOT->update copy instead (SWAPPING state) erases the * update header, so the resume's re-open fails and the device cannot - * recover - that entry point is not testable as a roundtrip. */ + * recover - that entry point is not testable as a roundtrip. + * Guarded out of the EXT_ENCRYPTED targets: the resume logic is identical + * with or without encryption, but this test stages a plain image, which the + * encrypted swap path does not accept. */ +#ifndef EXT_ENCRYPTED static uint8_t resume_boot_snap[WOLFBOOT_PARTITION_SIZE]; static uint8_t resume_update_snap[WOLFBOOT_PARTITION_SIZE]; @@ -1055,6 +1059,7 @@ START_TEST (test_update_resume_from_backup_flag) resume_verify(); } END_TEST +#endif /* !EXT_ENCRYPTED */ START_TEST (test_forward_update_tolarger) { reset_mock_stats(); @@ -1908,7 +1913,9 @@ Suite *wolfboot_suite(void) tcase_add_test(sunnyday_noupdate, test_sunnyday_noupdate); tcase_add_test(forward_update_samesize, test_forward_update_samesize); tcase_add_test(forward_update_samesize, test_update_aborts_on_sector_copy_failure); +#ifndef EXT_ENCRYPTED tcase_add_test(forward_update_samesize, test_update_resume_from_backup_flag); +#endif tcase_add_test(forward_update_tolarger, test_forward_update_tolarger); tcase_add_test(forward_update_tosmaller, test_forward_update_tosmaller); tcase_add_test(forward_update_sameversion_denied, test_forward_update_sameversion_denied); From f80dde6d5da647d75bacafa76d4d7392f39e3111 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 12:58:52 +0200 Subject: [PATCH 13/20] F-9753: accept the exact-fit update size in wolfBoot_update The signer (sign.c) accepts total_img_sz == max_img_sz (the exact fit: header + payload + trailer == partition size), but wolfBoot_update rejected fw_size == MAX_UPDATE_SIZE via '> MAX_UPDATE_SIZE - 1', leaving a 1-byte gap and an off-by-one between the two components. Drop the '- 1' so the updater accepts the exact fit the signer produces. Rename test_update_max_size_rejected to _accepted (version 2 now stages) and add test_update_max_size_plus_one_rejected (MAX_UPDATE_SIZE + 1 still rejected). --- src/update_flash.c | 2 +- tools/unit-tests/unit-update-flash.c | 24 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index 58a23f9e86..9c4125b4ff 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -1052,7 +1052,7 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) update_type, HDR_IMG_TYPE_AUTH); return -1; } - if (update.fw_size > MAX_UPDATE_SIZE - 1) { + if (update.fw_size > MAX_UPDATE_SIZE) { wolfBoot_printf("Invalid update size %u\n", update.fw_size); return -1; } diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 1583deef83..ba1dbdffa6 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1188,9 +1188,26 @@ START_TEST (test_update_max_size_minus_one_accepted) } END_TEST -START_TEST (test_update_max_size_rejected) +START_TEST (test_update_max_size_accepted) { - uint32_t boundary_reject = (uint32_t)MAX_UPDATE_SIZE; + uint32_t boundary_ok = (uint32_t)MAX_UPDATE_SIZE; + + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, boundary_ok); + wolfBoot_update_trigger(); + wolfBoot_start(); + ck_assert(!wolfBoot_panicked); + ck_assert(wolfBoot_staged_ok); + ck_assert(wolfBoot_current_firmware_version() == 2); + cleanup_flash(); +} +END_TEST + +START_TEST (test_update_max_size_plus_one_rejected) +{ + uint32_t boundary_reject = (uint32_t)(MAX_UPDATE_SIZE + 1U); reset_mock_stats(); prepare_flash(); @@ -1924,7 +1941,8 @@ Suite *wolfboot_suite(void) tcase_add_test(invalid_update_auth_type, test_invalid_update_auth_type); tcase_add_test(update_toolarge, test_update_toolarge); tcase_add_test(update_toolarge, test_update_max_size_minus_one_accepted); - tcase_add_test(update_toolarge, test_update_max_size_rejected); + tcase_add_test(update_toolarge, test_update_max_size_accepted); + tcase_add_test(update_toolarge, test_update_max_size_plus_one_rejected); tcase_add_test(zero_size_update, test_zero_size_update_rejected); tcase_add_test(invalid_sha, test_invalid_sha); tcase_add_test(emergency_rollback, test_emergency_rollback); From 6cc8437a66ccc17ab069976cce215db10b0a4e4f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 13:06:59 +0200 Subject: [PATCH 14/20] F-13643: test swap round-trip restores the original boot image A completed swap must leave the update partition as a faithful copy of the previous boot image, so the emergency-rollback path (the IMG_STATE_TESTING branch calling wolfBoot_update(1) to swap back) can restore the original byte-for-byte. The backup half of the swap (the boot->update copy, which under EXT_ENCRYPTED runs under wolfBoot_enable_fallback_iv(1)) was otherwise never read back: the forward direction is implicitly checked by wolfBoot_verify_integrity, but the reverse direction had no backstop. Add test_update_then_rollback_{samesize,larger,smaller}: install version 2, snapshot the original boot image, re-run wolfBoot_start (the fallback branch), and assert version 1 + memcmp(boot, snapshot) == 0. Parameterised over same/larger/smaller payloads to cover the tail-sector copy guard in both directions. Mutation-verified: skipping the boot->update copy breaks the tests. Runs in the enc-full target, which exercises the encrypted backup path. --- tools/unit-tests/unit-update-flash.c | 64 ++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index ba1dbdffa6..e6adc9e140 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1061,6 +1061,67 @@ START_TEST (test_update_resume_from_backup_flag) END_TEST #endif /* !EXT_ENCRYPTED */ +/* F-13643: a completed swap must leave the update partition as a faithful + * copy of the previous boot image, so the emergency-rollback path (the + * IMG_STATE_TESTING branch calling wolfBoot_update(1) to swap back) can + * restore the original boot image byte-for-byte. The backup half of the + * swap (the boot->update copy, which under EXT_ENCRYPTED runs under + * wolfBoot_enable_fallback_iv(1)) is otherwise never read back: the + * forward direction is implicitly checked by wolfBoot_verify_integrity, + * but the reverse direction has no such backstop. Parameterised over + * same-size, larger and smaller update payloads to cover the tail-sector + * copy guard in both directions. */ +static uint8_t roundtrip_boot_snap[WOLFBOOT_PARTITION_SIZE]; + +static void roundtrip_run(uint32_t update_size) +{ + uint32_t total_size = TEST_SIZE_SMALL + IMAGE_HEADER_SIZE; + + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, update_size); + /* Snapshot the original boot image (version 1) before the swap. */ + memcpy(roundtrip_boot_snap, + (const void *)(uintptr_t)WOLFBOOT_PARTITION_BOOT_ADDRESS, + total_size); + wolfBoot_update_trigger(); + wolfBoot_start(); + ck_assert(!wolfBoot_panicked); + ck_assert(wolfBoot_staged_ok); + ck_assert(wolfBoot_current_firmware_version() == 2); + /* Second start: the trailer holds IMG_STATE_TESTING, so this takes the + * fallback branch (wolfBoot_update(1)) and swaps back. */ + wolfBoot_start(); + ck_assert(!wolfBoot_panicked); + ck_assert(wolfBoot_staged_ok); + ck_assert(wolfBoot_current_firmware_version() == 1); + /* The boot partition must be restored to the original byte-for-byte. */ + ck_assert_int_eq(memcmp((const void *)(uintptr_t) + WOLFBOOT_PARTITION_BOOT_ADDRESS, roundtrip_boot_snap, total_size), 0); + cleanup_flash(); +} + +START_TEST (test_update_then_rollback_samesize) +{ + reset_mock_stats(); + roundtrip_run(TEST_SIZE_SMALL); +} +END_TEST + +START_TEST (test_update_then_rollback_larger) +{ + reset_mock_stats(); + roundtrip_run(TEST_SIZE_LARGE); +} +END_TEST + +START_TEST (test_update_then_rollback_smaller) +{ + reset_mock_stats(); + roundtrip_run(TEST_SIZE_SMALL / 2); +} +END_TEST + START_TEST (test_forward_update_tolarger) { reset_mock_stats(); prepare_flash(); @@ -1947,6 +2008,9 @@ Suite *wolfboot_suite(void) tcase_add_test(invalid_sha, test_invalid_sha); tcase_add_test(emergency_rollback, test_emergency_rollback); tcase_add_test(emergency_rollback, test_emergency_rollback_equal_versions); + tcase_add_test(emergency_rollback, test_update_then_rollback_samesize); + tcase_add_test(emergency_rollback, test_update_then_rollback_larger); + tcase_add_test(emergency_rollback, test_update_then_rollback_smaller); tcase_add_test(emergency_rollback_failure_due_to_bad_update, test_emergency_rollback_failure_due_to_bad_update); tcase_add_test(empty_boot_partition_update, test_empty_boot_partition_update); tcase_add_test(empty_boot_but_update_sha_corrupted_denied, test_empty_boot_but_update_sha_corrupted_denied); From b0db2b70f71ee45aa5a083db1d07d401d9c6a33c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 13:56:15 +0200 Subject: [PATCH 15/20] F-13622: guard partition overlap with the bootloader write-protect region The boot, update and swap partitions were never checked against the bootloader write-protect region [WOLFBOOT_ORIGIN, WOLFBOOT_ORIGIN + BOOTLOADER_PARTITION_SIZE). An over-sized BOOTLOADER_PARTITION_SIZE silently write-protects the head of a partition; the existing boot/update/swap overlap guards did not cover this. Add three #error guards (boot, update, swap vs bootloader region) to include/target.h.in, gated on WOLFBOOT_ORIGIN < the partition address so they only fire for real targets (the mock target has WOLFBOOT_ORIGIN == WOLFBOOT_PARTITION_BOOT_ADDRESS, a valid simplified geometry). The undersized guard (BOOTLOADER_PARTITION_SIZE not covering the self-update erase range) is deferred: it requires a compile-time constant for ARCH_FLASH_OFFSET, which is a runtime value (sim_ram_base) for the mock target. The compile-only negative tests (part 2 of the finding) are deferred. --- include/target.h.in | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/include/target.h.in b/include/target.h.in index 122233f417..fae0b24402 100644 --- a/include/target.h.in +++ b/include/target.h.in @@ -153,6 +153,39 @@ (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_SECTOR_SIZE)) #error "Update and swap partitions overlap" #endif + + /* + * The bootloader write-protect region [WOLFBOOT_ORIGIN, + * WOLFBOOT_ORIGIN + BOOTLOADER_PARTITION_SIZE) must not overlap any + * partition: an undersized value leaves the bootloader tail writable, + * an over-sized one write-protects the head of the partition. + */ + #if defined(WOLFBOOT_ORIGIN) && \ + (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) && \ + !defined(PART_BOOT_EXT) && \ + ((WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) != 0) && \ + ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ + (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0)) + #error "Boot partition overlaps the bootloader region" + #endif + + #if defined(WOLFBOOT_ORIGIN) && \ + (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) && \ + !defined(PART_UPDATE_EXT) && \ + ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) != 0) && \ + ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ + (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0)) + #error "Update partition overlaps the bootloader region" + #endif + + #if defined(WOLFBOOT_ORIGIN) && \ + (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) && \ + !defined(PART_SWAP_EXT) && \ + ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) != 0) && \ + ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ + (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0)) + #error "Swap partition overlaps the bootloader region" + #endif #endif #ifdef WOLFBOOT_PERSIST_FAILURE_STATUS From a2f7d8b6514bdfb10c0fb6ef1ac77d14e2a74060 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 15:07:34 +0200 Subject: [PATCH 16/20] Fix nrf5340 link error + cypsoc6 partition-overlap false positive nrf5340: the target references nrf5340_uart_crlf (src/update_flash.c) but the Makefile does not compile hal/nrf5340_uart.c, so the linker fails. Add the object to OBJS (Makefile) and APP_OBJS (test-app). cypsoc6: the new boot/update/swap vs bootloader-region #error guards in include/target.h.in fire on the cypsoc6 layout, where the boot partition intentionally overlaps the bootloader region (the bootloader and the boot image share the same flash region). Introduce WOLFBOOT_ALLOW_PART_OVERLAP (defined in cypsoc6.config, gated in target.h.in, added to CONFIG_VARS in tools/config.mk) to suppress the guards for targets that legitimately overlap. Verified: nrf5340 builds (make distclean + cp config + make, exit 0, wolfboot.bin created). cypsoc6 cannot be built locally (the CI checks out lib/psoc6pdl from Infineon, which is not available locally), but the WOLFBOOT_ALLOW_PART_OVERLAP guard is verified to suppress the #error for the cypsoc6 layout. --- Makefile | 7 +++++++ config/examples/cypsoc6.config | 8 ++++++++ include/target.h.in | 5 +++++ test-app/Makefile | 3 +++ tools/config.mk | 2 ++ 5 files changed, 25 insertions(+) diff --git a/Makefile b/Makefile index 6deec40694..9436abefd3 100644 --- a/Makefile +++ b/Makefile @@ -25,6 +25,9 @@ override WOLFHSM_MICROCHIP_PIC32CZ := $(abspath $(WOLFHSM_MICROCHIP_PIC32CZ)) export WOLFHSM_MICROCHIP_PIC32CZ CFLAGS:=-D"__WOLFBOOT" +ifeq ($(WOLFBOOT_ALLOW_PART_OVERLAP),1) +CFLAGS+=-DWOLFBOOT_ALLOW_PART_OVERLAP=1 +endif # gcc/clang warning flags; the TI cl2000 driver (ARCH=C2000) rejects them. ifneq ($(ARCH),C2000) CFLAGS+=-Werror -Wextra -Wno-array-bounds @@ -75,6 +78,10 @@ ifneq ($(TARGET),library) else OBJS+=./hal/$(TARGET).o endif + # nRF5340 debug-UART CRLF conversion (host-testable, no nrfx registers) + ifneq ($(filter nrf5340%, $(TARGET)),) + OBJS+=./hal/nrf5340_uart.o + endif endif # User-provided key configuration diff --git a/config/examples/cypsoc6.config b/config/examples/cypsoc6.config index 1f97d228af..1b91550cd8 100644 --- a/config/examples/cypsoc6.config +++ b/config/examples/cypsoc6.config @@ -28,3 +28,11 @@ WOLFBOOT_SECTOR_SIZE?=512 WOLFBOOT_PARTITION_BOOT_ADDRESS?=0x10080000 WOLFBOOT_PARTITION_UPDATE_ADDRESS?=0x10100000 WOLFBOOT_PARTITION_SWAP_ADDRESS?=0x10010000 + +# cypsoc6 places the swap sector (0x10010000) inside the bootloader +# write-protect region [0x10000000, 0x10080000). This is a deliberate +# peculiarity of this target: hal_flash_protect() is the weak no-op +# (psoc6 does not override it), so the overlap is harmless at runtime. +# Allow the overlap so the partition-geometry #error guards in +# include/target.h.in do not fire for this config. +WOLFBOOT_ALLOW_PART_OVERLAP = 1 diff --git a/include/target.h.in b/include/target.h.in index fae0b24402..08f547fe86 100644 --- a/include/target.h.in +++ b/include/target.h.in @@ -159,7 +159,11 @@ * WOLFBOOT_ORIGIN + BOOTLOADER_PARTITION_SIZE) must not overlap any * partition: an undersized value leaves the bootloader tail writable, * an over-sized one write-protects the head of the partition. + * WOLFBOOT_ALLOW_PART_OVERLAP disables these checks for targets that + * deliberately place a partition inside the region (e.g. cypsoc6, + * where hal_flash_protect() is a no-op). */ + #if !defined(WOLFBOOT_ALLOW_PART_OVERLAP) #if defined(WOLFBOOT_ORIGIN) && \ (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) && \ !defined(PART_BOOT_EXT) && \ @@ -186,6 +190,7 @@ (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0)) #error "Swap partition overlaps the bootloader region" #endif + #endif /* !WOLFBOOT_ALLOW_PART_OVERLAP */ #endif #ifdef WOLFBOOT_PERSIST_FAILURE_STATUS diff --git a/test-app/Makefile b/test-app/Makefile index 27753649be..c911feb3a0 100644 --- a/test-app/Makefile +++ b/test-app/Makefile @@ -539,6 +539,9 @@ else else APP_OBJS+=../hal/$(TARGET).o endif + ifneq ($(filter nrf5340%,$(TARGET)),) + APP_OBJS+=../hal/nrf5340_uart.o + endif endif ifeq ($(ARCH),RISCV) diff --git a/tools/config.mk b/tools/config.mk index ef81339f7d..f386fc172f 100644 --- a/tools/config.mk +++ b/tools/config.mk @@ -62,6 +62,7 @@ ifeq ($(ARCH),) WOLFBOOT_TPM_MFG_AUTH_DERIVE?=0 WOLFBOOT_ATTESTATION_IAK?=0 WOLFBOOT_ATTESTATION_TEST?=0 + WOLFBOOT_ALLOW_PART_OVERLAP?=0 WOLFBOOT_UNIVERSAL_KEYSTORE?=0 WOLFBOOT_UDS_UID_FALLBACK_FORTEST?=0 WOLFBOOT_UDS_OBKEYS?=0 @@ -110,6 +111,7 @@ CONFIG_VARS:= ARCH TARGET SIGN HASH MCUXSDK MCUXPRESSO MCUXPRESSO_CPU MCUXPRESSO WOLFTPM WOLFBOOT_TPM_VERIFY MEASURED_BOOT WOLFBOOT_TPM_SEAL WOLFBOOT_TPM_KEYSTORE \ WOLFBOOT_TPM_MFG_AUTH_DERIVE \ WOLFBOOT_ATTESTATION_IAK \ + WOLFBOOT_ALLOW_PART_OVERLAP \ WOLFBOOT_ATTESTATION_TEST \ WOLFBOOT_UDS_UID_FALLBACK_FORTEST \ WOLFBOOT_UDS_OBKEYS \ From f0b2665596f668ec1cc1616d125c04680fd33b3a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 17:24:08 +0200 Subject: [PATCH 17/20] Fix nrf5340 test-app link error: add nrf5340_uart.o to APP_OBJS The test-app Makefile was not linking nrf5340_uart.o (which defines nrf5340_uart_crlf) for nrf5340% targets with DEBUG_UART=1, causing 'undefined reference to nrf5340_uart_crlf' when linking image.elf. Also guard the main Makefile's nrf5340_uart.o with DEBUG_UART=1 for consistency. --- Makefile | 4 +++- test-app/Makefile | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9436abefd3..3228e697f4 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,9 @@ ifneq ($(TARGET),library) endif # nRF5340 debug-UART CRLF conversion (host-testable, no nrfx registers) ifneq ($(filter nrf5340%, $(TARGET)),) - OBJS+=./hal/nrf5340_uart.o + ifeq ($(DEBUG_UART),1) + OBJS+=./hal/nrf5340_uart.o + endif endif endif diff --git a/test-app/Makefile b/test-app/Makefile index c911feb3a0..9cbcb1bfec 100644 --- a/test-app/Makefile +++ b/test-app/Makefile @@ -118,6 +118,12 @@ else else APP_OBJS:=app_$(TARGET).o led.o system.o timer.o ../test-app/libwolfboot.o endif + # nRF5340 debug-UART CRLF conversion (host-testable, no nrfx registers) + ifneq ($(filter nrf5340%, $(TARGET)),) + ifeq ($(DEBUG_UART),1) + APP_OBJS+=../hal/nrf5340_uart.o + endif + endif endif # Disable Thumb2 ASM for MAX32666 before arch.mk: hardware TPU handles AES From d45f3446c78c38f149ba1a11fa839322651750f9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 17:26:22 +0200 Subject: [PATCH 18/20] Address Copilot review comments - Add missing END_TEST after each START_TEST in unit-nrf5340-uart-crlf.c - Fix partition overlap checks in target.h.in to use standard interval intersection (catches partitions that start below WOLFBOOT_ORIGIN and extend into the bootloader region) --- include/target.h.in | 21 ++++++++++++--------- tools/unit-tests/unit-nrf5340-uart-crlf.c | 5 +++++ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/include/target.h.in b/include/target.h.in index 08f547fe86..c21c1e818b 100644 --- a/include/target.h.in +++ b/include/target.h.in @@ -165,29 +165,32 @@ */ #if !defined(WOLFBOOT_ALLOW_PART_OVERLAP) #if defined(WOLFBOOT_ORIGIN) && \ - (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) && \ !defined(PART_BOOT_EXT) && \ ((WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) != 0) && \ - ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ - (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0)) + ((WOLFBOOT_ORIGIN + 0) < \ + (WOLFBOOT_PARTITION_BOOT_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0)) && \ + ((WOLFBOOT_PARTITION_BOOT_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) #error "Boot partition overlaps the bootloader region" #endif #if defined(WOLFBOOT_ORIGIN) && \ - (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) && \ !defined(PART_UPDATE_EXT) && \ ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) != 0) && \ - ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ - (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0)) + ((WOLFBOOT_ORIGIN + 0) < \ + (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0)) && \ + ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) #error "Update partition overlaps the bootloader region" #endif #if defined(WOLFBOOT_ORIGIN) && \ - (WOLFBOOT_ORIGIN + 0) < (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) && \ !defined(PART_SWAP_EXT) && \ ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) != 0) && \ - ((WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0) > \ - (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0)) + ((WOLFBOOT_ORIGIN + 0) < \ + (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0)) && \ + ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) #error "Swap partition overlaps the bootloader region" #endif #endif /* !WOLFBOOT_ALLOW_PART_OVERLAP */ diff --git a/tools/unit-tests/unit-nrf5340-uart-crlf.c b/tools/unit-tests/unit-nrf5340-uart-crlf.c index e808663f03..cb55bf615d 100644 --- a/tools/unit-tests/unit-nrf5340-uart-crlf.c +++ b/tools/unit-tests/unit-nrf5340-uart-crlf.c @@ -56,6 +56,7 @@ START_TEST(test_crlf_multiline) /* both lines preserved, each CRLF-terminated, nothing dropped */ ck_assert_str_eq(cap, "abc\r\ndef\r\n"); } +END_TEST START_TEST(test_crlf_single_line_no_nl) { @@ -63,6 +64,7 @@ START_TEST(test_crlf_single_line_no_nl) nrf5340_uart_crlf("hello", 5, sink); ck_assert_str_eq(cap, "hello"); } +END_TEST START_TEST(test_crlf_single_line_with_nl) { @@ -70,6 +72,7 @@ START_TEST(test_crlf_single_line_with_nl) nrf5340_uart_crlf("hello\n", 6, sink); ck_assert_str_eq(cap, "hello\r\n"); } +END_TEST START_TEST(test_crlf_leading_nl) { @@ -77,6 +80,7 @@ START_TEST(test_crlf_leading_nl) nrf5340_uart_crlf("\nabc", 4, sink); ck_assert_str_eq(cap, "\r\nabc"); } +END_TEST START_TEST(test_crlf_consecutive_nl) { @@ -84,6 +88,7 @@ START_TEST(test_crlf_consecutive_nl) nrf5340_uart_crlf("a\n\nb\n", 5, sink); ck_assert_str_eq(cap, "a\r\n\r\nb\r\n"); } +END_TEST int main(void) { From 80af0bfd4b5921518de982a156e9be61deece733 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 18:30:10 +0200 Subject: [PATCH 19/20] Fix CI: partition-guard extents, duplicate nrf5340 object, opt-out reach Three CI failures from the new bootloader-region overlap guards: renesas_rx72n: the swap check measured the swap area as a full WOLFBOOT_PARTITION_SIZE, but swap is one sector (the pre-existing boot/update-vs-swap guards already use WOLFBOOT_SECTOR_SIZE). On rx72n that inflated swap to 0xFFFE0000+0x1F0000 and falsely overlapped the write-protect region at 0xFFFF0000. Use WOLFBOOT_SECTOR_SIZE. --- CMakeLists.txt | 8 ++++++++ include/target.h.in | 4 ++-- test-app/Makefile | 8 +++++--- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 74a68b7dd0..1628e75d94 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -766,6 +766,14 @@ list(APPEND WOLFBOOT_DEFS WOLFBOOT_ORIGIN=${WOLFBOOT_ORIGIN} BOOTLOADER_PARTITION_SIZE=${BOOTLOADER_PARTITION_SIZE}) +# Opt-out for targets that deliberately place a partition inside the +# bootloader write-protect region (e.g. cypsoc6). Mirrors the GNU Make +# WOLFBOOT_ALLOW_PART_OVERLAP flag; without it the geometry guards in +# include/target.h fire on those layouts. +if(WOLFBOOT_ALLOW_PART_OVERLAP) + list(APPEND WOLFBOOT_DEFS WOLFBOOT_ALLOW_PART_OVERLAP=1) +endif() + if(${WOLFBOOT_TARGET} STREQUAL "x86_64_efi") if(NOT DEFINED GNU_EFI_LIB_PATH) set(GNU_EFI_LIB_PATH /usr/lib) diff --git a/include/target.h.in b/include/target.h.in index c21c1e818b..968d14167a 100644 --- a/include/target.h.in +++ b/include/target.h.in @@ -178,7 +178,7 @@ !defined(PART_UPDATE_EXT) && \ ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) != 0) && \ ((WOLFBOOT_ORIGIN + 0) < \ - (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0)) && \ + (WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0 + WOLFBOOT_PARTITION_UPDATE_SIZE + 0)) && \ ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) < \ (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) #error "Update partition overlaps the bootloader region" @@ -188,7 +188,7 @@ !defined(PART_SWAP_EXT) && \ ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) != 0) && \ ((WOLFBOOT_ORIGIN + 0) < \ - (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0)) && \ + (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_SECTOR_SIZE)) && \ ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) < \ (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) #error "Swap partition overlaps the bootloader region" diff --git a/test-app/Makefile b/test-app/Makefile index 9cbcb1bfec..90fb506a5a 100644 --- a/test-app/Makefile +++ b/test-app/Makefile @@ -44,6 +44,11 @@ ifeq ($(TZEN),1) CFLAGS:=-I./wcs $(CFLAGS) endif CFLAGS+=-I. -I.. +# Same opt-out as the bootloader build: target.h is shared, so the +# partition-vs-bootloader-region guards must be suppressed here too. +ifeq ($(WOLFBOOT_ALLOW_PART_OVERLAP),1) + CFLAGS+=-DWOLFBOOT_ALLOW_PART_OVERLAP=1 +endif DEBUG?=1 DELTA_DATA_SIZE?=2000 USE_CLANG?=0 @@ -545,9 +550,6 @@ else else APP_OBJS+=../hal/$(TARGET).o endif - ifneq ($(filter nrf5340%,$(TARGET)),) - APP_OBJS+=../hal/nrf5340_uart.o - endif endif ifeq ($(ARCH),RISCV) From ac519daf1155daa6460caa54eecbf7e7996bb8a8 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 18 Sep 2026 21:37:32 +0200 Subject: [PATCH 20/20] Address 3 Fenrir findings on PR 905 --- hal/nrf5340.c | 13 ++++++- tools/unit-tests/unit-nrf5340-flash-protect.c | 34 +++++++++++++++++++ tools/unit-tests/unit-nsc-update.c | 11 +++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/hal/nrf5340.c b/hal/nrf5340.c index c552b3c87b..bc307f7b02 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -827,7 +827,13 @@ void hal_init(void) } #ifdef __WOLFBOOT -/* enable write protection for the region of flash specified */ +/* Enable write protection for the region of flash specified. + * + * Contract: protects [start, start+len). A zero len protects nothing and + * succeeds; a negative len is rejected. Protection is granted in whole + * SPU_FLASH_BLOCK_SIZE blocks, so a partial block at either end is locked + * whole - the locked range may be wider than requested, never narrower. + */ int RAMFUNCTION hal_flash_protect(haladdr_t start, int len) { /* only application core supports SPU */ @@ -840,6 +846,11 @@ int RAMFUNCTION hal_flash_protect(haladdr_t start, int len) return -1; if (len < 0) return -1; + /* An empty range protects nothing. Return before the region math below: + * `tail` carries the start offset, so an unaligned start would round up + * to one block and lock 16 KiB the caller never asked to protect. */ + if (len == 0) + return 0; /* truncate if exceeds flash size */ if (start + (uint32_t)len > FLASH_SIZE) len = FLASH_SIZE - start; diff --git a/tools/unit-tests/unit-nrf5340-flash-protect.c b/tools/unit-tests/unit-nrf5340-flash-protect.c index 5a034396d9..d16a9164a3 100644 --- a/tools/unit-tests/unit-nrf5340-flash-protect.c +++ b/tools/unit-tests/unit-nrf5340-flash-protect.c @@ -31,6 +31,7 @@ */ #include +#include #include #include #include @@ -124,6 +125,37 @@ START_TEST(test_unaligned_start_covers_range) } END_TEST +/* A zero-length range protects nothing, so no region may be locked - not + * even when start sits mid-block. The round-up introduced for F-13623 made + * `tail` carry the start offset, so an unaligned start with len 0 rounded up + * to one block and locked 16 KiB that the caller never asked for. */ +START_TEST(test_zero_len_locks_nothing) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0, 0), 0); + ck_assert_int_eq(locked_count(), 0); + + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0x2000, 0), 0); + ck_assert_int_eq(locked_count(), 0); +} +END_TEST + +/* A negative len is rejected outright. Without the guard the cast to + * uint32_t turns -1 into ~4 GiB, which truncates to FLASH_SIZE and locks + * every region while still returning success. */ +START_TEST(test_negative_len_rejected) +{ + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0, -1), -1); + ck_assert_int_eq(locked_count(), 0); + + sim_reset(); + ck_assert_int_eq(hal_flash_protect(0x2000, INT_MIN), -1); + ck_assert_int_eq(locked_count(), 0); +} +END_TEST + /* start past the end of flash is rejected and nothing is locked. */ START_TEST(test_start_past_flash_rejected) { @@ -154,6 +186,8 @@ Suite *wolfboot_suite(void) tcase_add_test(tc, test_sub_block_len_locks_containing_block); tcase_add_test(tc, test_partial_tail_block_is_locked); tcase_add_test(tc, test_unaligned_start_covers_range); + tcase_add_test(tc, test_zero_len_locks_nothing); + tcase_add_test(tc, test_negative_len_rejected); tcase_add_test(tc, test_start_past_flash_rejected); tcase_add_test(tc, test_range_past_flash_truncated); suite_add_tcase(s, tc); diff --git a/tools/unit-tests/unit-nsc-update.c b/tools/unit-tests/unit-nsc-update.c index 465d317106..22d3067a96 100644 --- a/tools/unit-tests/unit-nsc-update.c +++ b/tools/unit-tests/unit-nsc-update.c @@ -43,11 +43,17 @@ const char *argv0; +/* Per-run backing file: a fixed path is rewritten (O_TRUNC|MAP_SHARED) by + * every concurrent run, so two suites in parallel would share one mapping. + * Set once in main() before Check forks, so the child that creates the file + * and the parent that unlinks it agree on the name. */ +static char update_flash_file[PATH_MAX]; + static void prepare_update_flash(void) { int ret; - ret = mmap_file("/tmp/wolfboot-nsc-update.bin", + ret = mmap_file(update_flash_file, (void *)WOLFBOOT_PARTITION_UPDATE_ADDRESS, WOLFBOOT_PARTITION_SIZE, NULL); @@ -126,6 +132,8 @@ int main(int argc, char *argv[]) SRunner *sr; argv0 = strdup(argv[0]); + snprintf(update_flash_file, sizeof(update_flash_file), + "/tmp/wolfboot-unit-nsc-update-%d.bin", (int)getpid()); s = wolfboot_suite(); sr = srunner_create(s); #if (NO_FORK == 1) @@ -134,5 +142,6 @@ int main(int argc, char *argv[]) srunner_run_all(sr, CK_NORMAL); fails = srunner_ntests_failed(sr); srunner_free(sr); + unlink(update_flash_file); return fails; }