Make users of `dec_slice_of` safe.

Make `dec_slice_of` return a slice rather than a pointer-length pair,
freeing its users from having to call the unsafe `slice::from_raw_parts`.

This requires splitting `dec_slice_of` and `dec_ptr` into mut and
non-mut versions, and reorganizing poll_oneoff a little to avoid
borrow-checker errors -- decoded slices do alias the main memory, so
make sure functions only need one or the other.

src/hostcalls.rs

@@ -542,13 +542,12 @@ pub fn path_open(
     }
 
     let path = match dec_slice_of::<u8>(memory, path_ptr, path_len) {
-        Ok((ptr, len)) => OsStr::from_bytes(unsafe { std::slice::from_raw_parts(ptr, len) }),
+        Ok(slice) => OsStr::from_bytes(slice),
         Err(e) => return enc_errno(e),
     };
 
     let (dir, path) = match path_get(
         wasi_ctx,
-        memory,
         dirfd,
         dirflags,
         path,
@@ -619,14 +618,11 @@ pub fn random_get(
 ) -> wasm32::__wasi_errno_t {
     use rand::{thread_rng, RngCore};
 
-    let buf_len = dec_usize(buf_len);
-    let buf_ptr = match dec_ptr(memory, buf_ptr, buf_len) {
-        Ok(ptr) => ptr,
+    let buf = match dec_slice_of_mut::<u8>(memory, buf_ptr, buf_len) {
+        Ok(buf) => buf,
         Err(e) => return enc_errno(e),
     };
 
-    let buf = unsafe { std::slice::from_raw_parts_mut(buf_ptr, buf_len) };
-
     thread_rng().fill_bytes(buf);
 
     return wasm32::__WASI_ESUCCESS;
@@ -643,16 +639,14 @@ pub fn poll_oneoff(
         return wasm32::__WASI_EINVAL;
     }
     enc_pointee(memory, nevents, 0).unwrap();
-    let input_slice_ =
+    let input_slice =
         dec_slice_of::<wasm32::__wasi_subscription_t>(memory, input, nsubscriptions).unwrap();
-    let input_slice = unsafe { slice::from_raw_parts(input_slice_.0, input_slice_.1) };
-
-    let output_slice_ =
-        dec_slice_of::<wasm32::__wasi_event_t>(memory, output, nsubscriptions).unwrap();
-    let output_slice = unsafe { slice::from_raw_parts_mut(output_slice_.0, output_slice_.1) };
 
     let input: Vec<_> = input_slice.iter().map(|x| dec_subscription(x)).collect();
 
+    let output_slice =
+        dec_slice_of_mut::<wasm32::__wasi_event_t>(memory, output, nsubscriptions).unwrap();
+
     let timeout = input
         .iter()
         .filter_map(|event| match event {
@@ -713,11 +707,16 @@ pub fn poll_oneoff(
             Ok(ready) => break ready as usize,
         }
     };
-    if ready == 0 {
-        return poll_oneoff_handle_timeout_event(memory, output_slice, nevents, timeout);
+    let events_count = if ready == 0 {
+        poll_oneoff_handle_timeout_event(output_slice, nevents, timeout)
+    } else {
+        let events = fd_events.iter().zip(poll_fds.iter()).take(ready);
+        poll_oneoff_handle_fd_event(output_slice, nevents, events)
+    };
+    if let Err(e) = enc_pointee(memory, nevents, events_count) {
+        return enc_errno(e);
     }
-    let events = fd_events.iter().zip(poll_fds.iter()).take(ready);
-    poll_oneoff_handle_fd_event(memory, output_slice, nevents, events)
+    wasm32::__WASI_ESUCCESS
 }
 
 pub fn fd_filestat_get(
@@ -761,12 +760,11 @@ pub fn path_filestat_get(
     let dirfd = dec_fd(dirfd);
     let dirflags = dec_lookupflags(dirflags);
     let path = match dec_slice_of::<u8>(memory, path_ptr, path_len) {
-        Ok((ptr, len)) => OsStr::from_bytes(unsafe { std::slice::from_raw_parts(ptr, len) }),
+        Ok(slice) => OsStr::from_bytes(slice),
         Err(e) => return enc_errno(e),
     };
     let (dir, path) = match path_get(
         wasi_ctx,
-        memory,
         dirfd,
         dirflags,
         path,
@@ -804,12 +802,11 @@ pub fn path_create_directory(
 
     let dirfd = dec_fd(dirfd);
     let path = match dec_slice_of::<u8>(memory, path_ptr, path_len) {
-        Ok((ptr, len)) => OsStr::from_bytes(unsafe { std::slice::from_raw_parts(ptr, len) }),
+        Ok(slice) => OsStr::from_bytes(slice),
         Err(e) => return enc_errno(e),
     };
     let (dir, path) = match path_get(
         wasi_ctx,
-        memory,
         dirfd,
         0,
         path,
@@ -843,12 +840,11 @@ pub fn path_unlink_file(
 
     let dirfd = dec_fd(dirfd);
     let path = match dec_slice_of::<u8>(memory, path_ptr, path_len) {
-        Ok((ptr, len)) => OsStr::from_bytes(unsafe { std::slice::from_raw_parts(ptr, len) }),
+        Ok(slice) => OsStr::from_bytes(slice),
         Err(e) => return enc_errno(e),
     };
     let (dir, path) = match path_get(
         wasi_ctx,
-        memory,
         dirfd,
         0,
         path,
@@ -1143,11 +1139,10 @@ struct FdEventData {
 }
 
 fn poll_oneoff_handle_timeout_event(
-    memory: &mut [u8],
     output_slice: &mut [wasm32::__wasi_event_t],
     nevents: wasm32::uintptr_t,
     timeout: Option<ClockEventData>,
-) -> wasm32::__wasi_errno_t {
+) -> wasm32::size_t {
     if let Some(ClockEventData { userdata, .. }) = timeout {
         let output_event = host::__wasi_event_t {
             userdata,
@@ -1161,24 +1156,18 @@ fn poll_oneoff_handle_timeout_event(
             },
         };
         output_slice[0] = enc_event(output_event);
-        if let Err(e) = enc_pointee(memory, nevents, 1) {
-            return enc_errno(e);
-        }
+        1
     } else {
         // shouldn't happen
-        if let Err(e) = enc_pointee(memory, nevents, 0) {
-            return enc_errno(e);
-        }
+        0
     }
-    wasm32::__WASI_ESUCCESS
 }
 
 fn poll_oneoff_handle_fd_event<'t>(
-    memory: &mut [u8],
     output_slice: &mut [wasm32::__wasi_event_t],
     nevents: wasm32::uintptr_t,
     events: impl Iterator<Item = (&'t FdEventData, &'t nix::poll::PollFd)>,
-) -> wasm32::__wasi_errno_t {
+) -> wasm32::size_t {
     let mut output_slice_cur = output_slice.iter_mut();
     let mut revents_count = 0;
     for (fd_event, poll_fd) in events {
@@ -1250,10 +1239,7 @@ fn poll_oneoff_handle_fd_event<'t>(
         *output_slice_cur.next().unwrap() = enc_event(output_event);
         revents_count += 1;
     }
-    if let Err(e) = enc_pointee(memory, nevents, revents_count) {
-        return enc_errno(e);
-    }
-    wasm32::__WASI_ESUCCESS
+    revents_count
 }
 
 /// Normalizes a path to ensure that the target path is located under the directory provided.
@@ -1261,7 +1247,6 @@ fn poll_oneoff_handle_fd_event<'t>(
 /// This is a workaround for not having Capsicum support in the OS.
 pub fn path_get<P: AsRef<OsStr>>(
     wasi_ctx: &WasiCtx,
-    memory: &mut [u8],
     dirfd: host::__wasi_fd_t,
     dirflags: host::__wasi_lookupflags_t,
     path: P,

src/memory.rs

@@ -23,7 +23,22 @@ macro_rules! bail_errno {
     };
 }
 
-pub fn dec_ptr(
+fn dec_ptr(
+    memory: &[u8],
+    ptr: wasm32::uintptr_t,
+    len: usize,
+) -> Result<*const u8, host::__wasi_errno_t> {
+    // check for overflow
+    let checked_len = (ptr as usize).checked_add(len).ok_or(host::__WASI_EFAULT)?;
+
+    // translate the pointer
+    memory
+        .get(ptr as usize..checked_len)
+        .ok_or(host::__WASI_EFAULT)
+        .map(|mem| mem.as_ptr())
+}
+
+fn dec_ptr_mut(
     memory: &mut [u8],
     ptr: wasm32::uintptr_t,
     len: usize,
@@ -38,7 +53,19 @@ pub fn dec_ptr(
         .map(|mem| mem.as_mut_ptr())
 }
 
-pub fn dec_ptr_to<'memory, T>(
+fn dec_ptr_to<'memory, T>(
+    memory: &'memory [u8],
+    ptr: wasm32::uintptr_t,
+) -> Result<&'memory T, host::__wasi_errno_t> {
+    // check that the ptr is aligned
+    if ptr as usize % align_of::<T>() != 0 {
+        bail_errno!(__WASI_EINVAL);
+    }
+
+    dec_ptr(memory, ptr, size_of::<T>()).map(|p| unsafe { &*(p as *const T) })
+}
+
+fn dec_ptr_to_mut<'memory, T>(
     memory: &'memory mut [u8],
     ptr: wasm32::uintptr_t,
 ) -> Result<&'memory mut T, host::__wasi_errno_t> {
@@ -47,13 +74,10 @@ pub fn dec_ptr_to<'memory, T>(
         bail_errno!(__WASI_EINVAL);
     }
 
-    dec_ptr(memory, ptr, size_of::<T>()).map(|p| unsafe { &mut *(p as *mut T) })
+    dec_ptr_mut(memory, ptr, size_of::<T>()).map(|p| unsafe { &mut *(p as *mut T) })
 }
 
-pub fn dec_pointee<T>(
-    memory: &mut [u8],
-    ptr: wasm32::uintptr_t,
-) -> Result<T, host::__wasi_errno_t> {
+pub fn dec_pointee<T>(memory: &[u8], ptr: wasm32::uintptr_t) -> Result<T, host::__wasi_errno_t> {
     dec_ptr_to::<T>(memory, ptr).map(|p| unsafe { ptr::read(p) })
 }
 
@@ -62,14 +86,13 @@ pub fn enc_pointee<T>(
     ptr: wasm32::uintptr_t,
     t: T,
 ) -> Result<(), host::__wasi_errno_t> {
-    dec_ptr_to::<T>(memory, ptr).map(|p| unsafe { ptr::write(p, t) })
+    dec_ptr_to_mut::<T>(memory, ptr).map(|p| unsafe { ptr::write(p, t) })
 }
 
-pub fn dec_slice_of<'memory, T>(
-    memory: &'memory mut [u8],
+fn check_slice_of<T>(
     ptr: wasm32::uintptr_t,
     len: wasm32::size_t,
-) -> Result<(&'memory mut T, usize), host::__wasi_errno_t> {
+) -> Result<(usize, usize), host::__wasi_errno_t> {
     // check alignment, and that length doesn't overflow
     if ptr as usize % align_of::<T>() != 0 {
         return Err(host::__WASI_EINVAL);
@@ -81,8 +104,27 @@ pub fn dec_slice_of<'memory, T>(
         return Err(host::__WASI_EOVERFLOW);
     };
 
-    let ptr = dec_ptr(memory, ptr, len_bytes)? as *mut T;
-    Ok((unsafe { &mut *ptr }, len))
+    Ok((len, len_bytes))
+}
+
+pub fn dec_slice_of<'memory, T>(
+    memory: &'memory [u8],
+    ptr: wasm32::uintptr_t,
+    len: wasm32::size_t,
+) -> Result<&'memory [T], host::__wasi_errno_t> {
+    let (len, len_bytes) = check_slice_of::<T>(ptr, len)?;
+    let ptr = dec_ptr(memory, ptr, len_bytes)? as *const T;
+    Ok(unsafe { slice::from_raw_parts(ptr, len) })
+}
+
+pub fn dec_slice_of_mut<'memory, T>(
+    memory: &'memory mut [u8],
+    ptr: wasm32::uintptr_t,
+    len: wasm32::size_t,
+) -> Result<&'memory mut [T], host::__wasi_errno_t> {
+    let (len, len_bytes) = check_slice_of::<T>(ptr, len)?;
+    let ptr = dec_ptr_mut(memory, ptr, len_bytes)? as *mut T;
+    Ok(unsafe { slice::from_raw_parts_mut(ptr, len) })
 }
 
 pub fn enc_slice_of<T>(
@@ -102,7 +144,7 @@ pub fn enc_slice_of<T>(
     };
 
     // get the pointer into guest memory, and copy the bytes
-    let ptr = dec_ptr(memory, ptr, len_bytes)? as *mut libc::c_void;
+    let ptr = dec_ptr_mut(memory, ptr, len_bytes)? as *mut libc::c_void;
     unsafe {
         libc::memcpy(ptr, slice.as_ptr() as *const libc::c_void, len_bytes);
     }
@@ -138,7 +180,7 @@ macro_rules! dec_enc_scalar {
 }
 
 pub fn dec_ciovec(
-    memory: &mut [u8],
+    memory: &[u8],
     ciovec: &wasm32::__wasi_ciovec_t,
 ) -> Result<host::__wasi_ciovec_t, host::__wasi_errno_t> {
     let len = dec_usize(ciovec.buf_len);
@@ -149,12 +191,11 @@ pub fn dec_ciovec(
 }
 
 pub fn dec_ciovec_slice(
-    memory: &mut [u8],
+    memory: &[u8],
     ptr: wasm32::uintptr_t,
     len: wasm32::size_t,
 ) -> Result<Vec<host::__wasi_ciovec_t>, host::__wasi_errno_t> {
     let slice = dec_slice_of::<wasm32::__wasi_ciovec_t>(memory, ptr, len)?;
-    let slice = unsafe { slice::from_raw_parts(slice.0, slice.1) };
     slice.iter().map(|iov| dec_ciovec(memory, iov)).collect()
 }