Refactoring of pkcs11_store.c module #481
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awaiting feedback (zd18366) |
dgarske
requested changes
Jul 31, 2024
src/pkcs11_store.c
Outdated
| obj = vault_descriptors[found]; | ||
| memcpy(&obj->hdr, vault_base + found * KEYVAULT_OBJ_SIZE, sizeof(struct obj_hdr)); | ||
| } | ||
| obj = XMALLOC(sizeof(struct store_object), NULL, DYNAMIC_TYPE_TMP_BUFFER); |
There was a problem hiding this comment.
Consider cast (struct store_object*).
src/pkcs11_store.c
Outdated
| memcpy(&obj->hdr, vault_base + found * KEYVAULT_OBJ_SIZE, sizeof(struct obj_hdr)); | ||
| } | ||
| obj = XMALLOC(sizeof(struct store_object), NULL, DYNAMIC_TYPE_TMP_BUFFER); | ||
| if (!obj) { |
There was a problem hiding this comment.
Prefer obj == NULL... thus not assuming NULL == 0.
src/pkcs11_store.c
Outdated
| obj = vault_descriptors[found]; | ||
| memcpy(&obj->hdr, vault_base + found * KEYVAULT_OBJ_SIZE, sizeof(struct obj_hdr)); | ||
| } | ||
| obj = XMALLOC(sizeof(struct store_object), NULL, DYNAMIC_TYPE_TMP_BUFFER); |
There was a problem hiding this comment.
Consider version that doesn't require heap..
There was a problem hiding this comment.
I have removed the dynamic allocation in the last refactoring, however it seems that wolfPKCS11 still requires a heap, so I re-introduced the _sbrk in the designated pool.
There was a problem hiding this comment.
Ideally, since wolfPKCS11 uses its own area, this heap can be in a bank that is securely erased on powerdown
danielinux
changed the title
danielinux
force-pushed
the
pkcs11-store-fixes
branch
4 times, most recently
from
997847b
to
5b9278d
Compare
danielinux
force-pushed
the
pkcs11-store-fixes
branch
from
5b9278d
to
7384289
Compare
danielinux
force-pushed
the
pkcs11-store-fixes
branch
from
ac83256
to
ec8ad08
Compare
dgarske
previously approved these changes
Sep 5, 2024
|
Ready to review |
dgarske
previously approved these changes
Sep 5, 2024
danielinux
force-pushed
the
pkcs11-store-fixes
branch
from
f7704eb
to
6737d7e
Compare
dgarske
approved these changes
Sep 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pkcs11_store.cThis new design uses two sectors at the beginning of the keyvault partition.
The first sector contains the allocation tables for the nodes in flash, including
a bitmap to keep track of the free slots.
The second sector is a pre-commit 'backup' sector, used to update the content of the
vault in two steps. This guarantees powerfail protection.
All hal_flash_erase / hal_flash_write operations are now on entire sectors.
Sector content is cached to memory, then written to the backup sector, then
committed to the actual destination sector.