fw_meta: reduce raw pointer and unsafe usage #71
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
The length field in the firmware metadata takes into account the size
of the length field itself plus the associated UUID. To get the size
of the contained tables, one must substract those two values. In
parse_fw_meta_data(), this was done incorrectly due to a lack of
parenthesis and operator precedence working from left to right.
For comparison, this is already done correctly in find_table():
let len = orig_len - (mem::size_of::<Uuid>() + mem::size_of::<u16>());
Since the metadata is parsed backwards, the excessive computed length
would result in accesses to the memory before the firmware metadata.
This region would always be mapped, so no memory faults could end up
occuring.
Signed-off-by: Carlos López <[email protected]>
The functions that do the parsing are contained to the fw_meta module, so they should return errors related to firmware. This simplifies error handling in a few places. Signed-off-by: Carlos López <[email protected]>
cclaudio
approved these changes
Aug 14, 2023
Refactor parse_fw_meta_data() to avoid using unsafe and raw pointers as much as possible. This mainly consists in properly typing the metadata structures and going from raw pointers to typed references or values as soon as possible. Since the firmware metadata comes from the hypervisor, it can be considered untrusted in a confidential computing model, so extra care is taken when using lengths and offsets to index into memory. Signed-off-by: Carlos López <[email protected]>
Signed-off-by: Carlos López <[email protected]>
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.
Refactor
parse_fw_meta_data()to avoid using unsafe and raw pointers as much as possible. This mainly consists in properly typing the metadata structures and going from raw pointers to typed references or values as soon as possible. Since the firmware metadata comes from the hypervisor, it can be considered untrusted in a confidential computing model, so extra care is taken when using lengths and offsets to index into memory.All uses of
unsafeare documented, and all potential memory safety issues should no longer be present, making the parsing much more robust.