diff --git a/src/pkcs11_store.c b/src/pkcs11_store.c index bc0b0644..44fccadb 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 a599617f..df3647cb 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; }