Skip to content

Commit 7b01bdc

Browse files
zulinx86bchalios
authored andcommitted
chore(vmm): Add pointer alignment check for Queue
Raw pointers of `struct Queue` are assumed to be aligned properly; otherwise some methods (e.g. `read_volatile()`) will panic. Such an alignment is possible when restored from a broken/fuzzed snapshot. Add pointer alignment check and exit with an error early instead of panic. Signed-off-by: Takahiro Itazuri <[email protected]>
1 parent cea24c5 commit 7b01bdc

File tree

1 file changed

+89
-6
lines changed

1 file changed

+89
-6
lines changed

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

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ pub enum QueueError {
3434
DescIndexOutOfBounds(u16),
3535
/// Failed to write value into the virtio queue used ring: {0}
3636
MemoryError(#[from] vm_memory::GuestMemoryError),
37+
/// Pointer is not aligned properly: {0:#x} not {1}-byte aligned.
38+
PointerNotAligned(usize, u8),
3739
}
3840

3941
/// A virtio descriptor constraints with C representative.
@@ -323,11 +325,36 @@ impl Queue {
323325
.get_slice_ptr(mem, self.used_ring_address, self.used_ring_size())?
324326
.cast();
325327

326-
// Disable it for kani tests, otherwise it will hit this assertion
327-
// and fail.
328-
#[cfg(not(kani))]
329-
if self.actual_size() < self.len() {
330-
return Err(QueueError::InvalidQueueSize(self.len(), self.actual_size()));
328+
// All the above pointers are expected to be aligned properly; otherwise some methods (e.g.
329+
// `read_volatile()`) will panic. Such an unalignment is possible when restored from a
330+
// broken/fuzzed snapshot.
331+
//
332+
// Specification of those pointers' alignments
333+
// https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-350007
334+
// > ================ ==========
335+
// > Virtqueue Part Alignment
336+
// > ================ ==========
337+
// > Descriptor Table 16
338+
// > Available Ring 2
339+
// > Used Ring 4
340+
// > ================ ==========
341+
if !self.desc_table_ptr.cast::<u128>().is_aligned() {
342+
return Err(QueueError::PointerNotAligned(
343+
self.desc_table_ptr as usize,
344+
16,
345+
));
346+
}
347+
if !self.avail_ring_ptr.is_aligned() {
348+
return Err(QueueError::PointerNotAligned(
349+
self.avail_ring_ptr as usize,
350+
2,
351+
));
352+
}
353+
if !self.used_ring_ptr.cast::<u32>().is_aligned() {
354+
return Err(QueueError::PointerNotAligned(
355+
self.used_ring_ptr as usize,
356+
4,
357+
));
331358
}
332359

333360
Ok(())
@@ -1234,7 +1261,6 @@ mod verification {
12341261

12351262
#[cfg(test)]
12361263
mod tests {
1237-
12381264
use vm_memory::Bytes;
12391265

12401266
pub use super::*;
@@ -1644,6 +1670,63 @@ mod tests {
16441670
assert_eq!(q.used_ring_avail_event_get(), 1);
16451671
}
16461672

1673+
#[test]
1674+
fn test_initialize_with_aligned_pointer() {
1675+
let mut q = Queue::new(0);
1676+
1677+
let random_addr = 0x321;
1678+
// Descriptor table must be 16-byte aligned.
1679+
q.desc_table_address = GuestAddress(random_addr / 16 * 16);
1680+
// Available ring must be 2-byte aligned.
1681+
q.avail_ring_address = GuestAddress(random_addr / 2 * 2);
1682+
// Used ring must be 4-byte aligned.
1683+
q.avail_ring_address = GuestAddress(random_addr / 4 * 4);
1684+
1685+
let mem = single_region_mem(0x1000);
1686+
q.initialize(&mem).unwrap();
1687+
}
1688+
1689+
#[test]
1690+
fn test_initialize_with_misaligned_pointer() {
1691+
let mut q = Queue::new(0);
1692+
let mem = single_region_mem(0x1000);
1693+
1694+
// Descriptor table must be 16-byte aligned.
1695+
q.desc_table_address = GuestAddress(0xb);
1696+
match q.initialize(&mem) {
1697+
Ok(_) => panic!("Unexpected success"),
1698+
Err(QueueError::PointerNotAligned(addr, alignment)) => {
1699+
assert_eq!(addr % 16, 0xb);
1700+
assert_eq!(alignment, 16);
1701+
}
1702+
Err(e) => panic!("Unexpected error {e:#?}"),
1703+
}
1704+
q.desc_table_address = GuestAddress(0x0);
1705+
1706+
// Available ring must be 2-byte aligned.
1707+
q.avail_ring_address = GuestAddress(0x1);
1708+
match q.initialize(&mem) {
1709+
Ok(_) => panic!("Unexpected success"),
1710+
Err(QueueError::PointerNotAligned(addr, alignment)) => {
1711+
assert_eq!(addr % 2, 0x1);
1712+
assert_eq!(alignment, 2);
1713+
}
1714+
Err(e) => panic!("Unexpected error {e:#?}"),
1715+
}
1716+
q.avail_ring_address = GuestAddress(0x0);
1717+
1718+
// Used ring must be 4-byte aligned.
1719+
q.used_ring_address = GuestAddress(0x3);
1720+
match q.initialize(&mem) {
1721+
Ok(_) => panic!("unexpected success"),
1722+
Err(QueueError::PointerNotAligned(addr, alignment)) => {
1723+
assert_eq!(addr % 4, 0x3);
1724+
assert_eq!(alignment, 4);
1725+
}
1726+
Err(e) => panic!("Unexpected error {e:#?}"),
1727+
}
1728+
}
1729+
16471730
#[test]
16481731
fn test_queue_error_display() {
16491732
let err = QueueError::MemoryError(vm_memory::GuestMemoryError::InvalidGuestAddress(

0 commit comments

Comments
 (0)