fix(sallyport): avoid UB by removing implicit reference from indexing…

… with range

This potential source of UB was discovered while upgrading the Rust toolchain,
which upgrades us to a new version of Miri with stricter rules around raw pointers.
Specifically, an expression like `addr_of_mut!((*(ptr))[offset..])` is deliberately
attempting to operate only on raw pointers while avoiding any intermediate references,
since references have invariants that raw pointers do not.
However, there is in fact an implicit reference here that is created as a result of the
indexing operation. This is both surprising and not surprising, for interesting reasons.

First, it should not be surprising because indexing is governed by the Index traits,
whose methods function return references, so their presence here is natural.

On the other hand, it is surprising because Rust already special cases `(*ptr)[foo]` when `ptr`
is a raw slice and `foo` is not a range to avoid the Index traits entirely, which allows it to
avoid emitting an intermediate reference.

The ideal solution here is for Rust to be smart enough to not introduce the intermediate
reference here at all, which is tracked at rust-lang/rust#73987 .

In addition, while investigating this issue I brought it up to the Unsafe Code Guidelines team,
who saw fit to file rust-lang/rust#99437 as a more specific example
of the potential perils of the current behavior.

Signed-off-by: bstrie <[email protected]>

crates/sallyport/src/guest/alloc/phase_alloc.rs

@@ -7,7 +7,6 @@ use crate::Result;
 use core::alloc::Layout;
 use core::marker::PhantomData;
 use core::mem::{align_of, size_of};
-use core::ptr::addr_of_mut;
 use core::ptr::NonNull;
 
 pub(crate) mod phase {
@@ -76,29 +75,41 @@ impl<'a> Alloc<'a, phase::Init> {
 }
 
 impl<'a> Alloc<'a, phase::Stage> {
-    /// Allocates a memory region of `layout.size()` bytes with padding required to ensure alignment
-    /// and returns a tuple of non-null pointer and byte offset of start of that aligned region on success.
+    /// Allocates a memory region of `layout.size()` bytes with padding required to ensure
+    /// alignment and returns a tuple of non-null pointer and byte offset of start of that aligned
+    /// region on success.
     #[inline]
     fn allocate_layout(&mut self, layout: Layout) -> Result<(NonNull<[u8]>, usize)> {
         let free = self.ptr.len();
         let pad_size = self.ptr.cast::<u8>().as_ptr().align_offset(layout.align());
         let layout_size = layout.size();
+
         if free < pad_size.checked_add(layout_size).ok_or(EOVERFLOW)? {
             return Err(ENOMEM);
         }
 
-        let ptr = unsafe { addr_of_mut!((*(self.ptr.as_ptr()))[pad_size..]) };
+        let raw_slice = self.ptr.as_ptr();
+
+        // SAFETY: we know raw_slice is nonnull, and we know pad_size is in-bounds
+        let padded_raw_slice = unsafe { raw_slice.get_unchecked_mut(pad_size..) };
+
+        // SAFETY: we know padded_raw_slice is nonnull, and we know layout_size is in-bounds
+        let new_region = unsafe { padded_raw_slice.get_unchecked_mut(..layout_size) };
+
+        // SAFETY: we know padded_raw_slice is nonnull, and we know layout_size is in-bounds
+        let remainder = unsafe { padded_raw_slice.get_unchecked_mut(layout_size..) };
+
         let offset = self.offset + pad_size;
+
         *self = Self {
-            ptr: unsafe { NonNull::new_unchecked(addr_of_mut!((*(ptr))[layout_size..])) },
+            // SAFETY: we know remainder is nonnull
+            ptr: unsafe { NonNull::new_unchecked(remainder) },
             offset: offset + layout_size,
-
             phase: PhantomData,
         };
-        Ok((
-            unsafe { NonNull::new_unchecked(addr_of_mut!((*(ptr))[..layout_size])) },
-            offset,
-        ))
+
+        // SAFETY: we know new_region is nonnull
+        Ok((unsafe { NonNull::new_unchecked(new_region) }, offset))
     }
 
     #[inline]

crates/sallyport/src/lib.rs

@@ -11,6 +11,7 @@
 #![feature(core_ffi_c)]
 #![feature(c_size_t)]
 #![feature(nonnull_slice_from_raw_parts)]
+#![feature(slice_ptr_get)]
 #![feature(slice_ptr_len)]
 
 pub mod elf;