hv1_emulator: bounds-check guest-controlled synic indices#3686
Open
rei141 wants to merge 1 commit into
Open
Conversation
`signal_event` indexed the SINT array with an unchecked `sint_index` and the SIEFP page with an unchecked, guest-controllable `flag`; `post_message` indexed (and shifted by) an unchecked `sint_index`. An out-of-range value (e.g. a guest-configured event flag >= 2048) caused an out-of-bounds panic. Reject out-of-range `sint_index`/`flag` instead of indexing past the fixed-size array/page.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens SynIC emulation against guest/caller-influenced out-of-range indices to prevent panics from out-of-bounds array/page indexing.
Changes:
- Add bounds checks for
sint_indexbefore indexing the VP SINT array. - Validate
flagagainst the number of event-flag bits owned per SINT in the SIEFP page. - Add bounds checks for
sint_indexinProcessorSynic::post_messageto avoid invalid array access/bit shifts.
Comment on lines
+245
to
+251
| // Each SINT owns this many event flags in the SIEFP page. `flag` is | ||
| // guest-controllable (e.g. via a guest-configured event port), so an | ||
| // out-of-range value must be rejected rather than indexing past the | ||
| // page below (which would be a guest-triggerable OOB panic). | ||
| if flag >= (HV_PAGE_SIZE_USIZE / NUM_SINTS) * 8 { | ||
| return Ok(false); | ||
| } |
Comment on lines
+240
to
+242
| if sint_index >= NUM_SINTS { | ||
| return Ok(false); | ||
| } |
Comment on lines
+450
to
+452
| if sint_index as usize >= NUM_SINTS { | ||
| return Err(HvError::InvalidSynicState); | ||
| } |
Contributor
|
If the guest wants to crash itself it always can, I'm not sure this is worthwhile. |
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.
Summary
signal_eventindexed the SINT array with an uncheckedsint_indexand the SIEFP page with an unchecked,guest-controllable
flag;post_messageindexed (and shifted by) an uncheckedsint_index. An out-of-range value — e.g.a guest-configured event flag >= 2048, which reaches
signal_eventvia a guest-opened channel'sevent_flag— caused anout-of-bounds panic in the paravisor.
Fix
signal_event: returnOk(false)(the existing benign "not signaled" path used for masked/proxy SINTs) whensint_index >= NUM_SINTSorflag >= (HV_PAGE_SIZE_USIZE / NUM_SINTS) * 8(= 2048, the number of event flags one SINTowns in the SIEFP page).
post_message: return the existingHvError::InvalidSynicStatewhensint_index >= NUM_SINTS, guarding both the arrayindex and the
1 << sint_indexshift.Impact
Denial of service (out-of-bounds array/page index → panic), not memory unsafety. Scoped to the attacking guest's own VM —
an unprivileged VTL0 guest crashing the VTL2 paravisor that serves it (a self-DoS); no cross-VM/host impact. The paravisor
should not panic on guest-controlled indices regardless.