Skip to content

Commit 17c443c

Browse files
committed
iovec: fix kani proof for write_volatile_at
IoVecBufferMut type now uses IovDeque as its backing memory. IovDeque is performing a custom memory allocation, using memfd_create() and a combination of mmap() calls in order to provide a memory layout where the iovec objects stored in the IovDeque will always be in consecutive memory. kani doesn't really get along with these system calls, which breaks our proof for IoVecBufferMut::write_volatile_at. Substitute memory allocation and deallocation with plain calls to std::alloc::(de)alloc when we run kani proofs. Also provide a stub for IovDeque::push_back to provide the same memory layout invariants. Signed-off-by: Babis Chalios <[email protected]>
1 parent cb65529 commit 17c443c

File tree

2 files changed

+84
-5
lines changed

2 files changed

+84
-5
lines changed

src/vmm/src/devices/virtio/iov_deque.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ pub enum IovDequeError {
7171
// so making a slice out of them does not require any copies.
7272
#[derive(Debug)]
7373
pub struct IovDeque {
74-
iov: *mut libc::iovec,
75-
start: u16,
76-
len: u16,
74+
pub iov: *mut libc::iovec,
75+
pub start: u16,
76+
pub len: u16,
7777
}
7878

7979
// SAFETY: This is `Send`. We hold sole ownership of the underlying buffer.
@@ -215,7 +215,7 @@ impl IovDeque {
215215

216216
/// Returns `true` if the [`IovDeque`] is full, `false` otherwise
217217
#[inline(always)]
218-
fn is_full(&self) -> bool {
218+
pub fn is_full(&self) -> bool {
219219
self.len() == FIRECRACKER_MAX_QUEUE_SIZE
220220
}
221221

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

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,7 @@ mod tests {
800800
}
801801

802802
#[cfg(kani)]
803+
#[allow(dead_code)] // Avoid warning when using stubs
803804
mod verification {
804805
use std::mem::ManuallyDrop;
805806

@@ -808,7 +809,9 @@ mod verification {
808809
use vm_memory::VolatileSlice;
809810

810811
use super::{IoVecBuffer, IoVecBufferMut, IoVecVec};
812+
use crate::arch::PAGE_SIZE;
811813
use crate::devices::virtio::iov_deque::IovDeque;
814+
use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE;
812815

813816
// Maximum memory size to use for our buffers. For the time being 1KB.
814817
const GUEST_MEMORY_SIZE: usize = 1 << 10;
@@ -820,6 +823,50 @@ mod verification {
820823
// >= 1.
821824
const MAX_DESC_LENGTH: usize = 4;
822825

826+
mod stubs {
827+
use super::*;
828+
829+
/// This is a stub for the `IovDeque::push_back` method.
830+
///
831+
/// `IovDeque` relies on a special allocation of two pages of virtual memory, where both of
832+
/// these point to the same underlying physical page. This way, the contents of the first
833+
/// page of virtual memory are automatically mirrored in the second virtual page. We do
834+
/// that in order to always have the elements that are currently in the ring buffer in
835+
/// consecutive (virtual) memory.
836+
///
837+
/// To build this particular memory layout we create a new `memfd` object, allocate memory
838+
/// with `mmap` and call `mmap` again to make sure both pages point to the page allocated
839+
/// via the `memfd` object. These ffi calls make kani complain, so here we mock the
840+
/// `IovDeque` object memory with a normal memory allocation of two pages worth of data.
841+
///
842+
/// This stub helps imitate the effect of mirroring without all the elaborate memory
843+
/// allocation trick.
844+
pub fn push_back(deque: &mut IovDeque, iov: iovec) {
845+
// This should NEVER happen, since our ring buffer is as big as the maximum queue size.
846+
// We also check for the sanity of the VirtIO queues, in queue.rs, which means that if
847+
// we ever try to add something in a full ring buffer, there is an internal
848+
// bug in the device emulation logic. Panic here because the device is
849+
// hopelessly broken.
850+
assert!(
851+
!deque.is_full(),
852+
"The number of `iovec` objects is bigger than the available space"
853+
);
854+
855+
let offset = (deque.start + deque.len) as usize;
856+
let mirror = if offset >= FIRECRACKER_MAX_QUEUE_SIZE as usize {
857+
offset - FIRECRACKER_MAX_QUEUE_SIZE as usize
858+
} else {
859+
offset + FIRECRACKER_MAX_QUEUE_SIZE as usize
860+
};
861+
862+
// SAFETY: self.iov is a valid pointer and `self.start + self.len` is within range (we
863+
// asserted before that the buffer is not full).
864+
unsafe { deque.iov.add(offset).write_volatile(iov) };
865+
unsafe { deque.iov.add(mirror).write_volatile(iov) };
866+
deque.len += 1;
867+
}
868+
}
869+
823870
fn create_iovecs(mem: *mut u8, size: usize, nr_descs: usize) -> (IoVecVec, u32) {
824871
let mut vecs: Vec<iovec> = Vec::with_capacity(nr_descs);
825872
let mut len = 0u32;
@@ -850,8 +897,37 @@ mod verification {
850897
}
851898
}
852899

900+
fn create_iov_deque() -> IovDeque {
901+
// SAFETY: safe because the layout has non-zero size
902+
let mem = unsafe {
903+
std::alloc::alloc(std::alloc::Layout::from_size_align_unchecked(
904+
2 * PAGE_SIZE,
905+
PAGE_SIZE,
906+
))
907+
};
908+
IovDeque {
909+
iov: mem.cast(),
910+
start: kani::any_where(|&start| start < FIRECRACKER_MAX_QUEUE_SIZE),
911+
len: 0,
912+
}
913+
}
914+
915+
/// During `IovDeque::drop` we call `memunmap` to deallocate the backing memory of the
916+
/// `IovDeque`. For the purpose of the proof we substitute the allocation with a normal
917+
/// allocation, so deallocate the memory here.
918+
pub fn drop_iov_deque(deque: &mut IovDeque) {
919+
// We previously allocated this memory with a call to `std::alloc::alloc` with the same
920+
// allocator
921+
unsafe {
922+
std::alloc::dealloc(
923+
deque.iov.cast(),
924+
std::alloc::Layout::from_size_align_unchecked(2 * PAGE_SIZE, PAGE_SIZE),
925+
)
926+
}
927+
}
928+
853929
fn create_iovecs_mut(mem: *mut u8, size: usize, nr_descs: usize) -> (IovDeque, u32) {
854-
let mut vecs = IovDeque::new().unwrap();
930+
let mut vecs = create_iov_deque();
855931
let mut len = 0u32;
856932
for _ in 0..nr_descs {
857933
// The `IoVecBufferMut` constructors ensure that the memory region described by every
@@ -960,6 +1036,7 @@ mod verification {
9601036
#[kani::proof]
9611037
#[kani::unwind(5)]
9621038
#[kani::solver(cadical)]
1039+
#[kani::stub(IovDeque::push_back, stubs::push_back)]
9631040
fn verify_write_to_iovec() {
9641041
for nr_descs in 0..MAX_DESC_LENGTH {
9651042
let mut iov_mut = IoVecBufferMut::any_of_length(nr_descs);
@@ -984,6 +1061,8 @@ mod verification {
9841061
.unwrap(),
9851062
buf.len().min(iov_mut.len().saturating_sub(offset) as usize)
9861063
);
1064+
drop_iov_deque(&mut iov_mut.vecs);
1065+
std::mem::forget(iov_mut.vecs);
9871066
}
9881067
}
9891068
}

0 commit comments

Comments
 (0)