Skip to content

Commit 2f8155c

Browse files
roypatManciukic
authored andcommitted
refactor: factor out vring component alignment checks
Factor out the alignment checks on the vring components into get_slice_ptr, instead of writing them out 3 times in initialize(). While we're at it, also explain why its okay to only alignment check the GPA and not the HVAs as well. Signed-off-by: Patrick Roy <[email protected]>
1 parent 197f803 commit 2f8155c

File tree

1 file changed

+23
-33
lines changed

1 file changed

+23
-33
lines changed

src/vmm/src/devices/virtio/queue.rs

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::num::Wrapping;
99
use std::sync::atomic::{Ordering, fence};
1010

1111
use crate::logger::error;
12+
use crate::utils::u64_to_usize;
1213
use crate::vstate::memory::{Address, Bitmap, ByteValued, GuestAddress, GuestMemory};
1314

1415
pub const VIRTQ_DESC_F_NEXT: u16 = 0x1;
@@ -32,7 +33,7 @@ pub enum QueueError {
3233
/// Failed to write value into the virtio queue used ring: {0}
3334
MemoryError(#[from] vm_memory::GuestMemoryError),
3435
/// Pointer is not aligned properly: {0:#x} not {1}-byte aligned.
35-
PointerNotAligned(usize, u8),
36+
PointerNotAligned(usize, usize),
3637
}
3738

3839
/// Error type indicating the guest configured a virtio queue such that the avail_idx field would
@@ -310,31 +311,32 @@ impl Queue {
310311
+ std::mem::size_of::<u16>()
311312
}
312313

313-
fn get_slice_ptr<M: GuestMemory>(
314+
fn get_aligned_slice_ptr<T, M: GuestMemory>(
314315
&self,
315316
mem: &M,
316317
addr: GuestAddress,
317318
len: usize,
318-
) -> Result<*mut u8, QueueError> {
319+
alignment: usize,
320+
) -> Result<*mut T, QueueError> {
321+
// Guest memory base address is page aligned, so as long as alignment divides page size,
322+
// It suffices to check that the GPA is properly aligned (e.g. we don't need to recheck
323+
// the HVA).
324+
if addr.0 & (alignment as u64 - 1) != 0 {
325+
return Err(QueueError::PointerNotAligned(
326+
u64_to_usize(addr.0),
327+
alignment,
328+
));
329+
}
330+
319331
let slice = mem.get_slice(addr, len).map_err(QueueError::MemoryError)?;
320332
slice.bitmap().mark_dirty(0, len);
321-
Ok(slice.ptr_guard_mut().as_ptr())
333+
Ok(slice.ptr_guard_mut().as_ptr().cast())
322334
}
323335

324336
/// Set up pointers to the queue objects in the guest memory
325337
/// and mark memory dirty for those objects
326338
pub fn initialize<M: GuestMemory>(&mut self, mem: &M) -> Result<(), QueueError> {
327-
self.desc_table_ptr = self
328-
.get_slice_ptr(mem, self.desc_table_address, self.desc_table_size())?
329-
.cast();
330-
self.avail_ring_ptr = self
331-
.get_slice_ptr(mem, self.avail_ring_address, self.avail_ring_size())?
332-
.cast();
333-
self.used_ring_ptr = self
334-
.get_slice_ptr(mem, self.used_ring_address, self.used_ring_size())?
335-
.cast();
336-
337-
// All the above pointers are expected to be aligned properly; otherwise some methods (e.g.
339+
// All the below pointers are verified to be aligned properly; otherwise some methods (e.g.
338340
// `read_volatile()`) will panic. Such an unalignment is possible when restored from a
339341
// broken/fuzzed snapshot.
340342
//
@@ -347,24 +349,12 @@ impl Queue {
347349
// > Available Ring 2
348350
// > Used Ring 4
349351
// > ================ ==========
350-
if !self.desc_table_ptr.cast::<u128>().is_aligned() {
351-
return Err(QueueError::PointerNotAligned(
352-
self.desc_table_ptr as usize,
353-
16,
354-
));
355-
}
356-
if !self.avail_ring_ptr.is_aligned() {
357-
return Err(QueueError::PointerNotAligned(
358-
self.avail_ring_ptr as usize,
359-
2,
360-
));
361-
}
362-
if !self.used_ring_ptr.cast::<u32>().is_aligned() {
363-
return Err(QueueError::PointerNotAligned(
364-
self.used_ring_ptr as usize,
365-
4,
366-
));
367-
}
352+
self.desc_table_ptr =
353+
self.get_aligned_slice_ptr(mem, self.desc_table_address, self.desc_table_size(), 16)?;
354+
self.avail_ring_ptr =
355+
self.get_aligned_slice_ptr(mem, self.avail_ring_address, self.avail_ring_size(), 2)?;
356+
self.used_ring_ptr =
357+
self.get_aligned_slice_ptr(mem, self.used_ring_address, self.used_ring_size(), 4)?;
368358

369359
Ok(())
370360
}

0 commit comments

Comments
 (0)