From 1d3b576f03195c8c9078bc23644156261119226a Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 1 Sep 2026 15:47:22 +0200 Subject: [PATCH] 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 d79fe614..d84d16b0 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 c12aa347..c8bc786d 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 bb0a7f72..cf07f691 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; }