SecureStore: Add member initializers for inc_handle_t #11810
Merged
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.
Description (required)
_inc_set_handle is new'd in SecureStore::init(), then its members are referenced in various functions without being explicitly initialized first. These pre-existing values can confuse the SecureStore's internal state and cause various undesired behaviors.
Note: At least on the ARM GCC versions that I've tested with (6.3.1 and 7.2.1), the default initialization is also achieved by using
new inc_set_handle_t();
instead ofnew inc_set_handle_t;
(note the added parentheses). I chose to add explicit initializers instead because a.) it is hard to tell whether this behavior is guaranteed by spec or just how GCC happens to be implemented and b.) the explicit initializers make it more clear what is going on (and are not prone to failure if a future change forgets to use parentheses with new).Summary of change (What the change is for and why)
Fix use of uninitialized memory contents in SecureStore.
Documentation (Details of any document updates required)
Pull request type (required)
Test results (required)
A full Greentea test log will be uploaded soon (probably tomorrow).
Reviewers (optional)
@ARMMbed/team-cypress
Release Notes (required for feature/major PRs)
Summary of changes
Impact of changes
Migration actions required