virtio/console: validate control transmit messages - #3
Merged
Merged
Conversation
Guest control messages currently index ports and derived queues without validating the supplied port ID. Reject unknown PORT_READY and PORT_OPEN IDs before any port or queue access so malformed guest input cannot panic the VMM or change console state. Complete driver-to-device control chains with a used length of zero because the device does not write into them. Assisted-by: Codex:gpt-5.6-sol
The control transmit path currently reads an object from the head address without validating descriptor direction, declared length, chained layout, or guest memory ranges. That can read outside the driver-declared buffer and leave unreadable elements stranded. Validate the complete readable chain, accept scatter/gather layouts totaling exactly one control message, reject writable or malformed chains, and parse through the established descriptor Reader. Return every popped chain once with no device-written bytes. Assisted-by: Codex:gpt-5.6-sol
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
1 participant
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
Why
The control transmit path reads an eight-byte object from the head descriptor address without validating the guest-supplied port ID, descriptor direction, declared length, chained layout, or guest memory ranges. Unknown port IDs can reach unchecked port and queue indexing, while malformed or unreadable inputs can be processed outside their declared bounds or left uncompleted.
This change uses the established descriptor
Reader, preserves valid control-event behavior, and treats malformed input as a local robustness failure without adding guest-visible side effects.Validation
cargo fmt -- --checkcd examples && cargo fmt -- --checkcd tests && cargo fmt -- --checkxcrun clang-format -n -Werroroverinit/**/*.{c,h}python3 .github/scripts/check-ai-trailers.py c652b56ca6fe28a038bf4be5beb39fa54b4247c0cargo clippy --locked -- -D warningson macOS arm64 with the fixed empty init fixturecargo teston macOS arm64 with the fixed empty init fixturefocused
krun-devicestests pass independently at each commitfork CI passed AI trailers, formatting, Linux x86_64/aarch64 code quality, macOS code quality, Linux x86_64 units, Linux x86_64 examples, macOS cross-compilation, and Linux-to-FreeBSD cross-compilation
self-hosted Linux aarch64 unit/examples jobs remain queued because the fork has no matching runner available
the unrestricted integration workflow was intentionally canceled; only the fixed
multiport-consoleguest case is authorized for this preparationthe fixed
multiport-consoleintegration case passed twice on macOS arm64/HVF, including one retained-artifact run:OK - 1/1 passedthe local runner used an explicit Xcode
libclangloader path, the Makefile's aarch64 musl cross-linker flags, and a task-local dylib built from the official libkrunfw v5.5.0 prebuilt aarch64 source bundleReview status
This fork-local PR is preparation-only. It is intentionally missing human
Signed-off-bytrailers until the human contributor reviews the changes and explicitly accepts DCO responsibility. It must not be submitted upstream before that review and separate authorization.Shutdown/output handling, duplicate
PORT_OPEN, PR libkrun#739 notification behavior, partial writes, and FD lifetime/flags are out of scope.