fix(vmclock): validate guest address is plugged before activate write#5883
Merged
Manciukic merged 5 commits intoJun 10, 2026
Merged
Conversation
Manciukic
force-pushed
the
fix-vmclock-sigsegv
branch
2 times, most recently
from
da46f10 to
60d258b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5883 +/- ##
==========================================
+ Coverage 83.00% 83.01% +0.01%
==========================================
Files 277 277
Lines 30106 30129 +23
==========================================
+ Hits 24989 25012 +23
Misses 5117 5117
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Manciukic
added
the
Status: Awaiting review
Manciukic
force-pushed
the
fix-vmclock-sigsegv
branch
from
53cddc3 to
dc87382
Compare
ShadowCurse
reviewed
May 18, 2026
ShadowCurse
left a comment
ShadowCurse
left a comment
Contributor
There was a problem hiding this comment.
This feels like plugging a hole instead of fixing the issue (and all "fix the snapshot fuzzer" fixes fall into this category). It introduces unnecessary complexity (however small or big) without any real benefit.
Technically each write_slice/write_obj etc may do this type of check, yet it is absolutely valid to crash if somehow FC is tricked to writing to PROT_NONE memory.
We will need to rethink this snapshot fuzzing situation better sooner than later.
Manciukic
force-pushed
the
fix-vmclock-sigsegv
branch
2 times, most recently
from
1935bf2 to
ad362f1
Compare
Contributor
Author
|
I agree it's just plugging a hole, but until we have resolved the underlying issue either with the fuzzer or with the checks on read/write in the region (which are tracked internally), we need a way to make the fuzzer green. |
ShadowCurse
previously approved these changes
May 22, 2026
JackThomson2
reviewed
Jun 5, 2026
Change slots_intersecting_range to return (GuestMemorySlot, bool) tuples, where the bool indicates whether the slot is plugged. This avoids needing a separate lock to check plugged state after finding intersecting slots. Signed-off-by: Riccardo Mancini <mancio@amazon.com>
Add check_range_plugged() to GuestRegionMmapExt and GuestMemoryExtension trait. These methods verify that a guest address range falls within plugged memory slots before performing writes, using slots_intersecting_range to find the relevant slots. Unplugged hotpluggable regions are mprotect'd to PROT_NONE, so writing to them causes SIGSEGV. This provides a safe way to validate before writing. Signed-off-by: Riccardo Mancini <mancio@amazon.com>
Use check_range_plugged() before writing the vmclock struct to guest memory during snapshot restore. A malformed snapshot can place the vmclock guest_address in an unplugged (PROT_NONE) region, causing SIGSEGV on the write_slice call. Found by the snapshot fuzzer. Signed-off-by: Riccardo Mancini <mancio@amazon.com>
Apply the same check_range_plugged guard to VmGenId::activate() that was added to VmClock::activate(). A malformed snapshot can place the VMGenId guest_address in an unplugged (PROT_NONE) region, causing a SIGSEGV on the write_slice call during restore. Signed-off-by: Riccardo Mancini <mancio@amazon.com>
Manciukic
force-pushed
the
fix-vmclock-sigsegv
branch
from
f061d5f to
5015211
Compare
JackThomson2
approved these changes
Jun 10, 2026
ShadowCurse
approved these changes
Jun 10, 2026
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
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.
Summary
Fix SIGSEGV during snapshot restore when a malformed snapshot places the vmclock
guest_addressin an unplugged (PROT_NONE) hotpluggable memory region.Crashing on such accesses is the right thing to do as they're unexpected, but we try to keep the snapshot restoration path clean to test it via the fuzzer.
Root Cause
pluggedbitvec marking slots as unpluggedrestore_memory_regionscallsmprotect(PROT_NONE)on unplugged slotsVmClock::activatewrites toguest_address(from snapshot) which falls in the unplugged regionwrite_slice→copy_nonoverlapping→ SIGSEGV on the protected pageChanges
slots_intersecting_rangeto return(GuestMemorySlot, bool)tuples, exposing the plugged state alongside each slot.check_range_plugged()toGuestRegionMmapExtandGuestMemoryExtensiontrait. Validates that a guest address range falls within plugged memory slots before performing writes.check_range_plugged()beforewrite_sliceinVmClock::activate, converting the SIGSEGV into a graceful error.Testing
test_check_range_pluggedcovering plugged, unplugged, spanning, and zero-length casestest_slots_intersecting_rangeupdated for new return type