Fenrir fixes 2026 08 21

.gitignore

@@ -478,12 +478,14 @@ tools/unit-tests/unit-versal-qspi-dma
 tools/unit-tests/unit-xspi-tfd-index
 tools/unit-tests/unit-zynq-erase-loop
 tools/unit-tests/unit-zynq-ext-write
+tools/unit-tests/unit-sama5d3-ext-read
 # sources generated by the unit-test extraction rules
 tools/unit-tests/aurix_erased_extract.h
 tools/unit-tests/nvm_cache_scrub_extract.h
 tools/unit-tests/nxp_ls1028a_host.c
 tools/unit-tests/nxp_p1021_host.c
 tools/unit-tests/nxp_t10xx_fixup_extract.h
+tools/unit-tests/sama5d3_read_extract.h
 tools/unit-tests/sdhci_host.c
 tools/unit-tests/stm32l5_write_extract.h
 tools/unit-tests/stm32u5_write_extract.h

docs/Targets.md

@@ -5736,6 +5736,14 @@ A first stage loader is required to load the wolfBoot image into DDR for executi
 | fsl_qe_ucode_1021_10_A.bin   | 0x01F00000  |
 | swap block                   | 0x02200000  |
 
+The QE microcode programmed at `0x01F00000` is validated only for
+structural integrity (header magic, version, and size bounds) before it
+is activated; it is not cryptographically authenticated. This is out of
+scope for the example configuration: the microcode region sits inside
+the update partition, so replacing it requires the same flash write
+access that would let an attacker replace the update image itself,
+which wolfBoot's image authentication already protects against.
+
 ### Building wolfBoot for NXP P1021 PPC
 
 By default wolfBoot will use `powerpc-linux-gnu-` cross-compiler prefix. These tools can be installed with the Debian package `gcc-powerpc-linux-gnu` (`sudo apt install gcc-powerpc-linux-gnu`).
@@ -5888,6 +5896,13 @@ Note: On T1040, FMAN and QE firmware share the same 128KB NOR erase sector
 (0xEFF00000-0xEFF1FFFF). They must be programmed together in a single
 erase/write operation.
 
+The QE and FMan microcode in these NOR regions is validated only for
+structural integrity (header magic, version, and size bounds) before it
+is activated; it is not cryptographically authenticated. This is out of
+scope for the example configurations: programming these regions requires
+local write access to the board's flash, which also allows replacing the
+wolfBoot image itself - a threat the signed image flow already covers.
+
 ### Design
 
 Both T1024 and T1040 use a two-stage boot. Stage1 runs XIP from NOR flash,

hal/nxp_t10xx.c

@@ -3522,7 +3522,7 @@ int hal_dts_fixup(void* dts_addr)
 
     /* fixup the fman clock */
     off = fdt_node_offset_by_compatible(fdt, -1, "fsl,fman");
-    if (off != !FDT_ERR_NOTFOUND) {
+    if (off != -FDT_ERR_NOTFOUND) {
         fdt_fixup_val(fdt, off, "fman@", "clock-frequency", hal_get_bus_clk());
     }
 
@@ -3617,9 +3617,9 @@ int hal_dts_fixup(void* dts_addr)
 
     /* fix SDHC */
     off = fdt_node_offset_by_compatible(fdt, -1, "fsl,esdhc");
-    if (off != !FDT_ERR_NOTFOUND) {
+    if (off != -FDT_ERR_NOTFOUND) {
         fdt_fixup_val(fdt, off, "sdhc@", "clock-frequency", hal_get_bus_clk());
-        fdt_fixup_str(fdt, off, "cpu", "status", "okay");
+        fdt_fixup_str(fdt, off, "sdhc@", "status", "okay");
     }
 
 #endif /* !BUILD_LOADER_STAGE1 */

hal/sama5d3.c

@@ -601,28 +601,29 @@ static int nand_check_bad_block(uint32_t block)
 int ext_flash_read(uintptr_t address, uint8_t *data, int len)
 {
     uint8_t buffer_page[NAND_FLASH_PAGE_SIZE];
-    uint32_t block = div_u(address, nand_flash.block_size); /* The block where the address falls in */
-    uint32_t page = div_u(address, nand_flash.page_size); /* The page where the address falls in */
-    uint32_t start_page_in_block = mod(page, nand_flash.pages_per_block); /* The start page within this block */
-    uint32_t in_block_offset = mod(address, nand_flash.block_size); /* The offset of the address within the block */
-    uint32_t remaining = nand_flash.block_size - in_block_offset; /* How many bytes remaining to read in the first block */
-    int len_to_read = len;
-    uint8_t *buffer = data;
-    uint32_t i;
-    int copy = 0;
+    uintptr_t addr = address;
+    uint8_t *dst = data;
+    uint32_t in_page = mod(address, nand_flash.page_size); /* The offset of the address within the page */
+    int remaining = len;
     int ret;
 
-    if (len < (int)nand_flash.page_size) {
-        buffer = buffer_page;
-        copy = 1;
-        len_to_read = nand_flash.page_size;
-    }
+    if (len <= 0)
+        return 0;
 
-    while (len_to_read > 0) {
-        uint32_t sz = len_to_read;
-        uint32_t pages_to_read;
-        if (sz > remaining)
-            sz = remaining;
+    while (remaining > 0) {
+        uint32_t block;
+        uint32_t page;
+        uint32_t page_in_block;
+        uint32_t chunk;
+
+        /* Bytes available from the current address to the end of its page */
+        chunk = nand_flash.page_size - in_page;
+        if (chunk > (uint32_t)remaining)
+            chunk = (uint32_t)remaining;
+
+        block = div_u(addr, nand_flash.block_size);
+        page = div_u(addr, nand_flash.page_size);
+        page_in_block = mod(page, nand_flash.pages_per_block);
 
         do {
             ret = nand_check_bad_block(block);
@@ -632,32 +633,23 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len)
             }
         } while (ret < 0);
 
-        /* Amount of pages to be read from this block */
-        pages_to_read = div_u((sz + nand_flash.page_size - 1), nand_flash.page_size);
-
-        if (pages_to_read * nand_flash.page_size > remaining)
-            pages_to_read--;
-
-        /* Read (remaining) pages off a block */
-        for (i = 0; i < pages_to_read; i++) {
-            nand_read_page(block, start_page_in_block + i, buffer);
-            if (sz > nand_flash.page_size)
-                sz = nand_flash.page_size;
-            len_to_read -= sz;
-            buffer += sz;
+        if ((in_page == 0) && (chunk == nand_flash.page_size)) {
+            /* Full page at the page head: read straight into the caller's
+             * buffer. */
+            nand_read_page(block, page_in_block, dst);
         }
-        /* The block is over, move to the next one */
-        block++;
-        start_page_in_block = 0;
-        remaining = nand_flash.block_size;
-    }
-    if (copy) {
-        uint32_t *dst = (uint32_t *)data;
-        uint32_t *src = (uint32_t *)buffer_page;
-        uint32_t tot_len = (uint32_t)len;
-        for (i = 0; i < (tot_len >> 2); i++) {
-            dst[i] = src[i];
+        else {
+            /* Partial first/last page: stage through the page buffer and
+             * copy from the column offset, so a full page is never written
+             * past the end of the caller's buffer. */
+            nand_read_page(block, page_in_block, buffer_page);
+            memcpy(dst, buffer_page + in_page, chunk);
         }
+
+        dst += chunk;
+        remaining -= (int)chunk;
+        addr += chunk;
+        in_page = mod(addr, nand_flash.page_size);
     }
     return len;
 }
@@ -716,20 +708,23 @@ static void dbgu_init(void) {
 
 int ext_flash_write(uintptr_t address, const uint8_t *data, int len)
 {
-    /* TODO */
+    /* SAMA5D3 NAND page program is not implemented: this HAL only
+     * supports reading NAND. Report failure instead of pretending the
+     * data was programmed. */
     (void)address;
     (void)data;
     (void)len;
 
-    return 0;
+    return -1;
 }
 
 int ext_flash_erase(uintptr_t address, int len)
 {
-    /* TODO */
+    /* SAMA5D3 NAND block erase is not implemented: see ext_flash_write. */
     (void)address;
     (void)len;
-    return 0;
+
+    return -1;
 }
 
 /* SAMA5D3 NAND flash does not have an enable pin */

include/fdt.h

@@ -73,6 +73,18 @@ struct fdt_property {
 #define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
 #define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
 
+/* Bounds for the attacker-influenced fdt_totalsize before relocating or
+ * forwarding a DTB. MIN is the FDT v17 header size (also enforced by the
+ * signer): fdt_check_header validates magic/version but not totalsize, so a
+ * crafted header with a tiny totalsize must be rejected rather than
+ * loaded/forwarded as a partial tree. The MAX default assumes a staging
+ * window at WOLFBOOT_LOAD_DTS_ADDRESS of at least 1 MiB; targets with a
+ * smaller window must override WOLFBOOT_DTS_MAX_SIZE (see hal/nxp_ppc.h). */
+#ifndef WOLFBOOT_DTS_MAX_SIZE
+#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U)
+#endif
+#define WOLFBOOT_DTS_MIN_SIZE (40U)
+
 #define FDT_FIRST_SUPPORTED_VERSION 0x10
 #define FDT_LAST_SUPPORTED_VERSION  0x11
 

src/fdt.c

@@ -462,10 +462,24 @@ static int fdt_subnode_offset_namelen(const void *fdt, int offset,
 int fdt_check_header(const void *fdt)
 {
     if (fdt_magic(fdt) == FDT_MAGIC) {
+        uint32_t off_rsv = fdt_off_mem_rsvmap(fdt);
+        uint32_t off_struct = fdt_off_dt_struct(fdt);
+        uint32_t size_struct = fdt_size_dt_struct(fdt);
+        uint32_t off_strings = fdt_off_dt_strings(fdt);
+        uint32_t size_strings = fdt_size_dt_strings(fdt);
+
         if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
             return -FDT_ERR_BADVERSION;
         if (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION)
             return -FDT_ERR_BADVERSION;
+        /* The three structural areas must sit inside the blob and not
+         * overlap: reservation map, structure block and string table,
+         * in that order. The additions are made in 64-bit so the size
+         * fields cannot wrap around the comparison. */
+        if (off_rsv > off_struct
+            || (uint64_t)off_struct + size_struct > off_strings
+            || (uint64_t)off_strings + size_strings > fdt_totalsize(fdt))
+            return -FDT_ERR_BADSTRUCTURE;
     }
     else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
         if (fdt_size_dt_struct(fdt) == 0)
@@ -579,6 +593,17 @@ const char* fdt_get_string(const void *fdt, int stroffset, int *lenp)
     uint32_t strsize = fdt_size_dt_strings(fdt);
     const char *s;
     const char *end;
+    int err;
+
+    /* off_dt_strings/size_dt_strings are attacker-influenceable header
+     * fields; validate the layout against totalsize before forming the
+     * string-table pointer. */
+    err = fdt_check_header(fdt);
+    if (err != 0) {
+        if (lenp)
+            *lenp = err;
+        return NULL;
+    }
 
     if ((stroffset < 0) || ((uint32_t)stroffset >= strsize)) {
         if (lenp)
@@ -726,21 +751,28 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
         int len;
         const char *prop = (const char*)fdt_getprop(fdt, offset, "compatible",
             &len);
-        /* property list may contain multiple null terminated strings */
-        while (prop != NULL && len >= complen) {
+        /* property list may contain multiple null terminated strings.
+         * Locate each entry's NUL terminator within the declared length
+         * first, then compare: the entry must be exactly as long as the
+         * wanted string, so no byte is ever read past the property and
+         * an unterminated trailing entry can neither match nor be
+         * misread as one. */
+        while (prop != NULL && len > 0) {
             const char* nextprop;
-            if (memcmp(compatible, prop, complen+1) == 0) {
-                return offset;
-            }
+            int entrylen;
+
             nextprop = memchr(prop, '\0', len);
-            if (nextprop != NULL) {
-                len -= (nextprop - prop) + 1;
-                prop = nextprop + 1;
-            }
-            else {
+            if (nextprop == NULL) {
                 /* No NUL terminator within the declared length, break. */
                 break;
             }
+            entrylen = (int)(nextprop - prop);
+            if (entrylen == complen &&
+                memcmp(compatible, prop, complen) == 0) {
+                return offset;
+            }
+            len -= entrylen + 1;
+            prop = nextprop + 1;
         }
     }
     return offset;
@@ -908,6 +940,22 @@ int fdt_fixup_val64(void* fdt, int off, const char* node, const char* name,
 
 
 /* FIT Specific */
+
+/* Returns the property value only when it is a NUL-terminated C string
+ * within its declared length, else NULL: property values are opaque
+ * byte arrays and the names taken from them are passed to
+ * fdt_find_node_offset()/strcmp(), which strlen() them. */
+static const char* fit_getprop_string(const void* fdt, int offset,
+    const char* name)
+{
+    int len = 0;
+    const char* val = (const char*)fdt_getprop(fdt, offset, name, &len);
+
+    if (val == NULL || len <= 0 || memchr(val, '\0', len) == NULL)
+        return NULL;
+    return val;
+}
+
 const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_dt,
     const char** pramdisk, const char** pfpga)
 {
@@ -936,19 +984,16 @@ const char* fit_find_images(void* fdt, const char** pkernel, const char** pflat_
         if (conf == NULL)
 #endif
         {
-            val = fdt_getprop(fdt, off, "default", &len);
-            if (val != NULL && len > 0) {
-                conf = (const char*)val;
-            }
+            conf = fit_getprop_string(fdt, off, "default");
         }
     }
     if (conf != NULL) {
         off = fdt_find_node_offset(fdt, -1, conf);
         if (off > 0) {
-            kernel = fdt_getprop(fdt, off, "kernel", &len);
-            flat_dt = fdt_getprop(fdt, off, "fdt", &len);
-            ramdisk = fdt_getprop(fdt, off, "ramdisk", &len);
-            fpga = fdt_getprop(fdt, off, "fpga", &len);
+            kernel = fit_getprop_string(fdt, off, "kernel");
+            flat_dt = fit_getprop_string(fdt, off, "fdt");
+            ramdisk = fit_getprop_string(fdt, off, "ramdisk");
+            fpga = fit_getprop_string(fdt, off, "fpga");
         }
     }
     if (kernel == NULL) {
@@ -1187,11 +1232,20 @@ static void* fit_load_image_inner(void* fdt, const char* image, int* lenp,
              * raw. */
             comp = (const char*)fdt_getprop(fdt, off, "compression",
                 &complen);
-            if (comp != NULL && complen > 0) {
-                if (strcmp(comp, "gzip") == 0) {
+            /* Compare within the declared property length: the value
+             * must be exactly "gzip" or "none" (NUL-terminated). Any
+             * other shape - including an unterminated value - fails
+             * closed instead of being strncmp()'d past the property. */
+            if (comp != NULL) {
+                if (complen == 5 && comp[4] == '\0' &&
+                    memcmp(comp, "gzip", 4) == 0) {
                     is_gzip = 1;
                 }
-                else if (strcmp(comp, "none") != 0) {
+                else if (complen == 5 && comp[4] == '\0' &&
+                    memcmp(comp, "none", 4) == 0) {
+                    /* uncompressed */
+                }
+                else {
                     is_unknown_comp = 1;
                 }
             }