ShMem is easy to misuse

[langston-barrett]

Consider the following program:

use libafl_bolts::shmem::{ShMem as _, ShMemProvider as _};

fn main() {
    let mut prov = libafl_bolts::shmem::MmapShMemProvider::default();
    let shmem = prov.new_on_shmem([0x01u8; 256]).unwrap();
    let v_ptr = shmem.as_ptr_of::<Vec<u8>>().unwrap();
    let v = unsafe { (*v_ptr).clone() };
    println!("{}", v[5]);
}

This results in an assertion failure in std (though it could easily also result in a segfault, depending on your machine):

cargo -q run -r

memory allocation of 72340172838076673 bytes failed
zsh: abort (core dumped)  cargo -q run -r

The problem is that the type passed to ShMem::as_ptr_of is different from that passed to ShMemProvider::new_on_shmem. In this trivial example, this is easy to see, but one can certainly imagine a larger program where the construction- and use-sites are further away and drift out of sync.

You might say that this isn't that big of a deal, because it requires dereferencing a raw pointer, which is unsafe. In particular, doing so requires you to ensure that there is a valid value of the pointer's type at that address. I would argue that this interface is unnecessarily flexible, and makes it too easy to do the wrong thing without realizing it.

One easy enhancement would be to add a typed wrapper around ShMem:

pub trait ShMemProvider {
    fn new_on_shmem<T: Sized + 'static>(&mut self, value: T) -> Result<TypedShMem<Self::ShMem, T>, Error>;
}

pub struct TypedShMem<S: ShMem, T: Sized + 'static> {
    inner: S,
    phantom: PhantomData<T>
}

impl<S: ShMem, T: Sized + 'static> TypedShMem<S, T> {
    pub fn into_inner(self) -> S {  self.inner  }
    pub fn as_ptr(&self) -> *const T {  self.inner.as_ptr_of::<T>().unwrap() }
    pub fn as_mut_ptr(&mut self) -> *mut T { self.inner.as_mut_ptr_of::<T>().unwrap()  }
}

(I would advocate against impl DerefMut<Target = T> for TypedShMem<S, T> due to #2807.) This retains all of the flexibility of the current approach, while making it easier for the type system to nudge users in the right direction.

This is just a suggestion, feel free to close if you disagree with my reasoning!

[domenukk]

Looks like as_ptr_of is never used. Would you say we should simply remove the function?

[langston-barrett]

I bet as_{mut_,}_ptr_of (or something like it) will have to be used if the DerefMut<Target = [u8]> instance is removed (#2807).