vmbus_serial_host: implement RX_CLEAR_BUFFER instead of todo!()#3687
Open
rei141 wants to merge 1 commit into
Open
vmbus_serial_host: implement RX_CLEAR_BUFFER instead of todo!()#3687rei141 wants to merge 1 commit into
rei141 wants to merge 1 commit into
Conversation
The RX_CLEAR_BUFFER host notification is guest-triggerable but reached `todo!()`, so a guest could panic the serial device. Implement it by clearing the buffered RX bytes.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements handling for a guest-issued “clear RX buffer” notification to avoid guest-triggerable panics and correctly discard buffered RX data.
Changes:
- Replaces a
todo!()panic with logic to clear the buffered RX bytes onRX_CLEAR_BUFFER. - Adds explanatory comments documenting why this must be non-panicking.
| // The guest asked to discard buffered host->guest RX data. This | ||
| // is guest-controlled, so it must not panic (the previous | ||
| // `todo!()` was a guest-triggerable DoS); just clear the buffer. | ||
| self.state.rx_bytes.clear(); |
smalis-msft
reviewed
Jun 8, 2026
| match notification { | ||
| HostNotifications::RX_CLEAR_BUFFER => { | ||
| todo!("clear rx buffer unimplemented") | ||
| // The guest asked to discard buffered host->guest RX data. This |
Contributor
There was a problem hiding this comment.
This comment isn't necessary, it's just restating the obvious and mentioning now obsolete history.
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
The
RX_CLEAR_BUFFERhost notification is guest-triggerable but reachedtodo!("clear rx buffer unimplemented"), so aguest could panic the serial device in the paravisor after the VERSION handshake.
Fix
Implement the branch by clearing the buffered host->guest RX data (
self.state.rx_bytes.clear();). This is both thesecurity fix (no longer panics on guest input) and the intended functionality.
Impact
Denial of service (reachable
todo!()panic), not memory unsafety. Scoped to the attacking guest's own VM — anunprivileged VTL0 guest crashing the VTL2 paravisor that serves it (a self-DoS); no cross-VM/host impact.