Repository: bytecodealliance/wasmtime
Pull request #2617 (2021-01-28 16:48:21)

Add knobs to limit memories/tables in a `Store`
Fuzzing has turned up that module linking can create large amounts of
tables and memories in addition to instances. For example if N instances
are allowed and M tables are allowed per-instance, then currently
wasmtime allows MxN tables (which is quite a lot). This is causing some
wasm-smith-generated modules to exceed resource limits while fuzzing!

This commits adds corresponding `max_tables` and `max_memories`
functions to sit alongside the `max_instances` configuration.
Additionally fuzzing now by default configures all of these to a
somewhat low value to avoid too much resource usage while fuzzing.




Patch:
diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs
index d31eba04c..a35bd186a 100644
--- a/crates/fuzzing/src/generators.rs
+++ b/crates/fuzzing/src/generators.rs
@@ -75,7 +75,7 @@ pub struct Config {
 impl Config {
     /// Converts this to a `wasmtime::Config` object
     pub fn to_wasmtime(&self) -> wasmtime::Config {
-        let mut cfg = wasmtime::Config::new();
+        let mut cfg = crate::fuzz_default_config(wasmtime::Strategy::Auto).unwrap();
         cfg.debug_info(self.debug_info)
             .static_memory_maximum_size(self.static_memory_maximum_size.unwrap_or(0).into())
             .static_memory_guard_size(self.static_memory_guard_size.unwrap_or(0).into())
diff --git a/crates/fuzzing/src/lib.rs b/crates/fuzzing/src/lib.rs
index 7ef338241..d131149dd 100644
--- a/crates/fuzzing/src/lib.rs
+++ b/crates/fuzzing/src/lib.rs
@@ -39,6 +39,9 @@ pub fn fuzz_default_config(strategy: wasmtime::Strategy) -> anyhow::Result<wasmt
         .wasm_bulk_memory(true)
         .wasm_reference_types(true)
         .wasm_module_linking(true)
+        .max_instances(100)
+        .max_tables(100)
+        .max_memories(100)
         .strategy(strategy)?;
     Ok(config)
 }
diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs
index 6581f9e0d..7dc19abbc 100644
--- a/crates/fuzzing/src/oracles.rs
+++ b/crates/fuzzing/src/oracles.rs
@@ -101,6 +101,9 @@ pub fn instantiate_with_config(
         Ok(_) => {}
         // Allow traps which can happen normally with `unreachable`
         Err(e) if e.downcast_ref::<Trap>().is_some() => {}
+        // Allow resource exhaustion since this is something that our wasm-smith
+        // generator doesn't guarantee is forbidden.
+        Err(e) if e.to_string().contains("resource limit exceeded") => {}
         Err(e) => panic!("failed to instantiate {}", e),
     }
 }
diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs
index 80b10548e..3edbfebb2 100644
--- a/crates/wasmtime/src/config.rs
+++ b/crates/wasmtime/src/config.rs
@@ -34,6 +34,8 @@ pub struct Config {
     pub(crate) features: WasmFeatures,
     pub(crate) wasm_backtrace_details_env_used: bool,
     pub(crate) max_instances: usize,
+    pub(crate) max_tables: usize,
+    pub(crate) max_memories: usize,
 }
 
 impl Config {
@@ -81,6 +83,8 @@ impl Config {
                 ..WasmFeatures::default()
             },
             max_instances: 10_000,
+            max_tables: 10_000,
+            max_memories: 10_000,
         };
         ret.wasm_backtrace_details(WasmBacktraceDetails::Environment);
         return ret;
@@ -655,11 +659,35 @@ impl Config {
     /// this `Store`.
     ///
     /// Instantiation will fail with an error if this limit is exceeded.
+    ///
+    /// This value defaults to 10,000.
     pub fn max_instances(&mut self, instances: usize) -> &mut Self {
         self.max_instances = instances;
         self
     }
 
+    /// Configures the maximum number of tables which can be created within
+    /// this `Store`.
+    ///
+    /// Instantiation will fail with an error if this limit is exceeded.
+    ///
+    /// This value defaults to 10,000.
+    pub fn max_tables(&mut self, tables: usize) -> &mut Self {
+        self.max_tables = tables;
+        self
+    }
+
+    /// Configures the maximum number of memories which can be created within
+    /// this `Store`.
+    ///
+    /// Instantiation will fail with an error if this limit is exceeded.
+    ///
+    /// This value defaults to 10,000.
+    pub fn max_memories(&mut self, memories: usize) -> &mut Self {
+        self.max_memories = memories;
+        self
+    }
+
     pub(crate) fn target_isa(&self) -> Box<dyn TargetIsa> {
         self.isa_flags
             .clone()
diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs
index 02ad1a1b9..47115d90a 100644
--- a/crates/wasmtime/src/instance.rs
+++ b/crates/wasmtime/src/instance.rs
@@ -257,7 +257,7 @@ impl<'a> Instantiator<'a> {
     ///   defined here.
     fn step(&mut self) -> Result<Option<(StoreInstanceHandle, Option<RuntimeInstance>)>> {
         if self.cur.initializer == 0 {
-            self.store.bump_instance_count()?;
+            self.store.bump_resource_counts(&self.cur.module)?;
         }
 
         // Read the current module's initializer and move forward the
diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs
index be4aaf156..2a85fa61c 100644
--- a/crates/wasmtime/src/store.rs
+++ b/crates/wasmtime/src/store.rs
@@ -67,8 +67,11 @@ pub(crate) struct StoreInner {
     /// Set of all compiled modules that we're holding a strong reference to
     /// the module's code for. This includes JIT functions, trampolines, etc.
     modules: RefCell<HashSet<ArcModuleCode>>,
-    /// The number of instantiated instances in this store.
+
+    // Numbers of resources instantiated in this store.
     instance_count: Cell<usize>,
+    memory_count: Cell<usize>,
+    table_count: Cell<usize>,
 }
 
 struct HostInfoKey(VMExternRef);
@@ -112,6 +115,8 @@ impl Store {
                 frame_info: Default::default(),
                 modules: Default::default(),
                 instance_count: Default::default(),
+                memory_count: Default::default(),
+                table_count: Default::default(),
             }),
         }
     }
@@ -216,12 +221,40 @@ impl Store {
         }
     }
 
-    pub(crate) fn bump_instance_count(&self) -> Result<()> {
-        let n = self.inner.instance_count.get();
-        self.inner.instance_count.set(n + 1);
-        if n >= self.engine().config().max_instances {
-            bail!("instance limit of {} exceeded", n);
+    pub(crate) fn bump_resource_counts(&self, module: &Module) -> Result<()> {
+        let config = self.engine().config();
+
+        fn bump(slot: &Cell<usize>, max: usize, amt: usize, desc: &str) -> Result<()> {
+            let new = slot.get().saturating_add(amt);
+            if new > max {
+                bail!(
+                    "resource limit exceeded: {} count too high at {}",
+                    desc,
+                    new
+                );
+            }
+            slot.set(new);
+            Ok(())
         }
+
+        let module = module.env_module();
+        let memories = module.memory_plans.len() - module.num_imported_memories;
+        let tables = module.table_plans.len() - module.num_imported_tables;
+
+        bump(
+            &self.inner.instance_count,
+            config.max_instances,
+            1,
+            "instance",
+        )?;
+        bump(
+            &self.inner.memory_count,
+            config.max_memories,
+            memories,
+            "memory",
+        )?;
+        bump(&self.inner.table_count, config.max_tables, tables, "table")?;
+
         Ok(())
     }
 


Test patch:
diff --git a/tests/all/module_linking.rs b/tests/all/module_linking.rs
index b81a9deb3..829ddd9b8 100644
--- a/tests/all/module_linking.rs
+++ b/tests/all/module_linking.rs
@@ -218,6 +218,81 @@ fn limit_instances() -> Result<()> {
     )?;
     let store = Store::new(&engine);
     let err = Instance::new(&store, &module, &[]).err().unwrap();
-    assert!(err.to_string().contains("instance limit of 10 exceeded"));
+    assert!(
+        err.to_string().contains("resource limit exceeded"),
+        "bad error: {}",
+        err
+    );
+    Ok(())
+}
+
+#[test]
+fn limit_memories() -> Result<()> {
+    let mut config = Config::new();
+    config.wasm_module_linking(true);
+    config.wasm_multi_memory(true);
+    config.max_memories(10);
+    let engine = Engine::new(&config);
+    let module = Module::new(
+        &engine,
+        r#"
+            (module
+              (module $m0
+                (memory 1 1)
+                (memory 1 1)
+                (memory 1 1)
+                (memory 1 1)
+                (memory 1 1)
+              )
+
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+            )
+        "#,
+    )?;
+    let store = Store::new(&engine);
+    let err = Instance::new(&store, &module, &[]).err().unwrap();
+    assert!(
+        err.to_string().contains("resource limit exceeded"),
+        "bad error: {}",
+        err
+    );
+    Ok(())
+}
+
+#[test]
+fn limit_tables() -> Result<()> {
+    let mut config = Config::new();
+    config.wasm_module_linking(true);
+    config.max_tables(10);
+    let engine = Engine::new(&config);
+    let module = Module::new(
+        &engine,
+        r#"
+            (module
+              (module $m0
+                (table 1 1 funcref)
+                (table 1 1 funcref)
+                (table 1 1 funcref)
+                (table 1 1 funcref)
+                (table 1 1 funcref)
+              )
+
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+              (instance (instantiate $m0))
+            )
+        "#,
+    )?;
+    let store = Store::new(&engine);
+    let err = Instance::new(&store, &module, &[]).err().unwrap();
+    assert!(
+        err.to_string().contains("resource limit exceeded"),
+        "bad error: {}",
+        err
+    );
     Ok(())
 }