uhyve icon indicating copy to clipboard operation
uhyve copied to clipboard

Maybe unsound in virtio's write_ fn groups

Open lwz23 opened this issue 11 months ago • 1 comments

Hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project. I notice the following code:

pub fn write_selected_queue(&mut self, dest: &[u8]) {
		self.selected_queue_num = unsafe {
			#[allow(clippy::cast_ptr_alignment)]
			*(dest.as_ptr() as *const u16)
		};
	}

	// Register virtqueue
	pub fn write_pfn(&mut self, dest: &[u8], mem: &MmapMemory) {
		let status = self.read_status_reg();
		if status & STATUS_FEATURES_OK != 0
			&& status & STATUS_DRIVER_OK == 0
			&& self.selected_queue_num as usize == self.virt_queues.len()
		{
			let gpa = GuestPhysAddr::new(unsafe {
				#[allow(clippy::cast_ptr_alignment)]
				*(dest.as_ptr() as *const u64)
			});
			let hva = mem.host_address(gpa).unwrap();
			let queue = unsafe { Virtqueue::new(hva as *mut u8, QUEUE_LIMIT) };
			self.virt_queues.push(queue);
		}
	}

	pub fn write_requested_features(&mut self, dest: &[u8]) {
		if self.read_status_reg() == STATUS_ACKNOWLEDGE | STATUS_DRIVER {
			let requested_features = unsafe {
				#[allow(clippy::cast_ptr_alignment)]
				*(dest.as_ptr() as *const u32)
			};
			self.requested_features =
				(self.requested_features | requested_features) & HOST_FEATURES;
		}
	}

the problem is dest.as_ptr() as *const u16/32/64 , indices is u8 which is 1-bytes aligned, and convert it to a higher align type may result in unaligned problem which volited to safety precondition of deference a raw pointer. According to rust's security specifications, any pub function that can cause UB should be marked as unsafe, so I decide to report it.

POC

pub struct VirtioNetPciDevice {
	//registers: PciRegisters, //Add more
	requested_features: u32,
	selected_queue_num: u16,
	//virt_queues: Vec<Virtqueue>,
	//iface: Option<Mutex<Iface>>,
	mac_addr: [u8; 6],
}
impl VirtioNetPciDevice {
    pub fn write_selected_queue(&mut self, dest: &[u8]) {
        self.selected_queue_num = unsafe {
            #[allow(clippy::cast_ptr_alignment)]
            *(dest.as_ptr() as *const u16)
        };
    }
    
}
fn main() {
    let mut device = VirtioNetPciDevice {
        requested_features: 0,
        selected_queue_num: 0,
        mac_addr: [0; 6],
    };


    let mut dest = [0u8; 3]; 

    dest[1] = 1;
    dest[2] = 2;

    device.write_selected_queue(&dest); 
}

result:

PS E:\Github\lwz> cargo +nightly miri run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s
     Running `C:\Users\ROG\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe runner target\miri\x86_64-pc-windows-msvc\debug\lwz.exe`
warning: fields `requested_features` and `mac_addr` are never read
 --> src\main.rs:3:2
  |
1 | pub struct VirtioNetPciDevice {
  |            ------------------ fields in this struct
2 |     //registers: PciRegisters, //Add more
3 |     requested_features: u32,
  |     ^^^^^^^^^^^^^^^^^^
...
7 |     mac_addr: [u8; 6],
  |     ^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

error: Undefined Behavior: accessing memory based on pointer with alignment 1, but alignment 2 is required
  --> src\main.rs:13:13
   |
13 |             *(dest.as_ptr() as *const u16)
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory based on pointer with alignment 1, but alignment 2 is required
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `VirtioNetPciDevice::write_selected_queue` at src\main.rs:13:13: 13:43
note: inside `main`
  --> src\main.rs:31:5
   |
31 |     device.write_selected_queue(&dest);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error; 1 warning emitted

error: process didn't exit successfully: `C:\Users\ROG\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo-miri.exe runner target\miri\x86_64-pc-windows-msvc\debug\lwz.exe` (exit code: 1)
PS E:\Github\lwz> 

lwz23 avatar Jan 06 '25 08:01 lwz23

maybe same problem for https://github.com/hermit-os/uhyve/blob/8d2f7e5cc9a17bdae285a87f10a6634284f42129/src/virtqueue.rs#L38 https://github.com/hermit-os/uhyve/blob/8d2f7e5cc9a17bdae285a87f10a6634284f42129/src/virtqueue.rs#L46 https://github.com/hermit-os/uhyve/blob/8d2f7e5cc9a17bdae285a87f10a6634284f42129/src/virtqueue.rs#L53

lwz23 avatar Jan 06 '25 08:01 lwz23