Skip to content

Commit 1e680c6

Browse files
committed
kani: Update the iovec harnesses
We need to update the iovec harnesses to cover the generalized read_at and write_at functions. We need to work around some kani performance limitations again, where if we use the `SmallVec` optimization is used, then kani gets stuck in post processing (I will open an issue over at kani once this PR is merged). Signed-off-by: Patrick Roy <[email protected]>
1 parent 2740cac commit 1e680c6

File tree

1 file changed

+70
-21
lines changed

1 file changed

+70
-21
lines changed

src/vmm/src/devices/virtio/iovec.rs

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ pub enum IoVecError {
2222
GuestMemory(#[from] GuestMemoryError),
2323
}
2424

25+
// Using SmallVec in the kani proofs causes kani to use unbounded amounts of memory
26+
// during post-processing, and then crash.
27+
// TODO: remove new-type once kani performance regression are resolved
28+
#[cfg(kani)]
29+
type IoVecVec = Vec<iovec>;
30+
#[cfg(not(kani))]
31+
type IoVecVec = SmallVec<[iovec; 4]>;
32+
2533
/// This is essentially a wrapper of a `Vec<libc::iovec>` which can be passed to `libc::writev`.
2634
///
2735
/// It describes a buffer passed to us by the guest that is scattered across multiple
@@ -30,15 +38,15 @@ pub enum IoVecError {
3038
#[derive(Debug)]
3139
pub struct IoVecBuffer {
3240
// container of the memory regions included in this IO vector
33-
vecs: SmallVec<[iovec; 4]>,
41+
vecs: IoVecVec,
3442
// Total length of the IoVecBuffer
3543
len: usize,
3644
}
3745

3846
impl IoVecBuffer {
3947
/// Create an `IoVecBuffer` from a `DescriptorChain`
4048
pub fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
41-
let mut vecs = SmallVec::new();
49+
let mut vecs = IoVecVec::new();
4250
let mut len = 0usize;
4351

4452
let mut next_descriptor = Some(head);
@@ -165,15 +173,15 @@ impl IoVecBuffer {
165173
#[derive(Debug)]
166174
pub struct IoVecBufferMut {
167175
// container of the memory regions included in this IO vector
168-
vecs: SmallVec<[iovec; 4]>,
176+
vecs: IoVecVec,
169177
// Total length of the IoVecBufferMut
170178
len: usize,
171179
}
172180

173181
impl IoVecBufferMut {
174182
/// Create an `IoVecBufferMut` from a `DescriptorChain`
175183
pub fn from_descriptor_chain(head: DescriptorChain) -> Result<Self, IoVecError> {
176-
let mut vecs = SmallVec::new();
184+
let mut vecs = IoVecVec::new();
177185
let mut len = 0usize;
178186

179187
for desc in head {
@@ -547,9 +555,11 @@ mod verification {
547555
use std::mem::ManuallyDrop;
548556

549557
use libc::{c_void, iovec};
550-
use smallvec::SmallVec;
558+
use vm_memory::bitmap::BitmapSlice;
559+
use vm_memory::volatile_memory::Error;
560+
use vm_memory::VolatileSlice;
551561

552-
use super::{IoVecBuffer, IoVecBufferMut};
562+
use super::{IoVecBuffer, IoVecBufferMut, IoVecVec};
553563

554564
// Maximum memory size to use for our buffers. For the time being 1KB.
555565
const GUEST_MEMORY_SIZE: usize = 1 << 10;
@@ -561,7 +571,7 @@ mod verification {
561571
// >= 1.
562572
const MAX_DESC_LENGTH: usize = 4;
563573

564-
fn create_iovecs(mem: *mut u8, size: usize) -> (SmallVec<[iovec; 4]>, usize) {
574+
fn create_iovecs(mem: *mut u8, size: usize) -> (IoVecVec, usize) {
565575
let nr_descs: usize = kani::any_where(|&n| n <= MAX_DESC_LENGTH);
566576
let mut vecs: Vec<iovec> = Vec::with_capacity(nr_descs);
567577
let mut len = 0usize;
@@ -579,7 +589,7 @@ mod verification {
579589
len += iov_len;
580590
}
581591

582-
(vecs.into(), len)
592+
(vecs, len)
583593
}
584594

585595
impl kani::Arbitrary for IoVecBuffer {
@@ -608,6 +618,42 @@ mod verification {
608618
}
609619
}
610620

621+
// A mock for the Read-/WriteVolatile implementation for u8 slices that does
622+
// not go through rust-vmm's machinery (which would cause kani get stuck during post processing)
623+
struct KaniBuffer<'a>(&'a mut [u8]);
624+
625+
impl vm_memory::ReadVolatile for KaniBuffer<'_> {
626+
fn read_volatile<B: BitmapSlice>(
627+
&mut self,
628+
buf: &mut VolatileSlice<B>,
629+
) -> Result<usize, vm_memory::VolatileMemoryError> {
630+
let count = buf.len().min(self.0.len());
631+
unsafe {
632+
std::ptr::copy_nonoverlapping(self.0.as_ptr(), buf.ptr_guard_mut().as_ptr(), count);
633+
}
634+
self.0 = std::mem::take(&mut self.0).split_at_mut(count).1;
635+
Ok(count)
636+
}
637+
}
638+
639+
impl vm_memory::WriteVolatile for KaniBuffer<'_> {
640+
fn write_volatile<B: BitmapSlice>(
641+
&mut self,
642+
buf: &VolatileSlice<B>,
643+
) -> Result<usize, vm_memory::VolatileMemoryError> {
644+
let count = buf.len().min(self.0.len());
645+
unsafe {
646+
std::ptr::copy_nonoverlapping(
647+
buf.ptr_guard_mut().as_ptr(),
648+
self.0.as_mut_ptr(),
649+
count,
650+
);
651+
}
652+
self.0 = std::mem::take(&mut self.0).split_at_mut(count).1;
653+
Ok(count)
654+
}
655+
}
656+
611657
#[kani::proof]
612658
#[kani::unwind(5)]
613659
#[kani::solver(cadical)]
@@ -622,14 +668,15 @@ mod verification {
622668
// checking the data copied is not exactly trivial.
623669
//
624670
// What we can verify is the bytes that we read out from guest memory:
625-
// * `None`, if `offset` is past the guest memory.
626-
// * `Some(bytes)`, otherwise. In this case, `bytes` is:
627671
// - `buf.len()`, if `offset + buf.len() < iov.len()`;
628672
// - `iov.len() - offset`, otherwise.
629-
match iov.read_at(&mut buf, offset) {
630-
None => assert!(offset >= iov.len()),
631-
Some(bytes) => assert_eq!(bytes, buf.len().min(iov.len() - offset)),
632-
}
673+
// Furthermore, we know our Read-/WriteVolatile implementation above is infallible, so
674+
// provided that the logic inside read_volatile_at is correct, we should always get Ok(...)
675+
assert_eq!(
676+
iov.read_volatile_at(&mut KaniBuffer(&mut buf), offset, GUEST_MEMORY_SIZE)
677+
.unwrap(),
678+
buf.len().min(iov.len().saturating_sub(offset))
679+
);
633680
}
634681

635682
#[kani::proof]
@@ -638,21 +685,23 @@ mod verification {
638685
fn verify_write_to_iovec() {
639686
let mut iov_mut: IoVecBufferMut = kani::any();
640687

641-
let buf = kani::vec::any_vec::<u8, GUEST_MEMORY_SIZE>();
688+
let mut buf = kani::vec::any_vec::<u8, GUEST_MEMORY_SIZE>();
642689
let offset: usize = kani::any();
643690

644691
// We can't really check the contents that the operation here writes into `IoVecBufferMut`,
645692
// because our `IoVecBufferMut` being completely arbitrary can contain overlapping memory
646693
// regions, so checking the data copied is not exactly trivial.
647694
//
648695
// What we can verify is the bytes that we write into guest memory:
649-
// * `None`, if `offset` is past the guest memory.
650-
// * `Some(bytes)`, otherwise. In this case, `bytes` is:
651696
// - `buf.len()`, if `offset + buf.len() < iov.len()`;
652697
// - `iov.len() - offset`, otherwise.
653-
match iov_mut.write_at(&buf, offset) {
654-
None => assert!(offset >= iov_mut.len()),
655-
Some(bytes) => assert_eq!(bytes, buf.len().min(iov_mut.len() - offset)),
656-
}
698+
// Furthermore, we know our Read-/WriteVolatile implementation above is infallible, so
699+
// provided that the logic inside write_volatile_at is correct, we should always get Ok(...)
700+
assert_eq!(
701+
iov_mut
702+
.write_volatile_at(&mut KaniBuffer(&mut buf), offset, GUEST_MEMORY_SIZE)
703+
.unwrap(),
704+
buf.len().min(iov_mut.len().saturating_sub(offset))
705+
);
657706
}
658707
}

0 commit comments

Comments
 (0)