From 3e0645dd50e8f428fc0074649b34818db33c0a9d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 25 Aug 2026 16:08:16 +0200 Subject: [PATCH 1/2] 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 954ac338f4..c5122de2a5 100644 --- a/options.mk +++ b/options.mk @@ -1099,6 +1099,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 323c929a5b..f3a7defb1a 100644 --- a/src/pkcs11_callable.c +++ b/src/pkcs11_callable.c @@ -1506,6 +1506,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 a53679aec6..1a49c98ee4 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)); } @@ -489,7 +710,7 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) if ((handle == NULL) || (handle->hdr == NULL) || (handle->buffer == NULL)) return -1; - obj_size = handle->hdr->size; + obj_size = handle->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -514,6 +735,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; @@ -522,7 +744,7 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) if ((handle->flags & STORE_FLAGS_READONLY) != 0) return -1; - obj_size = handle->hdr->size; + obj_size = handle->size; if (obj_size > KEYVAULT_OBJ_SIZE) return -1; @@ -541,18 +763,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 5291223f65..169fb28ee5 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 8266dfd3bfdc81191ffb847a593469d33d990e79 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 25 Aug 2026 17:30:04 +0200 Subject: [PATCH 2/2] 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 1a49c98ee4..c755b5d8df 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; @@ -722,7 +720,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 169fb28ee5..a6e7d00b79 100644 --- a/tools/unit-tests/unit-pkcs11_store.c +++ b/tools/unit-tests/unit-pkcs11_store.c @@ -511,6 +511,90 @@ START_TEST(test_shorter_overwrite_erases_residual_key_material) } 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 */ @@ -523,6 +607,8 @@ Suite *wolfboot_suite(void) TCase* tcase_delete_corrupted = tcase_create("delete_corrupted_pos"); TCase* tcase_find_bounds = tcase_create("find_bounds"); TCase* tcase_remanence = tcase_create("shorter_overwrite_erases_residual"); + 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); @@ -530,6 +616,8 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase_delete_corrupted, test_delete_object_corrupted_pos_no_oob); tcase_add_test(tcase_find_bounds, test_find_object_search_stops_at_header_sector); tcase_add_test(tcase_remanence, test_shorter_overwrite_erases_residual_key_material); + 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); @@ -537,6 +625,8 @@ Suite *wolfboot_suite(void) suite_add_tcase(s, tcase_delete_corrupted); suite_add_tcase(s, tcase_find_bounds); suite_add_tcase(s, tcase_remanence); + suite_add_tcase(s, tcase_interleaved); + suite_add_tcase(s, tcase_concurrent_read); return s; }