Skip to content

Commit

Permalink
Mitigate possible disk image misuse
Browse files Browse the repository at this point in the history
If the user specifies a Raw disk image, there are possible scenarios
where the guest could misuse the disk image. In one such scenario, the
guest could write a different image header into the first sector of the
file. Mitigate this by forcing the guest to write to the first four
bytes as a whole, and not byte-by-byte in any order. Additionally,
mitigate this by verifying that if the offset into the disk image is
zero and the length of the buffer to be written is greater than or equal
to four, probe the buffer's first four bytes to make sure it's not a
QCOW magic string ("QFI\xfb"). If any of these conditions are met, then
reject the write.

Signed-off-by: Jake Correnti <[email protected]>
  • Loading branch information
jakecorrenti committed Nov 20, 2024
1 parent 71d75e7 commit 7593e31
Showing 1 changed file with 62 additions and 6 deletions.
68 changes: 62 additions & 6 deletions src/devices/src/virtio/file_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ use std::io::{Error, ErrorKind, Result};
use std::os::unix::io::AsRawFd;

#[cfg(feature = "blk")]
use imago::io_buffers::{IoVector, IoVectorMut};
use imago::io_buffers::{IoBuffer, IoVector, IoVectorMut};
use vm_memory::VolatileSlice;

use libc::{c_int, c_void, read, readv, size_t, write, writev};

use super::bindings::{off64_t, pread64, preadv64, pwrite64, pwritev64};
#[cfg(feature = "blk")]
use super::block::device::DiskProperties;
use super::block::{device::DiskProperties, disk};

/// A trait for setting the size of a file.
/// This is equivalent to File's `set_len` method, but
Expand Down Expand Up @@ -440,9 +440,12 @@ impl FileReadWriteAtVolatile for DiskProperties {
}

fn write_at_volatile(&self, slice: VolatileSlice, offset: u64) -> Result<usize> {
let slices = &[&slice];
let iovec = IoVector::from_volatile_slice(slices);
self.file().writev(iovec.0, offset)?;
let slices = &[slice];

let mut _copied_head: Option<IoBuffer> = None;
let iovec = create_iovec(self, slices, offset, &mut _copied_head)?;

self.file().writev(iovec, offset)?;
Ok(slice.len())
}

Expand All @@ -451,7 +454,9 @@ impl FileReadWriteAtVolatile for DiskProperties {
return Ok(0);
}

let (iovec, _guard) = IoVector::from_volatile_slice(bufs);
let mut _copied_head: Option<IoBuffer> = None;
let iovec = create_iovec(self, bufs, offset, &mut _copied_head)?;

let full_length = iovec
.len()
.try_into()
Expand All @@ -460,3 +465,54 @@ impl FileReadWriteAtVolatile for DiskProperties {
Ok(full_length)
}
}

#[cfg(feature = "blk")]
fn create_iovec<'a>(
disk: &DiskProperties,
bufs: &'a [VolatileSlice],
offset: u64,
head_buffer: &'a mut Option<IoBuffer>,
) -> Result<IoVector<'a>> {
match disk.image_format() {
disk::ImageType::Raw => {
let (iovec, _guard) = IoVector::from_volatile_slice(bufs);
guard_header_bytes(disk.file(), iovec, offset, head_buffer)
}
_ => Ok(IoVector::from_volatile_slice(bufs).0),
}
}

#[cfg(feature = "blk")]
fn guard_header_bytes<'a>(
image: &imago::SyncFormatAccess<imago::file::File>,
iovec: IoVector<'a>,
offset: u64,
head_buffer: &'a mut Option<IoBuffer>,
) -> Result<IoVector<'a>> {
if (1..4).contains(&offset) || (offset + iovec.len() < 4) {
// Make sure the guest writes to the first four bytes as a whole. This is done to prevent a
// Raw disk image byte-by-byte, and in any order, writing the header of a formatted disk
// image into the first sector of the disk image.
return Err(Error::new(
ErrorKind::InvalidInput,
"partial write to first four bytes of disk image",
));
}

if offset > 0 {
return Ok(iovec);
}

let (head, tail) = iovec.split_at(std::cmp::max(image.mem_align(), 4) as u64);
*head_buffer = Some(head.try_into_owned(image.mem_align())?);
let head_slice = head_buffer.as_ref().unwrap().as_ref().into_slice();

if head_slice.get(0..4) == Some(&disk::QCOW_MAGIC.to_be_bytes()) {
return Err(Error::new(
ErrorKind::InvalidData,
"writing QCOW2 header to disk image",
));
}

Ok(tail.with_inserted(0, head_slice))
}

0 comments on commit 7593e31

Please sign in to comment.