Fenrir fixes 2026 08 24

.github/workflows/test-library.yml

@@ -174,16 +174,8 @@ jobs:
 
           echo "$output"
 
-          # TODO hal/library.c does not currently return an error code during failure
-          # Test only looks for the word "Failure"
-          # See https://github.com/wolfSSL/wolfBoot/pull/625
-
-          # If the tool printed "Failure", treat it as a failure regardless of exit code
-          if echo "$output" | grep -F "Failure" >/dev/null; then
-              status=1
-          fi
-
-          # Must have failed (non-zero exit)
+          # A rejected image must exit non-zero: wolfBoot_start() propagates
+          # the verify error to the process exit code
           if [ "$status" -eq 0 ]; then
             echo "Expected failure, but exit code was 0"
             exit 1

hal/hifive1.c

@@ -492,16 +492,18 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
     page = address >> 8;
 
     while (j < (uint32_t)len) {
-        if ((off > 0) || (len < FLASH_PAGE_SIZE)) {
+        uint32_t remaining = (uint32_t)len - j;
+
+        if ((off > 0) || (remaining < FLASH_PAGE_SIZE)) {
             uint8_t *orig = (uint8_t *)(FLASH_BASE + (page << 8));
             int rel_len;
             rel_len = FLASH_PAGE_SIZE - off;
             if (swmode) {
                 fespi_hwmode();
                 swmode = 0;
             }
-            if (rel_len > len)
-                rel_len = len;
+            if (rel_len > (int)remaining)
+                rel_len = (int)remaining;
             for (i = 0; i < off; i++)
                 data_copy[i] = orig[i];
             for (i = off; i < off + rel_len; i++)

hal/library.c

@@ -165,7 +165,9 @@ int wolfBoot_start(void)
                         (int)os_image.signature_ok);
     }
 
-    return 0;
+    /* The error paths reach this point with ret < 0; propagate it so a
+     * rejected image does not look like a successful boot to the caller. */
+    return ret;
 }
 
 

hal/nxp_p1021.c

@@ -1835,6 +1835,7 @@ int ext_flash_erase(uintptr_t address, int len)
         wolfBoot_printf("erase page %d, status %x\n", page, status);
 #endif
         (void)status;
+        address += block_size;
         len -= block_size;
     }
 

hal/nxp_t10xx.c

@@ -3249,6 +3249,7 @@ static int hal_flash_status_wait(uint32_t sector, uint16_t mask, uint32_t timeou
 int hal_flash_write(uint32_t address, const uint8_t *data, int len)
 {
     uint32_t i, pos, sector, offset, xfer, nwords;
+    int ret = 0;
 
     /* adjust for flash base */
     if (address >= FLASH_BASE_ADDR)
@@ -3288,7 +3289,9 @@ int hal_flash_write(uint32_t address, const uint8_t *data, int len)
             FLASH_IO16_WRITE(sector, offset, 0);
             FLASH_IO16_WRITE(sector, offset, word);
             FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_BUFFER_CONFIRM);
-            hal_flash_status_wait(sector, 0x44, 200*1000);
+            ret = hal_flash_status_wait(sector, 0x44, 200*1000);
+            if (ret != 0)
+                return ret;
             address++;
             pos++;
             len--;
@@ -3317,7 +3320,9 @@ int hal_flash_write(uint32_t address, const uint8_t *data, int len)
         /* Typical 410us */
 
         /* poll for program completion - max 200ms */
-        hal_flash_status_wait(sector, 0x44, 200*1000);
+        ret = hal_flash_status_wait(sector, 0x44, 200*1000);
+        if (ret != 0)
+            return ret;
 
         address += xfer;
         len -= xfer;
@@ -3328,6 +3333,7 @@ int hal_flash_write(uint32_t address, const uint8_t *data, int len)
 int hal_flash_erase(uint32_t address, int len)
 {
     uint32_t sector;
+    int ret = 0;
 
     /* adjust for flash base */
     if (address >= FLASH_BASE_ADDR)
@@ -3350,7 +3356,9 @@ int hal_flash_erase(uint32_t address, int len)
         /* Typical is 200ms (max 1100ms) */
 
         /* poll for erase completion - max 1.1 sec */
-        hal_flash_status_wait(sector, 0x4C, 1100*1000);
+        ret = hal_flash_status_wait(sector, 0x4C, 1100*1000);
+        if (ret != 0)
+            return ret;
 
         address += FLASH_SECTOR_SIZE;
         len -= FLASH_SECTOR_SIZE;

hal/stm32c0.c

@@ -150,7 +150,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
 
     while (i < len) {
         flash_clear_errors();
-        if ((len - i > 3) && ((((address + i) & 0x07) == 0) &&
+        if ((len - i >= 8) && ((((address + i) & 0x07) == 0) &&
                 ((((uint32_t)data) + i) & 0x07) == 0)) {
             src = (uint32_t *)data;
             dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE);

hal/stm32g0.c

@@ -140,7 +140,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
 
     while (i < len) {
         flash_clear_errors();
-        if ((len - i > 3) && ((((address + i) & 0x07) == 0) &&
+        if ((len - i >= 8) && ((((address + i) & 0x07) == 0) &&
                 ((((uint32_t)data) + i) & 0x07) == 0)) {
             src = (uint32_t *)data;
             dst = (uint32_t *)address;

hal/stm32g4.c

@@ -54,7 +54,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
 
     while (i < len) {
         flash_clear_errors();
-        if ((len - i > 3) && ((((address + i) & 0x07) == 0) &&
+        if ((len - i >= 8) && ((((address + i) & 0x07) == 0) &&
                 ((((uint32_t)data) + i) & 0x07) == 0)) {
             src = (uint32_t *)data;
             dst = (uint32_t *)address;

src/elf.c

@@ -183,12 +183,14 @@ int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry,
 #ifndef ELF_PARSER
         if (mmu_cb != NULL) {
             if (mmu_cb(vaddr, paddr, mem_size) != 0) {
-#ifdef DEBUG_ELF
-            wolfBoot_printf(
-                "Fail to map %u bytes to %p (p %p)\r\n",
-                (uint32_t)mem_size, (void*)vaddr, (void*)paddr);
-#endif
-            continue;
+                /* Never silently drop a PT_LOAD segment: a failed mapping
+                 * leaves a hole in the image, and publishing the entry
+                 * point for a partially loaded ELF is worse than failing
+                 * the load. */
+                wolfBoot_printf("ELF: failed to map %u bytes to %p "
+                    "(p %p) -- aborting ELF load\r\n",
+                    (uint32_t)mem_size, (void*)vaddr, (void*)paddr);
+                return -6;
             }
         }
 

src/fdt.c

@@ -864,7 +864,7 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size)
     uint32_t off_str;
     uint32_t size_str;
     uint32_t total;
-    uint32_t data_end;
+    uint64_t data_end;
     uint32_t shift;
     uint32_t i;
 
@@ -877,9 +877,18 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size)
     off_str     = fdt_off_dt_strings(fdt);
     size_str    = fdt_size_dt_strings(fdt);
     total       = fdt_totalsize(fdt);
-    data_end    = off_str + size_str;
+    /* 64-bit: a wrapped 32-bit end would slip past the checks below
+     * and re-wrap into the memmove length. */
+    data_end    = (uint64_t)off_str + (uint64_t)size_str;
     shift       = (uint32_t)sizeof(struct fdt_reserve_entry); /* 16 */
 
+    /* Validate the layout before using it: the structure block must
+     * start after the reserve map's terminator, and the string block
+     * after the structure block. 64-bit comparisons: a wrapped 32-bit
+     * sum would pass the check. */
+    if (((uint64_t)off_rsv + shift > off_dt) || (off_str < off_dt)) {
+        return -FDT_ERR_BADSTRUCTURE;
+    }
     if ((data_end + shift) > total) {
         return -FDT_ERR_NOSPACE;
     }
@@ -889,14 +898,15 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size)
     i = 0;
     while ((rsv[i].address != 0ULL) || (rsv[i].size != 0ULL)) {
         i++;
-        if ((off_rsv + (i + 1U) * shift) > off_dt) {
+        if (((uint64_t)off_rsv + (uint64_t)(i + 1U) * shift) > off_dt) {
             return -FDT_ERR_BADSTRUCTURE;
         }
     }
 
-    /* Shift structure + strings down by 16 bytes. memmove handles overlap. */
+    /* Shift structure + strings down by 16 bytes. memmove handles overlap.
+     * The length comes from the validated 64-bit end. */
     memmove(base + off_dt + shift, base + off_dt,
-        (size_t)((off_str + size_str) - off_dt));
+        (size_t)(data_end - off_dt));
 
     /* Insert new entry where the old terminator was, write new terminator. */
     rsv[i].address = cpu_to_fdt64(address);

src/fwtpm_callable.c

@@ -42,6 +42,19 @@
 static FWTPM_CTX fwtpm_ctx;
 static int fwtpm_ready;
 
+/* Staging buffer for FWTPM_ProcessCommand(): the processor writes up to
+ * a max-sized response regardless of the capacity the caller offers, so
+ * it must never write into the caller's (possibly shorter) buffer. The
+ * response is copied out only after the produced length has been
+ * checked against the snapshotted capacity. */
+static uint8_t g_fwtpm_rsp_stage[WCS_FWTPM_MAX_COMMAND_SIZE];
+
+/* Command staging: the processor parses the packet more than once
+ * (authentication, then execution), and a DMA-capable non-secure
+ * attacker cannot rewrite secure memory, so both parses see the same
+ * bytes even if the NS command buffer changes in between. */
+static uint8_t g_fwtpm_cmd_stage[WCS_FWTPM_MAX_COMMAND_SIZE];
+
 extern unsigned int _start_heap;
 extern unsigned int _heap_size;
 
@@ -164,20 +177,31 @@ int CSME_NSE_API wcs_fwtpm_transmit(const uint8_t *cmd, uint32_t cmdSz,
         return BAD_FUNC_ARG;
     }
 
-    rspLen = (int)rspCapacity;
-    rc = FWTPM_ProcessCommand(&fwtpm_ctx, cmd, (int)cmdSz, rsp, &rspLen, 0);
+    /* Stage the command before invoking the processor (see the staging
+     * buffer comment above). */
+    memcpy(g_fwtpm_cmd_stage, cmd, cmdSz);
+
+    rspLen = (int)WCS_FWTPM_MAX_COMMAND_SIZE;
+    rc = FWTPM_ProcessCommand(&fwtpm_ctx, g_fwtpm_cmd_stage, (int)cmdSz,
+                              g_fwtpm_rsp_stage, &rspLen, 0);
     if (rc == TPM_RC_SUCCESS) {
-        wireSz = fwtpm_rsp_size(rsp, rspLen);
+        wireSz = fwtpm_rsp_size(g_fwtpm_rsp_stage, rspLen);
         if (wireSz > 0U && wireSz <= rspCapacity) {
+            memcpy(rsp, g_fwtpm_rsp_stage, wireSz);
             *rspSz = wireSz;
         }
         else if (rspLen >= 0 && (uint32_t)rspLen <= rspCapacity) {
+            memcpy(rsp, g_fwtpm_rsp_stage, (uint32_t)rspLen);
             *rspSz = (uint32_t)rspLen;
         }
         else {
             rc = TPM_RC_FAILURE;
         }
     }
+    /* Zero the staging before returning: the response may carry
+     * sensitive material (auth tags, unsealed data). */
+    memset(g_fwtpm_cmd_stage, 0, sizeof(g_fwtpm_cmd_stage));
+    memset(g_fwtpm_rsp_stage, 0, sizeof(g_fwtpm_rsp_stage));
     return rc;
 }
 

src/image.c

@@ -382,15 +382,12 @@ static void wolfBoot_verify_signature_ecc(uint8_t key_slot,
     #endif /* WOLFBOOT_ENABLE_WOLFHSM_CLIENT || (SERVER && CERT_CHAIN) */
         /* wc_ecc_verify_hash_ex() doesn't trigger a crypto callback, so we need
            to use wc_ecc_verify_hash instead. Unfortunately, that requires
-           converting the signature to intermediate DER format first */
-        mp_init(&r);
-        mp_init(&s);
-        mp_read_unsigned_bin(&r, sig, point_sz);
-        mp_read_unsigned_bin(&s, sig + point_sz, point_sz);
-        uint32_t rSz = mp_unsigned_bin_size(&r);
-        uint32_t sSz = mp_unsigned_bin_size(&s);
-        ret          = wc_ecc_rs_raw_to_sig(sig, rSz, &sig[point_sz], sSz,
-                                            (byte*)&tmpSigBuf, (word32*)&tmpSigSz);
+           converting the signature to intermediate DER format first. Both
+           fields are passed at full width: the raw signature is fixed-width
+           and left-zero-padded, and the conversion strips the padding. */
+        ret = wc_ecc_rs_raw_to_sig(sig, (word32)point_sz, &sig[point_sz],
+                                   (word32)point_sz,
+                                   (byte*)&tmpSigBuf, (word32*)&tmpSigSz);
         /* Verify the (temporary) DER representation of the signature */
         if (ret == 0) {
             VERIFY_FN(img, &verify_res, wc_ecc_verify_hash, tmpSigBuf, tmpSigSz,

src/riscv_sbi.c

@@ -157,10 +157,18 @@ typedef struct {
     volatile uint32_t init_magic;
     volatile int      hart_state[MPFS_NUM_HARTS];
     volatile uint32_t ipi_ops[MPFS_NUM_HARTS];
+    /* Fence-completion protocol: ipi_done[h] is incremented by hart h
+     * only after it has executed the fence ops it consumed; a requester
+     * snapshots it into ipi_wait_gen[h] before posting and waits until
+     * ipi_done[h] passes the snapshot. */
+    volatile uint32_t ipi_done[MPFS_NUM_HARTS];
+    volatile uint32_t ipi_wait_gen[MPFS_NUM_HARTS];
 } sbi_shared_state_t;
 #define SBI_SHARED ((sbi_shared_state_t *)SBI_SHARED_DTIM_ADDR)
-#define sbi_hart_state (SBI_SHARED->hart_state)
-#define sbi_ipi_ops    (SBI_SHARED->ipi_ops)
+#define sbi_hart_state   (SBI_SHARED->hart_state)
+#define sbi_ipi_ops      (SBI_SHARED->ipi_ops)
+#define sbi_ipi_done     (SBI_SHARED->ipi_done)
+#define sbi_ipi_wait_gen (SBI_SHARED->ipi_wait_gen)
 
 /* Per-hart IPI work flags, set by a requesting hart and consumed in the
  * target hart's M-mode software-interrupt handler. */
@@ -189,6 +197,8 @@ void sbi_hart_mark_started(unsigned long hartid)
         for (k = 0; k < (unsigned int)MPFS_NUM_HARTS; k++) {
             sbi_hart_state[k] = SBI_HSM_STOPPED;
             sbi_ipi_ops[k] = 0;
+            sbi_ipi_done[k] = 0;
+            sbi_ipi_wait_gen[k] = 0;
         }
         __asm__ volatile("fence rw, rw" ::: "memory");
         SBI_SHARED->init_magic = SBI_SHARED_MAGIC;
@@ -465,6 +475,12 @@ void sbi_ipi_irq(unsigned long hartid)
     if ((ops & SBI_IPI_OP_SSIP) != 0U || ops == 0U) {
         csr_set_bits(mip, MIP_SSIP);
     }
+    /* Publish completion only after the requested fences have executed:
+     * a requester treats the increment as this hart being done. */
+    if ((ops & (SBI_IPI_OP_FENCE_I | SBI_IPI_OP_SFENCE)) != 0U) {
+        (void)__atomic_add_fetch(&sbi_ipi_done[hartid], 1U,
+                                 __ATOMIC_ACQ_REL);
+    }
 }
 
 /* Post an IPI op to every hart in (mask << base) and ring its MSIP.
@@ -501,6 +517,11 @@ static void sbi_post_ipi(unsigned long mask, unsigned long base,
         if (sbi_hart_state[h] != SBI_HSM_STARTED) {
             continue; /* parked harts consume MSIP in their wake loop */
         }
+        /* Snapshot the completion counter before posting: the wait below
+         * completes when the target increments past this value. */
+        if ((op & (SBI_IPI_OP_FENCE_I | SBI_IPI_OP_SFENCE)) != 0U) {
+            sbi_ipi_wait_gen[h] = sbi_ipi_done[h];
+        }
         /* Atomic OR (amoor.w, rv64a): two harts may post to the same target
          * concurrently, and the target may be consuming (sbi_ipi_irq) at the
          * same time - a plain |= read-modify-write could drop an op. */
@@ -511,9 +532,11 @@ static void sbi_post_ipi(unsigned long mask, unsigned long base,
     __asm__ volatile("fence iorw, iorw" ::: "memory");
 }
 
-/* Wait (bounded) for the posted fence ops to be consumed by the targets.
- * The SBI remote-fence calls are synchronous; the bound guards against a
- * wedged target turning into a wedged caller. */
+/* Wait (bounded) for the posted fence ops to be consumed by the targets:
+ * the targets increment ipi_done after executing the fences, so this
+ * completes only when every target has finished its fence. The SBI
+ * remote-fence calls are synchronous; the bound guards against a wedged
+ * target turning into a wedged caller. */
 static void sbi_wait_ipi_done(unsigned long mask, unsigned long base,
     unsigned long self)
 {
@@ -535,7 +558,7 @@ static void sbi_wait_ipi_done(unsigned long mask, unsigned long base,
             continue;
         }
         spin = 10000000U;
-        while (sbi_ipi_ops[h] != 0U && spin > 0U) {
+        while (sbi_ipi_done[h] <= sbi_ipi_wait_gen[h] && spin > 0U) {
             spin--;
         }
     }