From 7a92ae9c86512eda4f80fbeaca91421d030e4624 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 18 Aug 2026 09:53:18 +0200 Subject: [PATCH] t2080: bound the FMan container before walking it The NOR-extent test ran after the microcode table had already been dereferenced, so a container near the top of the bank read past the window the guard exists to enforce -- a machine check on hardware. Bound the fixed part before the first dereference and the table before the walk. The accept/reject outcome is unchanged; the out-of-bank read is what goes away. Use the file's own QE_MAX_RISC for the count bound instead of a hand-written 2, matching the t10xx and p1021 siblings; the shared length formula already handles count > 1. --- hal/nxp_t2080.c | 37 ++++++++++++++++------- tools/unit-tests/unit-t2080-fman-loader.c | 19 +++++++++++- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/hal/nxp_t2080.c b/hal/nxp_t2080.c index af18aea1..ba75b25f 100644 --- a/hal/nxp_t2080.c +++ b/hal/nxp_t2080.c @@ -647,6 +647,7 @@ static int hal_fman_init(void) { const struct qe_firmware *fw = (const struct qe_firmware *)FMAN_FW_ADDR; const struct qe_header *hdr = &fw->header; + uint64_t fw_off, extent; unsigned int i; /* Guard: FMAN_FW_ADDR must lie in the NOR window, else the magic read @@ -662,6 +663,17 @@ static int hal_fman_init(void) return -1; } + /* Everything below reads through fw. The guard above only proved + * that FMAN_FW_ADDR itself is inside the NOR window, so bound the + * fixed part of the container (header + first microcode entry) + * before dereferencing any of it. */ + fw_off = (uint64_t)((uintptr_t)fw - (uintptr_t)FLASH_BASE_ADDR); + extent = (uint64_t)FLASH_BANK_SIZE - fw_off; + if (extent < (uint64_t)sizeof(struct qe_firmware)) { + wolfBoot_printf("FMAN: container truncated by NOR end, skipping\n"); + return -1; + } + /* Check firmware magic */ if (hdr->magic[0] != 'Q' || hdr->magic[1] != 'E' || hdr->magic[2] != 'F') { wolfBoot_printf("FMAN: no firmware at 0x%x\n", (unsigned)FMAN_FW_ADDR); @@ -678,19 +690,27 @@ static int hal_fman_init(void) wolfBoot_printf("FMAN: version %d unsupported\n", hdr->version); return -1; } - if (fw->count < 1 || fw->count > 2) { + if (fw->count < 1 || fw->count > QE_MAX_RISC) { wolfBoot_printf("FMAN: count %d invalid\n", fw->count); return -1; } { - uint64_t calc = sizeof(struct qe_firmware); - uint64_t fw_off = - (uint64_t)((uintptr_t)fw - (uintptr_t)FLASH_BASE_ADDR); - uint64_t extent = (uint64_t)FLASH_BANK_SIZE - fw_off; uint64_t length = hdr->length; + uint64_t table = (uint64_t)sizeof(struct qe_firmware) + + (uint64_t)(fw->count - 1) * sizeof(struct qe_microcode); + uint64_t calc; unsigned int k; - calc += (uint64_t)(fw->count - 1) * sizeof(struct qe_microcode); + /* Bound the microcode table and the declared image against the + * NOR before either is walked -- the table sits past the fixed + * part checked above, and the loop below dereferences it. */ + if (table > extent || length > extent) { + wolfBoot_printf("FMAN: image %lu exceeds NOR extent %lu\n", + (unsigned long)length, (unsigned long)extent); + return -1; + } + + calc = table; for (k = 0; k < fw->count; k++) calc += (uint64_t)4 * fw->microcode[k].count; @@ -706,11 +726,6 @@ static int hal_fman_init(void) return -1; } } - if (length > extent) { - wolfBoot_printf("FMAN: image %lu exceeds NOR extent %lu\n", - (unsigned long)length, (unsigned long)extent); - return -1; - } } for (i = 0; i < fw->count; i++) { diff --git a/tools/unit-tests/unit-t2080-fman-loader.c b/tools/unit-tests/unit-t2080-fman-loader.c index dbc9c7ec..ec4e7eed 100644 --- a/tools/unit-tests/unit-t2080-fman-loader.c +++ b/tools/unit-tests/unit-t2080-fman-loader.c @@ -121,7 +121,8 @@ static uint32_t make_blob(uint32_t code_offset, uint32_t code_words, uint32_t version, uint8_t count) { struct qe_firmware *fw = slot(); - uint64_t calc = sizeof(struct qe_firmware); + uint64_t calc = sizeof(struct qe_firmware) + + (uint64_t)(count ? count - 1 : 0) * sizeof(struct qe_microcode); uint32_t i; memset(g_nor, 0x00, sizeof(g_nor)); @@ -192,6 +193,21 @@ START_TEST(test_bad_version) } END_TEST +/* A container declaring more than one RISC must be accepted up to + * QE_MAX_RISC, which this file defines as 4. The bound was a hand + * written 2, so a 3-RISC container was rejected even though the length + * formula (shared with the t10xx and p1021 siblings) handles it. Only + * entry 0 carries code here; the others are zeroed, which the loader + * skips via the !code_offset test. */ +START_TEST(test_multi_risc_blob_accepted) +{ + make_blob(sizeof(struct qe_firmware) + + 2 * sizeof(struct qe_microcode), 8, 1, 3); + + ck_assert_int_eq(hal_fman_init(), 0); +} +END_TEST + /* count == 0 used to be accepted as a successful upload of nothing * (F-9762); it must now be rejected. */ START_TEST(test_zero_count_rejected) @@ -245,6 +261,7 @@ Suite *t2080_fman_loader_suite(void) tcase_add_test(tc, test_valid_blob); tcase_add_test(tc, test_bad_magic); tcase_add_test(tc, test_bad_version); + tcase_add_test(tc, test_multi_risc_blob_accepted); tcase_add_test(tc, test_zero_count_rejected); tcase_add_test(tc, test_mcode_offset_past_image_rejected); tcase_add_test(tc, test_image_past_nor_extent_rejected);