Fix soundness issues with MMIO and shared memory

Cargo.toml

@@ -8,6 +8,5 @@ description = "VirtIO guest drivers."
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-volatile = "0.3"
 log = "0.4"
 bitflags = "1.3"

src/blk.rs

@@ -1,22 +1,22 @@
 use super::*;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
+use crate::volatile::{volread, Volatile};
 use bitflags::*;
 use core::hint::spin_loop;
 use log::*;
-use volatile::Volatile;
 
 /// The virtio block device is a simple virtual block device (ie. disk).
 ///
 /// Read and write requests (and other exotic requests) are placed in the queue,
 /// and serviced (probably out of order) by the device except where noted.
-pub struct VirtIOBlk<'a, H: Hal, T: Transport> {
+pub struct VirtIOBlk<H: Hal, T: Transport> {
  transport: T,
- queue: VirtQueue<'a, H>,
+ queue: VirtQueue<H>,
  capacity: usize,
 }
 
-impl<H: Hal, T: Transport> VirtIOBlk<'_, H, T> {
+impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
  /// Create a new VirtIO-Blk driver.
  pub fn new(mut transport: T) -> Result<Self> {
  transport.begin_init(|features| {
@@ -28,21 +28,19 @@ impl<H: Hal, T: Transport> VirtIOBlk<'_, H, T> {
  });
 
  // read configuration space
- let config_space = transport.config_space().cast::<BlkConfig>();
- let config = unsafe { config_space.as_ref() };
+ let config = transport.config_space().cast::<BlkConfig>();
  info!("config: {:?}", config);
- info!(
- "found a block device of size {}KB",
- config.capacity.read() / 2
- );
+ // Safe because config is a valid pointer to the device configuration space.
+ let capacity = unsafe { volread!(config, capacity) };
+ info!("found a block device of size {}KB", capacity / 2);
 
  let queue = VirtQueue::new(&mut transport, 0, 16)?;
  transport.finish_init();
 
  Ok(VirtIOBlk {
  transport,
  queue,
- capacity: config.capacity.read() as usize,
+ capacity: capacity as usize,
  })
  }
 

src/console.rs

@@ -1,10 +1,10 @@
 use super::*;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
+use crate::volatile::{ReadOnly, WriteOnly};
 use bitflags::*;
 use core::{fmt, hint::spin_loop};
 use log::*;
-use volatile::{ReadOnly, WriteOnly};
 
 const QUEUE_RECEIVEQ_PORT_0: usize = 0;
 const QUEUE_TRANSMITQ_PORT_0: usize = 1;
@@ -14,8 +14,8 @@ const QUEUE_SIZE: u16 = 2;
 /// Emergency and cols/rows unimplemented.
 pub struct VirtIOConsole<'a, H: Hal, T: Transport> {
  transport: T,
- receiveq: VirtQueue<'a, H>,
- transmitq: VirtQueue<'a, H>,
+ receiveq: VirtQueue<H>,
+ transmitq: VirtQueue<H>,
  queue_buf_dma: DMA<H>,
  queue_buf_rx: &'a mut [u8],
  cursor: usize,
@@ -161,7 +161,7 @@ mod tests {
  cols: ReadOnly::new(0),
  rows: ReadOnly::new(0),
  max_nr_ports: ReadOnly::new(0),
- emerg_wr: WriteOnly::new(0),
+ emerg_wr: WriteOnly::default(),
  };
  let state = Arc::new(Mutex::new(State {
  status: DeviceStatus::empty(),

src/gpu.rs

@@ -1,10 +1,10 @@
 use super::*;
 use crate::queue::VirtQueue;
 use crate::transport::Transport;
+use crate::volatile::{ReadOnly, Volatile, WriteOnly};
 use bitflags::*;
 use core::{fmt, hint::spin_loop};
 use log::*;
-use volatile::{ReadOnly, Volatile, WriteOnly};
 
 /// A virtio based graphics adapter.
 ///
@@ -21,9 +21,9 @@ pub struct VirtIOGpu<'a, H: Hal, T: Transport> {
  /// DMA area of cursor image buffer.
  cursor_buffer_dma: Option<DMA<H>>,
  /// Queue for sending control commands.
- control_queue: VirtQueue<'a, H>,
+ control_queue: VirtQueue<H>,
  /// Queue for sending cursor commands.
- cursor_queue: VirtQueue<'a, H>,
+ cursor_queue: VirtQueue<H>,
  /// Queue buffer DMA
  queue_buf_dma: DMA<H>,
  /// Send buffer for queue.

src/input.rs

@@ -1,23 +1,23 @@
 use super::*;
 use crate::transport::Transport;
+use crate::volatile::{volread, volwrite, ReadOnly, WriteOnly};
 use alloc::boxed::Box;
 use bitflags::*;
 use log::*;
-use volatile::{ReadOnly, WriteOnly};
 
 /// Virtual human interface devices such as keyboards, mice and tablets.
 ///
 /// An instance of the virtio device represents one such input device.
 /// Device behavior mirrors that of the evdev layer in Linux,
 /// making pass-through implementations on top of evdev easy.
-pub struct VirtIOInput<'a, H: Hal, T: Transport> {
+pub struct VirtIOInput<H: Hal, T: Transport> {
  transport: T,
- event_queue: VirtQueue<'a, H>,
- status_queue: VirtQueue<'a, H>,
+ event_queue: VirtQueue<H>,
+ status_queue: VirtQueue<H>,
  event_buf: Box<[InputEvent; 32]>,
 }
 
-impl<H: Hal, T: Transport> VirtIOInput<'_, H, T> {
+impl<H: Hal, T: Transport> VirtIOInput<H, T> {
  /// Create a new VirtIO-Input driver.
  pub fn new(mut transport: T) -> Result<Self> {
  let mut event_buf = Box::new([InputEvent::default(); QUEUE_SIZE]);
@@ -75,12 +75,16 @@ impl<H: Hal, T: Transport> VirtIOInput<'_, H, T> {
  subsel: u8,
  out: &mut [u8],
  ) -> u8 {
- let mut config_space = self.transport.config_space().cast::<Config>();
- let config = unsafe { config_space.as_mut() };
- config.select.write(select as u8);
- config.subsel.write(subsel);
- let size = config.size.read();
- let data = config.data.read();
+ let config = self.transport.config_space().cast::<Config>();
+ let size;
+ let data;
+ // Safe because config points to a valid MMIO region for the config space.
+ unsafe {
+ volwrite!(config, select, select as u8);
+ volwrite!(config, subsel, subsel);
+ size = volread!(config, size);
+ data = volread!(config, data);
+ }
  out[..size as usize].copy_from_slice(&data[..size as usize]);
  size
  }

src/lib.rs

@@ -15,6 +15,7 @@ mod input;
 mod net;
 mod queue;
 mod transport;
+mod volatile;
 
 pub use self::blk::{BlkResp, RespStatus, VirtIOBlk};
 pub use self::console::VirtIOConsole;

src/net.rs

@@ -2,10 +2,10 @@ use core::mem::{size_of, MaybeUninit};
 
 use super::*;
 use crate::transport::Transport;
+use crate::volatile::{volread, ReadOnly, Volatile};
 use bitflags::*;
 use core::hint::spin_loop;
 use log::*;
-use volatile::{ReadOnly, Volatile};
 
 /// The virtio network device is a virtual ethernet card.
 ///
@@ -14,14 +14,14 @@ use volatile::{ReadOnly, Volatile};
 /// Empty buffers are placed in one virtqueue for receiving packets, and
 /// outgoing packets are enqueued into another for transmission in that order.
 /// A third command queue is used to control advanced filtering features.
-pub struct VirtIONet<'a, H: Hal, T: Transport> {
+pub struct VirtIONet<H: Hal, T: Transport> {
  transport: T,
  mac: EthernetAddress,
- recv_queue: VirtQueue<'a, H>,
- send_queue: VirtQueue<'a, H>,
+ recv_queue: VirtQueue<H>,
+ send_queue: VirtQueue<H>,
 }
 
-impl<H: Hal, T: Transport> VirtIONet<'_, H, T> {
+impl<H: Hal, T: Transport> VirtIONet<H, T> {
  /// Create a new VirtIO-Net driver.
  pub fn new(mut transport: T) -> Result<Self> {
  transport.begin_init(|features| {
@@ -31,10 +31,13 @@ impl<H: Hal, T: Transport> VirtIONet<'_, H, T> {
  (features & supported_features).bits()
  });
  // read configuration space
- let config_space = transport.config_space().cast::<Config>();
- let config = unsafe { config_space.as_ref() };
- let mac = config.mac.read();
- debug!("Got MAC={:?}, status={:?}", mac, config.status.read());
+ let config = transport.config_space().cast::<Config>();
+ let mac;
+ // Safe because config points to a valid MMIO region for the config space.
+ unsafe {
+ mac = volread!(config, mac);
+ debug!("Got MAC={:?}, status={:?}", mac, volread!(config, status));
+ }
 
  let queue_num = 2; // for simplicity
  let recv_queue = VirtQueue::new(&mut transport, QUEUE_RECEIVE, queue_num)?;