From febf29ad6141e083fcc67d7439cb04e9d593b7b3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 25 Aug 2026 16:08:16 +0200 Subject: [PATCH 01/12] pkcs11 store: batch sector commits to Store_Close Every wolfPKCS11 field write flushed the payload sector and the header sector to flash (2 erases + 2 programs of a full sector each), and the token store re-serializes all objects per C_CreateObject/C_DestroyObject, so those calls cost hundreds of sector erases and tens of seconds on flash with slow erase times. Cache modified sectors in RAM and commit them together when the store window closes: - sector cache sized to the worst-case span of one object plus the header sector (WOLFBOOT_PKCS11_STORE_CACHE_SECTORS), LRU eviction when exceeded - header sector commits last, so a committed header is the atomic commit point of the batch: power failure during a flush leaves the flash in either the pre-batch or the post-batch state - per-commit backup sector write preserved, keeping recovery of the sector in flight at failure time - delete_object commits on return (durability contract, unit-tested) - nodes table, bitmap, payload ids and the live object size (handle->size) are read from the cache when the sector is dirty Measured on an STM32H5 with 8KB sectors, wolfPKCS11 in the secure world: C_CreateObject 1.5s -> 0.15s, C_DestroyObject 1.3s -> 0.12s, 456 -> 40 sector erases per create, and the count no longer scales with the number of objects in the token. PKCS11_STORE_STATS (off by default) adds flash-activity counters and a test-app bench to quantify store traffic: make PKCS11_STORE_STATS=1. --- include/wolfboot/wcs_pkcs11.h | 6 + options.mk | 4 + src/pkcs11_callable.c | 24 ++ src/pkcs11_store.c | 357 ++++++++++++++++++++++----- test-app/Makefile | 3 + test-app/test_pkcs11.c | 197 +++++++++++++++ tools/unit-tests/unit-pkcs11_store.c | 9 +- 7 files changed, 531 insertions(+), 69 deletions(-) diff --git a/include/wolfboot/wcs_pkcs11.h b/include/wolfboot/wcs_pkcs11.h index 09c69e0bdd..d798458198 100644 --- a/include/wolfboot/wcs_pkcs11.h +++ b/include/wolfboot/wcs_pkcs11.h @@ -343,5 +343,11 @@ CK_RV CSME_NSE_API C_GetFunctionStatus_nsc_call(CK_SESSION_HANDLE hSession); CK_RV CSME_NSE_API C_CancelFunction_nsc_call(CK_SESSION_HANDLE hSession); CK_RV CSME_NSE_API C_WaitForSlotEvent_nsc_call(CK_FLAGS flags, CK_SLOT_ID_PTR pSlot, CK_VOID_PTR pReserved); +#ifdef PKCS11_STORE_STATS +CK_RV CSME_NSE_API C_StoreGetStats_nsc_call(uint32_t *pCommits, + uint32_t *pErases, uint32_t *pPrograms); +CK_RV CSME_NSE_API C_StoreResetStats_nsc_call(void); +#endif + #endif /* SECURE_PKCS11 */ #endif /* !WOLFBOOT_PKCS11_H */ diff --git a/options.mk b/options.mk index 9690a14b0a..5075ceffa7 100644 --- a/options.mk +++ b/options.mk @@ -1143,6 +1143,10 @@ ifeq ($(WOLFBOOT_DICE_HW),1) endif endif +ifeq ($(PKCS11_STORE_STATS),1) + CFLAGS+=-DPKCS11_STORE_STATS +endif + ifeq ($(WOLFCRYPT_TZ_PKCS11),1) CFLAGS+=-DSECURE_PKCS11 CFLAGS+=-DWOLFPKCS11_USER_SETTINGS diff --git a/src/pkcs11_callable.c b/src/pkcs11_callable.c index 474a0c1a4e..225ede4c32 100644 --- a/src/pkcs11_callable.c +++ b/src/pkcs11_callable.c @@ -1512,6 +1512,30 @@ CK_RV CSME_NSE_API C_CancelFunction_nsc_call(CK_SESSION_HANDLE hSession) return C_CancelFunction(hSession); } +#ifdef PKCS11_STORE_STATS +/* Flash-activity counters, implemented in src/pkcs11_store.c (the wolfBoot + * store backend); declared here to keep the wolfPKCS11 submodule untouched. */ +void wolfPKCS11_Store_GetStats(uint32_t *commits, uint32_t *erases, + uint32_t *programs); +void wolfPKCS11_Store_ResetStats(void); + +CK_RV CSME_NSE_API C_StoreGetStats_nsc_call(uint32_t *pCommits, + uint32_t *pErases, uint32_t *pPrograms) +{ + NSC_CHK(ns_ok(pCommits, sizeof(uint32_t))); + NSC_CHK(ns_ok(pErases, sizeof(uint32_t))); + NSC_CHK(ns_ok(pPrograms, sizeof(uint32_t))); + wolfPKCS11_Store_GetStats(pCommits, pErases, pPrograms); + return CKR_OK; +} + +CK_RV CSME_NSE_API C_StoreResetStats_nsc_call(void) +{ + wolfPKCS11_Store_ResetStats(); + return CKR_OK; +} +#endif + CK_RV CSME_NSE_API C_WaitForSlotEvent_nsc_call(CK_FLAGS flags, CK_SLOT_ID_PTR pSlot, CK_VOID_PTR pReserved) { /* pReserved must be NULL; the underlying call rejects anything else. */ diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 8351d6cde3..bc0b0644c5 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -109,6 +109,7 @@ struct obj_hdr struct store_handle { uint32_t flags; uint32_t pos; + uint32_t size; /* live object size; the flash node is updated at commit */ void *buffer; struct obj_hdr *hdr; uint32_t in_buffer_offset; @@ -119,20 +120,79 @@ struct store_handle { static struct store_handle openstores_handles[MAX_OPEN_STORES] = {}; -static uint8_t cached_sector[WOLFBOOT_SECTOR_SIZE]; +/* + * Sector cache: batches flash traffic within a Store_Open/Store_Close + * window. Sectors accumulate modifications in RAM and are committed + * together by cache_flush_all(). The header sector (offset 0) is + * always committed last, so a committed header is the atomic commit + * point of the whole batch: power failure during a flush leaves the + * flash in either the pre-batch or the post-batch state, never a mix. + */ +#define PKCS11_STORE_MAX_SECTORS \ + ((KEYVAULT_OBJ_SIZE + WOLFBOOT_SECTOR_SIZE - 1) / WOLFBOOT_SECTOR_SIZE \ + + 2) + +#ifndef WOLFBOOT_PKCS11_STORE_CACHE_SECTORS + #define WOLFBOOT_PKCS11_STORE_CACHE_SECTORS PKCS11_STORE_MAX_SECTORS +#endif +#if (WOLFBOOT_PKCS11_STORE_CACHE_SECTORS > PKCS11_STORE_MAX_SECTORS) + #error WOLFBOOT_PKCS11_STORE_CACHE_SECTORS exceeds worst case +#endif + +struct cache_entry { + uint8_t *sector; /* NULL when the slot is free */ + uint32_t offset; /* vault offset of the sector */ + uint32_t lru; /* last use tick */ +}; + +static uint8_t cache_sector_mem + [WOLFBOOT_PKCS11_STORE_CACHE_SECTORS][WOLFBOOT_SECTOR_SIZE]; +static struct cache_entry store_cache[WOLFBOOT_PKCS11_STORE_CACHE_SECTORS]; +static uint32_t cache_lru_tick; + +static uint8_t *cache_get_sector(uint32_t offset); +static void cache_flush_all(void); +static uint8_t *sector_ptr(uint32_t offset); +static uint8_t *sector0_ptr(void); + +/* Optional flash-activity instrumentation (PKCS11_STORE_STATS, not + * enabled by any shipping config): counts sector commits, erases and + * programs so a host test can quantify the store's flash traffic. */ +#ifdef PKCS11_STORE_STATS +static uint32_t stats_commits; +static uint32_t stats_erases; +static uint32_t stats_programs; + +void wolfPKCS11_Store_GetStats(uint32_t *commits, uint32_t *erases, + uint32_t *programs) +{ + *commits = stats_commits; + *erases = stats_erases; + *programs = stats_programs; +} + +void wolfPKCS11_Store_ResetStats(void) +{ + stats_commits = 0; + stats_erases = 0; + stats_programs = 0; +} +#endif static void bitmap_put(uint32_t pos, int val) { uint32_t octet = pos / 8; uint32_t bit = pos % 8; - uint8_t *bitmap = cached_sector + sizeof(uint32_t); + uint8_t *bitmap; /* Reject out-of-range positions (e.g. a power-fault-corrupted hdr->pos * left as erased flash) to avoid an out-of-bounds write past the - * bitmap, which lives within cached_sector. */ - if (pos >= KEYVAULT_MAX_ITEMS) + * bitmap, which lives within the header sector. */ + if (pos >= KEYVAULT_MAX_ITEMS) { return; + } + bitmap = cache_get_sector(0) + sizeof(uint32_t); if (val != 0) { bitmap[octet] |= (1 << bit); } else { @@ -144,7 +204,7 @@ static int bitmap_get(uint32_t pos) { uint32_t octet = pos / 8; uint32_t bit = pos % 8; - uint8_t *bitmap = vault_base + sizeof(uint32_t); + uint8_t *bitmap = sector0_ptr() + sizeof(uint32_t); return (bitmap[octet] & (1 << bit)) >> bit; } @@ -172,19 +232,136 @@ static int bitmap_find_free_pos(void) #define BACKUP_SECTOR_ADDRESS (vault_base + WOLFBOOT_SECTOR_SIZE) -static void cache_commit(uint32_t offset) +static struct cache_entry *cache_find(uint32_t offset) +{ + int i; + + for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + if ((store_cache[i].sector != NULL) && + (store_cache[i].offset == offset)) { + return &store_cache[i]; + } + } + return NULL; +} + +static void cache_commit_entry(struct cache_entry *entry) { hal_flash_unlock(); /* Write backup sector first */ hal_flash_erase((uintptr_t)BACKUP_SECTOR_ADDRESS, WOLFBOOT_SECTOR_SIZE); - hal_flash_write((uintptr_t)BACKUP_SECTOR_ADDRESS, cached_sector, WOLFBOOT_SECTOR_SIZE); + hal_flash_write((uintptr_t)BACKUP_SECTOR_ADDRESS, entry->sector, + WOLFBOOT_SECTOR_SIZE); /* Erase + write actual destination sector */ - hal_flash_erase((uintptr_t)vault_base + offset, WOLFBOOT_SECTOR_SIZE); - hal_flash_write((uintptr_t)vault_base + offset, cached_sector, WOLFBOOT_SECTOR_SIZE); + hal_flash_erase((uintptr_t)vault_base + entry->offset, + WOLFBOOT_SECTOR_SIZE); + hal_flash_write((uintptr_t)vault_base + entry->offset, entry->sector, + WOLFBOOT_SECTOR_SIZE); hal_flash_lock(); +#ifdef PKCS11_STORE_STATS + stats_commits++; + stats_erases += 2; + stats_programs += 2; +#endif +} + +/* + * Get a RAM copy of the vault sector at the given offset. Modifications + * stay in RAM until cache_flush_all() (or LRU eviction) commits them. + */ +static uint8_t *cache_get_sector(uint32_t offset) +{ + struct cache_entry *entry; + int i; + int free_slot = -1; + + entry = cache_find(offset); + if (entry != NULL) { + entry->lru = ++cache_lru_tick; + return entry->sector; + } + + for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + if (store_cache[i].sector == NULL) { + free_slot = i; + break; + } + } + if (free_slot < 0) { + /* No free slot: commit the least recently used entry */ + int oldest = 0; + + for (i = 1; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + if (store_cache[i].lru < store_cache[oldest].lru) { + oldest = i; + } + } + cache_commit_entry(&store_cache[oldest]); + store_cache[oldest].sector = NULL; + free_slot = oldest; + } + + entry = &store_cache[free_slot]; + entry->sector = &cache_sector_mem[free_slot][0]; + entry->offset = offset; + entry->lru = ++cache_lru_tick; + memcpy(entry->sector, vault_base + offset, WOLFBOOT_SECTOR_SIZE); + return entry->sector; +} + +/* + * Commit all cached sectors. Payload sectors first, the header sector + * (offset 0) last, so the header is the atomic commit point of the + * batch. + */ +static void cache_flush_all(void) +{ + int i; + int pass; + + for (pass = 0; pass < 2; pass++) { + for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + if (store_cache[i].sector == NULL) { + continue; + } + if ((pass == 0) == (store_cache[i].offset == 0)) { + continue; + } + cache_commit_entry(&store_cache[i]); + store_cache[i].sector = NULL; + } + } +} + +static void cache_reset(void) +{ + int i; + + for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + store_cache[i].sector = NULL; + } +} + +/* + * Read access to a vault sector: the RAM copy when the sector is + * cached, flash otherwise. Writes must go through cache_get_sector(). + */ +static uint8_t *sector_ptr(uint32_t offset) +{ + struct cache_entry *entry = cache_find(offset); + + if (entry != NULL) { + return entry->sector; + } + return vault_base + offset; +} + +static uint8_t *sector0_ptr(void) +{ + return sector_ptr(0); } static void restore_backup(uint32_t offset) @@ -195,46 +372,65 @@ static void restore_backup(uint32_t offset) hal_flash_write((uintptr_t)vault_base + offset, BACKUP_SECTOR_ADDRESS, WOLFBOOT_SECTOR_SIZE); hal_flash_lock(); +#ifdef PKCS11_STORE_STATS + stats_erases++; + stats_programs++; +#endif } static void check_vault(void) { - uint32_t *magic = (uint32_t *)vault_base; + uint32_t *magic; + uint32_t *backup_magic; + uint8_t *s0 = NULL; uint32_t total_vault_size = KEYVAULT_MAX_ITEMS * KEYVAULT_OBJ_SIZE; + cache_reset(); + if ((total_vault_size % WOLFBOOT_SECTOR_SIZE) != 0) total_vault_size = (total_vault_size / WOLFBOOT_SECTOR_SIZE) * WOLFBOOT_SECTOR_SIZE + WOLFBOOT_SECTOR_SIZE; + magic = (uint32_t *)vault_base; if (*magic != VAULT_HEADER_MAGIC) { - uint32_t *magic = (uint32_t *)BACKUP_SECTOR_ADDRESS; - if (*magic == VAULT_HEADER_MAGIC) { + backup_magic = (uint32_t *)BACKUP_SECTOR_ADDRESS; + if (*backup_magic == VAULT_HEADER_MAGIC) { restore_backup(0); return; } - memset(cached_sector, 0xFF, WOLFBOOT_SECTOR_SIZE); - magic = (uint32_t *)cached_sector; + s0 = cache_get_sector(0); + memset(s0, 0xFF, WOLFBOOT_SECTOR_SIZE); + magic = (uint32_t *)s0; *magic = VAULT_HEADER_MAGIC; - memset(cached_sector + sizeof(uint32_t), 0x00, BITMAP_SIZE); - cache_commit(0); + memset(s0 + sizeof(uint32_t), 0x00, BITMAP_SIZE); + cache_flush_all(); hal_flash_unlock(); hal_flash_erase((uintptr_t)vault_base + WOLFBOOT_SECTOR_SIZE * 2, total_vault_size); hal_flash_lock(); +#ifdef PKCS11_STORE_STATS + stats_erases += total_vault_size / WOLFBOOT_SECTOR_SIZE; +#endif } } static void delete_object(int32_t type, uint32_t tok_id, uint32_t obj_id) { - struct obj_hdr *hdr = (struct obj_hdr *)(cached_sector + STORE_PRIV_HDR_OFFSET); + struct obj_hdr *hdr; + uint8_t *s0; + + /* Deletions are durable on return, like the historical per-write + * commits: validate the vault (resets the cache) and commit the + * whole batch before returning. */ check_vault(); - memcpy(cached_sector, vault_base, WOLFBOOT_SECTOR_SIZE); + s0 = cache_get_sector(0); + hdr = (struct obj_hdr *)(s0 + STORE_PRIV_HDR_OFFSET); - while ((uintptr_t)hdr < ((uintptr_t)cached_sector + WOLFBOOT_SECTOR_SIZE)) { + while ((uintptr_t)hdr < ((uintptr_t)s0 + WOLFBOOT_SECTOR_SIZE)) { if ((hdr->token_id == tok_id) && (hdr->object_id == obj_id) && (hdr->type == type)) { hdr->token_id = PKCS11_INVALID_ID; hdr->object_id = PKCS11_INVALID_ID; bitmap_put(hdr->pos, 0); - cache_commit(0); + cache_flush_all(); return; } hdr++; @@ -248,20 +444,27 @@ static void delete_object(int32_t type, uint32_t tok_id, uint32_t obj_id) */ static uint8_t *find_object_buffer(int32_t type, uint32_t tok_id, uint32_t obj_id) { - struct obj_hdr *hdr = NODES_TABLE; + struct obj_hdr *hdr; uint32_t *tok_obj_stored = NULL; - while ((uintptr_t)hdr < ((uintptr_t)vault_base + WOLFBOOT_SECTOR_SIZE)) { + uint8_t *s0 = sector0_ptr(); + + hdr = (struct obj_hdr *)(s0 + STORE_PRIV_HDR_OFFSET); + while ((uintptr_t)hdr < ((uintptr_t)s0 + WOLFBOOT_SECTOR_SIZE)) { if ((hdr->token_id == tok_id) && (hdr->object_id == obj_id) && (hdr->type == type)) { - tok_obj_stored = (uint32_t *) (vault_base + (2 * WOLFBOOT_SECTOR_SIZE) + (hdr->pos * KEYVAULT_OBJ_SIZE)); + uint32_t obj_off = 2 * WOLFBOOT_SECTOR_SIZE + + hdr->pos * KEYVAULT_OBJ_SIZE; + uint32_t in_sector_off = obj_off % WOLFBOOT_SECTOR_SIZE; + uint32_t sector_base = obj_off - in_sector_off; + + tok_obj_stored = (uint32_t *)(sector_ptr(sector_base) + + in_sector_off); if ((tok_obj_stored[0] != tok_id) || (tok_obj_stored[1] != obj_id)) { /* Id's don't match. Try backup sector. */ - uint32_t in_sector_off = (hdr->pos * KEYVAULT_OBJ_SIZE) % - WOLFBOOT_SECTOR_SIZE; - uint32_t sector_base = hdr->pos * KEYVAULT_OBJ_SIZE + - 2 * WOLFBOOT_SECTOR_SIZE - in_sector_off; - tok_obj_stored = (uint32_t *)((BACKUP_SECTOR_ADDRESS + in_sector_off)); - if ((tok_obj_stored[0] == tok_id) && (tok_obj_stored[1] == obj_id)) { + tok_obj_stored = (uint32_t *)(BACKUP_SECTOR_ADDRESS + + in_sector_off); + if ((tok_obj_stored[0] == tok_id) && + (tok_obj_stored[1] == obj_id)) { /* Found backup! restoring... */ restore_backup(sector_base); } else { @@ -270,7 +473,7 @@ static uint8_t *find_object_buffer(int32_t type, uint32_t tok_id, uint32_t obj_i } } /* Object is now OK */ - return vault_base + 2 * WOLFBOOT_SECTOR_SIZE + hdr->pos * KEYVAULT_OBJ_SIZE; + return vault_base + obj_off; } hdr++; } @@ -280,32 +483,39 @@ static uint8_t *find_object_buffer(int32_t type, uint32_t tok_id, uint32_t obj_i static struct obj_hdr *find_object_header(int32_t type, uint32_t tok_id, uint32_t obj_id) { - struct obj_hdr *hdr = NODES_TABLE; - while ((uintptr_t)hdr < ((uintptr_t)vault_base + WOLFBOOT_SECTOR_SIZE)) { + struct obj_hdr *hdr; + uint8_t *s0 = sector0_ptr(); + + hdr = (struct obj_hdr *)(s0 + STORE_PRIV_HDR_OFFSET); + while ((uintptr_t)hdr < ((uintptr_t)s0 + WOLFBOOT_SECTOR_SIZE)) { if ((hdr->token_id == tok_id) && (hdr->object_id == obj_id) && (hdr->type == type)) { - return hdr; + /* Return the flash address of the node */ + return (struct obj_hdr *)(vault_base + + ((uint8_t *)hdr - (uint8_t *)s0)); } hdr++; } - return NULL; + return NULL; /* object not found */ } static struct obj_hdr *create_object(int32_t type, uint32_t tok_id, uint32_t obj_id) { struct obj_hdr *hdr = NULL; uint32_t *tok_obj_id; + uint8_t *s0; + uint8_t *pay; + uint32_t sector_base, in_sector_off; /* Refuse to create an object that's already in store */ if (find_object_buffer(type, tok_id, obj_id) != NULL) { return NULL; } /* Caching sector 0 */ - memcpy(cached_sector, vault_base , WOLFBOOT_SECTOR_SIZE); - hdr = (struct obj_hdr *)(cached_sector + STORE_PRIV_HDR_OFFSET); - while ((uintptr_t)hdr < ((uintptr_t)cached_sector + WOLFBOOT_SECTOR_SIZE)) { + s0 = cache_get_sector(0); + hdr = (struct obj_hdr *)(s0 + STORE_PRIV_HDR_OFFSET); + while ((uintptr_t)hdr < ((uintptr_t)s0 + WOLFBOOT_SECTOR_SIZE)) { if (hdr->token_id == PKCS11_INVALID_ID) { - uint32_t sector_base, in_sector_off; int pos = bitmap_find_free_pos(); if (pos < 0) { return NULL; @@ -327,37 +537,39 @@ static struct obj_hdr *create_object(int32_t type, uint32_t tok_id, uint32_t obj hdr->size = 2 * sizeof(uint32_t); /* Set the bit to claim the position in flash */ bitmap_put(hdr->pos, 1); - cache_commit(0); /* Mark the beginning of the object in the sector, - * write the tok/obj ids + * write the tok/obj ids. Stays in the cache until the + * window is closed. */ - memcpy(cached_sector, vault_base + sector_base, - WOLFBOOT_SECTOR_SIZE); - tok_obj_id = (void*)(cached_sector + in_sector_off); + pay = cache_get_sector(sector_base); + tok_obj_id = (uint32_t *)(pay + in_sector_off); tok_obj_id[0] = tok_id; tok_obj_id[1] = obj_id; - cache_commit(sector_base); /* Return the address of the header in flash */ - return (struct obj_hdr *)(vault_base + ((uint8_t *)hdr - (uint8_t *)cached_sector)); + return (struct obj_hdr *)(vault_base + + ((uint8_t *)hdr - (uint8_t *)s0)); } hdr++; } return NULL; /* No space left in the nodes table */ } -static void update_store_size(struct obj_hdr *hdr, uint32_t size) +static void update_store_size(struct store_handle *handle, + struct obj_hdr *hdr, uint32_t size) { uint32_t off; + uint8_t *s0; struct obj_hdr *hdr_mem; + if (((uint8_t *)hdr) < vault_base || - ((uint8_t *)hdr > vault_base + WOLFBOOT_SECTOR_SIZE)) + ((uint8_t *)hdr > vault_base + WOLFBOOT_SECTOR_SIZE)) { return; - check_vault(); + } off = (uintptr_t)hdr - (uintptr_t)vault_base; - memcpy(cached_sector, vault_base, WOLFBOOT_SECTOR_SIZE); - hdr_mem = (struct obj_hdr *)(cached_sector + off); + s0 = cache_get_sector(0); + hdr_mem = (struct obj_hdr *)(s0 + off); hdr_mem->size = size; - cache_commit(0); + handle->size = size; } static void erase_object_payload(uint8_t *buf) @@ -375,16 +587,18 @@ static void erase_object_payload(uint8_t *buf) while (sector_base < erase_end) { uint32_t erase_start = erase_off; uint32_t erase_stop = sector_base + WOLFBOOT_SECTOR_SIZE; + uint8_t *s; - if (erase_start < sector_base) + if (erase_start < sector_base) { erase_start = sector_base; - if (erase_stop > erase_end) + } + if (erase_stop > erase_end) { erase_stop = erase_end; + } - memcpy(cached_sector, vault_base + sector_base, WOLFBOOT_SECTOR_SIZE); - memset(cached_sector + (erase_start - sector_base), 0xFF, + s = cache_get_sector(sector_base); + memset(s + (erase_start - sector_base), 0xFF, erase_stop - erase_start); - cache_commit(sector_base); sector_base += WOLFBOOT_SECTOR_SIZE; } } @@ -410,6 +624,7 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, { struct store_handle *handle; uint8_t *buf; + uint32_t hdr_off; int is_new = 0; /* Check if there is one handle available to open the slot */ @@ -455,12 +670,15 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, handle->flags |= STORE_FLAGS_OPEN; /* Set the 'readonly' flag in this handle if open with 'r' */ - if (read) + if (read) { handle->flags |= STORE_FLAGS_READONLY; - else { + /* Live size from the (possibly cached) header sector */ + hdr_off = (uintptr_t)handle->hdr - (uintptr_t)vault_base; + handle->size = ((struct obj_hdr *)(sector0_ptr() + hdr_off))->size; + } else { handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ - update_store_size(handle->hdr, 2 * sizeof(uint32_t)); + update_store_size(handle, handle->hdr, 2 * sizeof(uint32_t)); /* Erase object data sectors to clear residual key material from a * prior (longer) payload. New objects are already in a fresh sector * from create_object(), so only do this for existing objects. */ @@ -479,6 +697,9 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, void wolfPKCS11_Store_Close(void* store) { struct store_handle *handle = store; + /* Commit all pending sectors: the header sector last, so the header + * is the atomic commit point of the window. */ + cache_flush_all(); memset(handle, 0, sizeof(*handle)); } @@ -492,7 +713,7 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) if (len < 0) return -1; - obj_size = handle->hdr->size; + obj_size = handle->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -517,6 +738,7 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) uint32_t in_sector_offset = 0; uint32_t in_sector_len = 0; uint32_t sector_base = 0; + uint8_t *s; int written = 0; @@ -528,7 +750,7 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) if (len < 0) return -1; - obj_size = handle->hdr->size; + obj_size = handle->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -547,18 +769,17 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) if (in_sector_len > (uint32_t)(len - written)) in_sector_len = len - written; - /* Cache the corresponding sector */ - memcpy(cached_sector, (void *)(uintptr_t)sector_base, WOLFBOOT_SECTOR_SIZE); - /* Write content into cache */ - memcpy(cached_sector + in_sector_offset, buffer + written, in_sector_len); + /* Copy the write into the sector cache; the sector is committed + * at Store_Close (or on LRU eviction). */ + s = cache_get_sector( + (uint32_t)((uintptr_t)sector_base - (uintptr_t)vault_base)); + memcpy(s + in_sector_offset, buffer + written, in_sector_len); /* Adjust in_buffer position for the handle accordingly */ handle->in_buffer_offset += in_sector_len; written += in_sector_len; - /* Write sector to flash */ - cache_commit((uintptr_t)sector_base - (uintptr_t)vault_base); } obj_size += written; - update_store_size(handle->hdr, obj_size); + update_store_size(handle, handle->hdr, obj_size); return len; } diff --git a/test-app/Makefile b/test-app/Makefile index b39a19963e..43e955af23 100644 --- a/test-app/Makefile +++ b/test-app/Makefile @@ -399,6 +399,9 @@ ifeq ($(TZEN),1) ifeq ($(WOLFCRYPT_TZ_PKCS11),1) CFLAGS+=-DWOLFSSL_USER_SETTINGS -DWOLFTPM_USER_SETTINGS CFLAGS+=-DWOLFBOOT_PKCS11_APP -DSECURE_PKCS11 -DWOLFBOOT_TZ_PKCS11 + ifeq ($(PKCS11_STORE_STATS),1) + CFLAGS+=-DPKCS11_STORE_STATS + endif ifeq ($(PKCS11_TESTAPP),1) CFLAGS+=-DWOLFBOOT_PKCS11_TESTAPP endif diff --git a/test-app/test_pkcs11.c b/test-app/test_pkcs11.c index b1f9e3be1c..b3bb295fd8 100644 --- a/test-app/test_pkcs11.c +++ b/test-app/test_pkcs11.c @@ -18,6 +18,7 @@ #include "test_pkcs11.h" #include "wolfpkcs11/pkcs11.h" +#include "wolfboot/wcs_pkcs11.h" #include #include @@ -508,6 +509,196 @@ static int test_pkcs11_log_key_attrs(CK_SESSION_HANDLE session, return 0; } +#ifdef PKCS11_STORE_STATS +/* + * Store-traffic benchmark: C_CreateObject and C_DestroyObject of + * persistent (CKA_TOKEN=true) ECC P-256 objects. + * + * The target emits one marker line per completed operation plus the + * store's flash commit/erase/program counts; the host timestamps the + * serial lines, so wall time is measured outside the DUT (the secure + * world owns its own timers and must not be touched from here). + * The store runs in the secure world; every C_* call below crosses + * the NSC boundary, so the measured times include the transition + * overhead. + */ +#define PKCS11_BENCH_ROUNDS 3 + +static int bench_get_stats(uint32_t *commits, uint32_t *erases, + uint32_t *programs) +{ + return (int)C_StoreGetStats_nsc_call(commits, erases, programs); +} + +static void bench_log_op(const char *label, int round, + uint32_t c0, uint32_t e0, uint32_t p0, + uint32_t c1, uint32_t e1, uint32_t p1) +{ + printf("bench r%d %s commits=%lu erases=%lu programs=%lu\r\n", + round, label, + (unsigned long)(c1 - c0), + (unsigned long)(e1 - e0), + (unsigned long)(p1 - p0)); +} + +static int bench_create_pair(CK_SESSION_HANDLE session, int round, + CK_OBJECT_HANDLE *pub_obj, CK_OBJECT_HANDLE *priv_obj) +{ + CK_RV rv; + CK_OBJECT_CLASS pub_class = CKO_PUBLIC_KEY; + CK_OBJECT_CLASS priv_class = CKO_PRIVATE_KEY; + CK_KEY_TYPE key_type = CKK_EC; + CK_BBOOL ck_true = CK_TRUE; + CK_BYTE id[4]; + CK_BYTE label[20]; + int label_len = 0; + uint32_t c0, e0, p0, c1, e1, p1; + int ret; + CK_ATTRIBUTE pub_tmpl[] = { + { CKA_CLASS, &pub_class, sizeof(pub_class) }, + { CKA_KEY_TYPE, &key_type, sizeof(key_type) }, + { CKA_EC_PARAMS, (CK_VOID_PTR)test_ecc_p256_params, + sizeof(test_ecc_p256_params) }, + { CKA_VERIFY, &ck_true, sizeof(ck_true) }, + { CKA_TOKEN, &ck_true, sizeof(ck_true) }, + { CKA_ID, (CK_VOID_PTR)id, sizeof(id) }, + { CKA_LABEL, (CK_VOID_PTR)label, (CK_ULONG)label_len }, + { CKA_EC_POINT, (CK_VOID_PTR)test_ecc_p256_pub, + sizeof(test_ecc_p256_pub) } + }; + CK_ATTRIBUTE priv_tmpl[] = { + { CKA_CLASS, &priv_class, sizeof(priv_class) }, + { CKA_KEY_TYPE, &key_type, sizeof(key_type) }, + { CKA_EC_PARAMS, (CK_VOID_PTR)test_ecc_p256_params, + sizeof(test_ecc_p256_params) }, + { CKA_SIGN, &ck_true, sizeof(ck_true) }, + { CKA_TOKEN, &ck_true, sizeof(ck_true) }, + { CKA_PRIVATE, &ck_true, sizeof(ck_true) }, + { CKA_ID, (CK_VOID_PTR)id, sizeof(id) }, + { CKA_LABEL, (CK_VOID_PTR)label, (CK_ULONG)label_len }, + { CKA_VALUE, (CK_VOID_PTR)test_ecc_p256_priv, + sizeof(test_ecc_p256_priv) } + }; + + *pub_obj = CK_INVALID_HANDLE; + *priv_obj = CK_INVALID_HANDLE; + + id[0] = 0xB0; + id[1] = 0; + id[2] = 0; + id[3] = (CK_BYTE)(round + 1); + label_len = (int)snprintf((char *)label, sizeof(label), + "bench priv r%d", round); + priv_tmpl[7].ulValueLen = (CK_ULONG)label_len; + + ret = bench_get_stats(&c0, &e0, &p0); + if (ret != 0) + return -1; + + rv = wolfpkcs11nsFunctionList.C_CreateObject(session, priv_tmpl, + (CK_ULONG)(sizeof(priv_tmpl) / sizeof(priv_tmpl[0])), priv_obj); + ret = bench_get_stats(&c1, &e1, &p1); + if (ret != 0) + return -1; + bench_log_op("create_priv", round, c0, e0, p0, c1, e1, p1); + if (rv != CKR_OK) { + test_pkcs11_dump_rv("C_CreateObject(bench priv)", rv); + return -1; + } + + label_len = (int)snprintf((char *)label, sizeof(label), + "bench pub r%d", round); + pub_tmpl[6].ulValueLen = (CK_ULONG)label_len; + + ret = bench_get_stats(&c0, &e0, &p0); + if (ret != 0) + return -1; + + rv = wolfpkcs11nsFunctionList.C_CreateObject(session, pub_tmpl, + (CK_ULONG)(sizeof(pub_tmpl) / sizeof(pub_tmpl[0])), pub_obj); + ret = bench_get_stats(&c1, &e1, &p1); + if (ret != 0) { + (void)wolfpkcs11nsFunctionList.C_DestroyObject(session, + *priv_obj); + return -1; + } + bench_log_op("create_pub", round, c0, e0, p0, c1, e1, p1); + if (rv != CKR_OK) { + test_pkcs11_dump_rv("C_CreateObject(bench pub)", rv); + (void)wolfpkcs11nsFunctionList.C_DestroyObject(session, + *priv_obj); + *priv_obj = CK_INVALID_HANDLE; + return -1; + } + + return 0; +} + +static int bench_destroy_pair(CK_SESSION_HANDLE session, int round, + CK_OBJECT_HANDLE pub_obj, CK_OBJECT_HANDLE priv_obj) +{ + CK_RV rv; + uint32_t c0, e0, p0, c1, e1, p1; + int ret; + + ret = bench_get_stats(&c0, &e0, &p0); + if (ret != 0) + return -1; + + rv = wolfpkcs11nsFunctionList.C_DestroyObject(session, priv_obj); + ret = bench_get_stats(&c1, &e1, &p1); + if (ret != 0) + return -1; + bench_log_op("destroy_priv", round, c0, e0, p0, c1, e1, p1); + if (rv != CKR_OK) { + test_pkcs11_dump_rv("C_DestroyObject(bench priv)", rv); + return -1; + } + + ret = bench_get_stats(&c0, &e0, &p0); + if (ret != 0) + return -1; + + rv = wolfpkcs11nsFunctionList.C_DestroyObject(session, pub_obj); + ret = bench_get_stats(&c1, &e1, &p1); + if (ret != 0) + return -1; + bench_log_op("destroy_pub", round, c0, e0, p0, c1, e1, p1); + if (rv != CKR_OK) { + test_pkcs11_dump_rv("C_DestroyObject(bench pub)", rv); + return -1; + } + + return 0; +} + +static int test_pkcs11_bench(CK_SESSION_HANDLE session) +{ + int round; + int ret; + + printf("bench: start rounds=%d\r\n", PKCS11_BENCH_ROUNDS); + + (void)C_StoreResetStats_nsc_call(); + + for (round = 0; round < PKCS11_BENCH_ROUNDS; round++) { + CK_OBJECT_HANDLE pub_obj = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE priv_obj = CK_INVALID_HANDLE; + + ret = bench_create_pair(session, round, &pub_obj, &priv_obj); + if (ret < 0) + return -1; + ret = bench_destroy_pair(session, round, pub_obj, priv_obj); + if (ret < 0) + return -1; + } + + printf("bench: done\r\n"); + return 0; +} + +#endif /* PKCS11_STORE_STATS */ + int test_pkcs11_start(void) { int wc_ret; @@ -556,6 +747,12 @@ int test_pkcs11_start(void) } session_logged_in = 1; +#ifdef PKCS11_STORE_STATS + if (test_pkcs11_bench(session) < 0) { + printf("bench: failure (continuing)\r\n"); + } +#endif + key_state = test_pkcs11_find_keypair(session, &pub_obj, &priv_obj); if (key_state < 0) { ret = -1; diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index bf69221e79..a599617f24 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -315,10 +315,17 @@ START_TEST(test_cross_sector_write_preserves_length) handle = store; ck_assert_uint_eq(handle->in_buffer_offset, 2 * sizeof(uint32_t) + WOLFBOOT_SECTOR_SIZE); - ck_assert_uint_eq(handle->hdr->size, + /* The size is tracked live in the handle; the flash node is updated + * when the window is closed. */ + ck_assert_uint_eq(handle->size, 2 * sizeof(uint32_t) + WOLFBOOT_SECTOR_SIZE); wolfPKCS11_Store_Close(store); + /* After the close the committed node must carry the same size */ + ck_assert_uint_eq( + ((struct obj_hdr *)(vault_base + STORE_PRIV_HDR_OFFSET))->size, + 2 * sizeof(uint32_t) + WOLFBOOT_SECTOR_SIZE); + free(payload); } END_TEST From b5c9c366f5c938374affa49ffebe99e464fea73c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 25 Aug 2026 17:30:04 +0200 Subject: [PATCH 02/12] pkcs11 store: commit pending sectors in check_vault, read via cache check_vault() dropped the shared sector cache on every vault validation, silently losing the pending writes of any still-open window when another handle was opened or an object removed (MAX_OPEN_STORES allows 16). Flush instead - the atomic header-last commit - so an in-flight batch only gets an earlier commit point; its data is never discarded. wolfPKCS11_Store_Read() now reads through sector_ptr() like every other read in the file, so a sector still in the cache can never be read stale against a live size. Add unit tests covering the interleaved-window data loss and a concurrent reader observing a pending write; both fail without the check_vault fix. --- src/pkcs11_store.c | 43 +++++++++---- tools/unit-tests/unit-pkcs11_store.c | 90 ++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 13 deletions(-) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index bc0b0644c5..44fccadbbb 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -336,15 +336,6 @@ static void cache_flush_all(void) } } -static void cache_reset(void) -{ - int i; - - for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { - store_cache[i].sector = NULL; - } -} - /* * Read access to a vault sector: the RAM copy when the sector is * cached, flash otherwise. Writes must go through cache_get_sector(). @@ -385,7 +376,12 @@ static void check_vault(void) uint8_t *s0 = NULL; uint32_t total_vault_size = KEYVAULT_MAX_ITEMS * KEYVAULT_OBJ_SIZE; - cache_reset(); + /* The cache is shared across all open windows: commit any pending + * sectors before (re)validating instead of dropping them, or a + * still-open window would silently lose its writes. The flush is + * atomic (header last), so this only moves that window's commit + * point earlier, it never mixes batches. */ + cache_flush_all(); if ((total_vault_size % WOLFBOOT_SECTOR_SIZE) != 0) total_vault_size = (total_vault_size / WOLFBOOT_SECTOR_SIZE) * WOLFBOOT_SECTOR_SIZE + WOLFBOOT_SECTOR_SIZE; @@ -418,8 +414,8 @@ static void delete_object(int32_t type, uint32_t tok_id, uint32_t obj_id) uint8_t *s0; /* Deletions are durable on return, like the historical per-write - * commits: validate the vault (resets the cache) and commit the - * whole batch before returning. */ + * commits: validate the vault (commits pending sectors) and commit + * the whole batch before returning. */ check_vault(); s0 = cache_get_sector(0); hdr = (struct obj_hdr *)(s0 + STORE_PRIV_HDR_OFFSET); @@ -707,6 +703,8 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) { struct store_handle *handle = store; uint32_t obj_size = 0; + uint32_t src_off; + uint32_t remaining; if ((handle == NULL) || (handle->hdr == NULL) || (handle->buffer == NULL)) return -1; @@ -725,7 +723,26 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) len = (obj_size - handle->in_buffer_offset); if (len > 0) { - memcpy(buffer, (uint8_t *)(handle->buffer) + handle->in_buffer_offset, len); + /* Read through sector_ptr() like every other read in this file: + * the RAM copy when the sector is cached, flash otherwise, so a + * cached (not yet committed) sector can never be read stale. */ + src_off = (uint32_t)((uintptr_t)handle->buffer + + handle->in_buffer_offset - (uintptr_t)vault_base); + remaining = (uint32_t)len; + while (remaining > 0) { + uint32_t in_sector = src_off % WOLFBOOT_SECTOR_SIZE; + uint32_t chunk = WOLFBOOT_SECTOR_SIZE - in_sector; + uint8_t *s; + + if (chunk > remaining) { + chunk = remaining; + } + s = sector_ptr(src_off - in_sector); + memcpy(buffer, s + in_sector, chunk); + buffer += chunk; + src_off += chunk; + remaining -= chunk; + } handle->in_buffer_offset += len; } return len; diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index a599617f24..df3647cbe9 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -640,6 +640,90 @@ START_TEST(test_remove_erases_payload_from_flash) } END_TEST +/* A second write window opened while the first is still open must not + * discard the first window's pending writes: both objects survive. */ +START_TEST(test_interleaved_write_windows_both_persist) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok = 60; + void *store_a = NULL; + void *store_b = NULL; + void *store = NULL; + char first[] = "first window payload"; + char second[] = "second window payload"; + char rd[64]; + int ret; + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Window A: open + write, left open (dirty sector cache). */ + ret = wolfPKCS11_Store_Open(type, id_tok, 1, 0, &store_a); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store_a, first, (int)strlen(first) + 1); + ck_assert_int_eq(ret, (int)strlen(first) + 1); + + /* Window B while A is still open: the open re-validates the vault + * and must commit A's batch, not drop it. */ + ret = wolfPKCS11_Store_Open(type, id_tok, 2, 0, &store_b); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store_b, second, (int)strlen(second) + 1); + ck_assert_int_eq(ret, (int)strlen(second) + 1); + wolfPKCS11_Store_Close(store_b); + wolfPKCS11_Store_Close(store_a); + + ret = wolfPKCS11_Store_Open(type, id_tok, 1, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Read(store, rd, (int)sizeof(rd)); + ck_assert_int_eq(ret, (int)strlen(first) + 1); + ck_assert(strcmp(first, rd) == 0); + wolfPKCS11_Store_Close(store); + + ret = wolfPKCS11_Store_Open(type, id_tok, 2, 1, &store); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Read(store, rd, (int)sizeof(rd)); + ck_assert_int_eq(ret, (int)strlen(second) + 1); + ck_assert(strcmp(second, rd) == 0); + wolfPKCS11_Store_Close(store); +} +END_TEST + +/* A reader opened on the same object while a write window holds it must + * see that window's writes (committed by the reader's own vault + * validation), not a NOT_AVAILABLE error or erased flash. */ +START_TEST(test_concurrent_reader_sees_pending_writes) +{ + const int type = DYNAMIC_TYPE_RSA; + const CK_ULONG id_tok = 70; + void *store_w = NULL; + void *store_r = NULL; + char secret[] = "pending write"; + char rd[64]; + int ret; + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert_int_eq(ret, 0); + memset(vault_base, 0xEE, keyvault_size); + + ret = wolfPKCS11_Store_Open(type, id_tok, 1, 0, &store_w); + ck_assert_int_eq(ret, 0); + ret = wolfPKCS11_Store_Write(store_w, secret, (int)strlen(secret) + 1); + ck_assert_int_eq(ret, (int)strlen(secret) + 1); + + /* The write is still pending in the write window. A concurrent + * reader on the same object must observe it, not erased flash. */ + ret = wolfPKCS11_Store_Open(type, id_tok, 1, 1, &store_r); + ck_assert_int_eq(ret, 0); + memset(rd, 0, sizeof(rd)); + ret = wolfPKCS11_Store_Read(store_r, rd, (int)sizeof(rd)); + ck_assert_int_eq(ret, (int)strlen(secret) + 1); + ck_assert(strcmp(secret, rd) == 0); + wolfPKCS11_Store_Close(store_r); + wolfPKCS11_Store_Close(store_w); +} +END_TEST + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -654,6 +738,8 @@ Suite *wolfboot_suite(void) TCase* tcase_remanence = tcase_create("shorter_overwrite_erases_residual"); TCase* tcase_neg_len = tcase_create("rejects_negative_len"); TCase* tcase_remove_erase = tcase_create("remove_erases_payload"); + TCase* tcase_interleaved = tcase_create("interleaved_windows"); + TCase* tcase_concurrent_read = tcase_create("concurrent_reader"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -663,6 +749,8 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_remanence, test_shorter_overwrite_erases_residual_key_material); tcase_add_test(tcase_neg_len, test_store_rejects_negative_len); tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); + tcase_add_test(tcase_interleaved, test_interleaved_write_windows_both_persist); + tcase_add_test(tcase_concurrent_read, test_concurrent_reader_sees_pending_writes); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); @@ -672,6 +760,8 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_remanence); suite_add_tcase(s, tcase_neg_len); suite_add_tcase(s, tcase_remove_erase); + suite_add_tcase(s, tcase_interleaved); + suite_add_tcase(s, tcase_concurrent_read); return s; } From 241004367b3df9fc465d5af24bd1ab97f0a42c4f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 31 Aug 2026 15:42:59 +0200 Subject: [PATCH 03/12] pkcs11 store: read object size live from the cached header Store_Read and Store_Write used handle->size, a snapshot taken at Store_Open. The payload path reads through the sector cache, so once another window's batch (e.g. a write-open truncation) sat pending in the cache, the window saw live erased data under a stale size and returned 0xFF bytes past the true end instead of EOF. Pre-PR the size was read live from the flash header on every call, so the PR regressed that case. Read the size from the same (possibly cached) header sector the payload comes from, via store_live_size(), so size and data share one source of truth. Drop the now-dead handle->size snapshot; update_store_size() only writes the cached header node. Addresses PR #873 review comment (wolfSSL-Fenrir-bot, src/pkcs11_store.c:711). --- src/pkcs11_store.c | 32 +++++++++++++++++----------- tools/unit-tests/unit-pkcs11_store.c | 7 +++--- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 44fccadbbb..676ac1bca4 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -109,7 +109,6 @@ struct obj_hdr struct store_handle { uint32_t flags; uint32_t pos; - uint32_t size; /* live object size; the flash node is updated at commit */ void *buffer; struct obj_hdr *hdr; uint32_t in_buffer_offset; @@ -550,8 +549,7 @@ static struct obj_hdr *create_object(int32_t type, uint32_t tok_id, uint32_t obj return NULL; /* No space left in the nodes table */ } -static void update_store_size(struct store_handle *handle, - struct obj_hdr *hdr, uint32_t size) +static void update_store_size(struct obj_hdr *hdr, uint32_t size) { uint32_t off; uint8_t *s0; @@ -565,7 +563,6 @@ static void update_store_size(struct store_handle *handle, s0 = cache_get_sector(0); hdr_mem = (struct obj_hdr *)(s0 + off); hdr_mem->size = size; - handle->size = size; } static void erase_object_payload(uint8_t *buf) @@ -620,7 +617,6 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, { struct store_handle *handle; uint8_t *buf; - uint32_t hdr_off; int is_new = 0; /* Check if there is one handle available to open the slot */ @@ -668,13 +664,10 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, /* Set the 'readonly' flag in this handle if open with 'r' */ if (read) { handle->flags |= STORE_FLAGS_READONLY; - /* Live size from the (possibly cached) header sector */ - hdr_off = (uintptr_t)handle->hdr - (uintptr_t)vault_base; - handle->size = ((struct obj_hdr *)(sector0_ptr() + hdr_off))->size; } else { handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ - update_store_size(handle, handle->hdr, 2 * sizeof(uint32_t)); + update_store_size(handle->hdr, 2 * sizeof(uint32_t)); /* Erase object data sectors to clear residual key material from a * prior (longer) payload. New objects are already in a fresh sector * from create_object(), so only do this for existing objects. */ @@ -699,6 +692,21 @@ void wolfPKCS11_Store_Close(void* store) memset(handle, 0, sizeof(*handle)); } +/* Live object size from the (possibly cached) header sector: the same + * source of truth as the payload path, so a window's size and data + * cannot diverge while another window's batch is pending in the cache. */ +static uint32_t store_live_size(struct store_handle *handle) +{ + uint32_t off; + + if (((uint8_t *)handle->hdr) < vault_base || + ((uint8_t *)handle->hdr > vault_base + WOLFBOOT_SECTOR_SIZE)) { + return 0; + } + off = (uint32_t)((uintptr_t)handle->hdr - (uintptr_t)vault_base); + return ((struct obj_hdr *)(sector0_ptr() + off))->size; +} + int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) { struct store_handle *handle = store; @@ -711,7 +719,7 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) if (len < 0) return -1; - obj_size = handle->size; + obj_size = store_live_size(handle); if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -767,7 +775,7 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) if (len < 0) return -1; - obj_size = handle->size; + obj_size = store_live_size(handle); if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -796,7 +804,7 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) written += in_sector_len; } obj_size += written; - update_store_size(handle, handle->hdr, obj_size); + update_store_size(handle->hdr, obj_size); return len; } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index df3647cbe9..99bf68a107 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -315,9 +315,10 @@ START_TEST(test_cross_sector_write_preserves_length) handle = store; ck_assert_uint_eq(handle->in_buffer_offset, 2 * sizeof(uint32_t) + WOLFBOOT_SECTOR_SIZE); - /* The size is tracked live in the handle; the flash node is updated - * when the window is closed. */ - ck_assert_uint_eq(handle->size, + /* The size is tracked live in the (cached) header node; the flash + * node is committed when the window is closed. */ + ck_assert_uint_eq( + ((struct obj_hdr *)(sector0_ptr() + STORE_PRIV_HDR_OFFSET))->size, 2 * sizeof(uint32_t) + WOLFBOOT_SECTOR_SIZE); wolfPKCS11_Store_Close(store); From d6a5570f69e173911e469017e746283ca04e27d1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 31 Aug 2026 15:45:24 +0200 Subject: [PATCH 04/12] pkcs11 store: exercise the cached read path in the concurrent test test_concurrent_reader_sees_pending_writes only ever read from flash: the reader's Store_Open calls check_vault(), which flushes the sector cache, so the sector_ptr() read path in Store_Read was never exercised and the test passed identically against the pre-PR memcpy. Write more on the still-open writer after the reader is open. That batch lands only in the sector cache, so the reader can only see it through the cached read path and the live header size; a flash-only or snapshot-size read returns EOF here. Verified: the new assertion fails against the pre-fix store (ret == 0) and passes with the live-size fix. Addresses PR #873 review comments (wolfSSL-Fenrir-bot, tools/unit-tests/unit-pkcs11_store.c:587, both near-duplicate findings). --- tools/unit-tests/unit-pkcs11_store.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 99bf68a107..bb0a7f72ea 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -691,8 +691,11 @@ START_TEST(test_interleaved_write_windows_both_persist) END_TEST /* A reader opened on the same object while a write window holds it must - * see that window's writes (committed by the reader's own vault - * validation), not a NOT_AVAILABLE error or erased flash. */ + * see that window's writes, not a NOT_AVAILABLE error or erased flash. + * The first write is committed by the reader's own vault validation; the + * second is issued after the reader is open, so it sits only in the sector + * cache and must be read back through the cache (and the live header + * size), not from the pre-write flash. */ START_TEST(test_concurrent_reader_sees_pending_writes) { const int type = DYNAMIC_TYPE_RSA; @@ -700,6 +703,7 @@ START_TEST(test_concurrent_reader_sees_pending_writes) void *store_w = NULL; void *store_r = NULL; char secret[] = "pending write"; + char more[] = " more"; char rd[64]; int ret; @@ -720,6 +724,17 @@ START_TEST(test_concurrent_reader_sees_pending_writes) ret = wolfPKCS11_Store_Read(store_r, rd, (int)sizeof(rd)); ck_assert_int_eq(ret, (int)strlen(secret) + 1); ck_assert(strcmp(secret, rd) == 0); + + /* Write more on the still-open writer: this lands only in the sector + * cache (the reader's open already flushed the earlier batch to + * flash). The reader must see it through the cache and the live + * header size; a flash-only or snapshot-size read returns EOF here. */ + ret = wolfPKCS11_Store_Write(store_w, more, (int)strlen(more)); + ck_assert_int_eq(ret, (int)strlen(more)); + memset(rd, 0, sizeof(rd)); + ret = wolfPKCS11_Store_Read(store_r, rd, (int)sizeof(rd)); + ck_assert_int_eq(ret, (int)strlen(more)); + ck_assert(memcmp(more, rd, strlen(more)) == 0); wolfPKCS11_Store_Close(store_r); wolfPKCS11_Store_Close(store_w); } From b9ef442faf955df98106b057e08b31ad428a373d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 31 Aug 2026 19:45:43 +0200 Subject: [PATCH 05/12] pkcs11 store: harden sector cache (eviction, release scrub, open durability) Three fixes from the 2026-08-31 Fenrir PR review round: 1. cache_get_sector() LRU eviction could pick the header sector (offset 0) as the victim, committing it to flash while the batch's payload sectors were still only in RAM - a mixed pre/post-batch state that breaks the header-last atomic commit point cache_flush_all() relies on. The header is now exempt from victim selection; if it is the only cached sector, flush the whole batch instead (header-last is then trivial). 2. cache_flush_all() and LRU eviction released a slot by clearing .sector without wiping the buffer, so private-key bytes staged by Store_Write lingered in secure-world SRAM until the slot was next reused. The single staging buffer this cache replaced self-cleaned (the header sector overwrote it on every size update); per-sector slots do not. A new cache_release() wc_ForceZero()s the buffer before freeing the slot. 3. Store_Open in write mode set the size to 8 (truncation) in the cache only; with batched commits the payload sectors are flushed before the header, so a power loss during the erase/rewrite left the old size over a partly erased payload. The truncated header is now committed to flash before erase_object_payload(), so the empty state is the crash fallback. Addresses PR #873 review comments (wolfSSL-Fenrir-bot, src/pkcs11_store.c:301, :333, :670, 2026-08-31). Verification: tools/unit-tests unit-pkcs11_store 9/9 pass (incl. test_concurrent_reader_sees_pending_writes, test_shorter_overwrite_erases_residual_key_material, test_interleaved_write_windows_both_persist). --- src/pkcs11_store.c | 68 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index 676ac1bca4..d79fe614f6 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -33,6 +33,7 @@ #include #include +#include /* wc_ForceZero */ #ifndef KEYVAULT_OBJ_SIZE #define KEYVAULT_OBJ_SIZE 0x1000 /* 4KB per object */ @@ -267,6 +268,39 @@ static void cache_commit_entry(struct cache_entry *entry) #endif } +/* + * Release a cache slot: zeroize the sector buffer before marking the slot + * free. The buffer may hold private-key bytes (staged by Store_Write), and a + * released slot is not overwritten until the next object is cached, so the + * material would otherwise linger in secure-world SRAM. The single staging + * buffer this cache replaced was self-cleaning (overwritten by the header + * sector on every size update); per-sector slots are not, so the scrub moves + * here, mirroring the wc_ForceZero() in wolfhsm_flash_hal.c. + */ +static void cache_release(int i) +{ + if (store_cache[i].sector != NULL) { + wc_ForceZero(store_cache[i].sector, WOLFBOOT_SECTOR_SIZE); + store_cache[i].sector = NULL; + } +} + +/* + * Commit one cached sector to flash and release its slot. Used to make the + * Open-time truncation (size = 8) durable before the payload is erased, so + * the empty state survives a power loss independent of the header-last batch + * flush at Store_Close. + */ +static void cache_commit_offset(uint32_t offset) +{ + struct cache_entry *entry = cache_find(offset); + + if (entry != NULL) { + cache_commit_entry(entry); + cache_release((int)(entry - store_cache)); + } +} + /* * Get a RAM copy of the vault sector at the given offset. Modifications * stay in RAM until cache_flush_all() (or LRU eviction) commits them. @@ -290,17 +324,30 @@ static uint8_t *cache_get_sector(uint32_t offset) } } if (free_slot < 0) { - /* No free slot: commit the least recently used entry */ - int oldest = 0; + /* No free slot: commit the least recently used entry. The header + * sector (offset 0) is never a victim: committing it before the + * payload would break the header-last atomic commit point that + * cache_flush_all() relies on. If it is the only cached sector, + * flush the whole batch instead (header-last is then trivial). */ + int oldest = -1; - for (i = 1; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { - if (store_cache[i].lru < store_cache[oldest].lru) { + for (i = 0; i < WOLFBOOT_PKCS11_STORE_CACHE_SECTORS; i++) { + if (store_cache[i].offset == 0) { + continue; + } + if ((oldest < 0) || + (store_cache[i].lru < store_cache[oldest].lru)) { oldest = i; } } - cache_commit_entry(&store_cache[oldest]); - store_cache[oldest].sector = NULL; - free_slot = oldest; + if (oldest < 0) { + cache_flush_all(); + free_slot = 0; + } else { + cache_commit_entry(&store_cache[oldest]); + cache_release(oldest); + free_slot = oldest; + } } entry = &store_cache[free_slot]; @@ -330,7 +377,7 @@ static void cache_flush_all(void) continue; } cache_commit_entry(&store_cache[i]); - store_cache[i].sector = NULL; + cache_release(i); } } } @@ -668,6 +715,11 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ update_store_size(handle->hdr, 2 * sizeof(uint32_t)); + /* Make the truncation (size = 8) durable before erasing the + * payload: the empty state is the crash fallback, so a power loss + * during the erase/rewrite must leave the object reading back + * empty, never the old size over a partly erased payload. */ + cache_commit_offset(0); /* Erase object data sectors to clear residual key material from a * prior (longer) payload. New objects are already in a fresh sector * from create_object(), so only do this for existing objects. */ From 1d3b576f03195c8c9078bc23644156261119226a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 15:47:22 +0200 Subject: [PATCH 06/12] pkcs11 store: pin the Open-time durability rule with a power-fail test Adds power-fail injection to the flash mock and a test that cuts power at every flash operation of a rewrite window, asserting the object always reads back as the whole old payload, the whole new payload, or empty. Fails at op 3 without this fix, passes with it. unit-pkcs11_store 10/10; full unit-tests suite green. --- src/pkcs11_store.c | 24 ++++-- tools/unit-tests/unit-mock-flash.c | 27 +++++++ tools/unit-tests/unit-pkcs11_store.c | 117 +++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index d79fe614f6..d84d16b085 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -715,15 +715,23 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, handle->flags &= ~STORE_FLAGS_READONLY; /* Truncate the slot when opening in write mode */ update_store_size(handle->hdr, 2 * sizeof(uint32_t)); - /* Make the truncation (size = 8) durable before erasing the - * payload: the empty state is the crash fallback, so a power loss - * during the erase/rewrite must leave the object reading back - * empty, never the old size over a partly erased payload. */ - cache_commit_offset(0); - /* Erase object data sectors to clear residual key material from a - * prior (longer) payload. New objects are already in a fresh sector - * from create_object(), so only do this for existing objects. */ if (!is_new) { + /* Existing object: its committed payload is about to be + * destroyed, so make the truncation (size = 8) durable first. + * The empty state is the crash fallback, and a power loss + * during the erase/rewrite must leave the object reading back + * empty, never the old size over a partly erased payload. + * + * A new object needs no such commit: nothing of it is in flash + * yet, so its crash fallback is already "object absent", and + * the node claimed by create_object() is only published by the + * header-last flush at Store_Close. Committing the header here + * would cost a sector erase + program (twice, with the backup + * sector) on every create for no added guarantee. */ + cache_commit_offset(0); + /* Erase object data sectors to clear residual key material from + * a prior (longer) payload. New objects are already in a fresh + * sector from create_object(). */ erase_object_payload(buf); } } diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index c12aa3472b..c8bc786d0e 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -37,6 +37,31 @@ static int erased_vault = 0; static int hal_flash_write_fail = 0; const char *argv0; +#ifdef MOCK_KEYVAULT +/* Power-fail injection for the keyvault (pkcs11 store) tests. + * + * When vault_powerfail_at is >= 0, the vault flash operation with that + * 0-based index, and every operation after it, is abandoned: the mock + * longjmp()s back to the arming point instead of touching the backing + * store. That models a power loss part-way through a sector commit, which + * is the only way to observe the store's crash-consistency ordering. + * + * Disabled (-1) by default, so tests that do not arm it are unaffected. + */ +#include +static int vault_powerfail_at = -1; +static int vault_flash_ops; +static jmp_buf vault_powerfail_jmp; + +static void vault_flash_op(void) +{ + vault_flash_ops++; + if ((vault_powerfail_at >= 0) && (vault_flash_ops > vault_powerfail_at)) { + longjmp(vault_powerfail_jmp, 1); + } +} +#endif + #include @@ -73,6 +98,7 @@ int hal_flash_write(haladdr_t address, const uint8_t *data, int len) } #ifdef MOCK_KEYVAULT if ((address >= (const uintptr_t)vault_base) && (address < (const uintptr_t)vault_base + keyvault_size)) { + vault_flash_op(); for (i = 0; i < len; i++) { a[i] = data[i]; } @@ -116,6 +142,7 @@ int hal_flash_erase(haladdr_t address, int len) memset((void *)(uintptr_t)address, 0xFF, len); #ifdef MOCK_KEYVAULT } else if ((address >= (uintptr_t)vault_base) && (address < (uintptr_t)vault_base + keyvault_size)) { + vault_flash_op(); printf("Erasing vault from %p : %p bytes\n", address, len); erased_vault++; memset((void *)(uintptr_t)address, 0xFF, len); diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index bb0a7f72ea..cf07f69188 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -740,6 +740,119 @@ START_TEST(test_concurrent_reader_sees_pending_writes) } END_TEST +/* A power cycle loses every byte of RAM state the store keeps: the sector + * cache and the open-handle table. Flash content survives. */ +static void vault_power_cycle(void) +{ + memset(store_cache, 0, sizeof(store_cache)); + memset(cache_sector_mem, 0, sizeof(cache_sector_mem)); + memset(openstores_handles, 0, sizeof(openstores_handles)); + cache_lru_tick = 0; + locked = 1; +} + +static int vault_obj_write(int type, CK_ULONG tok, CK_ULONG obj, + const uint8_t *payload, int len) +{ + void *store = NULL; + int ret = wolfPKCS11_Store_Open(type, tok, obj, 0, &store); + + if (ret != 0) + return ret; + ret = wolfPKCS11_Store_Write(store, (unsigned char *)payload, len); + wolfPKCS11_Store_Close(store); + return ret; +} + +static int vault_obj_read(int type, CK_ULONG tok, CK_ULONG obj, + uint8_t *out, int max) +{ + void *store = NULL; + int ret = wolfPKCS11_Store_Open(type, tok, obj, 1, &store); + + if (ret != 0) + return -1; + ret = wolfPKCS11_Store_Read(store, out, max); + wolfPKCS11_Store_Close(store); + return ret; +} + +/* Rewriting an existing object destroys its committed payload. Whatever the + * moment power is lost inside the Open/Write/Close window, the next boot must + * read the object back as the complete old payload, the complete new payload, + * or empty - never a mix of old, new and erased bytes. + * + * That is what the Open-time commit of the truncated header (size = 8) buys: + * without it the previous generation's size stays committed over a payload + * that is being erased and rewritten underneath it. This test drives a power + * failure at every single flash operation of the window to pin the property. + */ +START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { + static uint8_t old_p[2000], new_p[300], rd[KEYVAULT_OBJ_SIZE]; + static uint8_t snapshot[KEYVAULT_OBJ_SIZE * KEYVAULT_MAX_ITEMS + + 2 * WOLFBOOT_SECTOR_SIZE]; + const int type = DYNAMIC_TYPE_ECC; + const CK_ULONG tok = 7, obj = 77; + int i, ret, ops, crash; + + for (i = 0; i < (int)sizeof(old_p); i++) + old_p[i] = (uint8_t)('A' + (i % 23)); + for (i = 0; i < (int)sizeof(new_p); i++) + new_p[i] = (uint8_t)('a' + (i % 19)); + + ret = mmap_file(vault_path, vault_base, keyvault_size, NULL); + ck_assert(ret == 0); + memset(vault_base, 0xEE, keyvault_size); + + /* Lay down the previous generation, no faults. */ + vault_power_cycle(); + vault_powerfail_at = -1; + ret = vault_obj_write(type, tok, obj, old_p, (int)sizeof(old_p)); + ck_assert_int_eq(ret, (int)sizeof(old_p)); + memcpy(snapshot, vault_base, keyvault_size); + + /* Count the flash operations a clean rewrite takes. */ + vault_power_cycle(); + vault_flash_ops = 0; + vault_powerfail_at = -1; + vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); + ops = vault_flash_ops; + ck_assert_int_gt(ops, 0); + + for (crash = 0; crash <= ops; crash++) { + memcpy(vault_base, snapshot, keyvault_size); + vault_power_cycle(); + vault_flash_ops = 0; + vault_powerfail_at = crash; + if (setjmp(vault_powerfail_jmp) == 0) { + vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); + } + /* Power returns. */ + vault_powerfail_at = -1; + vault_power_cycle(); + memset(rd, 0, sizeof(rd)); + ret = vault_obj_read(type, tok, obj, rd, (int)sizeof(rd)); + + if (ret == (int)sizeof(old_p)) { + ck_assert_msg(memcmp(rd, old_p, sizeof(old_p)) == 0, + "power fail at op %d: old-sized payload is not the old " + "payload", crash); + } + else if (ret == (int)sizeof(new_p)) { + ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, + "power fail at op %d: new-sized payload is not the new " + "payload", crash); + } + else { + ck_assert_msg(ret <= 0, + "power fail at op %d: object read back %d bytes, neither " + "generation nor empty", crash, ret); + } + } +} +END_TEST + + Suite *wolfboot_suite(void) { /* Suite initialization */ @@ -756,6 +869,7 @@ Suite *wolfboot_suite(void) TCase* tcase_remove_erase = tcase_create("remove_erases_payload"); TCase* tcase_interleaved = tcase_create("interleaved_windows"); TCase* tcase_concurrent_read = tcase_create("concurrent_reader"); + TCase* tcase_power_fail = tcase_create("power_fail_rewrite"); tcase_add_test(tcase_store_and_load_objs, test_store_and_load_objs); tcase_add_test(tcase_cross_sector_write, test_cross_sector_write_preserves_length); tcase_add_test(tcase_close, test_close_clears_handle_state); @@ -767,6 +881,8 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_remove_erase, test_remove_erases_payload_from_flash); tcase_add_test(tcase_interleaved, test_interleaved_write_windows_both_persist); tcase_add_test(tcase_concurrent_read, test_concurrent_reader_sees_pending_writes); + tcase_add_test(tcase_power_fail, + test_power_fail_during_rewrite_never_mixes_generations); suite_add_tcase(s, tcase_store_and_load_objs); suite_add_tcase(s, tcase_cross_sector_write); suite_add_tcase(s, tcase_close); @@ -778,6 +894,7 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_remove_erase); suite_add_tcase(s, tcase_interleaved); suite_add_tcase(s, tcase_concurrent_read); + suite_add_tcase(s, tcase_power_fail); return s; } From 42e6dba558a27e3162b5b0bf5a4dd24075923b68 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 09:48:59 +0200 Subject: [PATCH 07/12] pkcs11 store: invalidate the flash read cache after every commit The store commits sectors with hal_flash_erase()/hal_flash_write() and then reads them back through the memory map - sector_ptr(), cache_get_sector()'s refill, and the raw magic reads in check_vault(). On a part that caches flash reads (STM32 ICACHE) those reads can return pre-erase bytes. check_vault() is the worst case: a stale magic there does not merely read wrong, it triggers restore_backup() or a full vault re-initialisation, losing the token. --- hal/stm32c5.c | 10 +++-- hal/stm32u3.c | 8 +++- hal/stm32u5.c | 8 +++- include/hal.h | 14 +++++++ src/pkcs11_store.c | 21 ++++++++-- tools/unit-tests/Makefile | 8 ++++ tools/unit-tests/unit-mock-flash.c | 60 ++++++++++++++++++++++++++++ tools/unit-tests/unit-pkcs11_store.c | 2 + 8 files changed, 121 insertions(+), 10 deletions(-) diff --git a/hal/stm32c5.c b/hal/stm32c5.c index 252d94b501..b5571c8fae 100644 --- a/hal/stm32c5.c +++ b/hal/stm32c5.c @@ -123,7 +123,6 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) if ((sr & FLASH_SR_ERR_MASK) != 0) { flash_clear_errors(); FLASH_CR &= ~FLASH_CR_PG; - hal_cache_invalidate(); return -1; } @@ -131,7 +130,6 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) i += write_len; DSB(); } - hal_cache_invalidate(); return 0; } @@ -153,6 +151,12 @@ void RAMFUNCTION hal_flash_lock(void) flash_wait_complete(); if ((FLASH_CR & FLASH_CR_LOCK) == 0) FLASH_CR |= FLASH_CR_LOCK; + /* Drop the flash read cache at the end of the batch rather than in + * hal_flash_write()/hal_flash_erase(): every write/erase sequence + * ends with a lock, so one invalidate per batch replaces one per + * operation (and per error return), and every consumer is covered, + * not just the ones that remember to ask. */ + hal_cache_invalidate(); } void RAMFUNCTION hal_flash_opt_unlock(void) @@ -231,12 +235,10 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) if ((sr & FLASH_SR_ERR_MASK) != 0) { flash_clear_errors(); FLASH_CR &= ~FLASH_CR_PER; - hal_cache_invalidate(); return -1; } } FLASH_CR &= ~FLASH_CR_PER; - hal_cache_invalidate(); return 0; } diff --git a/hal/stm32u3.c b/hal/stm32u3.c index 762eaf5b4c..7fb9e3fea9 100644 --- a/hal/stm32u3.c +++ b/hal/stm32u3.c @@ -105,7 +105,6 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) FLASH_NS_CR &= ~FLASH_CR_PG; i += 8; } - hal_cache_invalidate(); return 0; } @@ -128,6 +127,12 @@ void RAMFUNCTION hal_flash_lock(void) flash_wait_complete(); if ((FLASH_NS_CR & FLASH_CR_LOCK) == 0) FLASH_NS_CR |= FLASH_CR_LOCK; + /* Drop the flash read cache at the end of the batch rather than in + * hal_flash_write()/hal_flash_erase(): every write/erase sequence + * ends with a lock, so one invalidate per batch replaces one per + * operation (and per error return), and every consumer is covered, + * not just the ones that remember to ask. */ + hal_cache_invalidate(); } void RAMFUNCTION hal_flash_opt_unlock(void) @@ -194,7 +199,6 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) flash_wait_complete(); } FLASH_NS_CR &= ~FLASH_CR_PER; - hal_cache_invalidate(); return 0; } diff --git a/hal/stm32u5.c b/hal/stm32u5.c index ac5725d4f1..c67e85ba5b 100644 --- a/hal/stm32u5.c +++ b/hal/stm32u5.c @@ -158,6 +158,12 @@ void RAMFUNCTION hal_flash_lock(void) #endif if ((FLASH_NS_CR & FLASH_CR_LOCK) == 0) FLASH_NS_CR |= FLASH_CR_LOCK; + /* Drop the flash read cache at the end of the batch rather than in + * hal_flash_write()/hal_flash_erase(): every write/erase sequence + * ends with a lock, so one invalidate per batch replaces one per + * operation (and per error return), and every consumer is covered, + * not just the ones that remember to ask. */ + hal_cache_invalidate(); } void RAMFUNCTION hal_flash_opt_unlock(void) @@ -619,7 +625,7 @@ void hal_cache_disable(void) ICACHE_CR &= ~ICACHE_CR_CEN; } -void hal_cache_invalidate(void) +void RAMFUNCTION hal_cache_invalidate(void) { /* only try and invalidate cache if enabled */ if ((ICACHE_CR & ICACHE_CR_CEN) == 0) diff --git a/include/hal.h b/include/hal.h index d2f2a0c8a2..f042023c66 100644 --- a/include/hal.h +++ b/include/hal.h @@ -92,6 +92,20 @@ uint64_t hal_get_timer_us(void); #endif void hal_flash_unlock(void); void hal_flash_lock(void); +/* + * Drop any CPU-side cache of flash contents. + * + * On parts where flash reads are cached (e.g. the STM32 ICACHE), the CPU can + * still see pre-erase bytes after hal_flash_write()/hal_flash_erase() have + * completed. Any code that writes flash and then reads it back through the + * memory map must call this in between. + * + * src/libwolfboot.c provides a weak no-op, so targets without such a cache + * need not implement it; a HAL that has one overrides it and must also call + * it from its own hal_flash_lock(), which is where every write/erase batch + * ends. + */ +void hal_cache_invalidate(void); /* * Lock the flash region [address, address + len) against writes. * Return 0 on success, or a negative value on failure. diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index d84d16b085..5ad3dde1e7 100644 --- a/src/pkcs11_store.c +++ b/src/pkcs11_store.c @@ -245,6 +245,21 @@ static struct cache_entry *cache_find(uint32_t offset) return NULL; } +/* + * End a flash-mutating batch. Every sequence in this file is bracketed by + * hal_flash_unlock() ... store_flash_lock(), and the store reads committed + * sectors back through the memory map (sector_ptr(), cache_get_sector()'s + * refill, and the raw magic reads in check_vault()). On a part that caches + * flash reads those reads can return pre-erase bytes, so the cache is + * dropped here rather than relying on the HAL to do it internally: the + * store's correctness must not depend on which HAL it is linked against. + */ +static void store_flash_lock(void) +{ + hal_flash_lock(); + hal_cache_invalidate(); +} + static void cache_commit_entry(struct cache_entry *entry) { hal_flash_unlock(); @@ -260,7 +275,7 @@ static void cache_commit_entry(struct cache_entry *entry) hal_flash_write((uintptr_t)vault_base + entry->offset, entry->sector, WOLFBOOT_SECTOR_SIZE); - hal_flash_lock(); + store_flash_lock(); #ifdef PKCS11_STORE_STATS stats_commits++; stats_erases += 2; @@ -408,7 +423,7 @@ static void restore_backup(uint32_t offset) hal_flash_erase((uintptr_t)vault_base + offset, WOLFBOOT_SECTOR_SIZE); hal_flash_write((uintptr_t)vault_base + offset, BACKUP_SECTOR_ADDRESS, WOLFBOOT_SECTOR_SIZE); - hal_flash_lock(); + store_flash_lock(); #ifdef PKCS11_STORE_STATS stats_erases++; stats_programs++; @@ -447,7 +462,7 @@ static void check_vault(void) cache_flush_all(); hal_flash_unlock(); hal_flash_erase((uintptr_t)vault_base + WOLFBOOT_SECTOR_SIZE * 2, total_vault_size); - hal_flash_lock(); + store_flash_lock(); #ifdef PKCS11_STORE_STATS stats_erases += total_vault_size / WOLFBOOT_SECTOR_SIZE; #endif diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 8b844f46ea..daddf0ee88 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -124,6 +124,7 @@ TESTS+=unit-rp2350-flash-write TESTS+=unit-fwtpm-rsp-overrun TESTS+=unit-fwtpm-cmd-toctou TESTS+=unit-fdt-memrsv-wrap +TESTS+=unit-pkcs11_store-stalecache TESTS+=unit-aurix-erased-fill TESTS+=unit-aurix-erased-fill-invert TESTS+=unit-t2080-fman-loader @@ -257,6 +258,10 @@ unit-enc-nvm-flagshome:CFLAGS+=-DNVM_FLASH_WRITEONCE -DMOCK_PARTITIONS \ unit-enc-nvm-flagshome:WOLFCRYPT_SRC+=$(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/chacha.c unit-delta:CFLAGS+=-DNVM_FLASH_WRITEONCE -DMOCK_PARTITIONS -DDELTA_UPDATES -DDELTA_BLOCK_SIZE=512 unit-pkcs11_store:CFLAGS+=-I$(WOLFBOOT_LIB_WOLFPKCS11) -DMOCK_PARTITIONS -DMOCK_KEYVAULT -DSECURE_PKCS11 -DWOLFPKCS11_USER_SETTINGS +# Same suite against a flash-read cache that only refreshes on +# hal_cache_invalidate(): fails if the store reads back a sector it just +# committed without dropping the cache first. +unit-pkcs11_store-stalecache:CFLAGS+=-I$(WOLFBOOT_LIB_WOLFPKCS11) -DMOCK_PARTITIONS -DMOCK_KEYVAULT -DSECURE_PKCS11 -DWOLFPKCS11_USER_SETTINGS -DMOCK_STALE_CACHE unit-psa_store:CFLAGS+=-I$(WOLFBOOT_LIB_WOLFPSA) -DMOCK_PARTITIONS -DMOCK_KEYVAULT -DWOLFCRYPT_TZ_PSA unit-update-flash:CFLAGS+=-DMOCK_PARTITIONS -DWOLFBOOT_NO_SIGN -DUNIT_TEST_AUTH \ -DWOLFBOOT_HASH_SHA256 -DPRINTF_ENABLED -DEXT_FLASH -DPART_UPDATE_EXT -DPART_SWAP_EXT \ @@ -939,6 +944,9 @@ unit-update-disk-fit: ../../include/target.h unit-update-disk-fit.c unit-pkcs11_store: ../../include/target.h unit-pkcs11_store.c gcc -o $@ $(WOLFCRYPT_SRC) unit-pkcs11_store.c $(CFLAGS) $(WOLFCRYPT_CFLAGS) $(LDFLAGS) +unit-pkcs11_store-stalecache: ../../include/target.h unit-pkcs11_store.c + gcc -o $@ $(WOLFCRYPT_SRC) unit-pkcs11_store.c $(CFLAGS) $(WOLFCRYPT_CFLAGS) $(LDFLAGS) + unit-psa_store: ../../include/target.h unit-psa_store.c gcc -o $@ $(WOLFCRYPT_SRC) unit-psa_store.c $(CFLAGS) $(WOLFCRYPT_CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index c8bc786d0e..7a0074f1aa 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -53,6 +53,40 @@ static int vault_powerfail_at = -1; static int vault_flash_ops; static jmp_buf vault_powerfail_jmp; +/* Stale-cache model (MOCK_STALE_CACHE). + * + * Models a part that caches flash reads, such as the STM32 ICACHE: flash + * operations land in a shadow buffer (the real flash contents) while + * vault_base keeps whatever the CPU last saw, and only + * hal_cache_invalidate() refreshes it. Code that writes a sector and reads + * it back without invalidating therefore observes pre-erase bytes, exactly + * as it would on silicon. Off by default, so the ordinary suite is + * unaffected. + */ +#ifdef MOCK_STALE_CACHE +static uint8_t *vault_shadow; +static int vault_shadow_valid; + +static void vault_cache_prime(void) +{ + if (!vault_shadow_valid) { + if (vault_shadow == NULL) { + vault_shadow = malloc(keyvault_size); + ck_assert_ptr_nonnull(vault_shadow); + } + memcpy(vault_shadow, vault_base, keyvault_size); + vault_shadow_valid = 1; + } +} + +/* Flash side of a vault write/erase: the CPU view is left untouched. */ +static uint8_t *vault_flash_at(uintptr_t address) +{ + vault_cache_prime(); + return vault_shadow + (address - (uintptr_t)vault_base); +} +#endif + static void vault_flash_op(void) { vault_flash_ops++; @@ -99,6 +133,9 @@ int hal_flash_write(haladdr_t address, const uint8_t *data, int len) #ifdef MOCK_KEYVAULT if ((address >= (const uintptr_t)vault_base) && (address < (const uintptr_t)vault_base + keyvault_size)) { vault_flash_op(); +#ifdef MOCK_STALE_CACHE + a = vault_flash_at(address); +#endif for (i = 0; i < len; i++) { a[i] = data[i]; } @@ -145,8 +182,12 @@ int hal_flash_erase(haladdr_t address, int len) vault_flash_op(); printf("Erasing vault from %p : %p bytes\n", address, len); erased_vault++; +#ifdef MOCK_STALE_CACHE + memset(vault_flash_at(address), 0xFF, len); +#else memset((void *)(uintptr_t)address, 0xFF, len); #endif +#endif #ifdef WOLFBOOT_DIAGNOSTICS_ADDRESS } else if ((address >= (haladdr_t)WOLFBOOT_DIAGNOSTICS_ADDRESS) && (address < (haladdr_t)WOLFBOOT_DIAGNOSTICS_ADDRESS + @@ -170,6 +211,20 @@ void hal_flash_lock(void) locked++; } +#ifdef MOCK_KEYVAULT +/* src/libwolfboot.c carries the weak default, but the keyvault suites do not + * include it (suites that do already have the symbol, hence the guard). + * Under MOCK_STALE_CACHE this is what makes flash visible to the CPU again. */ +void hal_cache_invalidate(void) +{ +#ifdef MOCK_STALE_CACHE + if (vault_shadow_valid) { + memcpy(vault_base, vault_shadow, keyvault_size); + } +#endif +} +#endif /* MOCK_KEYVAULT */ + void hal_prepare_boot(void) { } @@ -313,6 +368,11 @@ static int mmap_file(const char *path, uint8_t *address, uint32_t len, if (ret_address) *ret_address = mmaped_addr; +#if defined(MOCK_KEYVAULT) && defined(MOCK_STALE_CACHE) + /* New backing store: the shadow is re-primed from it on first use. */ + vault_shadow_valid = 0; +#endif + close(fd); return 0; } diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index cf07f69188..b3b5de6c51 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -749,6 +749,8 @@ static void vault_power_cycle(void) memset(openstores_handles, 0, sizeof(openstores_handles)); cache_lru_tick = 0; locked = 1; + /* A reboot also drops any CPU-side cache of flash. */ + hal_cache_invalidate(); } static int vault_obj_write(int type, CK_ULONG tok, CK_ULONG obj, From 9b06b907f86a790f197f36530dac48f66048913d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 18:42:54 +0200 Subject: [PATCH 08/12] hal: always define hal_cache_invalidate(), implement it on F4 and G4 The weak no-op sat inside NVM_FLASH_WRITEONCE, so pkcs11_store.c failed to link on the nrf5340/nrf54l TrustZone configs. STM32F4 and STM32G4 enable the flash instruction/data caches and never reset them, so give them a real one. --- hal/stm32f4.c | 26 ++++++++++++++++++++++++++ hal/stm32g4.c | 26 ++++++++++++++++++++++++++ hal/stm32g4.h | 2 ++ src/libwolfboot.c | 12 ++++++++---- 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/hal/stm32f4.c b/hal/stm32f4.c index 780849368f..96082ffb93 100644 --- a/hal/stm32f4.c +++ b/hal/stm32f4.c @@ -196,9 +196,35 @@ void RAMFUNCTION hal_flash_unlock(void) FLASH_KEYR = FLASH_KEY2; } +/* RM0090 3.5.1: the instruction and data caches keep lines fetched before an + * erase/program, so a read-back through the flash memory map can return + * pre-erase bytes. The reset bits are only writable while the corresponding + * cache is disabled, hence the disable/reset/re-enable dance. */ +void RAMFUNCTION hal_cache_invalidate(void) +{ + uint32_t acr = FLASH_ACR; + + if (acr & FLASH_ACR_ENABLE_INST_CACHE) { + FLASH_ACR &= ~FLASH_ACR_ENABLE_INST_CACHE; + FLASH_ACR |= FLASH_ACR_RESET_INST_CACHE; + FLASH_ACR &= ~FLASH_ACR_RESET_INST_CACHE; + FLASH_ACR |= FLASH_ACR_ENABLE_INST_CACHE; + } + if (acr & FLASH_ACR_ENABLE_DATA_CACHE) { + FLASH_ACR &= ~FLASH_ACR_ENABLE_DATA_CACHE; + FLASH_ACR |= FLASH_ACR_RESET_DATA_CACHE; + FLASH_ACR &= ~FLASH_ACR_RESET_DATA_CACHE; + FLASH_ACR |= FLASH_ACR_ENABLE_DATA_CACHE; + } +} + void RAMFUNCTION hal_flash_lock(void) { FLASH_CR |= FLASH_CR_LOCK; + /* Drop the stale cache lines at the end of the write/erase batch: every + * sequence in wolfBoot ends with a lock, so one invalidate per batch + * covers every consumer that reads flash back. */ + hal_cache_invalidate(); } diff --git a/hal/stm32g4.c b/hal/stm32g4.c index 08c994b69c..137f143654 100644 --- a/hal/stm32g4.c +++ b/hal/stm32g4.c @@ -98,11 +98,37 @@ void RAMFUNCTION hal_flash_unlock(void) } } +/* RM0440 3.3.3: the instruction and data caches keep lines fetched before an + * erase/program, so a read-back through the flash memory map can return + * pre-erase bytes. The reset bits are only writable while the corresponding + * cache is disabled, hence the disable/reset/re-enable dance. */ +void RAMFUNCTION hal_cache_invalidate(void) +{ + uint32_t acr = FLASH_ACR; + + if (acr & FLASH_ACR_ICEN) { + FLASH_ACR &= ~FLASH_ACR_ICEN; + FLASH_ACR |= FLASH_ACR_ICRST; + FLASH_ACR &= ~FLASH_ACR_ICRST; + FLASH_ACR |= FLASH_ACR_ICEN; + } + if (acr & FLASH_ACR_DCEN) { + FLASH_ACR &= ~FLASH_ACR_DCEN; + FLASH_ACR |= FLASH_ACR_DCRST; + FLASH_ACR &= ~FLASH_ACR_DCRST; + FLASH_ACR |= FLASH_ACR_DCEN; + } +} + void RAMFUNCTION hal_flash_lock(void) { flash_wait_complete(); if ((FLASH_CR & FLASH_CR_LOCK) == 0) FLASH_CR |= FLASH_CR_LOCK; + /* Drop the stale cache lines at the end of the write/erase batch: every + * sequence in wolfBoot ends with a lock, so one invalidate per batch + * covers every consumer that reads flash back. */ + hal_cache_invalidate(); } diff --git a/hal/stm32g4.h b/hal/stm32g4.h index 2cec5a8c6d..2c2e46ba6c 100644 --- a/hal/stm32g4.h +++ b/hal/stm32g4.h @@ -102,6 +102,8 @@ #define FLASH_ACR_PRFTEN (1 << 8) #define FLASH_ACR_ICEN (1 << 9) #define FLASH_ACR_DCEN (1 << 10) +#define FLASH_ACR_ICRST (1 << 11) +#define FLASH_ACR_DCRST (1 << 12) #define FLASH_ACR_LATENCY_4WS (0x4) /* G4 has a single BSY at bit 16 (no BSY1/BSY2 like G0). */ diff --git a/src/libwolfboot.c b/src/libwolfboot.c index dca3f52338..5569d64550 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -244,6 +244,14 @@ static const uint32_t wolfboot_magic_trail = WOLFBOOT_MAGIC_TRAIL; #define FLAGS_UPDATE_EXT() PARTN_IS_EXT(PART_UPDATE) #endif +/* Weak no-op default: targets whose flash reads are not cached need not + * implement this. It lives outside NVM_FLASH_WRITEONCE because callers such + * as src/pkcs11_store.c are built independently of that option. */ +void WEAKFUNCTION hal_cache_invalidate(void) +{ + /* if cache flushing is required implement in hal */ +} + #ifdef NVM_FLASH_WRITEONCE /* Some internal FLASH memory models don't allow * multiple writes after erase in the same page/area. @@ -282,10 +290,6 @@ static uint8_t get_base_offset(uint8_t *base, uintptr_t off) return *(uint8_t*)((uintptr_t)base - off); /* ignore array bounds error */ } -void WEAKFUNCTION hal_cache_invalidate(void) -{ - /* if cache flushing is required implement in hal */ -} #ifdef __CCRX__ #pragma section FRAM #endif From 3d5bebeefecafba14b22bfff6a78f5b9dd5b5380 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 19:23:57 +0200 Subject: [PATCH 09/12] pkcs11 store: tighten power-fail assert, rebase F4 footprint limits vault_obj_read() now separates "object absent" from a failed read, so the power-fail test rejects a corrupted vault instead of accepting any negative. hal_cache_invalidate() shrank 120 -> 44 bytes; the 48 it still costs every STM32F407 build is folded into the test-size-all limits. --- hal/stm32f4.c | 32 +++++++++--------- hal/stm32g4.c | 31 ++++++++--------- tools/test.mk | 50 +++++++++++++++------------- tools/unit-tests/unit-pkcs11_store.c | 21 +++++++++--- 4 files changed, 76 insertions(+), 58 deletions(-) diff --git a/hal/stm32f4.c b/hal/stm32f4.c index 96082ffb93..6bd2bd1c2c 100644 --- a/hal/stm32f4.c +++ b/hal/stm32f4.c @@ -196,26 +196,28 @@ void RAMFUNCTION hal_flash_unlock(void) FLASH_KEYR = FLASH_KEY2; } +#if ((FLASH_ACR_ENABLE_INST_CACHE << 2) != FLASH_ACR_RESET_INST_CACHE) || \ + ((FLASH_ACR_ENABLE_DATA_CACHE << 2) != FLASH_ACR_RESET_DATA_CACHE) +#error "STM32F4: flash cache reset bits are not two positions above the enables" +#endif /* RM0090 3.5.1: the instruction and data caches keep lines fetched before an * erase/program, so a read-back through the flash memory map can return - * pre-erase bytes. The reset bits are only writable while the corresponding - * cache is disabled, hence the disable/reset/re-enable dance. */ + * pre-erase bytes. A cache reset bit is only writable while its cache is + * disabled, and the reset bits sit two positions above the enable bits + * (ICEN 9 -> ICRST 11, DCEN 10 -> DCRST 12), so one shift covers both. */ void RAMFUNCTION hal_cache_invalidate(void) { uint32_t acr = FLASH_ACR; - - if (acr & FLASH_ACR_ENABLE_INST_CACHE) { - FLASH_ACR &= ~FLASH_ACR_ENABLE_INST_CACHE; - FLASH_ACR |= FLASH_ACR_RESET_INST_CACHE; - FLASH_ACR &= ~FLASH_ACR_RESET_INST_CACHE; - FLASH_ACR |= FLASH_ACR_ENABLE_INST_CACHE; - } - if (acr & FLASH_ACR_ENABLE_DATA_CACHE) { - FLASH_ACR &= ~FLASH_ACR_ENABLE_DATA_CACHE; - FLASH_ACR |= FLASH_ACR_RESET_DATA_CACHE; - FLASH_ACR &= ~FLASH_ACR_RESET_DATA_CACHE; - FLASH_ACR |= FLASH_ACR_ENABLE_DATA_CACHE; - } + uint32_t en = acr & (FLASH_ACR_ENABLE_INST_CACHE | + FLASH_ACR_ENABLE_DATA_CACHE); + uint32_t off = acr & ~en; + + if (en == 0) + return; + FLASH_ACR = off; /* disable the caches that were on */ + FLASH_ACR = off | (en << 2); /* set their reset bits */ + FLASH_ACR = off; /* release reset */ + FLASH_ACR = acr; /* restore the original enables */ } void RAMFUNCTION hal_flash_lock(void) diff --git a/hal/stm32g4.c b/hal/stm32g4.c index 137f143654..a880c0ad5e 100644 --- a/hal/stm32g4.c +++ b/hal/stm32g4.c @@ -98,26 +98,27 @@ void RAMFUNCTION hal_flash_unlock(void) } } +#if ((FLASH_ACR_ICEN << 2) != FLASH_ACR_ICRST) || \ + ((FLASH_ACR_DCEN << 2) != FLASH_ACR_DCRST) +#error "STM32G4: flash cache reset bits are not two positions above the enables" +#endif /* RM0440 3.3.3: the instruction and data caches keep lines fetched before an * erase/program, so a read-back through the flash memory map can return - * pre-erase bytes. The reset bits are only writable while the corresponding - * cache is disabled, hence the disable/reset/re-enable dance. */ + * pre-erase bytes. A cache reset bit is only writable while its cache is + * disabled, and the reset bits sit two positions above the enable bits + * (ICEN 9 -> ICRST 11, DCEN 10 -> DCRST 12), so one shift covers both. */ void RAMFUNCTION hal_cache_invalidate(void) { uint32_t acr = FLASH_ACR; - - if (acr & FLASH_ACR_ICEN) { - FLASH_ACR &= ~FLASH_ACR_ICEN; - FLASH_ACR |= FLASH_ACR_ICRST; - FLASH_ACR &= ~FLASH_ACR_ICRST; - FLASH_ACR |= FLASH_ACR_ICEN; - } - if (acr & FLASH_ACR_DCEN) { - FLASH_ACR &= ~FLASH_ACR_DCEN; - FLASH_ACR |= FLASH_ACR_DCRST; - FLASH_ACR &= ~FLASH_ACR_DCRST; - FLASH_ACR |= FLASH_ACR_DCEN; - } + uint32_t en = acr & (FLASH_ACR_ICEN | FLASH_ACR_DCEN); + uint32_t off = acr & ~en; + + if (en == 0) + return; + FLASH_ACR = off; /* disable the caches that were on */ + FLASH_ACR = off | (en << 2); /* set their reset bits */ + FLASH_ACR = off; /* release reset */ + FLASH_ACR = acr; /* restore the original enables */ } void RAMFUNCTION hal_flash_lock(void) diff --git a/tools/test.mk b/tools/test.mk index cbc82c3d6c..baaa3054ef 100644 --- a/tools/test.mk +++ b/tools/test.mk @@ -1231,55 +1231,57 @@ test-size-all: # Several limits below are raised relative to upstream. This branch bumps # lib/wolfssl to the ti_c25 merge for CHAR_BIT!=8 wide-byte support, which # costs 4-48 bytes depending on configuration (most of it in the no-ASM - # SP-math and ML-DSA paths). No wolfBoot code changed in these builds, and - # each new value is the measured size, matching upstream's convention. + # SP-math and ML-DSA paths). On top of that, every entry here is an + # STM32F407 build, so each one also carries the flat 48 bytes of + # hal_cache_invalidate() and its call from hal_flash_lock(). Each value + # is the measured size, matching upstream's convention. # Measured with the CI container (ghcr.io/wolfssl/wolfboot-ci-arm). - make test-size SIGN=NONE LIMIT=5116 NO_ARM_ASM=1 + make test-size SIGN=NONE LIMIT=5164 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED25519 LIMIT=12228 NO_ARM_ASM=1 + make test-size SIGN=ED25519 LIMIT=12276 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC256 LIMIT=18924 NO_ARM_ASM=1 + make test-size SIGN=ECC256 LIMIT=18972 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13968 NO_ARM_ASM=1 + make test-size SIGN=ECC256 NO_ASM=1 LIMIT=14016 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA2048 LIMIT=11816 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 LIMIT=11864 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12372 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12412 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA4096 LIMIT=12116 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 LIMIT=12164 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12660 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12708 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC384 LIMIT=19608 NO_ARM_ASM=1 + make test-size SIGN=ECC384 LIMIT=19656 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15328 NO_ARM_ASM=1 + make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15376 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED448 LIMIT=14256 NO_ARM_ASM=1 + make test-size SIGN=ED448 LIMIT=14288 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA3072 LIMIT=11956 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 LIMIT=12004 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12480 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12528 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS2048 LIMIT=13748 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS2048 LIMIT=13796 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS2048 NO_ASM=1 LIMIT=14304 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS2048 NO_ASM=1 LIMIT=14344 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS3072 LIMIT=13916 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS3072 LIMIT=13964 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS3072 NO_ASM=1 LIMIT=14436 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS3072 NO_ASM=1 LIMIT=14484 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS4096 LIMIT=14088 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS4096 LIMIT=14136 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS4096 NO_ASM=1 LIMIT=14628 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS4096 NO_ASM=1 LIMIT=14676 NO_ARM_ASM=1 make keysclean make test-size SIGN=LMS LMS_LEVELS=2 LMS_HEIGHT=5 LMS_WINTERNITZ=8 \ WOLFBOOT_SMALL_STACK=0 IMAGE_SIGNATURE_SIZE=2644 \ - IMAGE_HEADER_SIZE?=5288 LIMIT=8120 NO_ARM_ASM=1 + IMAGE_HEADER_SIZE?=5288 LIMIT=8168 NO_ARM_ASM=1 make keysclean make test-size SIGN=XMSS XMSS_PARAMS='XMSS-SHA2_10_256' \ IMAGE_SIGNATURE_SIZE=2500 IMAGE_HEADER_SIZE?=4096 \ - LIMIT=8772 NO_ARM_ASM=1 + LIMIT=8820 NO_ARM_ASM=1 make keysclean make clean - make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19630 \ + make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19678 \ IMAGE_SIGNATURE_SIZE=2420 IMAGE_HEADER_SIZE?=8192 diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index b3b5de6c51..0daa9a36af 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -766,14 +766,22 @@ static int vault_obj_write(int type, CK_ULONG tok, CK_ULONG obj, return ret; } +/* "The object is not in the vault" is a legitimate state after a power cut, + * but any other open/read failure means the vault or its metadata is + * damaged. Report the two distinctly so the power-fail test can insist on + * the former and fail on the latter. */ +#define VAULT_OBJ_ABSENT (-1000) + static int vault_obj_read(int type, CK_ULONG tok, CK_ULONG obj, uint8_t *out, int max) { void *store = NULL; int ret = wolfPKCS11_Store_Open(type, tok, obj, 1, &store); + if (ret == NOT_AVAILABLE_E) + return VAULT_OBJ_ABSENT; if (ret != 0) - return -1; + return ret; ret = wolfPKCS11_Store_Read(store, out, max); wolfPKCS11_Store_Close(store); return ret; @@ -846,9 +854,14 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { "payload", crash); } else { - ck_assert_msg(ret <= 0, - "power fail at op %d: object read back %d bytes, neither " - "generation nor empty", crash, ret); + /* Only two other outcomes are crash-safe: the object was never + * published, or it is present but truncated to empty by the + * Open-time durability commit. Every other return (a negative + * read error, or a partial payload length) means the vault came + * back damaged. */ + ck_assert_msg(ret == VAULT_OBJ_ABSENT || ret == 0, + "power fail at op %d: object read back %d, neither old " + "payload, new payload, empty, nor absent", crash, ret); } } } From b2d8200dd21c92428ee93557155c010d1cf161b7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 20:57:53 +0200 Subject: [PATCH 10/12] pkcs11 store: restore both mock flash views before each injected power fail Under MOCK_STALE_CACHE the shadow is the flash array and vault_base only the CPU's view, so restoring vault_base alone let the power cycle copy the last iteration's result back: no crash case started from the old generation. --- tools/unit-tests/unit-mock-flash.c | 17 +++++++++++++++++ tools/unit-tests/unit-pkcs11_store.c | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index 7a0074f1aa..66ec20d37c 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -87,6 +87,23 @@ static uint8_t *vault_flash_at(uintptr_t address) } #endif +/* Restore a snapshot into every representation of the vault flash. + * + * Under MOCK_STALE_CACHE the shadow *is* the flash array and vault_base is + * only the CPU's cached view of it, so putting the snapshot back into + * vault_base alone leaves the previous contents in the shadow -- and the + * next hal_cache_invalidate() (a power cycle does one) copies them straight + * back over the snapshot. A test that restores a known state before each + * injected fault must therefore reset both. */ +static void vault_restore_snapshot(const uint8_t *snapshot) +{ + memcpy(vault_base, snapshot, keyvault_size); +#ifdef MOCK_STALE_CACHE + vault_cache_prime(); + memcpy(vault_shadow, snapshot, keyvault_size); +#endif +} + static void vault_flash_op(void) { vault_flash_ops++; diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 0daa9a36af..8b4156509a 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -830,7 +830,7 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { ck_assert_int_gt(ops, 0); for (crash = 0; crash <= ops; crash++) { - memcpy(vault_base, snapshot, keyvault_size); + vault_restore_snapshot(snapshot); vault_power_cycle(); vault_flash_ops = 0; vault_powerfail_at = crash; From 7919445c6291117ded5c7c87e1ecbdb7db4053c9 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 21:17:52 +0200 Subject: [PATCH 11/12] pkcs11 store: pin the no-fault control in the power-fail test The crash == ops iteration injects nothing, so it must read back new_p; the empty/absent branch was letting a silently lost complete rewrite pass. Also assert the uninjected baseline write lands before measuring against it. --- tools/unit-tests/unit-pkcs11_store.c | 30 +++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 8b4156509a..95d6c02ca9 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -821,13 +821,22 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { ck_assert_int_eq(ret, (int)sizeof(old_p)); memcpy(snapshot, vault_base, keyvault_size); - /* Count the flash operations a clean rewrite takes. */ + /* Count the flash operations a clean rewrite takes, and confirm the + * uninjected rewrite actually lands: every fault iteration below is + * measured against this baseline, so if the fault-free path could not + * store new_p the whole test would be vacuous. */ vault_power_cycle(); vault_flash_ops = 0; vault_powerfail_at = -1; - vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); + ret = vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); + ck_assert_int_eq(ret, (int)sizeof(new_p)); ops = vault_flash_ops; ck_assert_int_gt(ops, 0); + vault_power_cycle(); + memset(rd, 0, sizeof(rd)); + ret = vault_obj_read(type, tok, obj, rd, (int)sizeof(rd)); + ck_assert_int_eq(ret, (int)sizeof(new_p)); + ck_assert_mem_eq(rd, new_p, sizeof(new_p)); for (crash = 0; crash <= ops; crash++) { vault_restore_snapshot(snapshot); @@ -843,7 +852,22 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { memset(rd, 0, sizeof(rd)); ret = vault_obj_read(type, tok, obj, rd, (int)sizeof(rd)); - if (ret == (int)sizeof(old_p)) { + if (crash == ops) { + /* No fault can land on this iteration: vault_flash_op() only + * jumps once the op counter exceeds vault_powerfail_at, and a + * clean rewrite performs exactly ops operations. It is the + * no-fault control, so the rewrite ran to completion and the + * new payload must be there. Letting it take the empty/absent + * branch below would let a silently lost write pass. */ + ck_assert_msg(ret == (int)sizeof(new_p), + "no-fault control (op %d): object read back %d, expected " + "the new payload (%d bytes)", crash, ret, + (int)sizeof(new_p)); + ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, + "no-fault control (op %d): payload is not the new payload", + crash); + } + else if (ret == (int)sizeof(old_p)) { ck_assert_msg(memcmp(rd, old_p, sizeof(old_p)) == 0, "power fail at op %d: old-sized payload is not the old " "payload", crash); From ba544022e2d1b41ffa312541454a9b61d0b5b050 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 10 Sep 2026 21:39:18 +0200 Subject: [PATCH 12/12] pkcs11 store: cover torn flash ops and require a present, truncated header The mock abandoned a faulting erase/program whole, so half-written sectors went untested; it can now tear one part-way and the sweep runs every crash point at 1/4, 1/2 and 3/4. An empty read must also leave the node in place with size 8, so a lost or torn header no longer passes as an empty rewrite. --- tools/unit-tests/unit-mock-flash.c | 48 ++++++++-- tools/unit-tests/unit-pkcs11_store.c | 135 +++++++++++++++++---------- 2 files changed, 127 insertions(+), 56 deletions(-) diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index 66ec20d37c..c0d296189e 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -104,12 +104,40 @@ static void vault_restore_snapshot(const uint8_t *snapshot) #endif } -static void vault_flash_op(void) +/* Torn-operation mode. + * + * By default an injected fault abandons the whole flash operation, so the + * sector is either untouched or fully written -- which real silicon does + * not promise. With vault_powerfail_torn set, the faulting operation + * instead applies vault_torn_num/vault_torn_den of its bytes and only then + * loses power, leaving a half-erased or half-programmed sector behind. + */ +static int vault_powerfail_torn; +static int vault_torn_num = 1; +static int vault_torn_den = 2; + +/* How many of an operation's len bytes actually reach flash. + * + * Returns len when no fault is due on this operation. When one is: in the + * default atomic mode this longjmp()s and never returns, leaving flash + * untouched; in torn mode it returns a short count, and the caller applies + * that prefix and then calls vault_flash_torn_abort(). + */ +static int vault_flash_op_len(int len) { vault_flash_ops++; if ((vault_powerfail_at >= 0) && (vault_flash_ops > vault_powerfail_at)) { - longjmp(vault_powerfail_jmp, 1); + if (!vault_powerfail_torn) { + longjmp(vault_powerfail_jmp, 1); + } + return (int)(((long)len * vault_torn_num) / vault_torn_den); } + return len; +} + +static void vault_flash_torn_abort(void) +{ + longjmp(vault_powerfail_jmp, 1); } #endif @@ -149,13 +177,16 @@ int hal_flash_write(haladdr_t address, const uint8_t *data, int len) } #ifdef MOCK_KEYVAULT if ((address >= (const uintptr_t)vault_base) && (address < (const uintptr_t)vault_base + keyvault_size)) { - vault_flash_op(); + int n = vault_flash_op_len(len); #ifdef MOCK_STALE_CACHE a = vault_flash_at(address); #endif - for (i = 0; i < len; i++) { + for (i = 0; i < n; i++) { a[i] = data[i]; } + if (n != len) { + vault_flash_torn_abort(); + } } #endif #ifdef WOLFBOOT_DIAGNOSTICS_ADDRESS @@ -196,14 +227,17 @@ int hal_flash_erase(haladdr_t address, int len) memset((void *)(uintptr_t)address, 0xFF, len); #ifdef MOCK_KEYVAULT } else if ((address >= (uintptr_t)vault_base) && (address < (uintptr_t)vault_base + keyvault_size)) { - vault_flash_op(); + int n = vault_flash_op_len(len); printf("Erasing vault from %p : %p bytes\n", address, len); erased_vault++; #ifdef MOCK_STALE_CACHE - memset(vault_flash_at(address), 0xFF, len); + memset(vault_flash_at(address), 0xFF, n); #else - memset((void *)(uintptr_t)address, 0xFF, len); + memset((void *)(uintptr_t)address, 0xFF, n); #endif + if (n != len) { + vault_flash_torn_abort(); + } #endif #ifdef WOLFBOOT_DIAGNOSTICS_ADDRESS } else if ((address >= (haladdr_t)WOLFBOOT_DIAGNOSTICS_ADDRESS) && diff --git a/tools/unit-tests/unit-pkcs11_store.c b/tools/unit-tests/unit-pkcs11_store.c index 95d6c02ca9..186ffbf990 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -772,6 +772,19 @@ static int vault_obj_write(int type, CK_ULONG tok, CK_ULONG obj, * the former and fail on the latter. */ #define VAULT_OBJ_ABSENT (-1000) +/* Committed (on-flash) header size for an object, or -1 when the vault holds + * no node for it at all. Read after a power cycle, so the sector cache is + * empty and this is what actually survived in flash. */ +static int vault_obj_committed_size(int type, CK_ULONG tok, CK_ULONG obj) +{ + struct obj_hdr *hdr = find_object_header(type, (uint32_t)tok, + (uint32_t)obj); + + if (hdr == NULL) + return -1; + return (int)hdr->size; +} + static int vault_obj_read(int type, CK_ULONG tok, CK_ULONG obj, uint8_t *out, int max) { @@ -803,7 +816,11 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { 2 * WOLFBOOT_SECTOR_SIZE]; const int type = DYNAMIC_TYPE_ECC; const CK_ULONG tok = 7, obj = 77; - int i, ret, ops, crash; + static const char *modestr[] = { "atomic", "torn 1/4", "torn 1/2", + "torn 3/4" }; + static const int torn_num[] = { 0, 1, 1, 3 }; + static const int torn_den[] = { 1, 4, 2, 4 }; + int i, ret, ops, crash, hdr_size, mode; for (i = 0; i < (int)sizeof(old_p); i++) old_p[i] = (uint8_t)('A' + (i % 23)); @@ -838,56 +855,76 @@ START_TEST (test_power_fail_during_rewrite_never_mixes_generations) { ck_assert_int_eq(ret, (int)sizeof(new_p)); ck_assert_mem_eq(rd, new_p, sizeof(new_p)); - for (crash = 0; crash <= ops; crash++) { - vault_restore_snapshot(snapshot); - vault_power_cycle(); - vault_flash_ops = 0; - vault_powerfail_at = crash; - if (setjmp(vault_powerfail_jmp) == 0) { - vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); - } - /* Power returns. */ - vault_powerfail_at = -1; - vault_power_cycle(); - memset(rd, 0, sizeof(rd)); - ret = vault_obj_read(type, tok, obj, rd, (int)sizeof(rd)); - - if (crash == ops) { - /* No fault can land on this iteration: vault_flash_op() only - * jumps once the op counter exceeds vault_powerfail_at, and a - * clean rewrite performs exactly ops operations. It is the - * no-fault control, so the rewrite ran to completion and the - * new payload must be there. Letting it take the empty/absent - * branch below would let a silently lost write pass. */ - ck_assert_msg(ret == (int)sizeof(new_p), - "no-fault control (op %d): object read back %d, expected " - "the new payload (%d bytes)", crash, ret, - (int)sizeof(new_p)); - ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, - "no-fault control (op %d): payload is not the new payload", - crash); - } - else if (ret == (int)sizeof(old_p)) { - ck_assert_msg(memcmp(rd, old_p, sizeof(old_p)) == 0, - "power fail at op %d: old-sized payload is not the old " - "payload", crash); - } - else if (ret == (int)sizeof(new_p)) { - ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, - "power fail at op %d: new-sized payload is not the new " - "payload", crash); - } - else { - /* Only two other outcomes are crash-safe: the object was never - * published, or it is present but truncated to empty by the - * Open-time durability commit. Every other return (a negative - * read error, or a partial payload length) means the vault came - * back damaged. */ - ck_assert_msg(ret == VAULT_OBJ_ABSENT || ret == 0, - "power fail at op %d: object read back %d, neither old " - "payload, new payload, empty, nor absent", crash, ret); + /* Sweep every crash point once per fault shape: first with the faulting + * flash operation abandoned whole, then with it torn part-way through, + * so half-erased and half-programmed sectors are covered as well. Real + * silicon does not promise that a sector write is all-or-nothing. */ + for (mode = 0; mode < (int)(sizeof(torn_num) / sizeof(torn_num[0])); + mode++) { + vault_powerfail_torn = (mode != 0); + vault_torn_num = torn_num[mode]; + vault_torn_den = torn_den[mode]; + for (crash = 0; crash <= ops; crash++) { + vault_restore_snapshot(snapshot); + vault_power_cycle(); + vault_flash_ops = 0; + vault_powerfail_at = crash; + if (setjmp(vault_powerfail_jmp) == 0) { + vault_obj_write(type, tok, obj, new_p, (int)sizeof(new_p)); + } + /* Power returns. */ + vault_powerfail_at = -1; + vault_power_cycle(); + memset(rd, 0, sizeof(rd)); + ret = vault_obj_read(type, tok, obj, rd, (int)sizeof(rd)); + + if (crash == ops) { + /* No fault can land on this iteration: vault_flash_op() only + * jumps once the op counter exceeds vault_powerfail_at, and a + * clean rewrite performs exactly ops operations. It is the + * no-fault control, so the rewrite ran to completion and the + * new payload must be there. Letting it take the empty/absent + * branch below would let a silently lost write pass. */ + ck_assert_msg(ret == (int)sizeof(new_p), + "%s no-fault control (op %d): object read back %d, " + "expected the new payload (%d bytes)", modestr[mode], + crash, ret, (int)sizeof(new_p)); + ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, + "%s no-fault control (op %d): payload is not the new " + "payload", modestr[mode], crash); + } + else if (ret == (int)sizeof(old_p)) { + ck_assert_msg(memcmp(rd, old_p, sizeof(old_p)) == 0, + "%s power fail at op %d: old-sized payload is not the old " + "payload", modestr[mode], crash); + } + else if (ret == (int)sizeof(new_p)) { + ck_assert_msg(memcmp(rd, new_p, sizeof(new_p)) == 0, + "%s power fail at op %d: new-sized payload is not the new " + "payload", modestr[mode], crash); + } + else { + /* The one crash-safe alternative to a whole generation is the + * truncated-but-present object the Open-time commit + * guarantees. The node must still be there -- the rewrite + * never calls create_object() for an existing object, so + * losing it outright (VAULT_OBJ_ABSENT) would be a real + * fault, not an empty + * rewrite -- and its committed size must be exactly the 8-byte + * tok/obj prefix, or the header itself came back torn. */ + ck_assert_msg(ret == 0, + "%s power fail at op %d: object read back %d, neither old " + "payload, new payload, nor empty", modestr[mode], crash, + ret); + hdr_size = vault_obj_committed_size(type, tok, obj); + ck_assert_msg(hdr_size == (int)(2 * sizeof(uint32_t)), + "%s power fail at op %d: empty read but committed header " + "size is %d, expected %d", modestr[mode], crash, hdr_size, + (int)(2 * sizeof(uint32_t))); + } } } + vault_powerfail_torn = 0; } END_TEST