diff --git a/hal/nxp_t10xx.c b/hal/nxp_t10xx.c index 0b92a586..5cf3dcc5 100644 --- a/hal/nxp_t10xx.c +++ b/hal/nxp_t10xx.c @@ -2045,6 +2045,7 @@ struct qe_firmware { static int qe_check_firmware(const struct qe_firmware *firmware, const char* t) { unsigned int i; + uint64_t mcode_end; #ifdef ENABLE_QE_CRC32 uint32_t crc; #endif @@ -2089,6 +2090,19 @@ static int qe_check_firmware(const struct qe_firmware *firmware, const char* t) return -1; } + /* The microcode must lie inside the declared image: the upload + * reads code_offset + 4*count bytes from the firmware start, so an + * out-of-image offset would copy arbitrary memory into QE IRAM and + * program arbitrary traps (F-8000). 64-bit so the sum cannot wrap. */ + for (i = 0; i < firmware->count; i++) { + mcode_end = (uint64_t)firmware->microcode[i].code_offset + + (uint64_t)4 * firmware->microcode[i].count; + if (mcode_end > length) { + wolfBoot_printf("%s: microcode %u out of bounds!\n", t, i); + return -1; + } + } + #ifdef ENABLE_QE_CRC32 /* Validate the CRC */ crc = *(uint32_t *)((void *)firmware + calc_size); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index ff8f365e..06a82c94 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -107,6 +107,7 @@ TESTS+=unit-zynq-erase-loop TESTS+=unit-zynq-ext-write TESTS+=unit-versal-qspi-dma TESTS+=unit-versal-ext-write +TESTS+=unit-t10xx-qe-firmware TESTS+=unit-sdhci-uhs-recover TESTS+=unit-sdhci-wait-busy TESTS+=unit-ti-hercules-write @@ -869,6 +870,21 @@ versal_ext_write_extract.h: ../../hal/versal.c unit-versal-ext-write: unit-versal-ext-write.c versal_qspidev_extract.h versal_ext_write_extract.h versal_host.h gcc -o $@ unit-versal-ext-write.c -DEXT_FLASH -DARCH_AARCH64 -DARCH_64BIT $(CFLAGS) $(LDFLAGS) +# unit-t10xx-qe-firmware runs the real qe_check_firmware() from +# hal/nxp_t10xx.c (F-8000: the per-microcode code_offset was never +# bounded against the declared image, so a self-consistent blob could +# make the upload copy arbitrary memory into QE IRAM). The qe_* +# structures and the check function are extracted verbatim; the check +# path needs no register access. +# NB: gawk 5.x mis-lexes an action brace directly after a regex +# literal, so each pattern is followed by a space; $$ is a literal $. +t10xx_qe_firmware_extract.h: ../../hal/nxp_t10xx.c + awk '/^\/\* Structure packing \*\/$$/ {f=1} f {print} f && /^\} QE_PACKED;$$/ {n++; if (n == 4) exit}' $< > $@ + awk '/^static int qe_check_firmware\(/ {f=1} f {print} f && /^\}/ {exit}' $< >> $@ + +unit-t10xx-qe-firmware: unit-t10xx-qe-firmware.c t10xx_qe_firmware_extract.h + gcc -o $@ unit-t10xx-qe-firmware.c -DWOLFBOOT_NO_PRINTF $(CFLAGS) $(LDFLAGS) + # unit-sdhci-uhs-recover drives disk_read()'s UHS recovery path from the # real src/sdhci.c (F-9735: any read error permanently switched the host # to 1.8V signaling with no rollback). sdhci_host.c (generated below) is diff --git a/tools/unit-tests/unit-t10xx-qe-firmware.c b/tools/unit-tests/unit-t10xx-qe-firmware.c new file mode 100644 index 00000000..01cf2a42 --- /dev/null +++ b/tools/unit-tests/unit-t10xx-qe-firmware.c @@ -0,0 +1,186 @@ +/* unit-t10xx-qe-firmware.c + * + * Regression test for F-8000: qe_check_firmware() in hal/nxp_t10xx.c + * validated the QE microcode header for self-consistency (magic, + * version, count, length == computed size) but never bounded the + * per-microcode code_offset against the declared image length. + * qe_upload_firmware() then reads code_offset + 4*count bytes from + * the firmware start and copies them into QE IRAM (and programs the + * microcode's trap table), so a structurally valid blob with an + * out-of-image offset makes wolfBoot copy arbitrary memory into the + * engine and run unauthenticated microcode with serial DMA enabled + * before the image is authenticated. + * + * The test extracts the real qe_* structures and qe_check_firmware() + * from hal/nxp_t10xx.c (generated by the Makefile) and builds the + * blobs in memory. The check-only path needs no register access. + * + * 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 "printf.h" + +/* Number of RISC processors the target exposes (real: hal/nxp_t10xx.c). */ +#define QE_MAX_RISC 1 + +/* The real qe_* structures and qe_check_firmware() from hal/nxp_t10xx.c + * (extracted by the Makefile). */ +#include "t10xx_qe_firmware_extract.h" + +#define BLOB_SZ 4096 +static uint8_t g_blob[BLOB_SZ]; + +/* Build a self-consistent blob: header, one microcode, 'code_words' + * words of code at 'code_offset', trailing CRC word. Returns the + * (self-consistent) declared length. */ +static size_t make_blob(uint32_t code_offset, uint32_t code_words, + uint32_t *code) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + size_t calc = sizeof(struct qe_firmware); + uint32_t i; + + memset(g_blob, 0, sizeof(g_blob)); + fw->header.length = 0; /* filled below */ + fw->header.magic[0] = 'Q'; + fw->header.magic[1] = 'E'; + fw->header.magic[2] = 'F'; + fw->header.version = 1; + fw->count = 1; + fw->split = 1; + fw->microcode[0].count = code_words; + fw->microcode[0].code_offset = code_offset; + strncpy((char *)fw->microcode[0].id, "test-ucode", 31); + calc += (size_t)4 * code_words; + + /* Materialize the code words only when they actually fit in the + * blob; an out-of-image offset is exactly what the bounds check + * must catch, and the check path never reads the words. */ + if (code_offset + 4 * code_words <= BLOB_SZ) { + for (i = 0; i < code_words; i++) + ((uint32_t *)(void *)(g_blob + code_offset))[i] = + 0xA0000000U + i; + } + /* Trailing CRC word placeholder (CRC not checked without + * ENABLE_QE_CRC32, but the slot must exist for the length). */ + if (calc + sizeof(uint32_t) <= BLOB_SZ) + *(uint32_t *)(void *)(g_blob + calc) = 0; + + fw->header.length = (uint32_t)(calc + sizeof(uint32_t)); + return fw->header.length; +} + +/* A valid, in-image blob is accepted. */ +START_TEST(test_valid_blob_accepted) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + size_t len = make_blob((uint32_t)sizeof(struct qe_firmware), 16, NULL); + + (void)fw; + ck_assert_uint_lt(len, BLOB_SZ); + ck_assert_int_eq(qe_check_firmware(fw, "QE"), 0); +} +END_TEST + +/* A self-consistent blob whose microcode offset lies past the image + * must be rejected. Pre-fix it was accepted and the upload read + * arbitrary memory past the blob. */ +START_TEST(test_mcode_offset_past_image_rejected) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + + (void)make_blob(0x100000, 16, NULL); + ck_assert_int_eq(qe_check_firmware(fw, "QE"), -1); +} +END_TEST + +/* A self-consistent blob whose microcode words extend past the end of + * the image must be rejected. */ +START_TEST(test_mcode_words_past_image_rejected) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + uint32_t base = (uint32_t)sizeof(struct qe_firmware); + uint32_t length; + + /* Four words: the declared length is base + 4*4 + 4, and stays + * self-consistent no matter where the words are pointed at. */ + length = (uint32_t)make_blob(base, 4, NULL); + ck_assert_uint_eq(length, base + 4 * 4 + sizeof(uint32_t)); + + /* Shift the offset 8 bytes into the image: the words now end 8 + * bytes past the declared end. The old self-consistency check + * passes (it never looked at code_offset); the bounds check must + * reject. */ + fw->microcode[0].code_offset = base + 8; + ck_assert_int_eq(qe_check_firmware(fw, "QE"), -1); +} +END_TEST + +/* The pre-existing header checks still hold. */ +START_TEST(test_bad_magic_rejected) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + + make_blob((uint32_t)sizeof(struct qe_firmware), 16, NULL); + fw->header.magic[0] = 'X'; + ck_assert_int_eq(qe_check_firmware(fw, "QE"), -1); +} +END_TEST + +START_TEST(test_bad_length_rejected) +{ + struct qe_firmware *fw = (struct qe_firmware *)g_blob; + + make_blob((uint32_t)sizeof(struct qe_firmware), 16, NULL); + fw->header.length += 4; + ck_assert_int_eq(qe_check_firmware(fw, "QE"), -1); +} +END_TEST + +Suite *t10xx_qe_firmware_suite(void) +{ + Suite *s = suite_create("t10xx-qe-firmware"); + TCase *tc = tcase_create("t10xx-qe-firmware"); + + tcase_add_test(tc, test_valid_blob_accepted); + tcase_add_test(tc, test_mcode_offset_past_image_rejected); + tcase_add_test(tc, test_mcode_words_past_image_rejected); + tcase_add_test(tc, test_bad_magic_rejected); + tcase_add_test(tc, test_bad_length_rejected); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = t10xx_qe_firmware_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +}