Skip to content

virtio: validate descriptor index from avail ring #2780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
benhillis wants to merge 2 commits into from
Closed

virtio: validate descriptor index from avail ring #2780

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af3f000
virtio: validate descriptor index from avail ring
Feb 10, 2026
0c6fd95
virtio: reject queue_size of zero
Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions vm/devices/virtio/virtio/src/queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ pub enum QueueError {
TooLong,
#[error("Invalid queue size {0}. Must be a power of 2.")]
InvalidQueueSize(u16),
#[error("descriptor index {0} is out of range")]
InvalidDescriptorIndex(u16),
Comment on lines +44 to +45
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The InvalidDescriptorIndex error message doesn’t include the queue size / valid range, which makes debugging guest issues harder. Consider including queue_size in the variant (e.g., store both the index and queue_size) and mention the valid range in the error text.

Copilot uses AI. Check for mistakes.
}

pub struct QueueDescriptor {
Expand Down Expand Up @@ -240,6 +242,10 @@ impl SplitQueueGetWork {
mem: GuestMemory,
params: QueueParams,
) -> Result<Self, QueueError> {
if params.size == 0 {
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new params.size == 0 check is redundant with QueueCoreGetWork::new’s is_power_of_two() validation (which already rejects 0), and it only checks for 0 (not other invalid sizes). To avoid inconsistent validation if SplitQueueGetWork::new is ever called directly, either remove this check (rely on the outer validation) or validate the full invariant here (non-zero power-of-two).

Suggested change
if params.size == 0 {
if !params.size.is_power_of_two() {

Copilot uses AI. Check for mistakes.
return Err(QueueError::InvalidQueueSize(params.size));
}

let queue_avail = mem
.subrange(
params.avail_addr,
Expand Down Expand Up @@ -313,13 +319,17 @@ impl SplitQueueGetWork {
}

pub fn get_available_descriptor_index(&self, wrapped_index: u16) -> Result<u16, QueueError> {
Ok(self
let desc_index = self
.queue_avail
.read_plain::<u16_le>(
spec::AVAIL_OFFSET_RING + spec::AVAIL_ELEMENT_SIZE * wrapped_index as u64,
)
.map_err(QueueError::Memory)?
.get())
.get();
if desc_index >= self.queue_size {
return Err(QueueError::InvalidDescriptorIndex(desc_index));
}
Comment thread
benhillis marked this conversation as resolved.
Ok(desc_index)
}

fn set_available_event(&self, index: u16) -> Result<(), QueueError> {
Expand Down