virtio-devices: copy VSock header from guest #7530
Open
+79
−27
Conversation
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
Member
|
@alyssais Is this something you could take a look at? I think you might be the most experienced in this code. |
VsockPacket::hdr holds a raw pointer to the address of the VSock packet header, which is in guest memory. It opens the door to double-fetch (or TOCTOU) race conditions. Therefore, VSockPacket::hdr content can't be trusted since it can be arbitrarily changed by the guest, at any time. To mitigate this, we can copy the header content to an array in VMM's memory that the guest can't modify. Signed-off-by: Thomas Leroy <[email protected]>
Member
|
Can do, but probably not this week. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This issue has been reported privately to @rbradford.
VsockPacket::hdr holds a raw pointer to the address of the VSock packet header, which is in guest memory. It opens the door to double-fetch (or TOCTOU) race conditions. Therefore, VSockPacket::hdr content can't be trusted since it can be arbitrarily changed by the guest, at any time.
To mitigate this, we can copy the header content to an array in VMM's memory that the guest can't modify.
I managed to trigger this race condition by changing the
lenfield header just before this line. This triggers a slice out-of-bounds and makes the VMM process panic. Since the packet header is allocated by the guest kernel, the PoC needs a guest kernel module to scan kernel memory and change the len field. I can provide the source files for the PoC.It seems that some (maybe most) of the VSock code comes from firecracker, which had the same issue but fixed it. This PR takes some pieces of code from the firecracker codebase.
Also, I'm not a VSock expert so a more experienced reviewer would be appreciated.