From 8cff2e879273ad02cbee17f86d52ddf984f99bd1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 18:50:47 +0200 Subject: [PATCH 01/12] F-5672: cache NS in_vec[1].len in ARM_TEE_PS_SET to close TOCTOU double-fetch The ARM_TEE_PS_SET branch of arm_tee_psa_call() read in_vec[1].len directly from non-secure memory four times: the WOLFBOOT_PS_MAX_DATA bounds check, the null-data guard, the copy gate, the XMEMCPY length, and the entry->size assignment. in_vec lives in NS RAM, and on ARMv8-M with AIRCR.PRIS=0 a higher-priority NS interrupt can preempt Secure thread-mode execution (and an NS-accessible DMA engine can mutate NS RAM independently) between the bounds check and the copy. An NS attacker racing either mechanism could grow the length past WOLFBOOT_PS_MAX_DATA after it was validated, overflowing the fixed entry->data[512] buffer in Secure SRAM into adjacent Secure globals. Snapshot the length once into a Secure-stack local (data_len) right after capturing the in_vec bases and use only that local for every subsequent check, the XMEMCPY operand, and entry->size. This follows the standard PSA/CMSE practice of copying NS-supplied scalars to the Secure stack before validating and using them. --- src/arm_tee_psa_ipc.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/arm_tee_psa_ipc.c b/src/arm_tee_psa_ipc.c index 417cc8c41b..d7f9624697 100644 --- a/src/arm_tee_psa_ipc.c +++ b/src/arm_tee_psa_ipc.c @@ -766,16 +766,23 @@ int32_t arm_tee_psa_call(psa_handle_t handle, int32_t type, const void *data; const psa_storage_create_flags_t *flags; struct wolfboot_ps_entry *entry; + size_t data_len; if (in_vec == NULL || in_len < 3) { return PSA_ERROR_INVALID_ARGUMENT; } uid = (const psa_storage_uid_t *)in_vec[0].base; data = in_vec[1].base; flags = (const psa_storage_create_flags_t *)in_vec[2].base; + /* Snapshot the NS-supplied length once into a Secure-stack local. + * in_vec lives in NS memory and may be mutated concurrently (a + * preempting NS interrupt or NS-accessible DMA), so re-reading + * in_vec[1].len after the bounds check would allow a TOCTOU + * double-fetch to grow the copy past WOLFBOOT_PS_MAX_DATA. */ + data_len = in_vec[1].len; if (uid == NULL || flags == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } - if (in_vec[1].len > WOLFBOOT_PS_MAX_DATA) { + if (data_len > WOLFBOOT_PS_MAX_DATA) { return PSA_ERROR_INSUFFICIENT_STORAGE; } entry = wolfboot_ps_find(*uid); @@ -787,13 +794,13 @@ int32_t arm_tee_psa_call(psa_handle_t handle, int32_t type, } else if ((entry->flags & PSA_STORAGE_FLAG_WRITE_ONCE) != 0U) { return PSA_ERROR_NOT_PERMITTED; } - if (in_vec[1].len > 0 && data == NULL) { + if (data_len > 0 && data == NULL) { return PSA_ERROR_INVALID_ARGUMENT; } - if (in_vec[1].len > 0) { - XMEMCPY(entry->data, data, in_vec[1].len); + if (data_len > 0) { + XMEMCPY(entry->data, data, data_len); } - entry->size = in_vec[1].len; + entry->size = data_len; entry->flags = *flags; return PSA_SUCCESS; } From 277e8013cc575bc568904c2d2b75c4a5ed80f04a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:13:36 +0200 Subject: [PATCH 02/12] F-4965: bound hdr->pos in bitmap_put to prevent OOB write from corrupted vault A power fault during cache_commit(0) can leave a node header in the keyvault with valid magic/tok/obj/type but pos left as erased flash (0xFFFFFFFF). On the next boot find_object_buffer() detects the data-sector mismatch and calls delete_object(), which reaches bitmap_put(0xFFFFFFFF, 0). bitmap_put computed octet = pos/8 and indexed cached_sector[4 + octet] with no bounds check, writing ~512 MB past the static sector buffer. Reject pos >= KEYVAULT_MAX_ITEMS so a corrupted header can no longer turn into an out-of-bounds write. Adds a unit test that seeds a node with pos=PKCS11_INVALID_ID and confirms delete_object() no longer faults. --- src/pkcs11_store.c | 6 ++++ tools/unit-tests/unit-pkcs11_store.c | 41 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index b1f0b9ede0..8965d4ca0c 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -127,6 +127,12 @@ static void bitmap_put(uint32_t pos, int val) uint32_t bit = pos % 8; uint8_t *bitmap = cached_sector + sizeof(uint32_t); + /* Reject out-of-range positions (e.g. a power-fault-corrupted hdr->pos + * left as erased flash) to avoid an out-of-bounds write past the + * bitmap, which lives within cached_sector. */ + if (pos >= KEYVAULT_MAX_ITEMS) + return; + if (val != 0) { bitmap[octet] |= (1 << bit); } else { diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index f1d8c0434a..7ea41d02f0 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -382,6 +382,44 @@ START_TEST(test_delete_object_ignores_metadata_prefix) } END_TEST +START_TEST(test_delete_object_corrupted_pos_no_oob) +{ + const int32_t type = DYNAMIC_TYPE_RSA; + const uint32_t tok_id = 0x0A0B0C0DU; + const uint32_t obj_id = 0x10203040U; + struct obj_hdr *hdr; + int ret; + + ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xFF, keyvault_size); + + /* Valid header magic and zeroed bitmap so check_vault() accepts the + * sector without restoring/reinitializing it. */ + ((uint32_t *)vault_base)[0] = VAULT_HEADER_MAGIC; + memset(vault_base + sizeof(uint32_t), 0x00, BITMAP_SIZE); + + /* Simulate a power-fault-corrupted node: valid tok/obj/type but the + * 'pos' field was never written and is left as erased flash + * (PKCS11_INVALID_ID). delete_object() must not turn this into an + * out-of-bounds bitmap_put(0xFFFFFFFF, 0). */ + hdr = NODES_TABLE; + hdr->token_id = tok_id; + hdr->object_id = obj_id; + hdr->type = type; + hdr->pos = PKCS11_INVALID_ID; + hdr->size = 2 * sizeof(uint32_t); + + delete_object(type, tok_id, obj_id); + + /* If we get here without a crash, the OOB write was avoided. The node + * should also have been invalidated. */ + ck_assert_uint_eq(NODES_TABLE->token_id, PKCS11_INVALID_ID); + ck_assert_uint_eq(NODES_TABLE->object_id, PKCS11_INVALID_ID); +} +END_TEST + START_TEST(test_find_object_search_stops_at_header_sector) { const int32_t type = DYNAMIC_TYPE_RSA; @@ -421,16 +459,19 @@ Suite *wolfboot_suite(void) TCase* tcase_cross_sector_write = tcase_create("cross_sector_write"); TCase* tcase_close = tcase_create("close_state"); TCase* tcase_delete_object = tcase_create("delete_object"); + TCase* tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase* tcase_find_bounds = tcase_create("find_bounds"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); tcase_add_test(tcase_delete_object, test_delete_object_ignores_metadata_prefix); + tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); suite_add_tcase(s, tcase_delete_object); + suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); return s; } From a8a9eec96bf9d6b313ce41dcf687d680f1822fbf Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:19:05 +0200 Subject: [PATCH 03/12] F-4711: bound e820 entries to prevent boot_params stack overflow e820_add_entry_cb() appended every FSP-supplied resource descriptor into boot_params->e820_table[] with no check against E820_MAX_ENTRIES_ZEROPAGE (128). A HOB list with more than 128 EFI_HOB_TYPE_RESOURCE_DESCRIPTOR entries therefore wrote FSP-controlled addr/size/type triples past the fixed-size table into the stack-allocated boot_params in load_linux(), corrupting adjacent fields and the saved return address. Reject any entry once the table is full (return non-zero, which aborts the HOB iteration). e820_entries stays uint8_t since it is a fixed-offset field in the Linux zero-page layout and the cap makes the 256 wrap unreachable. Add unit-linux-loader-e820 regression test (x86 32bit, standalone) that feeds 200 descriptors and asserts the table never overflows. --- src/x86/linux_loader.c | 6 +- tools/unit-tests/Makefile | 8 +++ tools/unit-tests/unit-linux-loader-e820.c | 79 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tools/unit-tests/unit-linux-loader-e820.c diff --git a/src/x86/linux_loader.c b/src/x86/linux_loader.c index b84e721faa..0dcc83009b 100644 --- a/src/x86/linux_loader.c +++ b/src/x86/linux_loader.c @@ -73,8 +73,12 @@ static int e820_add_entry_cb(uint64_t start, uint64_t length, uint32_t type, void *ctx) { struct boot_params *bp = (struct boot_params*)ctx; - struct boot_e820_entry *map = bp->e820_table + bp->e820_entries; + struct boot_e820_entry *map; + if (bp->e820_entries >= E820_MAX_ENTRIES_ZEROPAGE) + return -1; + + map = bp->e820_table + bp->e820_entries; map->addr = start; map->size = length; map->type = (type == EFI_RESOURCE_SYSTEM_MEMORY) ? E820_TYPE_RAM : diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index ee0b6398e4..7e92a88cc6 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -60,6 +60,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128 TESTS+=unit-tpm-check-rot-auth TESTS+=unit-tpm-api-names TESTS+=unit-fit-gzip unit-fit-nogzip +TESTS+=unit-linux-loader-e820 include unit-sign-encrypted-output.mkfrag @@ -264,6 +265,13 @@ unit-chacha20: ../../include/target.h unit-extflash.c unit-pci: unit-pci.c ../../src/pci.c gcc -o $@ $< $(CFLAGS) -DWOLFBOOT_USE_PCI $(LDFLAGS) +# linux_loader.c is x86 32bit only and pulls in inline asm guarded on 32bit; +# build standalone with -m32 and without coverage (no 32bit gcov/check libs). +unit-linux-loader-e820: ../../include/target.h unit-linux-loader-e820.c + gcc -m32 -o $@ unit-linux-loader-e820.c -I. -I../../src -I../../include \ + -g -DUNIT_TEST -DWOLFBOOT_FSP -DUCODE0_ADDRESS=0 \ + -DWOLFBOOT_LOAD_BASE=0x100000 + unit-boot-x86-fsp: ../../include/target.h unit-boot-x86_fsp.c gcc -o $@ $^ $(CFLAGS) -DWOLFBOOT_LOAD_BASE=0x100000 -DWOLFBOOT_FSP \ -DUCODE0_ADDRESS=0 -ffunction-sections -fdata-sections $(LDFLAGS) \ diff --git a/tools/unit-tests/unit-linux-loader-e820.c b/tools/unit-tests/unit-linux-loader-e820.c new file mode 100644 index 0000000000..02a6403063 --- /dev/null +++ b/tools/unit-tests/unit-linux-loader-e820.c @@ -0,0 +1,79 @@ +/* unit-linux-loader-e820.c + * + * Regression test for F-4711: e820_add_entry_cb() must not write past the + * fixed-size boot_params->e820_table[E820_MAX_ENTRIES_ZEROPAGE] when the FSP + * HOB list supplies more resource descriptors than the table can hold. + * + * Built for x86 32bit (the only target supported by linux_loader.c), without + * the check framework, since 32bit libcheck is not generally available. + */ + +#include +#include +#include +#include + +#include "x86/hob.h" +#include "x86/linux_loader.h" + +#include "../../src/x86/hob.c" +#include "../../src/x86/linux_loader.c" + +/* More descriptors than the table can hold, to force the overflow path. */ +#define N_ENTRIES 200 +#define CANARY_LEN 2048 +#define CANARY_BYTE 0xAA + +/* boot_params followed by a canary region: any write past e820_table runs + * through the rest of boot_params and into the canary. */ +static struct { + struct boot_params bp; + uint8_t canary[CANARY_LEN]; +} obj; + +static uint8_t hoblist[(N_ENTRIES + 1) * + sizeof(struct efi_hob_resource_descriptor)]; + +int main(void) +{ + int i; + uint8_t *p = hoblist; + struct efi_hob_resource_descriptor *rd; + struct efi_hob_generic_header *end; + + for (i = 0; i < N_ENTRIES; i++) { + rd = (struct efi_hob_resource_descriptor *)p; + memset(rd, 0, sizeof(*rd)); + rd->header.hob_type = EFI_HOB_TYPE_RESOURCE_DESCRIPTOR; + rd->header.hob_length = sizeof(struct efi_hob_resource_descriptor); + rd->resource_type = EFI_RESOURCE_SYSTEM_MEMORY; + rd->physical_start = 0x4141414141414141ULL; + rd->resource_length = 0x4242424242424242ULL; + p += sizeof(struct efi_hob_resource_descriptor); + } + end = (struct efi_hob_generic_header *)p; + end->hob_type = EFI_HOB_TYPE_END_OF_HOB_LIST; + end->hob_length = sizeof(struct efi_hob_generic_header); + + memset(&obj, 0, sizeof(obj)); + memset(obj.canary, CANARY_BYTE, CANARY_LEN); + + (void)memory_map_from_hoblist(&obj.bp, (struct efi_hob *)hoblist); + + printf("e820_entries=%u\n", obj.bp.e820_entries); + + for (i = 0; i < CANARY_LEN; i++) { + if (obj.canary[i] != CANARY_BYTE) { + printf("FAIL: e820_table overflow corrupted memory at +%d\n", i); + return 1; + } + } + if (obj.bp.e820_entries > E820_MAX_ENTRIES_ZEROPAGE) { + printf("FAIL: e820_entries=%u exceeds max %d\n", + obj.bp.e820_entries, E820_MAX_ENTRIES_ZEROPAGE); + return 1; + } + + printf("PASS\n"); + return 0; +} From 27664501231dc30e240e503f787d0b3518156fea Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:26:28 +0200 Subject: [PATCH 04/12] F-4710: reject oversized FDT property length to prevent ~4GB FIT memcpy fdt_next_tag() advanced its struct cursor with offset += sizeof(struct fdt_property) - FDT_TAGSIZE + fdt32_to_cpu(*lenp); using unsigned arithmetic. A property whose len field is 0xFFFFFFFF wrapped this to a +7 advance, so the malformed node slipped past the fdt_offset_ptr() bounds check at the end of fdt_next_tag(). The bogus length then propagated up through fdt_get_property_by_offset() / fdt_getprop() and was returned by fit_load_image() as *lenp = -1. In the MMU FIT boot path, wolfBoot_start() (src/update_ram.c:465) aliases that out-parameter through (int*)&dts_size, turning -1 into a uint32_t 0xFFFFFFFF, and the only guard before the relocation is dts_ptr != NULL, so memcpy(WOLFBOOT_LOAD_DTS_ADDRESS, dts_ptr, 0xFFFFFFFF) ran (CWE-680). A FIT subimage with no "load" property reaches this with the inner copy skipped, so the giant size hits the outer DTS relocation directly. Fix at the root: a property value can never exceed the blob, so reject any FDT_PROP whose declared length is greater than fdt_totalsize() before the cursor arithmetic. This closes the wrap for every caller of fdt_next_tag() (including the other fit_load_image() sinks), not just the DTS path. Legitimate properties (len <= size_dt_struct < totalsize) are unaffected. Add a regression test to unit-fdt: a hand-built FIT whose /images/kernel-1 "data" property declares len=0xFFFFFFFF must make fit_load_image_ex() fail closed (return NULL) instead of handing back a live pointer with a negative length. The test fails before this change and passes after. --- src/fdt.c | 9 ++++++ tools/unit-tests/unit-fdt.c | 59 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/fdt.c b/src/fdt.c index b31feee971..095653c79c 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -152,6 +152,15 @@ static uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) if (!lenp) { return FDT_END; /* premature end */ } + /* A property value can never be larger than the blob itself. + * Reject an oversized length up front: otherwise the unsigned + * cursor arithmetic below wraps (e.g. len=0xFFFFFFFF advances + * offset by only 7 bytes), the malformed node slips past the + * fdt_offset_ptr() bounds check, and the bogus length propagates + * to callers as a negative int (a ~4GB memcpy size). */ + if (fdt32_to_cpu(*lenp) > (uint32_t)fdt_totalsize(fdt)) { + return FDT_END; /* bad structure */ + } /* skip-name offset, length and value */ offset += sizeof(struct fdt_property) - FDT_TAGSIZE + fdt32_to_cpu(*lenp); diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index c52672fe48..96f9e6e235 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -78,6 +78,64 @@ START_TEST(test_fdt_get_string_returns_string_with_valid_offset) } END_TEST +/* Minimal FIT with a single /images/kernel-1 node whose `data` property + * declares len=0xFFFFFFFF. There is no `load` (and no `compression`), so + * fit_load_image_inner() takes the pass-through branch. Before the + * fdt_next_tag() length check, the oversized len wrapped the cursor + * arithmetic, slipped past the bounds check, and was handed back as + * *lenp = -1 - which update_ram.c then aliased into a ~4GB memcpy size. + * The loader must instead fail closed (return NULL). */ +static const uint8_t fit_data_len_overflow[] = { + /* header */ + 0xd0, 0x0d, 0xfe, 0xed, /* magic */ + 0x00, 0x00, 0x00, 0x81, /* totalsize = 129 */ + 0x00, 0x00, 0x00, 0x38, /* off_dt_struct = 56 */ + 0x00, 0x00, 0x00, 0x7c, /* off_dt_strings = 124 */ + 0x00, 0x00, 0x00, 0x28, /* off_mem_rsvmap = 40 */ + 0x00, 0x00, 0x00, 0x11, /* version = 17 */ + 0x00, 0x00, 0x00, 0x10, /* last_comp_version = 16 */ + 0x00, 0x00, 0x00, 0x00, /* boot_cpuid_phys */ + 0x00, 0x00, 0x00, 0x05, /* size_dt_strings = 5 */ + 0x00, 0x00, 0x00, 0x44, /* size_dt_struct = 68 */ + /* mem_rsvmap terminator (offset 40) */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* struct block (offset 56) */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE root */ + 0x00, 0x00, 0x00, 0x00, /* "" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE images */ + 0x69, 0x6d, 0x61, 0x67, 0x65, 0x73, 0x00, 0x00, /* "images\0\0" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE kernel-1 */ + 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x2d, 0x31, + 0x00, 0x00, 0x00, 0x00, /* "kernel-1\0\0\0\0" */ + 0x00, 0x00, 0x00, 0x03, /* FDT_PROP */ + 0xff, 0xff, 0xff, 0xff, /* len = 0xFFFFFFFF */ + 0x00, 0x00, 0x00, 0x00, /* nameoff = 0 ("data") */ + 0x00, 0x00, 0x00, 0x00, /* data (4 bytes) */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE kernel-1 */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE images */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE root */ + 0x00, 0x00, 0x00, 0x09, /* FDT_END */ + /* strings block (offset 124) */ + 0x64, 0x61, 0x74, 0x61, 0x00, /* "data\0" */ +}; + +START_TEST(test_fit_load_image_rejects_oversized_prop_len) +{ + static uint8_t fit_scratch[sizeof(fit_data_len_overflow)]; + int len = 0; + void *ret; + + memcpy(fit_scratch, fit_data_len_overflow, sizeof(fit_scratch)); + + ret = fit_load_image_ex(fit_scratch, "kernel-1", &len, 64 * 1024); + + /* Must fail closed: never return a live pointer with a negative + * length that a caller could turn into a giant memcpy size. */ + ck_assert_ptr_null(ret); +} +END_TEST + static Suite *fdt_suite(void) { Suite *s = suite_create("fdt"); @@ -85,6 +143,7 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_get_string_rejects_out_of_range_offset); tcase_add_test(tc, test_fdt_get_string_returns_string_with_valid_offset); + tcase_add_test(tc, test_fit_load_image_rejects_oversized_prop_len); suite_add_tcase(s, tc); return s; From d175c819cd1589bfbfcd08a4d2ff92f87846bfb0 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:33:51 +0200 Subject: [PATCH 05/12] F-4645: bound load_linux kernel size to prevent syssize*16 overflow load_linux() computed the protected-mode kernel size as the uint32_t product param.hdr.syssize * 16 (src/x86/linux_loader.c), where syssize is copied verbatim from the (authenticated) bzImage at offset 0x1f4. The multiplication wraps for any syssize > 0x0FFFFFFF: syssize=0x10000000 yields kernel_size=0 (DoS), and syssize=0x1FFFFFFF/0xFFFFFFFF yields kernel_size=0xFFFFFFF0 (~4 GiB). That value fed straight into memcpy((uint8_t*)KERNEL_LOAD_ADDRESS, linux_image + param_size, kernel_size) with no cap, overwriting wolfBoot stage2, FSP data, and the heap (CWE-190 -> CWE-680). Fix at the root: linux_kernel_size() computes syssize * 16 in 64-bit and rejects the image (panic) when the result is zero or does not fit in the destination window [KERNEL_LOAD_ADDRESS, tolum). tolum is the top of low usable memory the FSP already reports and that the ELF boot path uses as its load upper bound (src/boot_x86_fsp_payload.c). The kernel load only runs under WOLFBOOT_FSP (the non-FSP path panics earlier at the memory map step), so tolum is always available there. Add unit-linux-loader-syssize regression test (x86 32bit, standalone) that feeds the PoC overflow values and asserts they are rejected while a legitimate kernel and the exact-fit boundary are accepted. --- src/x86/linux_loader.c | 29 +++++- tools/unit-tests/Makefile | 6 ++ tools/unit-tests/unit-linux-loader-syssize.c | 94 ++++++++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tools/unit-tests/unit-linux-loader-syssize.c diff --git a/src/x86/linux_loader.c b/src/x86/linux_loader.c index 0dcc83009b..6d0ebda30f 100644 --- a/src/x86/linux_loader.c +++ b/src/x86/linux_loader.c @@ -110,10 +110,27 @@ static int linux_boot_params_fill_memory_map(struct boot_params *bp, #define KERNEL_LOAD_ADDRESS 0x100000 #define KERNEL_CMDLINE_ADDRESS 0x10000 +/* Compute the protected-mode kernel size (syssize * 16) in 64-bit to avoid the + * uint32_t multiplication wrap, and reject any image whose kernel would not fit + * in the destination window [KERNEL_LOAD_ADDRESS, load_limit). Returns 0 on + * success, -1 if the size is zero or out of range. */ +static int linux_kernel_size(uint32_t syssize, uint32_t load_limit, + uint32_t *kernel_size) +{ + uint64_t ksz = (uint64_t)syssize * 16u; + + if (load_limit <= KERNEL_LOAD_ADDRESS) + return -1; + if (ksz == 0 || ksz > (uint64_t)(load_limit - KERNEL_LOAD_ADDRESS)) + return -1; + *kernel_size = (uint32_t)ksz; + return 0; +} + void load_linux(uint8_t *linux_image, void *params, const char *cmd_line) { struct boot_params param = { 0 }; - uint32_t kernel_size, param_size; + uint32_t kernel_size, param_size, load_limit; uint8_t *image_boot_param; uint16_t end_of_header_off; uint8_t *_cmd_line; @@ -145,7 +162,15 @@ void load_linux(uint8_t *linux_image, void *params, const char *cmd_line) memcpy(_cmd_line, (uint8_t*)cmd_line, strlen(cmd_line)+1); param.hdr.type_of_loader = 0xff; param.hdr.cmd_line_ptr = (uint32_t)(uintptr_t)_cmd_line; - kernel_size = param.hdr.syssize * 16; +#ifdef WOLFBOOT_FSP + load_limit = ((struct stage2_parameter *)params)->tolum; +#else + load_limit = 0; +#endif /* WOLFBOOT_FSP */ + if (linux_kernel_size(param.hdr.syssize, load_limit, &kernel_size) != 0) { + wolfBoot_printf("invalid kernel size" ENDLINE); + wolfBoot_panic(); + } memcpy((uint8_t *)KERNEL_LOAD_ADDRESS, linux_image + param_size, kernel_size); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 7e92a88cc6..d1ff2f992b 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -61,6 +61,7 @@ TESTS+=unit-tpm-check-rot-auth TESTS+=unit-tpm-api-names TESTS+=unit-fit-gzip unit-fit-nogzip TESTS+=unit-linux-loader-e820 +TESTS+=unit-linux-loader-syssize include unit-sign-encrypted-output.mkfrag @@ -272,6 +273,11 @@ unit-linux-loader-e820: ../../include/target.h unit-linux-loader-e820.c -g -DUNIT_TEST -DWOLFBOOT_FSP -DUCODE0_ADDRESS=0 \ -DWOLFBOOT_LOAD_BASE=0x100000 +unit-linux-loader-syssize: ../../include/target.h unit-linux-loader-syssize.c + gcc -m32 -o $@ unit-linux-loader-syssize.c -I. -I../../src -I../../include \ + -g -DUNIT_TEST -DWOLFBOOT_FSP -DUCODE0_ADDRESS=0 \ + -DWOLFBOOT_LOAD_BASE=0x100000 + unit-boot-x86-fsp: ../../include/target.h unit-boot-x86_fsp.c gcc -o $@ $^ $(CFLAGS) -DWOLFBOOT_LOAD_BASE=0x100000 -DWOLFBOOT_FSP \ -DUCODE0_ADDRESS=0 -ffunction-sections -fdata-sections $(LDFLAGS) \ diff --git a/tools/unit-tests/unit-linux-loader-syssize.c b/tools/unit-tests/unit-linux-loader-syssize.c new file mode 100644 index 0000000000..9294b73a81 --- /dev/null +++ b/tools/unit-tests/unit-linux-loader-syssize.c @@ -0,0 +1,94 @@ +/* unit-linux-loader-syssize.c + * + * Regression test for F-4645: load_linux() computed the protected-mode kernel + * size as the uint32_t product param.hdr.syssize * 16, which wraps for any + * syssize > 0x0FFFFFFF. The wrapped value (0 or ~4 GiB) was passed straight to + * memcpy() into KERNEL_LOAD_ADDRESS with no bounds check. + * + * linux_kernel_size() now performs the multiplication in 64-bit and rejects a + * size that is zero or does not fit in the destination window + * [KERNEL_LOAD_ADDRESS, load_limit). This test exercises that helper directly. + * + * Built for x86 32bit (the only target supported by linux_loader.c), without + * the check framework, since 32bit libcheck is not generally available. + */ + +#include +#include +#include +#include + +#include "x86/hob.h" +#include "x86/linux_loader.h" + +#include "../../src/x86/hob.c" +#include "../../src/x86/linux_loader.c" + +/* A generous low-memory window (256 MiB) used as the destination limit. */ +#define LOAD_LIMIT 0x10000000u + +int main(void) +{ + uint32_t ksz; + int ret; + + /* The PoC overflow values: all wrap when computed as uint32_t. */ + const uint32_t wrap_cases[] = { + 0x10000000u, /* 32bit product == 0x00000000 (DoS) */ + 0x1FFFFFFFu, /* 32bit product == 0xFFFFFFF0 (~4 GiB) */ + 0xFFFFFFFFu, /* 32bit product == 0xFFFFFFF0 (~4 GiB) */ + }; + unsigned i; + + for (i = 0; i < sizeof(wrap_cases) / sizeof(wrap_cases[0]); i++) { + uint32_t syssize = wrap_cases[i]; + + /* Demonstrate the original wrap: the 32bit product no longer matches + * the true 64bit size, which is exactly why a bound is required. */ + if ((uint32_t)(syssize * 16u) == (uint64_t)syssize * 16u) { + printf("FAIL: case 0x%08x does not actually wrap\n", syssize); + return 1; + } + + ksz = 0xDEADBEEF; + ret = linux_kernel_size(syssize, LOAD_LIMIT, &ksz); + if (ret == 0) { + printf("FAIL: overflowing syssize 0x%08x accepted (ksz=0x%08x)\n", + syssize, ksz); + return 1; + } + } + + /* A size that exceeds the window (but does not wrap) must be rejected. */ + ret = linux_kernel_size(LOAD_LIMIT / 16u, LOAD_LIMIT, &ksz); + if (ret == 0) { + printf("FAIL: oversized kernel accepted\n"); + return 1; + } + + /* Zero-sized kernel must be rejected. */ + if (linux_kernel_size(0, LOAD_LIMIT, &ksz) == 0) { + printf("FAIL: zero-sized kernel accepted\n"); + return 1; + } + + /* A legitimate kernel that fits must be accepted with the exact size. */ + ksz = 0; + ret = linux_kernel_size(0x10000u, LOAD_LIMIT, &ksz); + if (ret != 0 || ksz != 0x10000u * 16u) { + printf("FAIL: valid kernel rejected (ret=%d ksz=0x%08x)\n", ret, ksz); + return 1; + } + + /* The largest kernel that exactly fills the window must be accepted. */ + ret = linux_kernel_size((LOAD_LIMIT - KERNEL_LOAD_ADDRESS) / 16u, + LOAD_LIMIT, &ksz); + if (ret != 0 || ksz != (LOAD_LIMIT - KERNEL_LOAD_ADDRESS)) { + printf("FAIL: exact-fit kernel rejected (ret=%d ksz=0x%08x)\n", + ret, ksz); + return 1; + } + + printf("PASS\n"); + return 0; +} From 091b4eabbec574bd5ad706ed24ccdf1ed8dddd5e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:41:59 +0200 Subject: [PATCH 06/12] F-4644: validate NS pointers in TPM CSME_NSE_API veneers with cmse_check_address_range The eleven CSME_NSE_API TPM veneers in src/tpm.c are cmse_nonsecure_entry gateways when built with TZEN=1 (e.g. config/examples/stm32h5-tz-tpm.config, which exposes them to the non-secure STM32H5 test app). Each accepted a typed pointer from the non-secure caller and immediately used it as a dereference or memset/XMEMSET target on the Secure side -- memset(caps), memset(handles), memset(getTime), XMEMSET(quoteResult), and the in/out forwards to TPM2_GetCapability / TPM2_ParseAttest. Because Secure code can write Secure SRAM, a malicious non-secure caller could pass a Secure pointer and turn any of these veneers into a confused-deputy write primitive against Secure memory. Validate every non-secure-supplied pointer with cmse_check_address_range (CMSE_NONSECURE, plus CMSE_MPU_READWRITE for write targets) before the first use, returning BAD_FUNC_ARG (or NULL for the string helpers) when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_TPM_NS_RW/WOLFBOOT_TPM_NS_R, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (where there is no security boundary) collapse to a plain non-NULL pass-through. Buffer sizes use the caller-provided/known capacities (name_sz, error_sz, *certSz, PCR digest size). Verified by building wolfboot.elf with the stm32h5-tz-tpm config (-mcmse); the bug itself is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build. --- src/tpm.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/tpm.c b/src/tpm.c index 98ed7f601d..71ec29d9e5 100644 --- a/src/tpm.c +++ b/src/tpm.c @@ -1230,14 +1230,39 @@ static int wolfRNG_GetSeedCB(OS_Seed* os, uint8_t* seed, uint32_t sz) /* API's that are callable from non-secure code */ + +/* Validate that a buffer supplied by the non-secure caller is fully + * accessible from the non-secure world before the secure side dereferences + * it. Without this check a non-secure caller could pass a pointer into Secure + * SRAM and turn these veneers into a confused-deputy write primitive against + * Secure memory. Outside of a CMSE secure build there is no security boundary, + * so the checks collapse to a simple non-NULL pass-through. */ +#if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) +#include +#define WOLFBOOT_TPM_NS_RW(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), \ + CMSE_NONSECURE | CMSE_MPU_READWRITE) +#define WOLFBOOT_TPM_NS_R(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) +#else +#define WOLFBOOT_TPM_NS_RW(p, sz) ((void*)(p)) +#define WOLFBOOT_TPM_NS_R(p, sz) ((void*)(p)) +#endif + int CSME_NSE_API wolfBoot_tpm2_caps(WOLFTPM2_CAPS* caps) { + if (WOLFBOOT_TPM_NS_RW(caps, sizeof(*caps)) == NULL) { + return BAD_FUNC_ARG; + } memset(caps, 0, sizeof(*caps)); return wolfTPM2_GetCapabilities(&wolftpm_dev, caps); } int CSME_NSE_API wolfBoot_tpm2_get_handles(TPM_HANDLE handle, TPML_HANDLE* handles) { + if (WOLFBOOT_TPM_NS_RW(handles, sizeof(*handles)) == NULL) { + return BAD_FUNC_ARG; + } memset(handles, 0, sizeof(*handles)); return wolfTPM2_GetHandles(handle, handles); } @@ -1249,6 +1274,9 @@ const char* CSME_NSE_API wolfBoot_tpm2_get_alg_name(TPM_ALG_ID alg, if (name == NULL || name_sz <= 0) { return NULL; } + if (WOLFBOOT_TPM_NS_RW(name, name_sz) == NULL) { + return NULL; + } s_name = TPM2_GetAlgName(alg); if (s_name != NULL && name != NULL && name_sz > 0) { strncpy(name, s_name, name_sz - 1); @@ -1267,6 +1295,9 @@ const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc, char* error, int er if (error == NULL || error_sz <= 0) { return NULL; } + if (WOLFBOOT_TPM_NS_RW(error, error_sz) == NULL) { + return NULL; + } s_error = TPM2_GetRCString(rc); if (s_error != NULL && error != NULL && error_sz > 0) { strncpy(error, s_error, error_sz - 1); @@ -1281,17 +1312,32 @@ const char* CSME_NSE_API wolfBoot_tpm2_get_rc_string(int rc, char* error, int er int CSME_NSE_API wolfBoot_tpm2_get_capability(GetCapability_In* in, GetCapability_Out* out) { + if (WOLFBOOT_TPM_NS_R(in, sizeof(*in)) == NULL || + WOLFBOOT_TPM_NS_RW(out, sizeof(*out)) == NULL) { + return BAD_FUNC_ARG; + } return (int)TPM2_GetCapability(in, out); } int CSME_NSE_API wolfBoot_tpm2_read_pcr(uint8_t pcrIndex, uint8_t* digest, int* digestSz) { + if (WOLFBOOT_TPM_NS_RW(digest, + TPM2_GetHashDigestSize(WOLFBOOT_TPM_PCR_ALG)) == NULL || + WOLFBOOT_TPM_NS_RW(digestSz, sizeof(*digestSz)) == NULL) { + return BAD_FUNC_ARG; + } return wolfTPM2_ReadPCR(&wolftpm_dev, pcrIndex, WOLFBOOT_TPM_PCR_ALG, digest, digestSz); } int CSME_NSE_API wolfBoot_tpm2_read_cert(uint32_t handle, uint8_t* cert, uint32_t* certSz) { + if (WOLFBOOT_TPM_NS_RW(certSz, sizeof(*certSz)) == NULL) { + return BAD_FUNC_ARG; + } + if (WOLFBOOT_TPM_NS_RW(cert, *certSz) == NULL) { + return BAD_FUNC_ARG; + } wolfTPM2_SetAuthPassword(&wolftpm_dev, 0, NULL); return wolfTPM2_NVReadCert(&wolftpm_dev, handle, cert, certSz); } @@ -1304,6 +1350,13 @@ int CSME_NSE_API wolfBoot_tpm2_get_aik(WOLFTPM2_KEY* aik, if (aik == NULL) { return BAD_FUNC_ARG; } + if (WOLFBOOT_TPM_NS_RW(aik, sizeof(*aik)) == NULL) { + return BAD_FUNC_ARG; + } + if (masterPassword != NULL && + WOLFBOOT_TPM_NS_R(masterPassword, masterPasswordSz) == NULL) { + return BAD_FUNC_ARG; + } /* Load existing AIK and set auth */ rc = wolfTPM2_ReadPublicKey(&wolftpm_dev, aik, TPM2_IAK_KEY_HANDLE); @@ -1330,6 +1383,10 @@ int CSME_NSE_API wolfBoot_tpm2_get_timestamp(WOLFTPM2_KEY* aik, GetTime_Out* get if (aik == NULL || getTime == NULL) { return BAD_FUNC_ARG; } + if (WOLFBOOT_TPM_NS_RW(aik, sizeof(*aik)) == NULL || + WOLFBOOT_TPM_NS_RW(getTime, sizeof(*getTime)) == NULL) { + return BAD_FUNC_ARG; + } memset(getTime, 0, sizeof(*getTime)); memset(&eh_handle, 0, sizeof(eh_handle)); @@ -1358,6 +1415,10 @@ int CSME_NSE_API wolfBoot_tpm2_get_timestamp(WOLFTPM2_KEY* aik, GetTime_Out* get int CSME_NSE_API wolfBoot_tpm2_parse_attest(const TPM2B_ATTEST* in, TPMS_ATTEST* out) { + if (WOLFBOOT_TPM_NS_R(in, sizeof(*in)) == NULL || + WOLFBOOT_TPM_NS_RW(out, sizeof(*out)) == NULL) { + return BAD_FUNC_ARG; + } return TPM2_ParseAttest(in, out); } @@ -1372,6 +1433,11 @@ int CSME_NSE_API wolfBoot_tpm2_quote(WOLFTPM2_KEY* aik, quoteResult == NULL) { return BAD_FUNC_ARG; } + if (WOLFBOOT_TPM_NS_RW(aik, sizeof(*aik)) == NULL || + WOLFBOOT_TPM_NS_R(pcrArray, pcrArraySz) == NULL || + WOLFBOOT_TPM_NS_RW(quoteResult, sizeof(*quoteResult)) == NULL) { + return BAD_FUNC_ARG; + } /* set auth for using the AIK */ wolfTPM2_SetAuthHandle(&wolftpm_dev, 0, &aik->handle); From e1dd13e4dad9f70f0ce21f3fcb724e188a114075 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:44:07 +0200 Subject: [PATCH 07/12] F-4417: validate NS pointer in wcs_get_random CMSE veneer with cmse_check_address_range wcs_get_random is a cmse_nonsecure_entry secure gateway (wc_callable.o is in SECURE_OBJS, built with -mcmse when WOLFCRYPT_TZ=1, e.g. config/examples/stm32l5-wolfcrypt-tz.config). The rand pointer and size arrive directly from the non-secure caller and were passed unchecked to wc_RNG_GenerateBlock, which writes size bytes through rand. Because Secure code can write Secure SRAM, a malicious NS caller could pass a Secure pointer and turn the veneer into a confused-deputy write primitive against Secure memory (RNG output aimed at key buffers, RNG state, or a Secure stack return address). Validate the full rand/size range with cmse_check_address_range (CMSE_NONSECURE | CMSE_MPU_READWRITE) before the write, returning BAD_FUNC_ARG when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_WCS_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (no security boundary) collapse to a plain non-NULL pass-through. The range check already rejects oversized lengths that overrun NS-accessible memory, so no separate size cap is needed. Same fix pattern as F-4644 (TPM veneers in tpm.c). Verified by compiling src/wc_callable.c with cortex-m33 -mcmse -DWOLFCRYPT_SECURE_MODE; the bug is a TrustZone partitioning issue that cannot be exercised on the host unit-test build. --- src/wc_callable.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/wc_callable.c b/src/wc_callable.c index 1dd9761232..04fd2fb557 100644 --- a/src/wc_callable.c +++ b/src/wc_callable.c @@ -34,10 +34,29 @@ #include "wolfboot/wcs_fwtpm.h" #endif +/* wcs_get_random is a cmse_nonsecure_entry veneer: the rand pointer and size + * arrive from the non-secure caller and are used as the write target of + * wc_RNG_GenerateBlock. Validate the whole range is accessible from the + * non-secure world before writing, otherwise an NS caller could aim the RNG + * output at Secure SRAM (a confused-deputy write primitive). Outside a CMSE + * secure build there is no security boundary, so the check collapses to a + * non-NULL pass-through. */ +#if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) +#include +#define WOLFBOOT_WCS_NS_RW(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), \ + CMSE_NONSECURE | CMSE_MPU_READWRITE) +#else +#define WOLFBOOT_WCS_NS_RW(p, sz) ((void*)(p)) +#endif + static WC_RNG wcs_rng; int CSME_NSE_API wcs_get_random(uint8_t *rand, uint32_t size) { + if (WOLFBOOT_WCS_NS_RW(rand, size) == NULL) { + return BAD_FUNC_ARG; + } return wc_RNG_GenerateBlock(&wcs_rng, rand, size); } From 2ebc304caed6caf59d66a30dc9cdda281a9c696f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:45:41 +0200 Subject: [PATCH 08/12] F-4416: validate NS pointers in wcs_fwtpm_transmit CMSE veneer wcs_fwtpm_transmit is a cmse_nonsecure_entry veneer that receives the command buffer (cmd), response buffer (rsp) and response-size pointer (rspSz) directly from the non-secure caller. It only checked for NULL and size bounds, then passed the pointers to FWTPM_ProcessCommand (reading cmd, writing rsp) and dereferenced rspSz. A non-secure caller could aim rsp at Secure SRAM (confused-deputy write, forging TPM responses) or cmd at Secure memory (leak through the command path). Validate each NS-supplied range with cmse_check_address_range before first use: cmd as read-only (cmdSz), rspSz as read-write (sizeof) before dereferencing it, and rsp as read-write (rspCapacity). The checks are wrapped in WCS_FWTPM_NS_R/WCS_FWTPM_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds collapse to a non-NULL pass-through. Matches the existing guards in wc_callable.c and tpm.c. Verified by building wolfboot.elf with the stm32h5-tz-fwtpm config (-mcmse); the bug is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build. --- src/fwtpm_callable.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/fwtpm_callable.c b/src/fwtpm_callable.c index 0aa514bbcf..ba605e7b27 100644 --- a/src/fwtpm_callable.c +++ b/src/fwtpm_callable.c @@ -17,6 +17,25 @@ #include "wolftpm/fwtpm/fwtpm_nv.h" #include "wolftpm/tpm2_types.h" +/* Validate that a buffer supplied by the non-secure caller is fully accessible + * from the non-secure world before the secure side dereferences it. Without + * this check a non-secure caller could pass a pointer into Secure SRAM and turn + * this veneer into a confused-deputy primitive (forging TPM responses into + * Secure memory or leaking Secure memory through the command path). Outside of a + * CMSE secure build there is no security boundary, so the checks collapse to a + * simple non-NULL pass-through. */ +#if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) +#include +#define WCS_FWTPM_NS_RW(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), \ + CMSE_NONSECURE | CMSE_MPU_READWRITE) +#define WCS_FWTPM_NS_R(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) +#else +#define WCS_FWTPM_NS_RW(p, sz) ((void*)(p)) +#define WCS_FWTPM_NS_R(p, sz) ((void*)(p)) +#endif + static FWTPM_CTX fwtpm_ctx; static int fwtpm_ready; @@ -129,11 +148,18 @@ int CSME_NSE_API wcs_fwtpm_transmit(const uint8_t *cmd, uint32_t cmdSz, cmdSz > WCS_FWTPM_MAX_COMMAND_SIZE) { return BAD_FUNC_ARG; } + if (WCS_FWTPM_NS_R(cmd, cmdSz) == NULL || + WCS_FWTPM_NS_RW(rspSz, sizeof(*rspSz)) == NULL) { + return BAD_FUNC_ARG; + } rspCapacity = *rspSz; if (rspCapacity == 0U || rspCapacity > WCS_FWTPM_MAX_COMMAND_SIZE) { return BAD_FUNC_ARG; } + if (WCS_FWTPM_NS_RW(rsp, rspCapacity) == NULL) { + return BAD_FUNC_ARG; + } rspLen = (int)rspCapacity; rc = FWTPM_ProcessCommand(&fwtpm_ctx, cmd, (int)cmdSz, rsp, &rspLen, 0); From 53601308aa5b4e80da8914034144160d985c3b33 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:49:13 +0200 Subject: [PATCH 09/12] F-4338: skip MMIO BAR with zero alignment to prevent length-wrap cursor stall A hostile or malformed PCIe endpoint can return a BAR size-probe readback whose address bits (31:4) are all zero but is itself non-zero (e.g. 0x8, only the prefetch indicator). This passes the bar_value == 0 guard, yields bar_align == 0, and makes length = (~bar_align) + 1 wrap to 0 in uint32_t. With length == 0 the allocator cursor *base = bar_value + length is left unchanged, so the next BAR is programmed onto the same MMIO address, colliding the windows of all following devices. Treat a BAR with no writable address bits as unimplemented and skip it via restore_bar before computing length. Legitimate MMIO BARs always have at least one writable address bit; IO BARs already force the high bits, so bar_align is never 0 for them. Add a unit test that simulates the malformed probe readback and verifies the BAR is restored (not programmed) and does not collide with the next BAR. --- src/pci.c | 9 +++++ tools/unit-tests/unit-pci.c | 66 +++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/pci.c b/src/pci.c index eafcb69517..83f5f83f48 100644 --- a/src/pci.c +++ b/src/pci.c @@ -481,6 +481,15 @@ static int pci_program_bar(uint8_t bus, uint8_t dev, uint8_t fun, (is_mmio ? "mm" : "io"), bus, dev, fun, bar_idx, (uint32_t)bar_value, *is_64bit ? "64bit" : "", is_prefetch ? "prefetch" : ""); + /* A BAR with no writable address bits (bar_align == 0) is unimplemented + * or malformed: (~0) + 1 would wrap to length 0, leaving the allocator + * cursor unchanged and colliding the next BAR onto the same address. + * Treat it as unimplemented and skip it. Legitimate MMIO BARs always + * have at least one writable address bit; IO BARs force the high bits + * above, so bar_align is never 0 for them. */ + if (bar_align == 0) + goto restore_bar; + align = length = (~bar_align) + 1; /* force pci address to be on page boundary */ if (align < 0x1000) diff --git a/tools/unit-tests/unit-pci.c b/tools/unit-tests/unit-pci.c index b4213d10f0..bf1196aeee 100644 --- a/tools/unit-tests/unit-pci.c +++ b/tools/unit-tests/unit-pci.c @@ -57,6 +57,8 @@ struct test_pci_bar_info { uint8_t is_prefetch; /* 1=prefetchable */ uint8_t io_hi16_zero;/* 1=IO BAR only decodes 16 bits (upper 16 of mask are 0) */ uint32_t upper_mask; /* 64-bit BARs: upper half probe mask (0 = use default 0xFFFFFFFF) */ + uint8_t has_raw_probe;/* 1=override probe readback with raw_probe (hostile/malformed BAR) */ + uint32_t raw_probe; /* raw value returned on probe when has_raw_probe is set */ }; struct test_pci_node { @@ -262,6 +264,8 @@ static uint32_t test_pci_bar_probe_mask(struct test_pci_node *n, int bar_idx) return 0; b = &n->bars[bar_idx]; + if (b->has_raw_probe) + return b->raw_probe; if (b->size > 0) { uint32_t mask; if (b->is_io) { @@ -1061,6 +1065,64 @@ START_TEST(test_program_bars_iteration) } END_TEST +/* test_program_bar_zero_align: a hostile/malformed MMIO BAR whose probe + * readback has all address bits (31:4) zero but is non-zero (e.g. 0x8, just + * the prefetch indicator) must be treated as unimplemented. Otherwise + * bar_align == 0 makes length = (~0)+1 wrap to 0, the allocator cursor is not + * advanced, and the following BAR is programmed onto the same address. */ +START_TEST(test_program_bar_zero_align) +{ + struct test_pci_topology t; + struct pci_enum_info info; + int dev_node; + uint32_t bar0_val, bar1_val; + + test_pci_init(&t); + dev_node = test_pci_add_dev(&t, 0, 0, 0x1234, 0x5678, TEST_PCI_ROOT_BUS); + /* BAR0: malformed prefetchable MMIO — probe returns only the prefetch + * bit (0x8), so bar_align == 0. Use a raw override; no power-of-2 size + * can produce this readback. */ + t.nodes[dev_node].bars[0].has_raw_probe = 1; + t.nodes[dev_node].bars[0].raw_probe = 0x00000008; + /* BAR1: normal 64KB prefetchable MMIO (same mem_pf window). */ + test_pci_dev_set_bar(&t, dev_node, 1, 0x10000, TEST_PCI_BAR_PF); + test_pci_commit(&t); + + /* Pre-fill BAR0 so we can confirm it is restored, not programmed. */ + { + uint32_t orig = 0xDEAD0008; + memcpy(&t.nodes[dev_node].cfg[PCI_BAR0_OFFSET], &orig, 4); + } + + memset(&info, 0, sizeof(info)); + info.mem = 0x80000000; + info.mem_limit = 0x88000000; + info.mem_pf = 0x90000000; + info.mem_pf_limit = 0xFFFFFFFF; + info.io = 0x2000; + + pci_enum_bus(0, &info); + + bar0_val = pci_config_read32(0, 0, 0, PCI_BAR0_OFFSET); + bar1_val = pci_config_read32(0, 0, 0, PCI_BAR0_OFFSET + 4); + + /* The malformed BAR0 must be skipped and restored to its original value, + * never programmed onto the MMIO window. */ + ck_assert_uint_eq(bar0_val, 0xDEAD0008); + + /* BAR1 takes the head of the prefetchable window. */ + ck_assert_uint_eq(bar1_val, 0x90000000); + + /* The two BARs must not collide on the same MMIO address. */ + ck_assert_uint_ne(bar0_val, bar1_val); + + /* mem_pf advanced only by BAR1's size. */ + ck_assert_uint_eq(info.mem_pf, 0x90000000 + 0x10000); + + test_pci_cleanup(&t); +} +END_TEST + /* test_program_bridge: parameterized bridge programming tests */ START_TEST(test_program_bridge) @@ -1681,6 +1743,10 @@ Suite *wolfboot_suite(void) tcase_add_test(tc_bars_iter, test_program_bars_iteration); suite_add_tcase(s, tc_bars_iter); + TCase *tc_bar_zalign = tcase_create("program-bar-zero-align"); + tcase_add_test(tc_bar_zalign, test_program_bar_zero_align); + suite_add_tcase(s, tc_bar_zalign); + TCase *tc_bridge = tcase_create("program-bridge"); tcase_add_test(tc_bridge, test_program_bridge); suite_add_tcase(s, tc_bridge); From 3bf9a7646286ff30f0a259d15b1c18ba5426c3ea Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:51:13 +0200 Subject: [PATCH 10/12] F-4337: validate NS pointer in wolfBoot_nsc_get_partition_state CMSE veneer wolfBoot_nsc_get_partition_state is a cmse_nonsecure_entry secure-gateway veneer (compiled with CSME_NSE_API under __WOLFBOOT && TZEN). The output pointer st arrives directly from the non-secure caller and was forwarded unchecked to wolfBoot_get_partition_state, which performs *st = *state unconditionally once the partition magic check passes (libwolfboot.c:735). Because the veneer runs in Secure state with full write access to Secure SRAM, a malicious NS caller could aim st at Secure memory and turn the veneer into a confused-deputy 1-byte write primitive (the partition-state byte, e.g. 0x00/0xFF) against Secure SRAM (magic fields, key-store flags, version counters). Validate the st range with cmse_check_address_range (CMSE_NONSECURE | CMSE_MPU_READWRITE) before forwarding, returning -1 when the range is not accessible from the non-secure world. The check is wrapped in WOLFBOOT_NSC_NS_RW, guarded by __ARM_FEATURE_CMSE == 3U so non-CMSE builds (no security boundary) collapse to a plain non-NULL pass-through. Same fix pattern as F-4416/F-4417/F-4644 (wc_callable.c, fwtpm_callable.c, tpm.c). Verified by compiling the veneer with cortex-m33 -mcmse: the expected bl cmse_check_address_range is emitted before the callee. The bug is a TrustZone Secure/Non-secure partitioning issue that cannot be exercised on the host unit-test build (the TZEN veneer block is not compiled there). --- src/libwolfboot.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index fed3205273..0a0cd4e399 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2419,6 +2419,23 @@ int wolfBoot_ram_decrypt(uint8_t *src, uint8_t *dst) #endif /* EXT_ENCRYPTED */ #if defined(__WOLFBOOT) && defined(TZEN) + +/* The wolfBoot_nsc_* functions are cmse_nonsecure_entry veneers callable by any + * non-secure code. Pointer arguments arrive directly from the NS caller and are + * used as the write target of the secure callee. Validate the whole range is + * accessible from the non-secure world before writing, otherwise an NS caller + * could aim the write at Secure SRAM (a confused-deputy write primitive). Outside + * a CMSE secure build there is no security boundary, so the check collapses to a + * non-NULL pass-through. Same fix pattern as F-4416/F-4417/F-4644. */ +#if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) +#include +#define WOLFBOOT_NSC_NS_RW(p, sz) \ + cmse_check_address_range((void*)(p), (size_t)(sz), \ + CMSE_NONSECURE | CMSE_MPU_READWRITE) +#else +#define WOLFBOOT_NSC_NS_RW(p, sz) ((void*)(p)) +#endif + CSME_NSE_API void wolfBoot_nsc_success(void) { @@ -2440,6 +2457,8 @@ uint32_t wolfBoot_nsc_get_image_version(uint8_t part) CSME_NSE_API int wolfBoot_nsc_get_partition_state(uint8_t part, uint8_t *st) { + if (WOLFBOOT_NSC_NS_RW(st, sizeof(uint8_t)) == NULL) + return -1; return wolfBoot_get_partition_state(part, st); } From f9d7b7996787ed7bc93a1aff5d2c27fd73b6099a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 20:00:11 +0200 Subject: [PATCH 11/12] Updated .gitignore with new unit tests --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 8096ea3c40..a75fcf7e9a 100644 --- a/.gitignore +++ b/.gitignore @@ -188,6 +188,8 @@ tools/unit-tests/unit-max-space tools/unit-tests/unit-sdhci-disk-unaligned tools/unit-tests/unit-fwtpm-stub tools/unit-tests/unit-gzip +tools/unit-tests/unit-linux-loader-e820 +tools/unit-tests/unit-linux-loader-syssize # Elf preprocessing tools From 3a8404b5e286d396b08c4b1ab53238fddabd2aa9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 20:55:52 +0200 Subject: [PATCH 12/12] Fix test regression, addressed copilot comments --- src/fdt.c | 24 +++++++++++----- src/fwtpm_callable.c | 21 ++++++++------ src/libwolfboot.c | 16 ++++++----- src/tpm.c | 18 ++++++------ src/wc_callable.c | 16 ++++++----- src/x86/linux_loader.c | 19 ++++++++++--- tools/unit-tests/Makefile | 12 ++++++++ tools/unit-tests/unit-linux-loader-syssize.c | 26 +++++++++++++++++ tools/unit-tests/unit-pkcs11_store.c | 30 ++++++++++++++------ 9 files changed, 132 insertions(+), 50 deletions(-) diff --git a/src/fdt.c b/src/fdt.c index 095653c79c..9f208c15e5 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -125,6 +125,8 @@ static uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) { const uint32_t *tagp, *lenp; uint32_t tag; + uint32_t proplen; + uint64_t next_off; int offset = startoffset; const char *p; @@ -152,22 +154,30 @@ static uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) if (!lenp) { return FDT_END; /* premature end */ } + proplen = fdt32_to_cpu(*lenp); /* A property value can never be larger than the blob itself. * Reject an oversized length up front: otherwise the unsigned * cursor arithmetic below wraps (e.g. len=0xFFFFFFFF advances * offset by only 7 bytes), the malformed node slips past the * fdt_offset_ptr() bounds check, and the bogus length propagates * to callers as a negative int (a ~4GB memcpy size). */ - if (fdt32_to_cpu(*lenp) > (uint32_t)fdt_totalsize(fdt)) { + if (proplen > (uint32_t)fdt_totalsize(fdt)) { return FDT_END; /* bad structure */ } - /* skip-name offset, length and value */ - offset += sizeof(struct fdt_property) - FDT_TAGSIZE - + fdt32_to_cpu(*lenp); - if (fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 && - ((offset - fdt32_to_cpu(*lenp)) % 8) != 0) { - offset += 4; + /* skip-name offset, length and value. Accumulate the next cursor in a + * 64-bit unsigned so neither the addition nor the narrowing back to the + * signed int offset can overflow, then re-validate it against the blob + * size before continuing. */ + next_off = (uint64_t)offset + + (sizeof(struct fdt_property) - FDT_TAGSIZE) + proplen; + if (fdt_version(fdt) < 0x10 && proplen >= 8 && + ((next_off - proplen) % 8) != 0) { + next_off += 4; } + if (next_off > (uint64_t)fdt_totalsize(fdt)) { + return FDT_END; /* bad structure */ + } + offset = (int)next_off; break; case FDT_END: diff --git a/src/fwtpm_callable.c b/src/fwtpm_callable.c index ba605e7b27..23d7b822ad 100644 --- a/src/fwtpm_callable.c +++ b/src/fwtpm_callable.c @@ -17,18 +17,21 @@ #include "wolftpm/fwtpm/fwtpm_nv.h" #include "wolftpm/tpm2_types.h" -/* Validate that a buffer supplied by the non-secure caller is fully accessible - * from the non-secure world before the secure side dereferences it. Without - * this check a non-secure caller could pass a pointer into Secure SRAM and turn - * this veneer into a confused-deputy primitive (forging TPM responses into - * Secure memory or leaking Secure memory through the command path). Outside of a - * CMSE secure build there is no security boundary, so the checks collapse to a - * simple non-NULL pass-through. */ +/* Validate that a buffer supplied by the non-secure caller lives in the + * non-secure world before the secure side dereferences it. Without this check a + * non-secure caller could pass a pointer into Secure SRAM and turn this veneer + * into a confused-deputy primitive (forging TPM responses into Secure memory or + * leaking Secure memory through the command path). The relevant property is the + * Secure/Non-secure attribution (SAU/IDAU): only CMSE_NONSECURE is checked, not + * the MPU read/write permission bits. The MPU bits depend on an enabled NS MPU + * (they read back as 0 when NO_MPU is set) and do not constrain Secure accesses + * to NS memory anyway, so requiring them would wrongly reject valid NS buffers. + * Outside of a CMSE secure build there is no security boundary, so the checks + * collapse to a simple non-NULL pass-through. */ #if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) #include #define WCS_FWTPM_NS_RW(p, sz) \ - cmse_check_address_range((void*)(p), (size_t)(sz), \ - CMSE_NONSECURE | CMSE_MPU_READWRITE) + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #define WCS_FWTPM_NS_R(p, sz) \ cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #else diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 0a0cd4e399..2d81cbcb13 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2422,16 +2422,18 @@ int wolfBoot_ram_decrypt(uint8_t *src, uint8_t *dst) /* The wolfBoot_nsc_* functions are cmse_nonsecure_entry veneers callable by any * non-secure code. Pointer arguments arrive directly from the NS caller and are - * used as the write target of the secure callee. Validate the whole range is - * accessible from the non-secure world before writing, otherwise an NS caller - * could aim the write at Secure SRAM (a confused-deputy write primitive). Outside - * a CMSE secure build there is no security boundary, so the check collapses to a - * non-NULL pass-through. Same fix pattern as F-4416/F-4417/F-4644. */ + * used as the write target of the secure callee. Validate the whole range lives + * in the non-secure world before writing, otherwise an NS caller could aim the + * write at Secure SRAM (a confused-deputy write primitive). The check verifies + * only the Secure/Non-secure attribution (CMSE_NONSECURE); the MPU read/write + * permission bits are deliberately not required, as they read back as 0 when the + * NS MPU is disabled (NO_MPU) and do not constrain Secure accesses to NS memory + * anyway. Outside a CMSE secure build there is no security boundary, so the check + * collapses to a non-NULL pass-through. Same fix pattern as F-4416/F-4417/F-4644. */ #if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) #include #define WOLFBOOT_NSC_NS_RW(p, sz) \ - cmse_check_address_range((void*)(p), (size_t)(sz), \ - CMSE_NONSECURE | CMSE_MPU_READWRITE) + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #else #define WOLFBOOT_NSC_NS_RW(p, sz) ((void*)(p)) #endif diff --git a/src/tpm.c b/src/tpm.c index 71ec29d9e5..dc132c5e24 100644 --- a/src/tpm.c +++ b/src/tpm.c @@ -1231,17 +1231,19 @@ static int wolfRNG_GetSeedCB(OS_Seed* os, uint8_t* seed, uint32_t sz) /* API's that are callable from non-secure code */ -/* Validate that a buffer supplied by the non-secure caller is fully - * accessible from the non-secure world before the secure side dereferences - * it. Without this check a non-secure caller could pass a pointer into Secure - * SRAM and turn these veneers into a confused-deputy write primitive against - * Secure memory. Outside of a CMSE secure build there is no security boundary, - * so the checks collapse to a simple non-NULL pass-through. */ +/* Validate that a buffer supplied by the non-secure caller lives in the + * non-secure world before the secure side dereferences it. Without this check a + * non-secure caller could pass a pointer into Secure SRAM and turn these veneers + * into a confused-deputy write primitive against Secure memory. The check + * verifies only the Secure/Non-secure attribution (CMSE_NONSECURE); the MPU + * read/write permission bits are deliberately not required, as they read back as + * 0 when the NS MPU is disabled (NO_MPU) and do not constrain Secure accesses to + * NS memory anyway. Outside of a CMSE secure build there is no security + * boundary, so the checks collapse to a simple non-NULL pass-through. */ #if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) #include #define WOLFBOOT_TPM_NS_RW(p, sz) \ - cmse_check_address_range((void*)(p), (size_t)(sz), \ - CMSE_NONSECURE | CMSE_MPU_READWRITE) + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #define WOLFBOOT_TPM_NS_R(p, sz) \ cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #else diff --git a/src/wc_callable.c b/src/wc_callable.c index 04fd2fb557..c32f61e238 100644 --- a/src/wc_callable.c +++ b/src/wc_callable.c @@ -36,16 +36,18 @@ /* wcs_get_random is a cmse_nonsecure_entry veneer: the rand pointer and size * arrive from the non-secure caller and are used as the write target of - * wc_RNG_GenerateBlock. Validate the whole range is accessible from the - * non-secure world before writing, otherwise an NS caller could aim the RNG - * output at Secure SRAM (a confused-deputy write primitive). Outside a CMSE - * secure build there is no security boundary, so the check collapses to a - * non-NULL pass-through. */ + * wc_RNG_GenerateBlock. Validate the whole range lives in the non-secure world + * before writing, otherwise an NS caller could aim the RNG output at Secure SRAM + * (a confused-deputy write primitive). The check verifies only the + * Secure/Non-secure attribution (CMSE_NONSECURE); the MPU read/write permission + * bits are deliberately not required, as they read back as 0 when the NS MPU is + * disabled (NO_MPU) and do not constrain Secure accesses to NS memory anyway. + * Outside a CMSE secure build there is no security boundary, so the check + * collapses to a non-NULL pass-through. */ #if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) #include #define WOLFBOOT_WCS_NS_RW(p, sz) \ - cmse_check_address_range((void*)(p), (size_t)(sz), \ - CMSE_NONSECURE | CMSE_MPU_READWRITE) + cmse_check_address_range((void*)(p), (size_t)(sz), CMSE_NONSECURE) #else #define WOLFBOOT_WCS_NS_RW(p, sz) ((void*)(p)) #endif diff --git a/src/x86/linux_loader.c b/src/x86/linux_loader.c index 6d0ebda30f..2dcdbdf251 100644 --- a/src/x86/linux_loader.c +++ b/src/x86/linux_loader.c @@ -112,16 +112,27 @@ static int linux_boot_params_fill_memory_map(struct boot_params *bp, /* Compute the protected-mode kernel size (syssize * 16) in 64-bit to avoid the * uint32_t multiplication wrap, and reject any image whose kernel would not fit - * in the destination window [KERNEL_LOAD_ADDRESS, load_limit). Returns 0 on - * success, -1 if the size is zero or out of range. */ + * in the destination window [KERNEL_LOAD_ADDRESS, load_limit). + * + * load_limit == 0 means "no destination window bound is known" (non-FSP builds, + * which do not expose a tolum): in that case only the wrap and zero-size cases + * are rejected, bounding the result to what fits in the uint32_t kernel_size. + * + * Returns 0 on success, -1 if the size is zero or out of range. */ static int linux_kernel_size(uint32_t syssize, uint32_t load_limit, uint32_t *kernel_size) { uint64_t ksz = (uint64_t)syssize * 16u; + uint64_t max_size; - if (load_limit <= KERNEL_LOAD_ADDRESS) + if (load_limit == 0) + max_size = (uint64_t)0xFFFFFFFFu; + else if (load_limit <= KERNEL_LOAD_ADDRESS) return -1; - if (ksz == 0 || ksz > (uint64_t)(load_limit - KERNEL_LOAD_ADDRESS)) + else + max_size = (uint64_t)(load_limit - KERNEL_LOAD_ADDRESS); + + if (ksz == 0 || ksz > max_size) return -1; *kernel_size = (uint32_t)ksz; return 0; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index d1ff2f992b..9f2e4f889d 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -60,8 +60,20 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128 TESTS+=unit-tpm-check-rot-auth TESTS+=unit-tpm-api-names TESTS+=unit-fit-gzip unit-fit-nogzip + +# linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit +# (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the +# tests when it can, so a default "make" stays portable on hosts without 32-bit +# support. Set ENABLE_32BIT_TESTS=1 to force them on (e.g. to fail loudly if the +# toolchain is unexpectedly missing multilib). +HAVE_M32 := $(shell printf 'int main(void){return 0;}' | gcc -m32 -x c - -o /dev/null >/dev/null 2>&1 && echo 1 || echo 0) +ENABLE_32BIT_TESTS ?= $(HAVE_M32) +ifeq ($(ENABLE_32BIT_TESTS),1) TESTS+=unit-linux-loader-e820 TESTS+=unit-linux-loader-syssize +else +$(info Skipping 32-bit x86 linux-loader unit tests: 'gcc -m32' unavailable (set ENABLE_32BIT_TESTS=1 to force)) +endif include unit-sign-encrypted-output.mkfrag diff --git a/tools/unit-tests/unit-linux-loader-syssize.c b/tools/unit-tests/unit-linux-loader-syssize.c index 9294b73a81..da988a4045 100644 --- a/tools/unit-tests/unit-linux-loader-syssize.c +++ b/tools/unit-tests/unit-linux-loader-syssize.c @@ -89,6 +89,32 @@ int main(void) return 1; } + /* load_limit == 0 is the non-FSP case ("no destination window bound"): + * a legitimate kernel must still be accepted (no spurious panic) ... */ + ksz = 0; + ret = linux_kernel_size(0x10000u, 0u, &ksz); + if (ret != 0 || ksz != 0x10000u * 16u) { + printf("FAIL: valid kernel rejected with no load_limit " + "(ret=%d ksz=0x%08x)\n", ret, ksz); + return 1; + } + + /* ... while the wrapping cases must still be rejected even without a + * window bound, since the 64-bit size exceeds the uint32_t kernel_size. */ + for (i = 0; i < sizeof(wrap_cases) / sizeof(wrap_cases[0]); i++) { + if (linux_kernel_size(wrap_cases[i], 0u, &ksz) == 0) { + printf("FAIL: overflowing syssize 0x%08x accepted with no " + "load_limit\n", wrap_cases[i]); + return 1; + } + } + + /* Zero-sized kernel must be rejected even without a window bound. */ + if (linux_kernel_size(0, 0u, &ksz) == 0) { + printf("FAIL: zero-sized kernel accepted with no load_limit\n"); + return 1; + } + printf("PASS\n"); return 0; } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 7ea41d02f0..200c72c11d 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -65,6 +65,11 @@ #define MOCK_ADDRESS 0xCF000000 uint8_t *vault_base = (uint8_t *)MOCK_ADDRESS; + +/* Backing file for the mock keyvault. Made unique per process (see main()) so + * concurrent test runs do not collide on a shared /tmp path, with a usable + * default in case a test is ever driven without main() initializing it. */ +char vault_path[64] = "/tmp/wolfboot-unit-keyvault.bin"; #include "unit-keystore.c" #include "pkcs11_store.c" const uint32_t keyvault_size = KEYVAULT_OBJ_SIZE * KEYVAULT_MAX_ITEMS + 2 * WOLFBOOT_SECTOR_SIZE; @@ -88,7 +93,7 @@ START_TEST (test_store_and_load_objs) { id_tok = 1; id_obj = 12; readonly = 0; - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert(ret == 0); memset(vault_base, 0xEE, keyvault_size); @@ -296,7 +301,7 @@ START_TEST(test_cross_sector_write_preserves_length) for (ret = 0; ret < WOLFBOOT_SECTOR_SIZE; ret++) payload[ret] = (unsigned char)(ret & 0xFF); - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert_int_eq(ret, 0); memset(vault_base, 0xEE, keyvault_size); @@ -327,7 +332,7 @@ START_TEST(test_close_clears_handle_state) struct store_handle *handle; int ret; - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert_int_eq(ret, 0); memset(vault_base, 0xEE, keyvault_size); @@ -360,7 +365,7 @@ START_TEST(test_delete_object_ignores_metadata_prefix) uint8_t bitmap_before[BITMAP_SIZE]; int ret; - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert_int_eq(ret, 0); memset(vault_base, 0xFF, keyvault_size); @@ -390,7 +395,7 @@ START_TEST(test_delete_object_corrupted_pos_no_oob) struct obj_hdr *hdr; int ret; - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert_int_eq(ret, 0); memset(vault_base, 0xFF, keyvault_size); @@ -429,7 +434,7 @@ START_TEST(test_find_object_search_stops_at_header_sector) uint32_t *payload_ids; int ret; - ret = mmap_file("/tmp/wolfboot-unit-keyvault.bin", vault_base, + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); ck_assert_int_eq(ret, 0); memset(vault_base, 0xFF, keyvault_size); @@ -479,10 +484,19 @@ Suite *wolfboot_suite(void) int main(void) { int fails; - Suite *s = wolfboot_suite(); - SRunner *sr = srunner_create(s); + Suite *s; + SRunner *sr; + + /* Use a per-process backing file so parallel test runs (or a stale file + * from a previous run) cannot collide on a shared /tmp path. */ + snprintf(vault_path, sizeof(vault_path), + "/tmp/wolfboot-unit-keyvault-%d.bin", (int)getpid()); + + s = wolfboot_suite(); + sr = srunner_create(s); srunner_run_all(sr, CK_NORMAL); fails = srunner_ntests_failed(sr); srunner_free(sr); + unlink(vault_path); return fails; }