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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add PSA interruptible key generation setup & abort APIs #9639
Add PSA interruptible key generation setup & abort APIs #9639
Changes from all commits
cca4dbe
1c3c5b1
75a412f
e3abcc3
bb06832
b16edbe
005b78c
1f5075b
df186be
1ea62b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: is this really the best way to copy attributes? I admittedly can't find any other operations copying attributes, but this feels dangerous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no pointers in the struct currently, there might be an issue if someone adds a pointer in the future and it soesn't seem very propable to add a pointer to attributes, if we decide that it is still dangerous what I can do is add copy_attributes() function that still does a shallow copy and later will do a deep copy if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question!
There used to be a pointer in the attributes (domain parameters). We removed it in 3.6.0. I don't recall that we ever copied the data at that pointer: either we never copied attributes anywhere (most likely — we used to copy a sub-structure that didn't have pointers), or we only copied attributes at a point where we knew there were no domain parameters, or we had a memory management bug.
At this point I can't think of anything in the proposed evolution of the PSA API that would make us add a pointer back. But just in case it would be good to write and use a copy function, that can be just a
static inline
that does an assignment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually there is one place where we are copying the attribute structure: in
psa_start_key_creation
. We used to copy a sub-structure (attributes.core
) that didn't have any embedded pointer, and we flattened the structure when we removed the pointer.So at this point it's a preexisting issue. (Of my doing, incidentally — I did the patch that flattened the structure.) It would be good to use a copy function for future-proofing. But we would also need to ensure that we're calling
psa_key_attributes_reset
on all paths where the attributes become dead. We used to do that, but one of the advantages of simplifying the attribute structure was that we wouldn't have to do it any longer.So at this point, I lean weakly towards not bothering. If we ever add a pointer into the attribute structure, we'll have to review the code carefully to ensure that all places that might possibly copy or free an attribute structure are covered by the copy/free function, anyway. We won't gain much just because there are places where the copy/free functions are used, since there'll be no way to know that it's missing in other places.