From ef03cf007f26cb3fa19786de1e095d8ca65fc1dd Mon Sep 17 00:00:00 2001 From: Mattia Moffa Date: Thu, 2 Jul 2026 16:34:18 +0200 Subject: [PATCH] Fix additional findings - Check diag_erase() return value in wolfBoot_record_failure() - Dedicated WOLFBOOT_FAILURE_PHASE_SELF_UPDATE - Mark the diagnostics flash write buffers XALIGNED_STACK(4) for HALs that access the source word by word - Ensure diagnostics, update, or swap partitions don't overlap the bootloader --- docs/API.md | 2 ++ include/target.h.in | 30 ++++++++++++++++++++++++++++++ include/wolfboot/wolfboot.h | 2 ++ src/libwolfboot.c | 12 ++++++------ src/update_flash.c | 4 ++-- 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/docs/API.md b/docs/API.md index aab60109..1776f1c6 100644 --- a/docs/API.md +++ b/docs/API.md @@ -98,6 +98,8 @@ The following events are recorded: - An update image was rejected before performing the update (`WOLFBOOT_FAILURE_PHASE_UPDATE`). +- A bootloader self-update image was rejected + (`WOLFBOOT_FAILURE_PHASE_SELF_UPDATE`). - A boot image failed verification, triggering an emergency update (`WOLFBOOT_FAILURE_PHASE_BOOT`). - The emergency-update image also failed verification, leaving the device diff --git a/include/target.h.in b/include/target.h.in index 9c1aa554..58eff29e 100644 --- a/include/target.h.in +++ b/include/target.h.in @@ -144,6 +144,26 @@ (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_SECTOR_SIZE)) #error "Update and swap partitions overlap" #endif + + #if !defined(PART_UPDATE_EXT) && defined(WOLFBOOT_ORIGIN) && \ + ((BOOTLOADER_PARTITION_SIZE + 0) != 0) && \ + ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) != 0) && \ + ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0 + WOLFBOOT_PARTITION_SIZE + 0) > \ + (WOLFBOOT_ORIGIN + 0)) && \ + ((WOLFBOOT_PARTITION_UPDATE_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) + #error "Update partition overlaps the bootloader" + #endif + + #if !defined(PART_SWAP_EXT) && defined(WOLFBOOT_ORIGIN) && \ + ((BOOTLOADER_PARTITION_SIZE + 0) != 0) && \ + ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) != 0) && \ + ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_SECTOR_SIZE) > \ + (WOLFBOOT_ORIGIN + 0)) && \ + ((WOLFBOOT_PARTITION_SWAP_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) + #error "Swap partition overlaps the bootloader" + #endif #endif #ifdef WOLFBOOT_PERSIST_FAILURE_STATUS @@ -186,6 +206,16 @@ (WOLFBOOT_PARTITION_SWAP_ADDRESS + 0 + WOLFBOOT_SECTOR_SIZE)) #error "Diagnostics region overlaps the swap partition" #endif + + #if defined(WOLFBOOT_ORIGIN) && \ + ((BOOTLOADER_PARTITION_SIZE + 0) != 0) && \ + ((WOLFBOOT_DIAGNOSTICS_ADDRESS + 0 + \ + WOLFBOOT_DIAGNOSTICS_SECTORS * WOLFBOOT_SECTOR_SIZE) > \ + (WOLFBOOT_ORIGIN + 0)) && \ + ((WOLFBOOT_DIAGNOSTICS_ADDRESS + 0) < \ + (WOLFBOOT_ORIGIN + 0 + BOOTLOADER_PARTITION_SIZE + 0)) + #error "Diagnostics region overlaps the bootloader" + #endif #endif #endif /* WOLFBOOT_PERSIST_FAILURE_STATUS */ diff --git a/include/wolfboot/wolfboot.h b/include/wolfboot/wolfboot.h index 2b7692d1..fb1b0beb 100644 --- a/include/wolfboot/wolfboot.h +++ b/include/wolfboot/wolfboot.h @@ -624,6 +624,8 @@ int wolfBoot_get_partition_state(uint8_t part, uint8_t *st); #define WOLFBOOT_FAILURE_PHASE_ROLLBACK 3 /* rolled back to a previous image */ #define WOLFBOOT_FAILURE_PHASE_RECOVERY 4 /* emergency-update image also failed * verification (device unbootable) */ +#define WOLFBOOT_FAILURE_PHASE_SELF_UPDATE 5 /* bootloader self-update image + * rejected */ /* Cause of the failure */ #define WOLFBOOT_FAILURE_CAUSE_HEADER 1 /* bad/invalid image header */ diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 9d63c8d1..dc771da6 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -1106,7 +1106,7 @@ static int RAMFUNCTION diag_write(haladdr_t addr, const void *buf, uint32_t len) static int RAMFUNCTION diag_write_header(haladdr_t sector_addr, uint32_t generation) { struct wolfBoot_diag_header hdr; - uint8_t slot[DIAG_HDR_SIZE]; + uint8_t slot[DIAG_HDR_SIZE] XALIGNED_STACK(4); XMEMSET(&hdr, 0, sizeof(hdr)); hdr.magic = DIAG_HDR_MAGIC; hdr.generation = generation; @@ -1154,7 +1154,7 @@ int RAMFUNCTION wolfBoot_record_failure(uint8_t phase, uint8_t cause, uint32_t gen[DIAG_N_SECTORS]; int count[DIAG_N_SECTORS]; struct wolfBoot_failure_record rec; - uint8_t slot[DIAG_RECORD_SIZE]; + uint8_t slot[DIAG_RECORD_SIZE] XALIGNED_STACK(4); uint32_t seq, active_gen; int active, active_count, ret; @@ -1162,8 +1162,8 @@ int RAMFUNCTION wolfBoot_record_failure(uint8_t phase, uint8_t cause, if (diag_scan(gen, count, &active) == 0) { /* Region empty: initialize sector 0 at generation 1. */ - diag_erase(DIAG_SECTOR_ADDR(0), DIAG_SECTOR_SIZE); - if (diag_write_header(DIAG_SECTOR_ADDR(0), 1) != 0) { + if (diag_erase(DIAG_SECTOR_ADDR(0), DIAG_SECTOR_SIZE) != 0 || + diag_write_header(DIAG_SECTOR_ADDR(0), 1) != 0) { diag_lock(); return -1; } @@ -1181,8 +1181,8 @@ int RAMFUNCTION wolfBoot_record_failure(uint8_t phase, uint8_t cause, /* Active sector full, or the next slot holds a torn write that cannot * be reprogrammed: rotate into the next sector. */ int target = (active + 1) % DIAG_N_SECTORS; - diag_erase(DIAG_SECTOR_ADDR(target), DIAG_SECTOR_SIZE); - if (diag_write_header(DIAG_SECTOR_ADDR(target), active_gen + 1) != 0) { + if (diag_erase(DIAG_SECTOR_ADDR(target), DIAG_SECTOR_SIZE) != 0 || + diag_write_header(DIAG_SECTOR_ADDR(target), active_gen + 1) != 0) { diag_lock(); return -1; } diff --git a/src/update_flash.c b/src/update_flash.c index a3e37275..9e433c26 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -240,14 +240,14 @@ void RAMFUNCTION wolfBoot_check_self_update(void) } if (wolfBoot_verify_integrity(&update) < 0) { #ifdef WOLFBOOT_PERSIST_FAILURE_STATUS - wolfBoot_record_verify_failure(WOLFBOOT_FAILURE_PHASE_UPDATE, + wolfBoot_record_verify_failure(WOLFBOOT_FAILURE_PHASE_SELF_UPDATE, PART_UPDATE, &update); #endif return; } if (wolfBoot_verify_authenticity(&update) < 0) { #ifdef WOLFBOOT_PERSIST_FAILURE_STATUS - wolfBoot_record_verify_failure(WOLFBOOT_FAILURE_PHASE_UPDATE, + wolfBoot_record_verify_failure(WOLFBOOT_FAILURE_PHASE_SELF_UPDATE, PART_UPDATE, &update); #endif return;