Skip to content

Commit 052b069

Browse files
bchaliosShadowCurse
authored andcommitted
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. Co-authored-by: Babis Chalios <bchalios@amazon.es> Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
1 parent 81edbaa commit 052b069

File tree

2 files changed

+68
-4
lines changed

2 files changed

+68
-4
lines changed

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

Lines changed: 3 additions & 3 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.

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,7 @@ mod tests {
793793
}
794794

795795
#[cfg(kani)]
796+
#[allow(dead_code)] // Avoid warning when using stubs
796797
mod verification {
797798
use std::mem::ManuallyDrop;
798799

@@ -801,7 +802,9 @@ mod verification {
801802
use vm_memory::VolatileSlice;
802803

803804
use super::{IoVecBuffer, IoVecBufferMut};
805+
use crate::arch::PAGE_SIZE;
804806
use crate::devices::virtio::iov_deque::IovDeque;
807+
use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE;
805808

806809
// Maximum memory size to use for our buffers. For the time being 1KB.
807810
const GUEST_MEMORY_SIZE: usize = 1 << 10;
@@ -813,6 +816,50 @@ mod verification {
813816
// >= 1.
814817
const MAX_DESC_LENGTH: usize = 4;
815818

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

893+
fn create_iov_deque() -> IovDeque {
894+
// SAFETY: safe because the layout has non-zero size
895+
let mem = unsafe {
896+
std::alloc::alloc(std::alloc::Layout::from_size_align_unchecked(
897+
2 * PAGE_SIZE,
898+
PAGE_SIZE,
899+
))
900+
};
901+
IovDeque {
902+
iov: mem.cast(),
903+
start: kani::any_where(|&start| start < FIRECRACKER_MAX_QUEUE_SIZE),
904+
len: 0,
905+
}
906+
}
907+
846908
fn create_iovecs_mut(mem: *mut u8, size: usize, nr_descs: usize) -> (IovDeque, u32) {
847-
let mut vecs = IovDeque::new().unwrap();
909+
let mut vecs = create_iov_deque();
848910
let mut len = 0u32;
849911
for _ in 0..nr_descs {
850912
// The `IoVecBufferMut` constructors ensure that the memory region described by every
@@ -953,6 +1015,7 @@ mod verification {
9531015
#[kani::proof]
9541016
#[kani::unwind(5)]
9551017
#[kani::solver(cadical)]
1018+
#[kani::stub(IovDeque::push_back, stubs::push_back)]
9561019
fn verify_write_to_iovec() {
9571020
for nr_descs in 0..MAX_DESC_LENGTH {
9581021
let mut iov_mut = IoVecBufferMut::any_of_length(nr_descs);
@@ -981,6 +1044,7 @@ mod verification {
9811044
.unwrap(),
9821045
buf.len().min(iov_mut.len().saturating_sub(offset) as usize)
9831046
);
1047+
std::mem::forget(iov_mut.vecs);
9841048
}
9851049
}
9861050
}

0 commit comments

Comments
 (0)