Skip to content

Commit 8bf9cc6

Browse files
authored
feat(virtq): add virtio-villain inspired packed virtq coverage (#1634)
Map applicable packed ring cases into deterministic in process virtq tests, add high-level producer/consumer safety coverage, and add a packed-ring fuzz target with binary seed inputs. See: https://github.com/weltling/virtio-villain/tree/main/tests/vring Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
1 parent 280afdd commit 8bf9cc6

10 files changed

Lines changed: 1164 additions & 5 deletions

File tree

.github/workflows/Fuzzing.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ jobs:
2222
strategy:
2323
fail-fast: false
2424
matrix:
25-
target: ['fuzz_host_print', 'fuzz_guest_call', 'fuzz_host_call', 'fuzz_guest_estimate_trace_event', 'fuzz_guest_trace']
25+
target: ['fuzz_host_print', 'fuzz_guest_call', 'fuzz_host_call', 'fuzz_guest_estimate_trace_event', 'fuzz_guest_trace', 'fuzz_virtq_packed_ring']
2626
uses: ./.github/workflows/dep_fuzzing.yml
2727
with:
2828
target: ${{ matrix.target }}

.github/workflows/ValidatePullRequest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ jobs:
216216
if: ${{ !cancelled() && !failure() }}
217217
strategy:
218218
matrix:
219-
target: ['fuzz_host_print', 'fuzz_guest_call', 'fuzz_host_call', 'fuzz_guest_estimate_trace_event', 'fuzz_guest_trace']
219+
target: ['fuzz_host_print', 'fuzz_guest_call', 'fuzz_host_call', 'fuzz_guest_estimate_trace_event', 'fuzz_guest_trace', 'fuzz_virtq_packed_ring']
220220
arch:
221221
- X64
222222
# arm64 fuzzing runs on the daily schedule (DailyArm64.yml) instead of on

Justfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ like-ci config=default-target hypervisor="kvm":
218218
just fuzz-like-ci fuzz_host_call {{config}} {{hypervisor}}
219219
just fuzz-like-ci fuzz_guest_estimate_trace_event {{config}} {{hypervisor}}
220220
just fuzz-like-ci fuzz_guest_trace {{config}} {{hypervisor}}
221+
just fuzz-like-ci fuzz_virtq_packed_ring {{config}} {{hypervisor}}
221222

222223
@# spelling
223224
typos

fuzz/Cargo.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ test = false
4848
doc = false
4949
bench = false
5050

51+
[[bin]]
52+
name = "fuzz_virtq_packed_ring"
53+
path = "fuzz_targets/virtq_packed_ring.rs"
54+
test = false
55+
doc = false
56+
bench = false
57+
5158
[features]
5259
default = []
5360
trace = ["hyperlight-host/trace_guest", "hyperlight-common/trace_guest"]

fuzz/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ which evaluates to the following command `cargo +nightly fuzz run fuzz_host_prin
1010

1111
As per Microsoft's Offensive Research & Security Engineering (MORSE) team, all host exposed functions that receive or interact with guest data must be continuously fuzzed for, at least, 500 million fuzz test cases without any crashes. Because `cargo-fuzz` doesn't support setting a maximum number of iterations; instead, we use the `--max_total_time` flag to set a maximum time to run the fuzzer. We have a GitHub action (acting like a CRON job) that runs the fuzzers for 24 hours every week.
1212

13-
Currently, we fuzz the parameters and return type to a hardcoded `PrintOutput` guest function, and the `HostPrint` host function. We plan to add more fuzzers in the future.
13+
Currently, we fuzz the parameters and return type to a hardcoded `PrintOutput` guest function, the `HostPrint` host function, and the packed virtqueue ring parser. We plan to add more fuzzers in the future.
1414

1515
## On Failure
1616

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
/*
2+
Copyright 2026 The Hyperlight Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
#![no_main]
18+
19+
use std::cell::UnsafeCell;
20+
use std::num::NonZeroU16;
21+
use std::ops::Range;
22+
use std::rc::Rc;
23+
24+
use hyperlight_common::virtq::{Descriptor, Layout, MemOps, RingConsumer};
25+
use libfuzzer_sys::{Corpus, fuzz_target};
26+
27+
const DEFAULT_QUEUE_SIZE: usize = 16;
28+
const MAX_QUEUE_SIZE: usize = 64;
29+
const MAX_DESCS: usize = 64;
30+
const PAYLOAD_SIZE: usize = 4096;
31+
const BASE_ADDR: u64 = 0x1000;
32+
const HEADER_SIZE: usize = 16;
33+
const DESC_SIZE: usize = 12;
34+
35+
#[derive(Clone, Debug)]
36+
struct FuzzDesc {
37+
addr_offset: u32,
38+
len: u32,
39+
id: u16,
40+
flags: u16,
41+
}
42+
43+
#[derive(Clone, Debug)]
44+
struct FuzzCase {
45+
queue_size: usize,
46+
driver_event_off_wrap: u16,
47+
driver_event_flags: u16,
48+
written_len: u32,
49+
poll_count: usize,
50+
descs: Vec<FuzzDesc>,
51+
}
52+
53+
#[derive(Clone)]
54+
struct FuzzMem {
55+
inner: Rc<FuzzMemInner>,
56+
}
57+
58+
struct FuzzMemInner {
59+
storage: UnsafeCell<Vec<u8>>,
60+
base_addr: u64,
61+
}
62+
63+
impl FuzzMem {
64+
fn new(base_addr: u64, size: usize) -> Self {
65+
Self {
66+
inner: Rc::new(FuzzMemInner {
67+
storage: UnsafeCell::new(vec![0; size]),
68+
base_addr,
69+
}),
70+
}
71+
}
72+
73+
fn range(&self, addr: u64, len: usize) -> Result<Range<usize>, ()> {
74+
let offset = addr.checked_sub(self.inner.base_addr).ok_or(())? as usize;
75+
let end = offset.checked_add(len).ok_or(())?;
76+
let storage_len = unsafe { &*self.inner.storage.get() }.len();
77+
if end > storage_len {
78+
return Err(());
79+
}
80+
81+
Ok(offset..end)
82+
}
83+
}
84+
85+
// SAFETY: `FuzzMem` bounds-checks every translated address against its owned
86+
// backing storage and reports failures instead of dereferencing invalid memory.
87+
unsafe impl MemOps for FuzzMem {
88+
type Error = ();
89+
90+
fn read(&self, addr: u64, dst: &mut [u8]) -> Result<(), Self::Error> {
91+
let range = self.range(addr, dst.len())?;
92+
let storage = unsafe { &*self.inner.storage.get() };
93+
dst.copy_from_slice(&storage[range]);
94+
Ok(())
95+
}
96+
97+
fn write(&self, addr: u64, src: &[u8]) -> Result<(), Self::Error> {
98+
let range = self.range(addr, src.len())?;
99+
let storage = unsafe { &mut *self.inner.storage.get() };
100+
storage[range].copy_from_slice(src);
101+
Ok(())
102+
}
103+
104+
fn load_acquire(&self, addr: u64) -> Result<u16, Self::Error> {
105+
let mut bytes = [0; 2];
106+
self.read(addr, &mut bytes)?;
107+
Ok(u16::from_le_bytes(bytes))
108+
}
109+
110+
fn store_release(&self, addr: u64, val: u16) -> Result<(), Self::Error> {
111+
self.write(addr, &val.to_le_bytes())
112+
}
113+
114+
unsafe fn as_slice(&self, addr: u64, len: usize) -> Result<&[u8], Self::Error> {
115+
let range = self.range(addr, len)?;
116+
let storage = unsafe { &*self.inner.storage.get() };
117+
Ok(&storage[range])
118+
}
119+
120+
#[allow(clippy::mut_from_ref)]
121+
unsafe fn as_mut_slice(&self, addr: u64, len: usize) -> Result<&mut [u8], Self::Error> {
122+
let range = self.range(addr, len)?;
123+
let storage = unsafe { &mut *self.inner.storage.get() };
124+
Ok(&mut storage[range])
125+
}
126+
}
127+
128+
fn write_driver_event(mem: &FuzzMem, layout: Layout, off_wrap: u16, flags: u16) -> Result<(), ()> {
129+
mem.write(
130+
layout.drv_evt_addr(),
131+
&[
132+
(off_wrap & 0xff) as u8,
133+
(off_wrap >> 8) as u8,
134+
(flags & 0xff) as u8,
135+
(flags >> 8) as u8,
136+
],
137+
)
138+
}
139+
140+
/// Parse a compact little-endian packed-ring blob:
141+
///
142+
/// ```text
143+
/// u16 queue_size
144+
/// u16 desc_count
145+
/// u16 driver_event_off_wrap
146+
/// u16 driver_event_flags
147+
/// u32 written_len
148+
/// u8 poll_count
149+
/// u8 reserved[3]
150+
/// desc[desc_count]:
151+
/// u32 addr_offset
152+
/// u32 len
153+
/// u16 id
154+
/// u16 flags
155+
/// ```
156+
fn parse_case(data: &[u8]) -> Option<FuzzCase> {
157+
if data.len() < HEADER_SIZE {
158+
return None;
159+
}
160+
161+
let read_u16 = |i: usize| u16::from_le_bytes([data[i], data[i + 1]]);
162+
let read_u32 = |i: usize| u32::from_le_bytes([data[i], data[i + 1], data[i + 2], data[i + 3]]);
163+
164+
let raw_queue_size = read_u16(0);
165+
let queue_size = normalize_queue_size(raw_queue_size);
166+
let desc_count = usize::from(read_u16(2)).min(MAX_DESCS).min(queue_size);
167+
168+
let driver_event_off_wrap = read_u16(4);
169+
let driver_event_flags = read_u16(6);
170+
let written_len = read_u32(8);
171+
let poll_count = usize::from(data[12]).min(8);
172+
173+
let desc_bytes = desc_count.checked_mul(DESC_SIZE)?;
174+
if data.len() < HEADER_SIZE.checked_add(desc_bytes)? {
175+
return None;
176+
}
177+
178+
let mut descs = Vec::with_capacity(desc_count);
179+
let mut offset = HEADER_SIZE;
180+
181+
for _ in 0..desc_count {
182+
descs.push(FuzzDesc {
183+
addr_offset: read_u32(offset),
184+
len: read_u32(offset + 4),
185+
id: read_u16(offset + 8),
186+
flags: read_u16(offset + 10),
187+
});
188+
offset += DESC_SIZE;
189+
}
190+
191+
Some(FuzzCase {
192+
queue_size,
193+
driver_event_off_wrap,
194+
driver_event_flags,
195+
written_len,
196+
poll_count,
197+
descs,
198+
})
199+
}
200+
201+
fn normalize_queue_size(raw: u16) -> usize {
202+
let raw = usize::from(raw);
203+
if raw == 0 || !raw.is_power_of_two() {
204+
return DEFAULT_QUEUE_SIZE;
205+
}
206+
207+
raw.min(MAX_QUEUE_SIZE)
208+
}
209+
210+
fn run_case(case: FuzzCase) -> Corpus {
211+
let Some(num_descs) = NonZeroU16::new(case.queue_size as u16) else {
212+
return Corpus::Reject;
213+
};
214+
215+
let ring_size = Layout::query_size(case.queue_size);
216+
let mem = FuzzMem::new(BASE_ADDR, ring_size + PAYLOAD_SIZE);
217+
let layout = match unsafe { Layout::from_base(BASE_ADDR, num_descs) } {
218+
Ok(layout) => layout,
219+
Err(_) => return Corpus::Reject,
220+
};
221+
222+
if write_driver_event(
223+
&mem,
224+
layout,
225+
case.driver_event_off_wrap,
226+
case.driver_event_flags,
227+
)
228+
.is_err()
229+
{
230+
return Corpus::Reject;
231+
}
232+
233+
let payload_base = BASE_ADDR + ring_size as u64;
234+
for (idx, fuzz_desc) in case.descs.iter().enumerate() {
235+
let payload_offset = fuzz_desc.addr_offset as usize % PAYLOAD_SIZE;
236+
let desc = Descriptor {
237+
addr: payload_base + payload_offset as u64,
238+
len: fuzz_desc.len,
239+
id: fuzz_desc.id,
240+
flags: fuzz_desc.flags,
241+
};
242+
let desc_addr = layout.desc_table_addr() + idx as u64 * Descriptor::SIZE as u64;
243+
if mem.write_val(desc_addr, desc).is_err() {
244+
return Corpus::Reject;
245+
}
246+
}
247+
248+
let mut consumer = RingConsumer::new(layout, mem);
249+
for _ in 0..case.poll_count {
250+
let Ok((id, _chain)) = consumer.poll_available() else {
251+
break;
252+
};
253+
254+
if consumer
255+
.submit_used_with_notify(id, case.written_len)
256+
.is_err()
257+
{
258+
break;
259+
}
260+
}
261+
262+
Corpus::Keep
263+
}
264+
265+
fuzz_target!(|data: &[u8]| -> Corpus {
266+
let Some(case) = parse_case(data) else {
267+
return Corpus::Reject;
268+
};
269+
270+
run_case(case)
271+
});

src/hyperlight_common/src/virtq/consumer.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,46 @@ mod tests {
697697
assert!(producer.poll().unwrap().is_some());
698698
}
699699

700+
#[test]
701+
fn test_villain_indirect_descriptor_does_not_mark_high_level_inflight() {
702+
let ring = make_ring(16);
703+
let mem = ring.mem();
704+
let mut consumer = VirtqConsumer::new(ring.layout(), mem, TestNotifier::new());
705+
706+
let mut desc = Descriptor::new(0x1000, 16, 0, DescFlags::INDIRECT);
707+
desc.mark_avail(true);
708+
ring.write_desc(0, desc);
709+
710+
assert!(matches!(
711+
consumer.poll(1024),
712+
Err(VirtqError::RingError(RingError::BadChain))
713+
));
714+
assert_eq!(consumer.inflight.count_ones(..), 0);
715+
assert_eq!(consumer.inner.num_inflight(), 0);
716+
}
717+
718+
#[test]
719+
fn test_villain_bad_chain_does_not_mark_high_level_inflight() {
720+
let ring = make_ring(16);
721+
let mem = ring.mem();
722+
let mut consumer = VirtqConsumer::new(ring.layout(), mem, TestNotifier::new());
723+
724+
let mut first = Descriptor::new(0x1000, 16, 0, DescFlags::NEXT | DescFlags::WRITE);
725+
first.mark_avail(true);
726+
ring.write_desc(0, first);
727+
728+
let mut second = Descriptor::new(0x2000, 16, 0, DescFlags::empty());
729+
second.mark_avail(true);
730+
ring.write_desc(1, second);
731+
732+
assert!(matches!(
733+
consumer.poll(1024),
734+
Err(VirtqError::RingError(RingError::BadChain))
735+
));
736+
assert_eq!(consumer.inflight.count_ones(..), 0);
737+
assert_eq!(consumer.inner.num_inflight(), 0);
738+
}
739+
700740
#[test]
701741
fn test_writable_chain_writes_single_segment() {
702742
let ring = make_ring(16);

src/hyperlight_common/src/virtq/desc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ bitflags! {
2929
/// Descriptor flags as defined by VIRTIO specification.
3030
///
3131
/// Note: The implementation never follows the indirect-table interpretation,
32-
/// so INDIRECT bit is effectively ignored.
32+
/// so descriptors carrying INDIRECT are rejected as malformed.
3333
#[repr(transparent)]
3434
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
3535
pub struct DescFlags: u16 {

0 commit comments

Comments
 (0)