Fix undefined behavior when transmuting slices. #18
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
|
@carllerche I think the failure is spurious but I am not sure. |
|
Ah yes, it appears so, merging #17 should fix the issues here. |
|
I merged #17. Would you be able to merge master into this branch? |
The representation of slices is unspecified meaning that a mem::transmute is not guaranteed to do the right thing although it currently does in practice so AFAICT this currently is not a security vulnerability. The fix is "simple": instead of transmuting the slices, we get the pointer and the length of a slice, and use `from_raw_parts` to construct the other by casting the pointer to the other pointer type. For this to work correctly, both types have to have the same size, since otherwise, the length of the first slice would be incorrect. Also, either both types have the same alignment, or the original pointer is suitably aligned for the second type. Since the types here are fixed and internal to the library, we assert these conditions using `debug_assert!`s only. As long as these transmutes are tested, any changes in layout that would introduce undefined behavior will be detected.
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.
The representation of slices is unspecified meaning that a mem::transmute is not
guaranteed to do the right thing although it currently does in practice so AFAICT
this currently is not a security vulnerability.
The fix is "simple": instead of transmuting the slices, we get the pointer and
the length of a slice, and use
from_raw_partsto construct the other bycasting the pointer to the other pointer type. For this to work correctly, both
types have to have the same size, since otherwise, the length of the first slice
would be incorrect. Also, either both types have the same alignment, or the
original pointer is suitably aligned for the second type. Since the types here
are fixed and internal to the library, we assert these conditions using
debug_assert!s only. As long as these transmutes are tested, any changes inlayout that would introduce undefined behavior will be detected.