F-8000: bound T10xx QE microcode offsets to the declared image

qe_check_firmware() validated the QE/FMAN microcode header for
self-consistency (magic, version, count, length == computed size,
optional CRC) but never bounded the per-microcode code_offset against
the declared image length. qe_upload_firmware() 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 made wolfBoot copy arbitrary memory
into the engine ahead of image authentication.

Reject the firmware when any microcode's words extend past the
declared end (64-bit sum, no wrap). This protects both the QE and the
FMAN firmware paths, which share the check.

Test: tools/unit-tests/unit-t10xx-qe-firmware.c extracts the real
qe_* structures and qe_check_firmware() from hal/nxp_t10xx.c and
builds the blobs in memory; pre-fix both out-of-image variants were
accepted (one of them segfaulting the test harness while building the
blob's claimed code).
pull/862/head
Daniele Lacamera 2026-08-18 05:00:02 +02:00
parent 0cbd17cf00
commit 91c020a2e0
3 changed files with 216 additions and 0 deletions

View File

@ -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);

View File

@ -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

View File

@ -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 <check.h>
#include <stdint.h>
#include <string.h>
#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;
}