Handle overflow in GcArrayLayout computation (#13115)

* Handle overflow in `GcArrayLayout` computation

Fixes #13034

* fix ci

Author: fitzgen

crates/environ/src/gc.rs

@@ -349,23 +349,24 @@ pub struct GcArrayLayout {
 impl GcArrayLayout {
     /// Get the total size of this array for a given length of elements.
     #[inline]
-    pub fn size_for_len(&self, len: u32) -> u32 {
+    pub fn size_for_len(&self, len: u32) -> Option<u32> {
         self.elem_offset(len)
     }
 
     /// Get the offset of the `i`th element in an array with this layout.
     #[inline]
-    pub fn elem_offset(&self, i: u32) -> u32 {
-        self.base_size + i * self.elem_size
+    pub fn elem_offset(&self, i: u32) -> Option<u32> {
+        let elem_offset = i.checked_mul(self.elem_size)?;
+        self.base_size.checked_add(elem_offset)
     }
 
     /// Get a `core::alloc::Layout` for an array of this type with the given
     /// length.
-    pub fn layout(&self, len: u32) -> Layout {
-        let size = self.size_for_len(len);
+    pub fn layout(&self, len: u32) -> Option<Layout> {
+        let size = self.size_for_len(len)?;
         let size = usize::try_from(size).unwrap();
         let align = usize::try_from(self.align).unwrap();
-        Layout::from_size_align(size, align).unwrap()
+        Layout::from_size_align(size, align).ok()
     }
 }
 

crates/wasmtime/src/runtime/gc/enabled/arrayref.rs

@@ -393,22 +393,21 @@ impl ArrayRef {
             "attempted to use a `ArrayRefPre` with the wrong store"
         );
 
-        // Type check the elements against the element type.
-        for elem in elems.clone() {
-            elem.ensure_matches_ty(store, allocator.ty.element_type().unpack())
-                .context("element type mismatch")?;
-        }
-
         let len = u32::try_from(elems.len()).unwrap();
 
-        // Allocate the array and write each field value into the appropriate
-        // offset.
+        // Allocate the array.
         let arrayref = store
             .require_gc_store_mut()?
             .alloc_uninit_array(allocator.type_index(), len, allocator.layout())
             .context("unrecoverable error when allocating new `arrayref`")?
             .map_err(|n| GcHeapOutOfMemory::new((), n))?;
 
+        // Type check the elements against the element type.
+        for elem in elems.clone() {
+            elem.ensure_matches_ty(store, allocator.ty.element_type().unpack())
+                .context("element type mismatch")?;
+        }
+
         // From this point on, if we get any errors, then the array is not
         // fully initialized, so we need to eagerly deallocate it before the
         // next GC where the collector might try to interpret one of the

crates/wasmtime/src/runtime/store/gc.rs

@@ -197,6 +197,8 @@ impl StoreOpaque {
             Ok(x) => Ok(x),
             Err(e) => match e.downcast::<crate::GcHeapOutOfMemory<T>>() {
                 Ok(oom) => {
+                    log::trace!("Got GC heap OOM: {oom}");
+
                     let (value, oom) = oom.take_inner();
                     let bytes_needed = oom.bytes_needed();
 

crates/wasmtime/src/runtime/types.rs

@@ -1567,7 +1567,7 @@ impl From<TagType> for ExternType {
 ///
 /// This is either a packed 8- or -16 bit integer, or else it is some unpacked
 /// Wasm value type.
-#[derive(Clone, Hash)]
+#[derive(Debug, Clone, Hash)]
 pub enum StorageType {
     /// `i8`, an 8-bit integer.
     I8,

crates/wasmtime/src/runtime/vm/gc/enabled/arrayref.rs

@@ -145,7 +145,7 @@ impl VMArrayRef {
         ty: &StorageType,
         index: u32,
     ) -> Val {
-        let offset = layout.elem_offset(index);
+        let offset = layout.elem_offset(index).unwrap();
         let data = store.unwrap_gc_store_mut().gc_object_data(self.as_gc_ref());
         match ty {
             StorageType::I8 => Val::I32(data.read_u8(offset).into()),
@@ -205,7 +205,7 @@ impl VMArrayRef {
     ) -> Result<()> {
         debug_assert!(val._matches_ty(&store, &ty.unpack())?);
 
-        let offset = layout.elem_offset(index);
+        let offset = layout.elem_offset(index).unwrap();
         let data = store.unwrap_gc_store_mut().gc_object_data(self.as_gc_ref());
         match val {
             Val::I32(i) if ty.is_i8() => data.write_i8(offset, truncate_i32_to_i8(i)),
@@ -319,7 +319,7 @@ impl VMArrayRef {
         val: Val,
     ) -> Result<()> {
         debug_assert!(val._matches_ty(&store, &ty.unpack())?);
-        let offset = layout.elem_offset(index);
+        let offset = layout.elem_offset(index).unwrap();
         let gcstore = store.require_gc_store_mut()?;
         match val {
             Val::I32(i) if ty.is_i8() => gcstore

crates/wasmtime/src/runtime/vm/gc/enabled/drc.rs

@@ -414,7 +414,7 @@ impl DrcHeap {
         let info = match gc_layout {
             GcLayout::Array(l) => {
                 if l.elems_are_gc_refs {
-                    debug_assert_eq!(l.elem_offset(0), GC_REF_ARRAY_ELEMS_OFFSET,);
+                    debug_assert_eq!(l.elem_offset(0), Some(GC_REF_ARRAY_ELEMS_OFFSET));
                 }
                 TraceInfo::Array {
                     gc_ref_elems: l.elems_are_gc_refs,
@@ -1070,9 +1070,12 @@ unsafe impl GcHeap for DrcHeap {
         length: u32,
         layout: &GcArrayLayout,
     ) -> Result<Result<VMArrayRef, u64>> {
+        let layout = layout
+            .layout(length)
+            .ok_or_else(|| format_err!("allocation size too large"))?;
         let gc_ref = match self.alloc_raw(
             VMGcHeader::from_kind_and_index(VMGcKind::ArrayRef, ty),
-            layout.layout(length),
+            layout,
         )? {
             Err(n) => return Ok(Err(n)),
             Ok(gc_ref) => gc_ref,

crates/wasmtime/src/runtime/vm/gc/enabled/null.rs

@@ -307,9 +307,12 @@ unsafe impl GcHeap for NullHeap {
         length: u32,
         layout: &GcArrayLayout,
     ) -> Result<Result<VMArrayRef, u64>> {
+        let layout = layout
+            .layout(length)
+            .ok_or_else(|| format_err!("allocation size too large"))?;
         self.alloc(
             VMGcHeader::from_kind_and_index(VMGcKind::ArrayRef, ty),
-            layout.layout(length),
+            layout,
         )
         .map(|r| {
             r.map(|r| {

tests/all/arrays.rs

@@ -1,3 +1,5 @@
+use crate::ErrorExt;
+
 use super::gc_store;
 use wasmtime::*;
 
@@ -876,3 +878,22 @@ fn instantiate_with_array_global() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn issue_13034_array_layout_overflow() -> Result<()> {
+    for (storage, len, val) in [
+        (StorageType::I8, u32::MAX, Val::I32(0)),
+        (StorageType::I8, u32::MAX - 1, Val::I32(0)),
+        (StorageType::I16, u32::MAX / 2, Val::I32(0)),
+        (ValType::I32.into(), u32::MAX / 4, Val::I32(0)),
+        (ValType::I64.into(), u32::MAX / 8, Val::I32(0)),
+    ] {
+        log::trace!("Testing [{storage}; {len}]");
+        let mut store = gc_store()?;
+        let array_ty = ArrayType::new(store.engine(), FieldType::new(Mutability::Const, storage));
+        let pre = ArrayRefPre::new(&mut store, array_ty);
+        let err = ArrayRef::new(&mut store, &pre, &val, len).unwrap_err();
+        err.assert_contains("allocation size too large");
+    }
+    Ok(())
+}

tests/misc_testsuite/gc/issue-13034.wast

@@ -0,0 +1,15 @@
+;;! gc = true
+
+(module
+ (type $i32_array (array (mut i32)))
+
+ (global $arr (mut (ref null $i32_array)) (ref.null $i32_array))
+
+ (func (export "run")
+   (global.set $arr
+     (array.new $i32_array (i32.const 0) (i32.const 1073741817))
+   )
+ )
+)
+
+(assert_trap (invoke "run") "allocation size too large")