fix(migtd): avoid sending uninitialized ExchangeInformation padding#914
Open
kali834x wants to merge 1 commit into
Open
fix(migtd): avoid sending uninitialized ExchangeInformation padding#914kali834x wants to merge 1 commit into
kali834x wants to merge 1 commit into
Conversation
ExchangeInformation is exchanged with the migration peer byte-for-byte via as_bytes(), but it had no repr(C) and the four bytes of trailing padding after max_ver were copied to the peer uninitialized during the key exchange. Pin the layout with repr(C) and replace the implicit padding with an explicit zeroed reserved field; field order keeps key/min_ver/max_ver at their existing offsets so the wire format is unchanged. Signed-off-by: Zibran Khan <kali834x@gmail.com>
This file contains hidden or 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
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.
ExchangeInformation carries the negotiated version range and the migration session key, and the source and destination MigTDs exchange it over the RA-TLS channel by writing the struct's raw bytes through as_bytes(). The struct had no repr(C), so the layout was the compiler's choice, and with the two u16 version fields in front of the 8-byte-aligned key there are four bytes of trailing padding after max_ver. as_bytes() copies the whole struct including that padding, and nothing writes those bytes when the value is built, so each side hands the peer four uninitialized bytes of its own memory during the key exchange. I noticed it while checking why this struct alone among the wire types in this module has no repr(C). Adding repr(C) and turning the implicit padding into an explicit zeroed reserved field closes that, and the field order keeps key, min_ver and max_ver at the offsets the compiler was already giving them, so the on-wire bytes are unchanged for peers running the current code.