Prevent using shared memories with Memory (#12022)

* Prevent using shared memories with `Memory`

This commit fixes a few issues where it was possible to represent a wasm
shared linear memory with the `wasmtime::Memory` type. This is not sound
because `wasmtime::Memory` provides safe Rust access to the bytes where
that is not possible with wasm shared memories. Shared memories in Rust
must be represented by `SharedMemory`, not `wasmtime::Memory`.

Specifically this commit prevents two vectors of this happening:

1. `Memory::new` now requires that the memory type specified is
   non-shared. Instead `SharedMemory::new` must be used instead.

2. Core dumps now skip over shared memories when iterating over all
   memories in the store. Supporting shared memories is left to a future
   feature request for now.

* CI fixes

Author: alexcrichton

crates/wasmtime/src/runtime/component/instance.rs

@@ -928,11 +928,11 @@ impl<'a> Instantiator<'a> {
     }
 
     fn extract_memory(&mut self, store: &mut StoreOpaque, memory: &ExtractMemory) {
-        let mem = match lookup_vmexport(store, self.id, &memory.export) {
-            crate::runtime::vm::Export::Memory { memory, .. } => memory,
+        let import = match lookup_vmexport(store, self.id, &memory.export) {
+            crate::runtime::vm::Export::Memory(memory) => memory.vmimport(store),
+            crate::runtime::vm::Export::SharedMemory(_, import) => import,
             _ => unreachable!(),
         };
-        let import = mem.vmimport(store);
         self.instance_mut(store)
             .set_runtime_memory(memory.index, import.from.as_non_null());
     }
@@ -1040,7 +1040,7 @@ impl<'a> Instantiator<'a> {
             return;
         }
 
-        let val = unsafe { crate::Extern::from_wasmtime_export(export, store) };
+        let val = crate::Extern::from_wasmtime_export(export, store);
         let ty = DefinitionType::from(store, &val);
         crate::types::matching::MatchCx::new(module.engine())
             .definition(&expected, &ty)

crates/wasmtime/src/runtime/coredump.rs

@@ -41,7 +41,8 @@ impl WasmCoreDump {
     pub(crate) fn new(store: &mut StoreOpaque, backtrace: WasmBacktrace) -> WasmCoreDump {
         let modules: Vec<_> = store.modules().all_modules().cloned().collect();
         let instances: Vec<Instance> = store.all_instances().collect();
-        let store_memories: Vec<Memory> = store.all_memories().collect();
+        let store_memories: Vec<Memory> =
+            store.all_memories().filter_map(|m| m.unshared()).collect();
 
         let mut store_globals: Vec<Global> = vec![];
         store.for_each_global(|_store, global| store_globals.push(global));
@@ -266,9 +267,13 @@ impl WasmCoreDump {
 
                 let memories = instance
                     .all_memories(store.0)
-                    .collect::<Vec<_>>()
-                    .into_iter()
-                    .map(|(_i, memory)| memory_to_idx[&memory.hash_key(&store.0)])
+                    .filter_map(|(_, m)| m.unshared())
+                    .map(|memory| {
+                        memory_to_idx
+                            .get(&memory.hash_key(&store.0))
+                            .copied()
+                            .unwrap_or(u32::MAX)
+                    })
                     .collect::<Vec<_>>();
 
                 let globals = instance

crates/wasmtime/src/runtime/externals.rs

@@ -125,18 +125,15 @@ impl Extern {
         }
     }
 
-    pub(crate) unsafe fn from_wasmtime_export(
+    pub(crate) fn from_wasmtime_export(
         wasmtime_export: crate::runtime::vm::Export,
         store: &StoreOpaque,
     ) -> Extern {
         match wasmtime_export {
             crate::runtime::vm::Export::Function(f) => Extern::Func(f),
-            crate::runtime::vm::Export::Memory { memory, shared } => {
-                if shared {
-                    Extern::SharedMemory(SharedMemory::from_memory(memory, store))
-                } else {
-                    Extern::Memory(memory)
-                }
+            crate::runtime::vm::Export::Memory(m) => Extern::Memory(m),
+            crate::runtime::vm::Export::SharedMemory(m, _) => {
+                Extern::SharedMemory(crate::SharedMemory::from_raw(m, store.engine().clone()))
             }
             crate::runtime::vm::Export::Global(g) => Extern::Global(g),
             crate::runtime::vm::Export::Table(t) => Extern::Table(t),

crates/wasmtime/src/runtime/instance.rs

@@ -480,7 +480,7 @@ impl Instance {
         // SAFETY: the store `id` owns this instance and all exports contained
         // within.
         let export = unsafe { self.id.get_mut(store).get_export_by_index_mut(id, entity) };
-        unsafe { Extern::from_wasmtime_export(export, store) }
+        Extern::from_wasmtime_export(export, store)
     }
 
     /// Looks up an exported [`Func`] value by name.
@@ -623,7 +623,7 @@ impl Instance {
     pub(crate) fn all_memories<'a>(
         &'a self,
         store: &'a StoreOpaque,
-    ) -> impl ExactSizeIterator<Item = (MemoryIndex, Memory)> + 'a {
+    ) -> impl ExactSizeIterator<Item = (MemoryIndex, vm::ExportMemory)> + 'a {
         let store_id = store.id();
         store[self.id].all_memories(store_id)
     }
@@ -716,8 +716,11 @@ impl OwnedImports {
             crate::runtime::vm::Export::Table(t) => {
                 self.tables.push(t.vmimport(store));
             }
-            crate::runtime::vm::Export::Memory { memory, .. } => {
-                self.memories.push(memory.vmimport(store));
+            crate::runtime::vm::Export::Memory(m) => {
+                self.memories.push(m.vmimport(store));
+            }
+            crate::runtime::vm::Export::SharedMemory(_, vmimport) => {
+                self.memories.push(*vmimport);
             }
             crate::runtime::vm::Export::Tag(t) => {
                 self.tags.push(t.vmimport(store));

crates/wasmtime/src/runtime/memory.rs

@@ -1,6 +1,6 @@
 use crate::Trap;
 use crate::prelude::*;
-use crate::runtime::vm::{self, VMStore};
+use crate::runtime::vm::{self, ExportMemory, VMStore};
 use crate::store::{StoreInstanceId, StoreOpaque, StoreResourceLimiter};
 use crate::trampoline::generate_memory_export;
 use crate::{AsContext, AsContextMut, Engine, MemoryType, StoreContext, StoreContextMut};
@@ -285,7 +285,13 @@ impl Memory {
         limiter: Option<&mut StoreResourceLimiter<'_>>,
         ty: MemoryType,
     ) -> Result<Memory> {
-        generate_memory_export(store, limiter, &ty, None).await
+        if ty.is_shared() {
+            bail!("shared memories must be created through `SharedMemory`")
+        }
+        Ok(generate_memory_export(store, limiter, &ty, None)
+            .await?
+            .unshared()
+            .unwrap())
     }
 
     /// Returns the underlying type of this memory.
@@ -638,7 +644,14 @@ impl Memory {
         }
     }
 
-    pub(crate) fn from_raw(instance: StoreInstanceId, index: DefinedMemoryIndex) -> Memory {
+    /// Creates a new memory from its raw component parts.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the memory pointed to by `instance` and
+    /// `index` is not a shared memory. For that `SharedMemory` must be used
+    /// instead.
+    pub(crate) unsafe fn from_raw(instance: StoreInstanceId, index: DefinedMemoryIndex) -> Memory {
         Memory { instance, index }
     }
 
@@ -649,12 +662,7 @@ impl Memory {
     }
 
     pub(crate) fn vmimport(&self, store: &StoreOpaque) -> crate::runtime::vm::VMMemoryImport {
-        let instance = &store[self.instance];
-        crate::runtime::vm::VMMemoryImport {
-            from: instance.memory_ptr(self.index).into(),
-            vmctx: instance.vmctx().into(),
-            index: self.index,
-        }
+        store[self.instance].get_defined_memory_vmimport(self.index)
     }
 
     pub(crate) fn comes_from_same_store(&self, store: &StoreOpaque) -> bool {
@@ -804,7 +812,6 @@ pub unsafe trait MemoryCreator: Send + Sync {
 pub struct SharedMemory {
     vm: crate::runtime::vm::SharedMemory,
     engine: Engine,
-    page_size_log2: u8,
 }
 
 impl SharedMemory {
@@ -820,13 +827,11 @@ impl SharedMemory {
 
         let tunables = engine.tunables();
         let ty = ty.wasmtime_memory();
-        let page_size_log2 = ty.page_size_log2;
         let memory = crate::runtime::vm::SharedMemory::new(ty, tunables)?;
 
         Ok(Self {
             vm: memory,
             engine: engine.clone(),
-            page_size_log2,
         })
     }
 
@@ -838,7 +843,7 @@ impl SharedMemory {
     /// Returns the size, in WebAssembly pages, of this wasm memory.
     pub fn size(&self) -> u64 {
         let byte_size = u64::try_from(self.data_size()).unwrap();
-        let page_size = u64::from(self.page_size());
+        let page_size = self.page_size();
         byte_size / page_size
     }
 
@@ -849,9 +854,8 @@ impl SharedMemory {
     /// `1`. Future extensions might allow any power of two as a page size.
     ///
     /// [the custom-page-sizes proposal]: https://github.com/WebAssembly/custom-page-sizes
-    pub fn page_size(&self) -> u32 {
-        debug_assert!(self.page_size_log2 == 0 || self.page_size_log2 == 16);
-        1 << self.page_size_log2
+    pub fn page_size(&self) -> u64 {
+        self.ty().page_size()
     }
 
     /// Returns the byte length of this memory.
@@ -912,7 +916,7 @@ impl SharedMemory {
             Some((old_size, _new_size)) => {
                 // For shared memory, the `VMMemoryDefinition` is updated inside
                 // the locked region.
-                Ok(u64::try_from(old_size).unwrap() / u64::from(self.page_size()))
+                Ok(u64::try_from(old_size).unwrap() / self.page_size())
             }
             None => bail!("failed to grow memory by `{delta}`"),
         }
@@ -1010,41 +1014,22 @@ impl SharedMemory {
         // Note `vm::assert_ready` shouldn't panic here because this isn't
         // actually allocating any new memory (also no limiter), so resource
         // limiting shouldn't kick in.
-        vm::assert_ready(generate_memory_export(
+        let memory = vm::assert_ready(generate_memory_export(
             store,
             None,
             &self.ty(),
             Some(&self.vm),
         ))
-        .unwrap()
-        .vmimport(store)
+        .unwrap();
+        match memory {
+            ExportMemory::Unshared(_) => unreachable!(),
+            ExportMemory::Shared(_shared, vmimport) => vmimport,
+        }
     }
 
-    /// Create a [`SharedMemory`] from an [`ExportMemory`] definition. This
-    /// function is available to handle the case in which a Wasm module exports
-    /// shared memory and the user wants host-side access to it.
-    pub(crate) fn from_memory(mem: Memory, store: &StoreOpaque) -> Self {
-        #![cfg_attr(
-            not(feature = "threads"),
-            expect(
-                unused_variables,
-                unreachable_code,
-                reason = "definitions cfg'd to dummy",
-            )
-        )]
-
-        let instance = mem.instance.get(store);
-        let memory = instance.get_defined_memory(mem.index);
-        let module = instance.env_module();
-        let page_size_log2 = module.memories[module.memory_index(mem.index)].page_size_log2;
-        match memory.as_shared_memory() {
-            Some(mem) => Self {
-                vm: mem.clone(),
-                engine: store.engine().clone(),
-                page_size_log2,
-            },
-            None => panic!("unable to convert from a shared memory"),
-        }
+    /// Creates a [`SharedMemory`] from its constituent parts.
+    pub(crate) fn from_raw(vm: crate::runtime::vm::SharedMemory, engine: Engine) -> Self {
+        SharedMemory { vm, engine }
     }
 }
 

crates/wasmtime/src/runtime/store.rs

@@ -97,15 +97,16 @@ use crate::runtime::vm::GcRootsList;
 use crate::runtime::vm::VMContRef;
 use crate::runtime::vm::mpk::ProtectionKey;
 use crate::runtime::vm::{
-    self, GcStore, Imports, InstanceAllocationRequest, InstanceAllocator, InstanceHandle,
-    Interpreter, InterpreterRef, ModuleRuntimeInfo, OnDemandInstanceAllocator, SendSyncPtr,
-    SignalHandler, StoreBox, Unwind, VMContext, VMFuncRef, VMGcRef, VMStore, VMStoreContext,
+    self, ExportMemory, GcStore, Imports, InstanceAllocationRequest, InstanceAllocator,
+    InstanceHandle, Interpreter, InterpreterRef, ModuleRuntimeInfo, OnDemandInstanceAllocator,
+    SendSyncPtr, SignalHandler, StoreBox, Unwind, VMContext, VMFuncRef, VMGcRef, VMStore,
+    VMStoreContext,
 };
 use crate::trampoline::VMHostGlobalContext;
 use crate::{Engine, Module, Val, ValRaw, module::ModuleRegistry};
 #[cfg(feature = "gc")]
 use crate::{ExnRef, Rooted};
-use crate::{Global, Instance, Memory, Table, Uninhabited};
+use crate::{Global, Instance, Table, Uninhabited};
 use alloc::sync::Arc;
 use core::fmt;
 use core::marker;
@@ -1728,7 +1729,7 @@ impl StoreOpaque {
     }
 
     /// Get all memories (host- or Wasm-defined) within this store.
-    pub fn all_memories<'a>(&'a self) -> impl Iterator<Item = Memory> + 'a {
+    pub fn all_memories<'a>(&'a self) -> impl Iterator<Item = ExportMemory> + 'a {
         // NB: Host-created memories have dummy instances. Therefore, we can get
         // all memories in the store by iterating over all instances (including
         // dummy instances) and getting each of their defined memories.

crates/wasmtime/src/runtime/trampoline.rs

@@ -14,7 +14,7 @@ use self::memory::create_memory;
 use self::table::create_table;
 use self::tag::create_tag;
 use crate::prelude::*;
-use crate::runtime::vm::SharedMemory;
+use crate::runtime::vm::{ExportMemory, SharedMemory};
 use crate::store::{StoreOpaque, StoreResourceLimiter};
 use crate::{MemoryType, TableType, TagType};
 use wasmtime_environ::{MemoryIndex, TableIndex, TagIndex};
@@ -24,7 +24,7 @@ pub async fn generate_memory_export(
     limiter: Option<&mut StoreResourceLimiter<'_>>,
     m: &MemoryType,
     preallocation: Option<&SharedMemory>,
-) -> Result<crate::Memory> {
+) -> Result<ExportMemory> {
     let id = store.id();
     let instance = create_memory(store, limiter, m, preallocation).await?;
     Ok(store

crates/wasmtime/src/runtime/vm/export.rs

@@ -1,3 +1,5 @@
+use crate::runtime::vm::{SharedMemory, VMMemoryImport};
+
 /// The value of an export passed from one instance to another.
 pub enum Export {
     /// A function export value.
@@ -6,12 +8,29 @@ pub enum Export {
     /// A table export value.
     Table(crate::Table),
 
-    /// A memory export value.
-    Memory { memory: crate::Memory, shared: bool },
+    /// An unshared memory export value.
+    Memory(crate::Memory),
+
+    /// A shared memory export value.
+    SharedMemory(SharedMemory, VMMemoryImport),
 
     /// A global export value.
     Global(crate::Global),
 
     /// A tag export value.
     Tag(crate::Tag),
 }
+
+pub enum ExportMemory {
+    Unshared(crate::Memory),
+    Shared(SharedMemory, VMMemoryImport),
+}
+
+impl ExportMemory {
+    pub fn unshared(self) -> Option<crate::Memory> {
+        match self {
+            ExportMemory::Unshared(m) => Some(m),
+            ExportMemory::Shared(..) => None,
+        }
+    }
+}