WolfSSL
/
wolfBoot
mirror of https://github.com/wolfSSL/wolfBoot.git
1
0
Fork 0
Code Issues Releases Wiki Activity

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.
pull/873/head
Daniele Lacamera 2026-08-25 17:30:04 +02:00
parent febf29ad61
commit b5c9c366f5
2 changed files with 120 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/pkcs11_store.c
View File

@ -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;

90
tools/unit-tests/unit-pkcs11_store.c
View File

@ -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;
}