From 4a6616bdfeec55264d33f5e695c7a9f969bc19fe Mon Sep 17 00:00:00 2001
From: Evan Richter <evanjrichter@gmail.com>
Date: Thu, 27 Jan 2022 02:05:33 -0600
Subject: [PATCH] [libafl_qemu] simplify emu::{read,write}_mem (#496)

Methods read_mem and write_mem now operate on &[u8], not &[T]

The generic T slice interface was prone to various footguns:
* i32 is the default Rust integer type, but buffers are often expected
  to hold u8. This means the following code writes 16 bytes to the
  guest, not 4:

      let buf = [0; 4];
      emu.write_mem(addr, &buf);

* If a buffer of 16-bit or larger integers (&[u64] for example) is
  needed to read/write, the user will need to consider host/guest
  endianness. The byte array methods in std are a good, explicit
  alternative.

  Perhaps libafl_qemu could expose/define "to/from guest endianness"
  helper functions or extension traits using the established cfg flags,
  so that guest endianness is always right by default.

* emu::read_mem causes insta-UB if a user did something like:

      let mut my_bool = false;
      emu.read_mem(addr, &mut my_bool);

  It's less surprising for users to just operate on plain-ol' bytes,
  which they can explicitly transmute if they wish.
---
 fuzzers/fuzzbench_qemu/src/fuzzer.rs |  4 ++--
 fuzzers/qemu_launcher/src/fuzzer.rs  |  4 ++--
 libafl_qemu/src/emu.rs               | 18 +++++-------------
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/fuzzers/fuzzbench_qemu/src/fuzzer.rs b/fuzzers/fuzzbench_qemu/src/fuzzer.rs
index bf974a67ed..effdca4e7b 100644
--- a/fuzzers/fuzzbench_qemu/src/fuzzer.rs
+++ b/fuzzers/fuzzbench_qemu/src/fuzzer.rs
@@ -187,9 +187,9 @@ fn fuzz(
     println!("Break at {:#x}", emu.read_reg::<_, u64>(Regs::Rip).unwrap());
 
     let stack_ptr: u64 = emu.read_reg(Regs::Rsp).unwrap();
-    let mut ret_addr = [0u64];
+    let mut ret_addr = [0; 8];
     unsafe { emu.read_mem(stack_ptr, &mut ret_addr) };
-    let ret_addr = ret_addr[0];
+    let ret_addr = u64::from_le_bytes(ret_addr);
 
     println!("Stack pointer = {:#x}", stack_ptr);
     println!("Return address = {:#x}", ret_addr);
diff --git a/fuzzers/qemu_launcher/src/fuzzer.rs b/fuzzers/qemu_launcher/src/fuzzer.rs
index 80bbe0bbcc..40c999d632 100644
--- a/fuzzers/qemu_launcher/src/fuzzer.rs
+++ b/fuzzers/qemu_launcher/src/fuzzer.rs
@@ -74,9 +74,9 @@ pub fn fuzz() {
 
     // Get the return address
     let stack_ptr: u64 = emu.read_reg(Regs::Rsp).unwrap();
-    let mut ret_addr = [0u64];
+    let mut ret_addr = [0; 8];
     unsafe { emu.read_mem(stack_ptr, &mut ret_addr) };
-    let ret_addr = ret_addr[0];
+    let ret_addr = u64::from_le_bytes(ret_addr);
 
     println!("Stack pointer = {:#x}", stack_ptr);
     println!("Return address = {:#x}", ret_addr);
diff --git a/libafl_qemu/src/emu.rs b/libafl_qemu/src/emu.rs
index 5c32e42d9b..79ab2a1812 100644
--- a/libafl_qemu/src/emu.rs
+++ b/libafl_qemu/src/emu.rs
@@ -3,7 +3,7 @@
 use core::{
     convert::Into,
     ffi::c_void,
-    mem::{size_of, transmute, MaybeUninit},
+    mem::{transmute, MaybeUninit},
     ptr::{addr_of, addr_of_mut, copy_nonoverlapping, null},
 };
 use libc::c_int;
@@ -349,13 +349,9 @@ impl Emulator {
     /// This will write to a translated guest address (using `g2h`).
     /// It just adds `guest_base` and writes to that location, without checking the bounds.
     /// This may only be safely used for valid guest addresses!
-    pub unsafe fn write_mem<T>(&self, addr: u64, buf: &[T]) {
+    pub unsafe fn write_mem(&self, addr: u64, buf: &[u8]) {
         let host_addr = self.g2h(addr);
-        copy_nonoverlapping(
-            buf.as_ptr() as *const _ as *const u8,
-            host_addr,
-            buf.len() * size_of::<T>(),
-        );
+        copy_nonoverlapping(buf.as_ptr(), host_addr, buf.len());
     }
 
     /// Read a value from a guest address.
@@ -364,13 +360,9 @@ impl Emulator {
     /// This will read from a translated guest address (using `g2h`).
     /// It just adds `guest_base` and writes to that location, without checking the bounds.
     /// This may only be safely used for valid guest addresses!
-    pub unsafe fn read_mem<T>(&self, addr: u64, buf: &mut [T]) {
+    pub unsafe fn read_mem(&self, addr: u64, buf: &mut [u8]) {
         let host_addr = self.g2h(addr);
-        copy_nonoverlapping(
-            host_addr as *const u8,
-            buf.as_mut_ptr() as *mut _ as *mut u8,
-            buf.len() * size_of::<T>(),
-        );
+        copy_nonoverlapping(host_addr, buf.as_mut_ptr(), buf.len());
     }
 
     #[must_use]