Repository navigation
Check whole virtqueue rings in guest memory - #533
Merged
Merged
Conversation
Ram::get_ptr() only checks that one value of the pointee type is backed by host memory, while callers like virtqueues use the pointer for an array of values. Its alignment check also tests the byte pointer before the cast, so it never fails. Add Ram::get_array_ptr(), which checks that all the values are in one contiguous host mapping, and that the pointer is aligned for the pointee type. Ram::get_ptr() becomes the case of a single value, which fixes its alignment check. Signed-off-by: Changyuan Lyu <changyuanl@google.com>
SplitQueue::new() checks only the first entry of the descriptor table, the available ring and the used ring, but the queue accesses entries up to the queue size through the same pointers. A driver that places a ring at the end of guest memory, or across two memory regions, makes the worker read and write host memory beyond the guest memory mapping. Check that each ring is backed by one contiguous host mapping in full. Fixes: fc059ad ("perf(virtio): lock memory layout while running") Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Like SplitQueue::new() did, PackedQueue::new() checks only the first descriptor of the ring, but the queue accesses descriptors up to the queue size. A driver that places the ring at the end of guest memory, or across two memory regions, makes the worker read and write host memory beyond the guest memory mapping. Check that the whole ring is backed by one contiguous host mapping, and reject queue sizes above 2^15, the maximum for packed virtqueues, which is also all that the 15-bit ring offset in WrappedIndex can address. Also stop at a descriptor chain as long as the ring, since a chain takes consecutive descriptors. Otherwise, a driver that sets VIRTQ_DESC_F_NEXT in every descriptor makes the worker loop forever, and a driver that changes the flags while the worker walks the chain can end it after more descriptors than the ring has, which moves the offset of the next chain beyond the ring. Fixes: f97eb72 ("feat(virtio): support packed queue") Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Lencerf
force-pushed
the
fix/virtqueue-ring-bounds
branch
from
November 9, 2026 21:01
f131e56 to
474cc60
Compare
Coverage Report for CI Build 42698485359Coverage increased (+0.09%) to 39.495%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
SplitQueue::new()andPackedQueue::new()check only the first entry of eachring in guest memory, but the queues access entries up to the queue size
through the same pointers. A driver that places a ring at the end of guest
memory, or across two memory regions, makes the device worker read and write
host memory beyond the guest memory mapping.
Ram::get_array_ptr(), which checks that a whole array is backed by onecontiguous host mapping, and fix the alignment check of
Ram::get_ptr(),which never failed.
virtqueues.
descriptor chain as long as the ring, which also keeps the ring offset of the
next chain within the ring.
Every commit passes
cargo +nightly fmt --check,cargo clippy --all-targets -D warningson x86_64-unknown-linux-gnu/musl, aarch64-unknown-linux-gnu andaarch64-apple-darwin, and
cargo test --workspacelocally.