[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.
This commit is contained in:
Evan Richter 2022-01-27 02:05:33 -06:00 committed by GitHub
parent 408431ba5c
commit 4a6616bdfe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 9 additions and 17 deletions

View File

@ -187,9 +187,9 @@ fn fuzz(
println!("Break at {:#x}", emu.read_reg::<_, u64>(Regs::Rip).unwrap()); println!("Break at {:#x}", emu.read_reg::<_, u64>(Regs::Rip).unwrap());
let stack_ptr: u64 = emu.read_reg(Regs::Rsp).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) }; 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!("Stack pointer = {:#x}", stack_ptr);
println!("Return address = {:#x}", ret_addr); println!("Return address = {:#x}", ret_addr);

View File

@ -74,9 +74,9 @@ pub fn fuzz() {
// Get the return address // Get the return address
let stack_ptr: u64 = emu.read_reg(Regs::Rsp).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) }; 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!("Stack pointer = {:#x}", stack_ptr);
println!("Return address = {:#x}", ret_addr); println!("Return address = {:#x}", ret_addr);

View File

@ -3,7 +3,7 @@
use core::{ use core::{
convert::Into, convert::Into,
ffi::c_void, ffi::c_void,
mem::{size_of, transmute, MaybeUninit}, mem::{transmute, MaybeUninit},
ptr::{addr_of, addr_of_mut, copy_nonoverlapping, null}, ptr::{addr_of, addr_of_mut, copy_nonoverlapping, null},
}; };
use libc::c_int; use libc::c_int;
@ -349,13 +349,9 @@ impl Emulator {
/// This will write to a translated guest address (using `g2h`). /// This will write to a translated guest address (using `g2h`).
/// It just adds `guest_base` and writes to that location, without checking the bounds. /// It just adds `guest_base` and writes to that location, without checking the bounds.
/// This may only be safely used for valid guest addresses! /// 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); let host_addr = self.g2h(addr);
copy_nonoverlapping( copy_nonoverlapping(buf.as_ptr(), host_addr, buf.len());
buf.as_ptr() as *const _ as *const u8,
host_addr,
buf.len() * size_of::<T>(),
);
} }
/// Read a value from a guest address. /// Read a value from a guest address.
@ -364,13 +360,9 @@ impl Emulator {
/// This will read from a translated guest address (using `g2h`). /// This will read from a translated guest address (using `g2h`).
/// It just adds `guest_base` and writes to that location, without checking the bounds. /// It just adds `guest_base` and writes to that location, without checking the bounds.
/// This may only be safely used for valid guest addresses! /// 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); let host_addr = self.g2h(addr);
copy_nonoverlapping( copy_nonoverlapping(host_addr, buf.as_mut_ptr(), buf.len());
host_addr as *const u8,
buf.as_mut_ptr() as *mut _ as *mut u8,
buf.len() * size_of::<T>(),
);
} }
#[must_use] #[must_use]