Skip to content

Add Packed Descriptor #310

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.

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

Merged
merged 4 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions fuzz/common/src/blk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub mod tests {
use virtio_bindings::bindings::virtio_blk::VIRTIO_BLK_T_IN;
use virtio_bindings::bindings::virtio_ring::{VRING_DESC_F_NEXT, VRING_DESC_F_WRITE};
use virtio_blk::request::{Request, RequestType};
use virtio_queue::{mock::MockSplitQueue, Descriptor};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue};
use vm_memory::{ByteValued, Bytes, GuestAddress, GuestMemoryMmap};

// The same as the RequestHeader type in virtio_blk, with exposed fields
Expand Down Expand Up @@ -91,7 +91,7 @@ pub mod tests {
},
];

let q_descriptors: Vec<Descriptor> =
let q_descriptors: Vec<RawDescriptor> =
descriptors.into_iter().map(|desc| desc.into()).collect();
let mut chain = vq.build_multiple_desc_chains(&q_descriptors).unwrap();

Expand Down
13 changes: 9 additions & 4 deletions fuzz/common/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use ::virtio_queue::{Descriptor, Queue, QueueOwnedT, QueueT};
use ::virtio_queue::{
desc::{split::Descriptor as SplitDescriptor, RawDescriptor},
Queue, QueueOwnedT, QueueT,
};
use std::fs::{self, File};
use std::path::{Path, PathBuf};
use std::sync::atomic::Ordering;
Expand Down Expand Up @@ -183,9 +186,11 @@ impl Into<Ordering> for LoadOrdering {
}
}

impl Into<Descriptor> for FuzzingDescriptor {
fn into(self) -> Descriptor {
Descriptor::new(self.addr, self.len, self.flags, self.next)
impl Into<RawDescriptor> for FuzzingDescriptor {
fn into(self) -> RawDescriptor {
RawDescriptor::from(SplitDescriptor::new(
self.addr, self.len, self.flags, self.next,
))
}
}

Expand Down
4 changes: 2 additions & 2 deletions fuzz/common/src/virtio_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub mod tests {

use crate::create_corpus_file;
use virtio_bindings::bindings::virtio_ring::{VRING_DESC_F_NEXT, VRING_DESC_F_WRITE};
use virtio_queue::{mock::MockSplitQueue, Descriptor, Queue, QueueT};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue, Queue, QueueT};
use vm_memory::{GuestAddress, GuestMemoryMmap};

pub fn create_basic_virtio_queue_ops() -> VirtioQueueInput {
Expand Down Expand Up @@ -51,7 +51,7 @@ pub mod tests {
// To be able to call the functions we actually need to create the environment for running.
let mem = GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
let vq = MockSplitQueue::new(&mem, DEFAULT_QUEUE_SIZE);
let q_descriptors: Vec<Descriptor> =
let q_descriptors: Vec<RawDescriptor> =
descriptors.iter().map(|desc| (*desc).into()).collect();
vq.build_multiple_desc_chains(&q_descriptors).unwrap();
let mut q: Queue = vq.create_queue().unwrap();
Expand Down
6 changes: 3 additions & 3 deletions fuzz/common/src/vsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ mod tests {
use crate::virtio_queue::DEFAULT_QUEUE_SIZE;
use std::io::Write;
use virtio_bindings::bindings::virtio_ring::{VRING_DESC_F_NEXT, VRING_DESC_F_WRITE};
use virtio_queue::desc::RawDescriptor;
use virtio_queue::mock::MockSplitQueue;
use virtio_queue::Descriptor;
use virtio_vsock::packet::VsockPacket;
use vm_memory::{Bytes, GuestAddress, GuestMemory, GuestMemoryMmap};

Expand Down Expand Up @@ -332,7 +332,7 @@ mod tests {
next: 0,
},
];
let q_descriptors: Vec<Descriptor> =
let q_descriptors: Vec<RawDescriptor> =
descriptors.iter().map(|desc| (*desc).into()).collect();
let mut chain = vq.build_multiple_desc_chains(&q_descriptors).unwrap();

Expand Down Expand Up @@ -382,7 +382,7 @@ mod tests {
next: 0,
},
];
let q_descriptors: Vec<Descriptor> =
let q_descriptors: Vec<RawDescriptor> =
descriptors.iter().map(|desc| (*desc).into()).collect();
let mut chain = vq.build_multiple_desc_chains(&q_descriptors).unwrap();

Expand Down
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/blk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use libfuzzer_sys::fuzz_target;
use std::hint::black_box;
use virtio_blk::request::Request;
use virtio_blk::stdio_executor::StdIoBackend;
use virtio_queue::{mock::MockSplitQueue, Descriptor};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue};
use vm_memory::{Bytes, GuestAddress, GuestMemoryMmap};

fuzz_target!(|data: &[u8]| {
Expand All @@ -23,7 +23,7 @@ fuzz_target!(|data: &[u8]| {

let vq = MockSplitQueue::create(&m, start_addr, DEFAULT_QUEUE_SIZE);

let descriptors: Vec<Descriptor> = fuzz_input
let descriptors: Vec<RawDescriptor> = fuzz_input
.descriptors
.iter()
.map(|desc| (*desc).into())
Expand Down
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/virtio_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use common::{
virtio_queue::{VirtioQueueInput, DEFAULT_QUEUE_SIZE},
};
use libfuzzer_sys::fuzz_target;
use virtio_queue::{mock::MockSplitQueue, Descriptor};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue};
use vm_memory::{GuestAddress, GuestMemoryMmap};

fuzz_target!(|data: &[u8]| {
Expand All @@ -20,7 +20,7 @@ fuzz_target!(|data: &[u8]| {
let start_addr = GuestAddress(0x1000);
let m = GuestMemoryMmap::<()>::from_ranges(&[(start_addr, 0x11000)]).unwrap();
let vq = MockSplitQueue::create(&m, start_addr, DEFAULT_QUEUE_SIZE);
let descriptors: Vec<Descriptor> = fuzz_input
let descriptors: Vec<RawDescriptor> = fuzz_input
.descriptors
.iter()
.map(|desc| (*desc).into())
Expand Down
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/virtio_queue_ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use common::{
};
use libfuzzer_sys::fuzz_target;
use std::convert::{Into, TryFrom};
use virtio_queue::{mock::MockSplitQueue, Descriptor, Queue, QueueState};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue, Queue, QueueState};
use vm_memory::{GuestAddress, GuestMemoryMmap};

fuzz_target!(|data: &[u8]| {
Expand All @@ -22,7 +22,7 @@ fuzz_target!(|data: &[u8]| {
let m = GuestMemoryMmap::<()>::from_ranges(&[(start_addr, 0x11000)]).unwrap();
let vq = MockSplitQueue::create(&m, start_addr, DEFAULT_QUEUE_SIZE);

let descriptors: Vec<Descriptor> = fuzz_input
let descriptors: Vec<RawDescriptor> = fuzz_input
.descriptors
.iter()
.map(|desc| (*desc).into())
Expand Down
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/vsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use common::virtio_queue::DEFAULT_QUEUE_SIZE;
use common::vsock::{InitFunction, VsockInput};
use libfuzzer_sys::fuzz_target;
use virtio_queue::{mock::MockSplitQueue, Descriptor};
use virtio_queue::{desc::RawDescriptor, mock::MockSplitQueue};
use virtio_vsock::packet::VsockPacket;
use vm_memory::{GuestAddress, GuestMemoryMmap};

Expand All @@ -19,7 +19,7 @@ fuzz_target!(|data: &[u8]| {
let m = GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0x1000), 0x11000)]).unwrap();
let vq = MockSplitQueue::create(&m, start_addr, DEFAULT_QUEUE_SIZE);

let descriptors: Vec<Descriptor> = fuzz_input
let descriptors: Vec<RawDescriptor> = fuzz_input
.descriptors
.iter()
.map(|desc| (*desc).into())
Expand Down
129 changes: 102 additions & 27 deletions virtio-blk/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ use virtio_bindings::bindings::virtio_blk::{
VIRTIO_BLK_T_OUT, VIRTIO_BLK_T_WRITE_ZEROES,
};

use virtio_queue::{Descriptor, DescriptorChain};
use virtio_queue::{
desc::{split::Descriptor as SplitDescriptor, RawDescriptor},
DescriptorChain,
};
use vm_memory::{ByteValued, Bytes, GuestAddress, GuestMemory, GuestMemoryError};

/// Block request parsing errors.
Expand Down Expand Up @@ -158,10 +161,11 @@ impl Request {
}

// Checks that a descriptor meets the minimal requirements for a valid status descriptor.
fn check_status_desc<M>(mem: &M, desc: Descriptor) -> Result<()>
fn check_status_desc<M>(mem: &M, desc: RawDescriptor) -> Result<()>
where
M: GuestMemory + ?Sized,
{
let desc = SplitDescriptor::from(desc);
// The status MUST always be writable.
if !desc.is_write_only() {
return Err(Error::UnexpectedReadOnlyDescriptor);
Expand All @@ -182,7 +186,8 @@ impl Request {
}

// Checks that a descriptor meets the minimal requirements for a valid data descriptor.
fn check_data_desc(desc: Descriptor, request_type: RequestType) -> Result<()> {
fn check_data_desc(desc: RawDescriptor, request_type: RequestType) -> Result<()> {
let desc = SplitDescriptor::from(desc);
// We do this check only for the device-readable buffers, as opposed to
// also check that the device doesn't want to read a device-writable buffer
// because this one is not a MUST (the device MAY do that for debugging or
Expand Down Expand Up @@ -234,14 +239,14 @@ impl Request {
let mut desc = desc_chain.next().ok_or(Error::DescriptorChainTooShort)?;

while desc.has_next() {
Request::check_data_desc(desc, request.request_type)?;
Request::check_data_desc(RawDescriptor::from(desc), request.request_type)?;

request.data.push((desc.addr(), desc.len()));
desc = desc_chain.next().ok_or(Error::DescriptorChainTooShort)?;
}
let status_desc = desc;

Request::check_status_desc(desc_chain.memory(), status_desc)?;
Request::check_status_desc(desc_chain.memory(), RawDescriptor::from(status_desc))?;

request.status_addr = status_desc.addr();
Ok(request)
Expand Down Expand Up @@ -298,9 +303,24 @@ mod tests {
// The `build_desc_chain` function will populate the `NEXT` related flags and field.
let v = [
// A device-writable request header descriptor.
Descriptor::new(0x10_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x20_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x30_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(
0x10_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x20_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x30_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
];
// Create a queue of max 16 descriptors and a descriptor chain based on the array above.
let queue = MockSplitQueue::new(&mem, 16);
Expand All @@ -320,10 +340,15 @@ mod tests {
);

let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x20_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x20_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
// A device-readable request status descriptor.
Descriptor::new(0x30_0000, 0x100, 0, 0),
RawDescriptor::from(SplitDescriptor::new(0x30_0000, 0x100, 0, 0)),
];
let mut chain = queue.build_desc_chain(&v[..3]).unwrap();

Expand All @@ -334,10 +359,20 @@ mod tests {
);

let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x20_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x20_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
// Status descriptor with len = 0.
Descriptor::new(0x30_0000, 0x0, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(
0x30_0000,
0x0,
VRING_DESC_F_WRITE as u16,
0,
)),
];
let mut chain = queue.build_desc_chain(&v[..3]).unwrap();
assert_eq!(
Expand All @@ -346,9 +381,14 @@ mod tests {
);

let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x20_0000, 0x100, 0, 0),
Descriptor::new(0x30_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(0x20_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x30_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
];
let mut chain = queue.build_desc_chain(&v[..3]).unwrap();

Expand Down Expand Up @@ -378,10 +418,25 @@ mod tests {

// Invalid status address.
let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x20_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x30_0000, 0x200, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x1100_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x20_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x30_0000,
0x200,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x1100_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
];
let req_header = RequestHeader {
request_type: VIRTIO_BLK_T_OUT,
Expand All @@ -403,10 +458,25 @@ mod tests {

// Valid descriptor chain for OUT.
let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x20_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x30_0000, 0x200, VRING_DESC_F_WRITE as u16, 0),
Descriptor::new(0x40_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x20_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x30_0000,
0x200,
VRING_DESC_F_WRITE as u16,
0,
)),
RawDescriptor::from(SplitDescriptor::new(
0x40_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
];
let req_header = RequestHeader {
request_type: VIRTIO_BLK_T_OUT,
Expand Down Expand Up @@ -448,8 +518,13 @@ mod tests {

// Valid descriptor chain for FLUSH.
let v = [
Descriptor::new(0x10_0000, 0x100, 0, 0),
Descriptor::new(0x40_0000, 0x100, VRING_DESC_F_WRITE as u16, 0),
RawDescriptor::from(SplitDescriptor::new(0x10_0000, 0x100, 0, 0)),
RawDescriptor::from(SplitDescriptor::new(
0x40_0000,
0x100,
VRING_DESC_F_WRITE as u16,
0,
)),
];
let req_header = RequestHeader {
request_type: VIRTIO_BLK_T_FLUSH,
Expand Down
Loading