Skip to content

Commit 72ac687

Browse files
committed
chore(vmm): Add pointer alignment check for Queue
Raw pointers of `struct Queue` is 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 11d8a01 commit 72ac687

File tree

1 file changed

+21
-0
lines changed

1 file changed

+21
-0
lines changed

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

Lines changed: 21 additions & 0 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}
38+
PointerNotAligned(String),
3739
}
3840

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

328+
// All the above pointers is 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. Note that the type of `used_ring_ptr` is `*mut u8` since it is
331+
// a mix of `u16` and `u32` as documented in `struct Queue`, but at least must be aligned
332+
// for `u16` (maybe strictly `u32`?).
333+
if !self.desc_table_ptr.is_aligned() {
334+
return Err(QueueError::PointerNotAligned(
335+
"Queue::desc_table_ptr".into(),
336+
));
337+
}
338+
if !self.avail_ring_ptr.is_aligned() {
339+
return Err(QueueError::PointerNotAligned(
340+
"Queue::avail_ring_ptr".into(),
341+
));
342+
}
343+
if !self.used_ring_ptr.cast::<u16>().is_aligned() {
344+
return Err(QueueError::PointerNotAligned("Queue::used_ring_ptr".into()));
345+
}
346+
326347
// Disable it for kani tests, otherwise it will hit this assertion
327348
// and fail.
328349
#[cfg(not(kani))]

0 commit comments

Comments
 (0)