From b9205b0b226e0930196ab993ad5abe42b39eafbf Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 17 Mar 2021 14:26:32 -0700 Subject: [PATCH 1/4] Add resource limiting to the Wasmtime API. This commit adds a `ResourceLimiter` trait to the Wasmtime API. When used in conjunction with `Store::new_with_limiter`, this can be used to monitor and prevent WebAssembly code from growing linear memories and tables. This is particularly useful when hosts need to take into account host resource usage to determine if WebAssembly code can consume more resources. A simple `StaticResourceLimiter` is also included with these changes that will simply limit the size of linear memories or tables for all instances created in the store based on static values. --- crates/runtime/src/instance.rs | 82 +++-- crates/runtime/src/instance/allocator.rs | 11 +- .../runtime/src/instance/allocator/pooling.rs | 12 +- .../src/instance/allocator/pooling/uffd.rs | 1 + crates/runtime/src/lib.rs | 2 +- crates/runtime/src/memory.rs | 32 +- crates/wasmtime/src/instance.rs | 1 + crates/wasmtime/src/lib.rs | 2 + crates/wasmtime/src/limiter.rs | 129 ++++++++ crates/wasmtime/src/memory.rs | 6 +- crates/wasmtime/src/store.rs | 36 ++- crates/wasmtime/src/trampoline.rs | 1 + crates/wasmtime/src/trampoline/func.rs | 2 + crates/wasmtime/src/trampoline/memory.rs | 4 + tests/all/limiter.rs | 279 ++++++++++++++++++ tests/all/main.rs | 1 + tests/all/memory_creator.rs | 11 +- 17 files changed, 579 insertions(+), 33 deletions(-) create mode 100644 crates/wasmtime/src/limiter.rs create mode 100644 tests/all/limiter.rs diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 7da8d11ee57c..4015ade5af60 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -37,6 +37,37 @@ mod allocator; pub use allocator::*; +/// Used by hosts to limit resource consumption of instances. +/// +/// An instance can be created with a resource limiter so that hosts can take into account +/// non-WebAssembly resource usage to determine if a linear memory or table should grow. +pub trait ResourceLimiter { + /// Notifies the resource limiter that an instance's linear memory has been requested to grow. + /// + /// * `current` is the current size of the linear memory in WebAssembly page units. + /// * `desired` is the desired size of the linear memory in WebAssembly page units. + /// * `maximum` is either the linear memory's maximum or a maximum from an instance allocator, + /// also in WebAssembly page units. A value of `None` indicates that the linear memory is + /// unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no + /// effect as the linear memory will not grow. + fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool; + + /// Notifies the resource limiter that an instance's table has been requested to grow. + /// + /// * `current` is the current number of elements in the table. + /// * `desired` is the desired number of elements in the table. + /// * `maximum` is either the table's maximum or a maximum from an instance allocator. + /// A value of `None` indicates that the table is unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no + /// effect as the table will not grow. + fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool; +} + /// Runtime representation of an instance value, which erases all `Instance` /// information since instances are just a collection of values. pub type RuntimeInstance = Rc>; @@ -69,6 +100,9 @@ pub(crate) struct Instance { /// Hosts can store arbitrary per-instance information here. host_state: Box, + /// The limiter to use for the instance, if there is one. + limiter: Option>, + /// Additional context used by compiled wasm code. This field is last, and /// represents a dynamically-sized array that extends beyond the nominal /// end of the struct (similar to a flexible array member). @@ -378,11 +412,21 @@ impl Instance { /// Returns `None` if memory can't be grown by the specified amount /// of pages. pub(crate) fn memory_grow(&self, memory_index: DefinedMemoryIndex, delta: u32) -> Option { - let result = self + let memory = self .memories .get(memory_index) - .unwrap_or_else(|| panic!("no memory for index {}", memory_index.index())) - .grow(delta); + .unwrap_or_else(|| panic!("no memory for index {}", memory_index.index())); + + if let Some(limiter) = &self.limiter { + let current = memory.size(); + let desired = current.checked_add(delta)?; + + if !limiter.memory_growing(current, desired, memory.maximum()) { + return None; + } + } + + let result = unsafe { memory.grow(delta) }; // Keep current the VMContext pointers used by compiled wasm code. self.set_memory(memory_index, self.memories[memory_index].vmmemory()); @@ -460,19 +504,27 @@ impl Instance { delta: u32, init_value: TableElement, ) -> Option { - unsafe { - let orig_size = self - .tables - .get(table_index) - .unwrap_or_else(|| panic!("no table for index {}", table_index.index())) - .grow(delta, init_value)?; + let table = self + .tables + .get(table_index) + .unwrap_or_else(|| panic!("no table for index {}", table_index.index())); - // Keep the `VMContext` pointers used by compiled Wasm code up to - // date. - self.set_table(table_index, self.tables[table_index].vmtable()); + if let Some(limiter) = &self.limiter { + let current = table.size(); + let desired = current.checked_add(delta)?; - Some(orig_size) + if !limiter.table_growing(current, desired, table.maximum()) { + return None; + } } + + let result = unsafe { table.grow(delta, init_value) }; + + // Keep the `VMContext` pointers used by compiled Wasm code up to + // date. + self.set_table(table_index, self.tables[table_index].vmtable()); + + result } pub(crate) fn defined_table_fill( @@ -818,10 +870,6 @@ pub struct InstanceHandle { } impl InstanceHandle { - pub(crate) unsafe fn new(instance: *mut Instance) -> Self { - Self { instance } - } - /// Create a new `InstanceHandle` pointing at the instance /// pointed to by the given `VMContext` pointer. /// diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 03618a69d17b..4d9ca727b1c2 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -1,6 +1,6 @@ use crate::externref::{ModuleInfoLookup, VMExternRefActivationsTable, EMPTY_MODULE_LOOKUP}; use crate::imports::Imports; -use crate::instance::{Instance, InstanceHandle, RuntimeMemoryCreator}; +use crate::instance::{Instance, InstanceHandle, ResourceLimiter, RuntimeMemoryCreator}; use crate::memory::{DefaultMemoryCreator, Memory}; use crate::table::{Table, TableElement}; use crate::traphandlers::Trap; @@ -59,6 +59,9 @@ pub struct InstanceAllocationRequest<'a> { /// The pointer to the module info lookup to use for the instance. pub module_info_lookup: Option<*const dyn ModuleInfoLookup>, + + /// The resource limiter to use for the instance. + pub limiter: Option>, } /// An link error while instantiating a module. @@ -637,6 +640,7 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { let tables = Self::create_tables(&req.module); let host_state = std::mem::replace(&mut req.host_state, Box::new(())); + let limiter = std::mem::take(&mut req.limiter); let handle = { let instance = Instance { @@ -649,6 +653,7 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { )), dropped_data: RefCell::new(EntitySet::with_capacity(req.module.passive_data.len())), host_state, + limiter, vmctx: VMContext {}, }; let layout = instance.alloc_layout(); @@ -657,7 +662,9 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { alloc::handle_alloc_error(layout); } ptr::write(instance_ptr, instance); - InstanceHandle::new(instance_ptr) + InstanceHandle { + instance: instance_ptr, + } }; initialize_vmcontext(handle.instance(), req); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index 5bc342dafcc6..64fe14f5b71b 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -370,6 +370,7 @@ impl InstancePool { dropped_elements: RefCell::new(EntitySet::new()), dropped_data: RefCell::new(EntitySet::new()), host_state: Box::new(()), + limiter: None, vmctx: VMContext {}, }, ); @@ -391,6 +392,7 @@ impl InstancePool { }; let host_state = std::mem::replace(&mut req.host_state, Box::new(())); + let limiter = std::mem::take(&mut req.limiter); unsafe { let instance = self.instance(index); @@ -401,6 +403,7 @@ impl InstancePool { instance.module.as_ref(), ); instance.host_state = host_state; + instance.limiter = limiter; Self::set_instance_memories( instance, @@ -411,7 +414,9 @@ impl InstancePool { initialize_vmcontext(instance, req); - Ok(InstanceHandle::new(instance as _)) + Ok(InstanceHandle { + instance: instance as _, + }) } } @@ -463,8 +468,9 @@ impl InstancePool { instance.tables.clear(); instance.dropped_elements.borrow_mut().clear(); - // Drop any host state + // Drop any host state and limiter instance.host_state = Box::new(()); + instance.limiter = None; self.free_list.lock().unwrap().push(index); } @@ -1371,6 +1377,7 @@ mod test { interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), module_info_lookup: None, + limiter: None, }, ) .expect("allocation should succeed"), @@ -1395,6 +1402,7 @@ mod test { interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), module_info_lookup: None, + limiter: None, }, ) { Err(InstantiationError::Limit(3)) => {} diff --git a/crates/runtime/src/instance/allocator/pooling/uffd.rs b/crates/runtime/src/instance/allocator/pooling/uffd.rs index 4c4b22caba1a..43ba9a654a9c 100644 --- a/crates/runtime/src/instance/allocator/pooling/uffd.rs +++ b/crates/runtime/src/instance/allocator/pooling/uffd.rs @@ -524,6 +524,7 @@ mod test { interrupts: ptr::null(), externref_activations_table: ptr::null_mut(), module_info_lookup: None, + limiter: None, }, ) .expect("instance should allocate"), diff --git a/crates/runtime/src/lib.rs b/crates/runtime/src/lib.rs index 20f28ffd9761..1c3ce53a3db4 100644 --- a/crates/runtime/src/lib.rs +++ b/crates/runtime/src/lib.rs @@ -40,7 +40,7 @@ pub use crate::imports::Imports; pub use crate::instance::{ InstanceAllocationRequest, InstanceAllocator, InstanceHandle, InstanceLimits, InstantiationError, LinkError, ModuleLimits, OnDemandInstanceAllocator, - PoolingAllocationStrategy, PoolingInstanceAllocator, RuntimeInstance, + PoolingAllocationStrategy, PoolingInstanceAllocator, ResourceLimiter, RuntimeInstance, }; pub use crate::jit_int::GdbJitImageRegistration; pub use crate::memory::{Memory, RuntimeLinearMemory, RuntimeMemoryCreator}; diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index 024d90124604..bc3aa8b63a08 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -33,6 +33,10 @@ pub trait RuntimeLinearMemory { /// Returns the number of allocated wasm pages. fn size(&self) -> u32; + /// Returns the maximum number of pages the memory can grow to. + /// Returns `None` if the memory is unbounded. + fn maximum(&self) -> Option; + /// Grow memory by the specified amount of wasm pages. /// /// Returns `None` if memory can't be grown by the specified amount @@ -105,6 +109,12 @@ impl RuntimeLinearMemory for MmapMemory { self.mmap.borrow().size } + /// Returns the maximum number of pages the memory can grow to. + /// Returns `None` if the memory is unbounded. + fn maximum(&self) -> Option { + self.maximum + } + /// Grow memory by the specified amount of wasm pages. /// /// Returns `None` if memory can't be grown by the specified amount @@ -226,6 +236,15 @@ impl Memory { } } + /// Returns the maximum number of pages the memory can grow to. + /// Returns `None` if the memory is unbounded. + pub fn maximum(&self) -> Option { + match &self.0 { + MemoryStorage::Static { maximum, .. } => Some(*maximum), + MemoryStorage::Dynamic(mem) => mem.maximum(), + } + } + /// Returns whether or not the underlying storage of the memory is "static". pub(crate) fn is_static(&self) -> bool { if let MemoryStorage::Static { .. } = &self.0 { @@ -239,7 +258,16 @@ impl Memory { /// /// Returns `None` if memory can't be grown by the specified amount /// of wasm pages. - pub fn grow(&self, delta: u32) -> Option { + /// + /// # Safety + /// + /// Resizing the memory can reallocate the memory buffer for dynamic memories. + /// An instance's `VMContext` may have pointers to the memory's base and will + /// need to be fixed up after growing the memory. + /// + /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates + /// this unsafety. + pub unsafe fn grow(&self, delta: u32) -> Option { match &self.0 { MemoryStorage::Static { base, @@ -266,7 +294,7 @@ impl Memory { let start = usize::try_from(old_size).unwrap() * WASM_PAGE_SIZE as usize; let len = usize::try_from(delta).unwrap() * WASM_PAGE_SIZE as usize; - make_accessible(unsafe { base.add(start) }, len).ok()?; + make_accessible(base.add(start), len).ok()?; size.set(new_size); diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index 04a47337ca9b..baef6cb7df5a 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -525,6 +525,7 @@ impl<'a> Instantiator<'a> { as *const VMExternRefActivationsTable as *mut _, module_info_lookup: Some(self.store.module_info_lookup()), + limiter: self.store.limiter(), })?; // After we've created the `InstanceHandle` we still need to run diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index 98a86a279ecc..4706c42b59ec 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -283,6 +283,7 @@ mod config; mod engine; mod externals; mod instance; +mod limiter; mod linker; mod memory; mod module; @@ -299,6 +300,7 @@ pub use crate::engine::*; pub use crate::externals::*; pub use crate::func::*; pub use crate::instance::Instance; +pub use crate::limiter::*; pub use crate::linker::*; pub use crate::memory::*; pub use crate::module::{FrameInfo, FrameSymbol, Module}; diff --git a/crates/wasmtime/src/limiter.rs b/crates/wasmtime/src/limiter.rs new file mode 100644 index 000000000000..31751fc30741 --- /dev/null +++ b/crates/wasmtime/src/limiter.rs @@ -0,0 +1,129 @@ +use crate::{Store, StoreInner}; +use std::rc::Weak; + +/// Used by hosts to limit resource consumption of instances. +/// +/// A [`Store`] can be created with a resource limiter so that hosts can take into account +/// non-WebAssembly resource usage to determine if a linear memory or table should grow. +pub trait ResourceLimiter { + /// Notifies the resource limiter that an instance's linear memory has been requested to grow. + /// + /// * `current` is the current size of the linear memory in WebAssembly page units. + /// * `desired` is the desired size of the linear memory in WebAssembly page units. + /// * `maximum` is either the linear memory's maximum or a maximum from an instance allocator, + /// also in WebAssembly page units. A value of `None` indicates that the linear memory is + /// unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no + /// effect as the linear memory will not grow. + fn memory_growing( + &self, + store: &Store, + current: u32, + desired: u32, + maximum: Option, + ) -> bool; + + /// Notifies the resource limiter that an instance's table has been requested to grow. + /// + /// * `current` is the current number of elements in the table. + /// * `desired` is the desired number of elements in the table. + /// * `maximum` is either the table's maximum or a maximum from an instance allocator, + /// A value of `None` indicates that the table is unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no + /// effect as the table will not grow. + fn table_growing( + &self, + store: &Store, + current: u32, + desired: u32, + maximum: Option, + ) -> bool; +} + +pub(crate) struct ResourceLimiterProxy { + store: Weak, + limiter: Box, +} + +impl ResourceLimiterProxy { + pub(crate) fn new(store: &Store, limiter: impl ResourceLimiter + 'static) -> Self { + Self { + store: store.weak(), + limiter: Box::new(limiter), + } + } +} + +impl wasmtime_runtime::ResourceLimiter for ResourceLimiterProxy { + fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { + self.limiter.memory_growing( + &Store::upgrade(&self.store).unwrap(), + current, + desired, + maximum, + ) + } + + fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { + self.limiter.table_growing( + &Store::upgrade(&self.store).unwrap(), + current, + desired, + maximum, + ) + } +} + +/// A resource limiter that statically limits how much memories and tables can grow. +pub struct StaticResourceLimiter { + memory_limit: Option, + table_limit: Option, +} + +impl StaticResourceLimiter { + /// Creates a new [`StaticResourceLimiter`]. + /// + /// The `memory_limit` parameter is the number of WebAssembly pages a linear memory can grow to. + /// If `None`, the limiter will not limit linear memory growth. + /// + /// The `table_limit` parameter is the number of elements a table can grow to. + /// If `None`, the limiter will not limit table growth. + pub fn new(memory_limit: Option, table_limit: Option) -> Self { + Self { + memory_limit, + table_limit, + } + } +} + +impl ResourceLimiter for StaticResourceLimiter { + fn memory_growing( + &self, + _store: &Store, + _current: u32, + desired: u32, + _maximum: Option, + ) -> bool { + match self.memory_limit { + Some(limit) if desired > limit => false, + _ => true, + } + } + + fn table_growing( + &self, + _store: &Store, + _current: u32, + desired: u32, + _maximum: Option, + ) -> bool { + match self.table_limit { + Some(limit) if desired > limit => false, + _ => true, + } + } +} diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 39b634927467..4ceaed7f56ae 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -454,7 +454,7 @@ impl Memory { .memory_index(unsafe { &*self.wasmtime_export.definition }); self.instance .memory_grow(index, delta) - .ok_or_else(|| anyhow!("failed to grow memory")) + .ok_or_else(|| anyhow!("failed to grow memory by `{}`", delta)) } pub(crate) unsafe fn from_wasmtime_memory( @@ -500,6 +500,10 @@ pub unsafe trait LinearMemory { /// Returns the number of allocated wasm pages. fn size(&self) -> u32; + /// Returns the maximum number of pages the memory can grow to. + /// Returns `None` if the memory is unbounded. + fn maximum(&self) -> Option; + /// Grow memory by the specified amount of wasm pages. /// /// Returns `None` if memory can't be grown by the specified amount diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 301b1192c10c..63714fc0b9a3 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1,6 +1,6 @@ use crate::{ module::ModuleRegistry, signatures::SignatureCollection, trampoline::StoreInstanceHandle, - Engine, Func, Module, Trap, + Engine, Func, Module, ResourceLimiter, ResourceLimiterProxy, Trap, }; use anyhow::{bail, Result}; use std::any::{Any, TypeId}; @@ -12,7 +12,7 @@ use std::future::Future; use std::hash::{Hash, Hasher}; use std::pin::Pin; use std::ptr; -use std::rc::Rc; +use std::rc::{Rc, Weak}; use std::sync::Arc; use std::task::{Context, Poll}; use wasmtime_runtime::{ @@ -89,6 +89,7 @@ pub(crate) struct StoreInner { current_poll_cx: Cell<*mut Context<'static>>, out_of_gas_behavior: Cell, context_values: RefCell>>, + limiter: RefCell>>, } #[derive(Copy, Clone)] @@ -121,7 +122,7 @@ impl Hash for HostInfoKey { impl Store { /// Creates a new store to be associated with the given [`Engine`]. - pub fn new(engine: &Engine) -> Store { + pub fn new(engine: &Engine) -> Self { // Ensure that wasmtime_runtime's signal handlers are configured. Note // that at the `Store` level it means we should perform this // once-per-thread. Platforms like Unix, however, only require this @@ -130,7 +131,7 @@ impl Store { wasmtime_runtime::init_traps(crate::module::GlobalModuleRegistry::is_wasm_pc) .expect("failed to initialize trap handling"); - Store { + Self { inner: Rc::new(StoreInner { engine: engine.clone(), interrupts: Arc::new(Default::default()), @@ -149,10 +150,24 @@ impl Store { current_poll_cx: Cell::new(ptr::null_mut()), out_of_gas_behavior: Cell::new(OutOfGas::Trap), context_values: RefCell::new(HashMap::new()), + limiter: RefCell::new(None), }), } } + /// Creates a new store to be associated with the given [`Engine`] and using the given + /// resource limiter for any instances created in the store. + pub fn new_with_limiter(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { + let store = Store::new(engine); + + { + let mut l = store.inner.limiter.borrow_mut(); + *l = Some(Arc::new(ResourceLimiterProxy::new(&store, limiter))); + } + + store + } + /// Gets a host function from the [`Config`](crate::Config) associated with this [`Store`]. /// /// Returns `None` if the given host function is not defined. @@ -202,6 +217,10 @@ impl Store { } } + pub(crate) fn limiter(&self) -> Option> { + self.inner.limiter.borrow().clone() + } + pub(crate) fn signatures(&self) -> &RefCell { &self.inner.signatures } @@ -301,6 +320,15 @@ impl Store { self.existing_instance_handle(InstanceHandle::from_vmctx(cx)) } + pub(crate) fn weak(&self) -> Weak { + Rc::downgrade(&self.inner) + } + + pub(crate) fn upgrade(weak: &Weak) -> Option { + let inner = weak.upgrade()?; + Some(Self { inner }) + } + #[cfg_attr(not(target_os = "linux"), allow(dead_code))] // not used on all platforms pub(crate) fn set_signal_handler(&self, handler: Option>>) { *self.inner.signal_handler.borrow_mut() = handler; diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index ee0bf25c9e40..2ab6d6b88666 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -78,6 +78,7 @@ fn create_handle( as *const VMExternRefActivationsTable as *mut _, module_info_lookup: Some(store.module_info_lookup()), + limiter: None, }, )?; diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index 0110e7b2a9fb..b381c561ea01 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -288,6 +288,7 @@ pub fn create_function( interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), module_info_lookup: None, + limiter: None, })?, trampoline, )) @@ -320,6 +321,7 @@ pub unsafe fn create_raw_function( interrupts: std::ptr::null(), externref_activations_table: std::ptr::null_mut(), module_info_lookup: None, + limiter: None, })?, ) } diff --git a/crates/wasmtime/src/trampoline/memory.rs b/crates/wasmtime/src/trampoline/memory.rs index 0876ead22554..9f3ea772b080 100644 --- a/crates/wasmtime/src/trampoline/memory.rs +++ b/crates/wasmtime/src/trampoline/memory.rs @@ -37,6 +37,10 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.size() } + fn maximum(&self) -> Option { + self.mem.maximum() + } + fn grow(&self, delta: u32) -> Option { self.mem.grow(delta) } diff --git a/tests/all/limiter.rs b/tests/all/limiter.rs new file mode 100644 index 000000000000..27956fad093d --- /dev/null +++ b/tests/all/limiter.rs @@ -0,0 +1,279 @@ +use anyhow::Result; +use std::cell::{Cell, RefCell}; +use std::rc::Rc; +use wasmtime::*; + +#[test] +fn test_static_limiter() -> Result<()> { + let engine = Engine::default(); + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; + + let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(Some(10), Some(5))); + + let instance = Instance::new(&store, &module, &[])?; + + let memory = instance.get_memory("m").unwrap(); + + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + + assert_eq!( + memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), + "failed to grow memory by `1`" + ); + + let table = instance.get_table("t").unwrap(); + + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + + assert_eq!( + table + .grow(1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + + Ok(()) +} + +#[test] +fn test_static_limiter_memory_only() -> Result<()> { + let engine = Engine::default(); + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; + + let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(Some(10), None)); + + let instance = Instance::new(&store, &module, &[])?; + + let memory = instance.get_memory("m").unwrap(); + + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + + assert_eq!( + memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), + "failed to grow memory by `1`" + ); + + let table = instance.get_table("t").unwrap(); + + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + + Ok(()) +} + +#[test] +fn test_static_limiter_table_only() -> Result<()> { + let engine = Engine::default(); + let module = Module::new( + &engine, + r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, + )?; + + let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(None, Some(5))); + + let instance = Instance::new(&store, &module, &[])?; + + let memory = instance.get_memory("m").unwrap(); + + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + memory.grow(1)?; + + let table = instance.get_table("t").unwrap(); + + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + + assert_eq!( + table + .grow(1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + + Ok(()) +} + +struct MemoryContext { + host_memory_used: usize, + wasm_memory_used: usize, + memory_limit: usize, + limit_exceeded: bool, +} + +struct HostMemoryLimiter(Rc>); + +impl ResourceLimiter for HostMemoryLimiter { + fn memory_growing( + &self, + store: &Store, + current: u32, + desired: u32, + maximum: Option, + ) -> bool { + if let Some(ctx) = store.get::>>() { + let mut ctx = ctx.borrow_mut(); + + // Check if the desired exceeds a maximum (either from Wasm or from the host) + if desired > maximum.unwrap_or(u32::MAX) { + ctx.limit_exceeded = true; + return false; + } + + assert_eq!(current as usize * 0x10000, ctx.wasm_memory_used); + let desired = desired as usize * 0x10000; + + if desired + ctx.host_memory_used > ctx.memory_limit { + ctx.limit_exceeded = true; + return false; + } + + ctx.wasm_memory_used = desired; + } + + true + } + + fn table_growing( + &self, + _store: &Store, + _current: u32, + _desired: u32, + _maximum: Option, + ) -> bool { + true + } +} + +impl Drop for HostMemoryLimiter { + fn drop(&mut self) { + self.0.set(true); + } +} + +#[test] +fn test_custom_limiter() -> Result<()> { + let mut config = Config::default(); + + // This approximates a function that would "allocate" resources that the host tracks. + // Here this is a simple function that increments the current host memory "used". + config.wrap_host_func("", "alloc", |caller: Caller, size: u32| -> u32 { + if let Some(ctx) = caller.store().get::>>() { + let mut ctx = ctx.borrow_mut(); + let size = size as usize; + + if size + ctx.host_memory_used + ctx.wasm_memory_used <= ctx.memory_limit { + ctx.host_memory_used += size; + return 1; + } + + ctx.limit_exceeded = true; + } + + 0 + }); + + let engine = Engine::new(&config)?; + let module = Module::new( + &engine, + r#"(module (import "" "alloc" (func $alloc (param i32) (result i32))) (memory (export "m") 0) (func (export "f") (param i32) (result i32) local.get 0 call $alloc))"#, + )?; + + let dropped = Rc::new(Cell::new(false)); + let store = Store::new_with_limiter(&engine, HostMemoryLimiter(dropped.clone())); + + assert!(store + .set(Rc::new(RefCell::new(MemoryContext { + host_memory_used: 0, + wasm_memory_used: 0, + memory_limit: 1 << 20, // 16 wasm pages is the limit for both wasm + host memory + limit_exceeded: false + }))) + .is_ok()); + + let linker = Linker::new(&store); + let instance = linker.instantiate(&module)?; + let memory = instance.get_memory("m").unwrap(); + + // Grow the memory by 640 KiB + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + + assert!( + !store + .get::>>() + .unwrap() + .borrow() + .limit_exceeded + ); + + // Grow the host "memory" by 384 KiB + let f = instance.get_typed_func::("f")?; + + assert_eq!(f.call(1 * 0x10000).unwrap(), 1); + assert_eq!(f.call(3 * 0x10000).unwrap(), 1); + assert_eq!(f.call(2 * 0x10000).unwrap(), 1); + + // Memory is at the maximum, but the limit hasn't been exceeded + assert!( + !store + .get::>>() + .unwrap() + .borrow() + .limit_exceeded + ); + + // Try to grow the memory again + assert_eq!( + memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), + "failed to grow memory by `1`" + ); + + assert!( + store + .get::>>() + .unwrap() + .borrow() + .limit_exceeded + ); + + // Try to grow the host "memory" again + assert_eq!(f.call(1).unwrap(), 0); + + assert!( + store + .get::>>() + .unwrap() + .borrow() + .limit_exceeded + ); + + drop(f); + drop(memory); + drop(instance); + drop(linker); + drop(store); + + assert!(dropped.get()); + + Ok(()) +} diff --git a/tests/all/main.rs b/tests/all/main.rs index 4c921e60a3b6..172d6b9fd951 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -13,6 +13,7 @@ mod import_calling_export; mod import_indexes; mod instance; mod invoke_func_via_table; +mod limiter; mod linker; mod memory_creator; mod module; diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index e877b5e22d6d..5c49414706ef 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -17,6 +17,7 @@ mod not_for_windows { struct CustomMemory { mem: *mut c_void, size: usize, + guard_size: usize, used_wasm_pages: RefCell, glob_page_counter: Arc>, } @@ -43,6 +44,7 @@ mod not_for_windows { Self { mem, size, + guard_size, used_wasm_pages: RefCell::new(num_wasm_pages), glob_page_counter: glob_counter, } @@ -63,6 +65,10 @@ mod not_for_windows { *self.used_wasm_pages.borrow() } + fn maximum(&self) -> Option { + Some((self.size as u32 - self.guard_size as u32) / WASM_PAGE_SIZE) + } + fn grow(&self, delta: u32) -> Option { let delta_size = (delta as usize).checked_mul(WASM_PAGE_SIZE as usize)?; @@ -70,11 +76,8 @@ mod not_for_windows { let prev_size = (prev_pages as usize).checked_mul(WASM_PAGE_SIZE as usize)?; let new_pages = prev_pages.checked_add(delta)?; - let new_size = (new_pages as usize).checked_mul(WASM_PAGE_SIZE as usize)?; - - let guard_size = unsafe { sysconf(_SC_PAGESIZE) as usize }; - if new_size > self.size - guard_size { + if new_pages > self.maximum().unwrap() { return None; } unsafe { From fd6d264ee8ec7a288f8668322d77f6b612f81f64 Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Tue, 23 Mar 2021 14:11:07 -0700 Subject: [PATCH 2/4] Code review feedback. * Implemented `StoreLimits` and `StoreLimitsBuilder`. * Moved `max_instances`, `max_memories`, `max_tables` out of `Config` and into `StoreLimits`. * Moved storage of the limiter in the runtime into `Memory` and `Table`. * Made `InstanceAllocationRequest` use a reference to the limiter. * Updated docs. * Made `ResourceLimiterProxy` generic to remove a level of indirection. * Fixed the limiter not being used for `wasmtime::Memory` and `wasmtime::Table`. --- crates/c-api/src/config.rs | 5 - crates/fuzzing/src/lib.rs | 7 - crates/fuzzing/src/oracles.rs | 27 +- crates/runtime/src/instance.rs | 36 +-- crates/runtime/src/instance/allocator.rs | 22 +- .../runtime/src/instance/allocator/pooling.rs | 22 +- crates/runtime/src/memory.rs | 86 ++++-- crates/runtime/src/table.rs | 96 ++++--- crates/wasmtime/src/config.rs | 39 --- crates/wasmtime/src/instance.rs | 2 +- crates/wasmtime/src/lib.rs | 4 +- crates/wasmtime/src/limiter.rs | 129 --------- crates/wasmtime/src/limits.rs | 256 ++++++++++++++++++ crates/wasmtime/src/store.rs | 59 ++-- crates/wasmtime/src/trampoline.rs | 2 +- tests/all/{limiter.rs => limits.rs} | 21 +- tests/all/main.rs | 2 +- tests/all/module_linking.rs | 9 +- 18 files changed, 500 insertions(+), 324 deletions(-) delete mode 100644 crates/wasmtime/src/limiter.rs create mode 100644 crates/wasmtime/src/limits.rs rename tests/all/{limiter.rs => limits.rs} (91%) diff --git a/crates/c-api/src/config.rs b/crates/c-api/src/config.rs index f84bc320cd8d..3e6e313ba9d1 100644 --- a/crates/c-api/src/config.rs +++ b/crates/c-api/src/config.rs @@ -176,8 +176,3 @@ pub extern "C" fn wasmtime_config_static_memory_guard_size_set(c: &mut wasm_conf pub extern "C" fn wasmtime_config_dynamic_memory_guard_size_set(c: &mut wasm_config_t, size: u64) { c.config.dynamic_memory_guard_size(size); } - -#[no_mangle] -pub extern "C" fn wasmtime_config_max_instances_set(c: &mut wasm_config_t, limit: usize) { - c.config.max_instances(limit); -} diff --git a/crates/fuzzing/src/lib.rs b/crates/fuzzing/src/lib.rs index 6e4e991c4c62..7ef338241109 100644 --- a/crates/fuzzing/src/lib.rs +++ b/crates/fuzzing/src/lib.rs @@ -39,13 +39,6 @@ pub fn fuzz_default_config(strategy: wasmtime::Strategy) -> anyhow::Result Store { + Store::new_with_limits( + &engine, + StoreLimitsBuilder::new() + // The limits here are chosen based on the default "maximum type size" + // configured in wasm-smith, which is 1000. This means that instances + // are allowed to, for example, export up to 1000 memories. We bump that + // a little bit here to give us some slop. + .instances(1100) + .tables(1100) + .memories(1100) + .build(), + ) +} + /// Methods of timing out execution of a WebAssembly module #[derive(Debug)] pub enum Timeout { @@ -95,7 +110,7 @@ pub fn instantiate_with_config( _ => false, }); let engine = Engine::new(&config).unwrap(); - let store = Store::new(&engine); + let store = create_store(&engine); let mut timeout_state = SignalOnDrop::default(); match timeout { @@ -203,7 +218,7 @@ pub fn differential_execution( config.wasm_module_linking(false); let engine = Engine::new(&config).unwrap(); - let store = Store::new(&engine); + let store = create_store(&engine); let module = Module::new(&engine, &wasm).unwrap(); @@ -348,7 +363,7 @@ pub fn make_api_calls(api: crate::generators::api::ApiCalls) { ApiCall::StoreNew => { log::trace!("creating store"); assert!(store.is_none()); - store = Some(Store::new(engine.as_ref().unwrap())); + store = Some(create_store(engine.as_ref().unwrap())); } ApiCall::ModuleNew { id, wasm } => { @@ -439,7 +454,7 @@ pub fn spectest(fuzz_config: crate::generators::Config, test: crate::generators: config.wasm_reference_types(false); config.wasm_bulk_memory(false); config.wasm_module_linking(false); - let store = Store::new(&Engine::new(&config).unwrap()); + let store = create_store(&Engine::new(&config).unwrap()); if fuzz_config.consume_fuel { store.add_fuel(u64::max_value()).unwrap(); } @@ -463,7 +478,7 @@ pub fn table_ops( let mut config = fuzz_config.to_wasmtime(); config.wasm_reference_types(true); let engine = Engine::new(&config).unwrap(); - let store = Store::new(&engine); + let store = create_store(&engine); if fuzz_config.consume_fuel { store.add_fuel(u64::max_value()).unwrap(); } @@ -578,7 +593,7 @@ pub fn differential_wasmi_execution(wasm: &[u8], config: &crate::generators::Con let mut wasmtime_config = config.to_wasmtime(); wasmtime_config.cranelift_nan_canonicalization(true); let wasmtime_engine = Engine::new(&wasmtime_config).unwrap(); - let wasmtime_store = Store::new(&wasmtime_engine); + let wasmtime_store = create_store(&wasmtime_engine); if config.consume_fuel { wasmtime_store.add_fuel(u64::max_value()).unwrap(); } diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 4015ade5af60..916aa90f26c8 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -66,6 +66,21 @@ pub trait ResourceLimiter { /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no /// effect as the table will not grow. fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool; + + /// The maximum number of instances that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + fn instances(&self) -> usize; + + /// The maximum number of tables that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + fn tables(&self) -> usize; + + /// The maximum number of tables that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + fn memories(&self) -> usize; } /// Runtime representation of an instance value, which erases all `Instance` @@ -100,9 +115,6 @@ pub(crate) struct Instance { /// Hosts can store arbitrary per-instance information here. host_state: Box, - /// The limiter to use for the instance, if there is one. - limiter: Option>, - /// Additional context used by compiled wasm code. This field is last, and /// represents a dynamically-sized array that extends beyond the nominal /// end of the struct (similar to a flexible array member). @@ -417,15 +429,6 @@ impl Instance { .get(memory_index) .unwrap_or_else(|| panic!("no memory for index {}", memory_index.index())); - if let Some(limiter) = &self.limiter { - let current = memory.size(); - let desired = current.checked_add(delta)?; - - if !limiter.memory_growing(current, desired, memory.maximum()) { - return None; - } - } - let result = unsafe { memory.grow(delta) }; // Keep current the VMContext pointers used by compiled wasm code. @@ -509,15 +512,6 @@ impl Instance { .get(table_index) .unwrap_or_else(|| panic!("no table for index {}", table_index.index())); - if let Some(limiter) = &self.limiter { - let current = table.size(); - let desired = current.checked_add(delta)?; - - if !limiter.table_growing(current, desired, table.maximum()) { - return None; - } - } - let result = unsafe { table.grow(delta, init_value) }; // Keep the `VMContext` pointers used by compiled Wasm code up to diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 4d9ca727b1c2..c4825874a019 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -61,7 +61,7 @@ pub struct InstanceAllocationRequest<'a> { pub module_info_lookup: Option<*const dyn ModuleInfoLookup>, /// The resource limiter to use for the instance. - pub limiter: Option>, + pub limiter: Option<&'a Arc>, } /// An link error while instantiating a module. @@ -593,12 +593,15 @@ impl OnDemandInstanceAllocator { } } - fn create_tables(module: &Module) -> PrimaryMap { + fn create_tables( + module: &Module, + limiter: &Option<&Arc>, + ) -> PrimaryMap { let num_imports = module.num_imported_tables; let mut tables: PrimaryMap = PrimaryMap::with_capacity(module.table_plans.len() - num_imports); for table in &module.table_plans.values().as_slice()[num_imports..] { - tables.push(Table::new_dynamic(table)); + tables.push(Table::new_dynamic(table, limiter)); } tables } @@ -606,6 +609,7 @@ impl OnDemandInstanceAllocator { fn create_memories( &self, module: &Module, + limiter: &Option<&Arc>, ) -> Result, InstantiationError> { let creator = self .mem_creator @@ -615,8 +619,10 @@ impl OnDemandInstanceAllocator { let mut memories: PrimaryMap = PrimaryMap::with_capacity(module.memory_plans.len() - num_imports); for plan in &module.memory_plans.values().as_slice()[num_imports..] { - memories - .push(Memory::new_dynamic(plan, creator).map_err(InstantiationError::Resource)?); + memories.push( + Memory::new_dynamic(plan, creator, limiter) + .map_err(InstantiationError::Resource)?, + ); } Ok(memories) } @@ -636,11 +642,10 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { &self, mut req: InstanceAllocationRequest, ) -> Result { - let memories = self.create_memories(&req.module)?; - let tables = Self::create_tables(&req.module); + let memories = self.create_memories(&req.module, &req.limiter)?; + let tables = Self::create_tables(&req.module, &req.limiter); let host_state = std::mem::replace(&mut req.host_state, Box::new(())); - let limiter = std::mem::take(&mut req.limiter); let handle = { let instance = Instance { @@ -653,7 +658,6 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { )), dropped_data: RefCell::new(EntitySet::with_capacity(req.module.passive_data.len())), host_state, - limiter, vmctx: VMContext {}, }; let layout = instance.alloc_layout(); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index 64fe14f5b71b..d9bb374ba50c 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -9,7 +9,7 @@ use super::{ initialize_instance, initialize_vmcontext, InstanceAllocationRequest, InstanceAllocator, - InstanceHandle, InstantiationError, + InstanceHandle, InstantiationError, ResourceLimiter, }; use crate::{instance::Instance, Memory, Mmap, Table, VMContext}; use anyhow::{anyhow, bail, Context, Result}; @@ -370,7 +370,6 @@ impl InstancePool { dropped_elements: RefCell::new(EntitySet::new()), dropped_data: RefCell::new(EntitySet::new()), host_state: Box::new(()), - limiter: None, vmctx: VMContext {}, }, ); @@ -392,7 +391,6 @@ impl InstancePool { }; let host_state = std::mem::replace(&mut req.host_state, Box::new(())); - let limiter = std::mem::take(&mut req.limiter); unsafe { let instance = self.instance(index); @@ -403,14 +401,20 @@ impl InstancePool { instance.module.as_ref(), ); instance.host_state = host_state; - instance.limiter = limiter; Self::set_instance_memories( instance, self.memories.get(index), self.memories.max_wasm_pages, + &req.limiter, + )?; + + Self::set_instance_tables( + instance, + self.tables.get(index), + self.tables.max_elements, + &req.limiter, )?; - Self::set_instance_tables(instance, self.tables.get(index), self.tables.max_elements)?; initialize_vmcontext(instance, req); @@ -468,9 +472,8 @@ impl InstancePool { instance.tables.clear(); instance.dropped_elements.borrow_mut().clear(); - // Drop any host state and limiter + // Drop any host state instance.host_state = Box::new(()); - instance.limiter = None; self.free_list.lock().unwrap().push(index); } @@ -479,6 +482,7 @@ impl InstancePool { instance: &mut Instance, mut memories: impl Iterator, max_pages: u32, + limiter: &Option<&Arc>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -493,6 +497,7 @@ impl InstancePool { memories.next().unwrap(), max_pages, commit_memory_pages, + limiter, ) .map_err(InstantiationError::Resource)?, ); @@ -509,6 +514,7 @@ impl InstancePool { instance: &mut Instance, mut tables: impl Iterator, max_elements: u32, + limiter: &Option<&Arc>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -522,7 +528,7 @@ impl InstancePool { instance .tables - .push(Table::new_static(plan, base as _, max_elements)); + .push(Table::new_static(plan, base as _, max_elements, limiter)); } let mut dropped_elements = instance.dropped_elements.borrow_mut(); diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index bc3aa8b63a08..a8dea35c62e5 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -4,12 +4,14 @@ use crate::mmap::Mmap; use crate::vmcontext::VMMemoryDefinition; +use crate::ResourceLimiter; use anyhow::Result; use more_asserts::{assert_ge, assert_le}; use std::cell::{Cell, RefCell}; use std::cmp::min; use std::convert::TryFrom; use std::ptr; +use std::sync::Arc; use wasmtime_environ::{MemoryPlan, MemoryStyle, WASM_MAX_PAGES, WASM_PAGE_SIZE}; /// A memory allocator @@ -199,12 +201,22 @@ enum MemoryStorage { } /// Represents an instantiation of a WebAssembly memory. -pub struct Memory(MemoryStorage); +pub struct Memory { + storage: MemoryStorage, + limiter: Option>, +} impl Memory { /// Create a new dynamic (movable) memory instance for the specified plan. - pub fn new_dynamic(plan: &MemoryPlan, creator: &dyn RuntimeMemoryCreator) -> Result { - Ok(Self(MemoryStorage::Dynamic(creator.new_memory(plan)?))) + pub fn new_dynamic( + plan: &MemoryPlan, + creator: &dyn RuntimeMemoryCreator, + limiter: &Option<&Arc>, + ) -> Result { + Ok(Self { + storage: MemoryStorage::Dynamic(creator.new_memory(plan)?), + limiter: limiter.cloned(), + }) } /// Create a new static (immovable) memory instance for the specified plan. @@ -213,33 +225,41 @@ impl Memory { base: *mut u8, maximum: u32, make_accessible: fn(*mut u8, usize) -> Result<()>, + limiter: &Option<&Arc>, ) -> Result { if plan.memory.minimum > 0 { make_accessible(base, plan.memory.minimum as usize * WASM_PAGE_SIZE as usize)?; } - Ok(Self(MemoryStorage::Static { - base, - size: Cell::new(plan.memory.minimum), - maximum: min(plan.memory.maximum.unwrap_or(maximum), maximum), - make_accessible, - #[cfg(all(feature = "uffd", target_os = "linux"))] - guard_page_faults: RefCell::new(Vec::new()), - })) + Ok(Self { + storage: MemoryStorage::Static { + base, + size: Cell::new(plan.memory.minimum), + maximum: min(plan.memory.maximum.unwrap_or(maximum), maximum), + make_accessible, + #[cfg(all(feature = "uffd", target_os = "linux"))] + guard_page_faults: RefCell::new(Vec::new()), + }, + limiter: limiter.cloned(), + }) } /// Returns the number of allocated wasm pages. pub fn size(&self) -> u32 { - match &self.0 { + match &self.storage { MemoryStorage::Static { size, .. } => size.get(), MemoryStorage::Dynamic(mem) => mem.size(), } } - /// Returns the maximum number of pages the memory can grow to. + /// Returns the maximum number of pages the memory can grow to at runtime. + /// /// Returns `None` if the memory is unbounded. + /// + /// The runtime maximum may not be equal to the maximum from the linear memory's + /// Wasm type when it is being constrained by an instance allocator. pub fn maximum(&self) -> Option { - match &self.0 { + match &self.storage { MemoryStorage::Static { maximum, .. } => Some(*maximum), MemoryStorage::Dynamic(mem) => mem.maximum(), } @@ -247,7 +267,7 @@ impl Memory { /// Returns whether or not the underlying storage of the memory is "static". pub(crate) fn is_static(&self) -> bool { - if let MemoryStorage::Static { .. } = &self.0 { + if let MemoryStorage::Static { .. } = &self.storage { true } else { false @@ -268,7 +288,16 @@ impl Memory { /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// this unsafety. pub unsafe fn grow(&self, delta: u32) -> Option { - match &self.0 { + if let Some(limiter) = &self.limiter { + let current = self.size(); + let desired = current.checked_add(delta)?; + + if !limiter.memory_growing(current, desired, self.maximum()) { + return None; + } + } + + match &self.storage { MemoryStorage::Static { base, size, @@ -306,7 +335,7 @@ impl Memory { /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm code. pub fn vmmemory(&self) -> VMMemoryDefinition { - match &self.0 { + match &self.storage { MemoryStorage::Static { base, size, .. } => VMMemoryDefinition { base: *base, current_length: size.get() as usize * WASM_PAGE_SIZE as usize, @@ -327,7 +356,7 @@ impl Memory { size: usize, reset: fn(*mut u8, usize) -> Result<()>, ) { - match &self.0 { + match &self.storage { MemoryStorage::Static { guard_page_faults, .. } => { @@ -348,7 +377,7 @@ impl Memory { /// This function will panic if called on a dynamic memory. #[cfg(all(feature = "uffd", target_os = "linux"))] pub(crate) fn reset_guard_pages(&self) -> Result<()> { - match &self.0 { + match &self.storage { MemoryStorage::Static { guard_page_faults, .. } => { @@ -373,13 +402,16 @@ impl Default for Memory { unreachable!() } - Self(MemoryStorage::Static { - base: ptr::null_mut(), - size: Cell::new(0), - maximum: 0, - make_accessible, - #[cfg(all(feature = "uffd", target_os = "linux"))] - guard_page_faults: RefCell::new(Vec::new()), - }) + Self { + storage: MemoryStorage::Static { + base: ptr::null_mut(), + size: Cell::new(0), + maximum: 0, + make_accessible, + #[cfg(all(feature = "uffd", target_os = "linux"))] + guard_page_faults: RefCell::new(Vec::new()), + }, + limiter: None, + } } } diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 8c857add455a..7be92eb1d133 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -3,19 +3,20 @@ //! `Table` is to WebAssembly tables what `LinearMemory` is to WebAssembly linear memories. use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; -use crate::{Trap, VMExternRef}; +use crate::{ResourceLimiter, Trap, VMExternRef}; use std::cell::{Cell, RefCell}; use std::cmp::min; use std::convert::TryInto; use std::ops::Range; use std::ptr; +use std::sync::Arc; use wasmtime_environ::wasm::TableElementType; use wasmtime_environ::{ir, TablePlan}; /// An element going into or coming out of a table. /// /// Table elements are stored as pointers and are default-initialized with `ptr::null_mut`. -#[derive(Clone, Debug)] +#[derive(Clone)] pub enum TableElement { /// A `funcref`. FuncRef(*mut VMCallerCheckedAnyfunc), @@ -92,7 +93,6 @@ impl From for TableElement { } } -#[derive(Debug)] enum TableStorage { Static { data: *mut *mut u8, @@ -108,38 +108,51 @@ enum TableStorage { } /// Represents an instance's table. -#[derive(Debug)] -pub struct Table(TableStorage); +pub struct Table { + storage: TableStorage, + limiter: Option>, +} impl Table { /// Create a new dynamic (movable) table instance for the specified table plan. - pub fn new_dynamic(plan: &TablePlan) -> Self { + pub fn new_dynamic(plan: &TablePlan, limiter: &Option<&Arc>) -> Self { let elements = RefCell::new(vec![ptr::null_mut(); plan.table.minimum as usize]); let ty = plan.table.ty.clone(); let maximum = plan.table.maximum; - Self(TableStorage::Dynamic { - elements, - ty, - maximum, - }) + Self { + storage: TableStorage::Dynamic { + elements, + ty, + maximum, + }, + limiter: limiter.cloned(), + } } /// Create a new static (immovable) table instance for the specified table plan. - pub fn new_static(plan: &TablePlan, data: *mut *mut u8, maximum: u32) -> Self { + pub fn new_static( + plan: &TablePlan, + data: *mut *mut u8, + maximum: u32, + limiter: &Option<&Arc>, + ) -> Self { let size = Cell::new(plan.table.minimum); let ty = plan.table.ty.clone(); let maximum = min(plan.table.maximum.unwrap_or(maximum), maximum); - Self(TableStorage::Static { - data, - size, - ty, - maximum, - }) + Self { + storage: TableStorage::Static { + data, + size, + ty, + maximum, + }, + limiter: limiter.cloned(), + } } /// Returns the type of the elements in this table. pub fn element_type(&self) -> TableElementType { - match &self.0 { + match &self.storage { TableStorage::Static { ty, .. } => *ty, TableStorage::Dynamic { ty, .. } => *ty, } @@ -147,7 +160,7 @@ impl Table { /// Returns whether or not the underlying storage of the table is "static". pub(crate) fn is_static(&self) -> bool { - if let TableStorage::Static { .. } = &self.0 { + if let TableStorage::Static { .. } = &self.storage { true } else { false @@ -156,15 +169,20 @@ impl Table { /// Returns the number of allocated elements. pub fn size(&self) -> u32 { - match &self.0 { + match &self.storage { TableStorage::Static { size, .. } => size.get(), TableStorage::Dynamic { elements, .. } => elements.borrow().len().try_into().unwrap(), } } - /// Returns the maximum number of elements. + /// Returns the maximum number of elements at runtime. + /// + /// Returns `None` if the table is unbounded. + /// + /// The runtime maximum may not be equal to the maximum from the table's Wasm type + /// when it is being constrained by an instance allocator. pub fn maximum(&self) -> Option { - match &self.0 { + match &self.storage { TableStorage::Static { maximum, .. } => Some(*maximum), TableStorage::Dynamic { maximum, .. } => maximum.clone(), } @@ -217,6 +235,15 @@ impl Table { /// Generally, prefer using `InstanceHandle::table_grow`, which encapsulates /// this unsafety. pub unsafe fn grow(&self, delta: u32, init_value: TableElement) -> Option { + if let Some(limiter) = &self.limiter { + let current = self.size(); + let desired = current.checked_add(delta)?; + + if !limiter.table_growing(current, desired, self.maximum()) { + return None; + } + } + let old_size = self.size(); let new_size = old_size.checked_add(delta)?; @@ -229,7 +256,7 @@ impl Table { debug_assert!(self.type_matches(&init_value)); // First resize the storage and then fill with the init value - match &self.0 { + match &self.storage { TableStorage::Static { size, .. } => { size.set(new_size); } @@ -319,7 +346,7 @@ impl Table { /// Return a `VMTableDefinition` for exposing the table to compiled wasm code. pub fn vmtable(&self) -> VMTableDefinition { - match &self.0 { + match &self.storage { TableStorage::Static { data, size, .. } => VMTableDefinition { base: *data as _, current_elements: size.get(), @@ -346,7 +373,7 @@ impl Table { where F: FnOnce(&[*mut u8]) -> R, { - match &self.0 { + match &self.storage { TableStorage::Static { data, size, .. } => unsafe { f(std::slice::from_raw_parts(*data, size.get() as usize)) }, @@ -361,7 +388,7 @@ impl Table { where F: FnOnce(&mut [*mut u8]) -> R, { - match &self.0 { + match &self.storage { TableStorage::Static { data, size, .. } => unsafe { f(std::slice::from_raw_parts_mut(*data, size.get() as usize)) }, @@ -463,11 +490,14 @@ impl Drop for Table { // The default table representation is an empty funcref table that cannot grow. impl Default for Table { fn default() -> Self { - Self(TableStorage::Static { - data: std::ptr::null_mut(), - size: Cell::new(0), - ty: TableElementType::Func, - maximum: 0, - }) + Self { + storage: TableStorage::Static { + data: std::ptr::null_mut(), + size: Cell::new(0), + ty: TableElementType::Func, + maximum: 0, + }, + limiter: None, + } } } diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index 8351fea426db..f133fa787b77 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -383,9 +383,6 @@ pub struct Config { pub(crate) max_wasm_stack: usize, 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, #[cfg(feature = "async")] pub(crate) async_stack_size: usize, host_funcs: HostFuncMap, @@ -422,9 +419,6 @@ impl Config { max_wasm_stack: 1 << 20, wasm_backtrace_details_env_used: false, features: WasmFeatures::default(), - max_instances: 10_000, - max_tables: 10_000, - max_memories: 10_000, #[cfg(feature = "async")] async_stack_size: 2 << 20, host_funcs: HostFuncMap::new(), @@ -1196,39 +1190,6 @@ impl Config { self } - /// Configures the maximum number of instances 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_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 - } - /// Defines a host function for the [`Config`] for the given callback. /// /// Use [`Store::get_host_func`](crate::Store::get_host_func) to get a [`Func`](crate::Func) representing the function. diff --git a/crates/wasmtime/src/instance.rs b/crates/wasmtime/src/instance.rs index baef6cb7df5a..7a30f850aafd 100644 --- a/crates/wasmtime/src/instance.rs +++ b/crates/wasmtime/src/instance.rs @@ -525,7 +525,7 @@ impl<'a> Instantiator<'a> { as *const VMExternRefActivationsTable as *mut _, module_info_lookup: Some(self.store.module_info_lookup()), - limiter: self.store.limiter(), + limiter: self.store.limiter().as_ref(), })?; // After we've created the `InstanceHandle` we still need to run diff --git a/crates/wasmtime/src/lib.rs b/crates/wasmtime/src/lib.rs index 4706c42b59ec..332f63b4d22e 100644 --- a/crates/wasmtime/src/lib.rs +++ b/crates/wasmtime/src/lib.rs @@ -283,7 +283,7 @@ mod config; mod engine; mod externals; mod instance; -mod limiter; +mod limits; mod linker; mod memory; mod module; @@ -300,7 +300,7 @@ pub use crate::engine::*; pub use crate::externals::*; pub use crate::func::*; pub use crate::instance::Instance; -pub use crate::limiter::*; +pub use crate::limits::*; pub use crate::linker::*; pub use crate::memory::*; pub use crate::module::{FrameInfo, FrameSymbol, Module}; diff --git a/crates/wasmtime/src/limiter.rs b/crates/wasmtime/src/limiter.rs deleted file mode 100644 index 31751fc30741..000000000000 --- a/crates/wasmtime/src/limiter.rs +++ /dev/null @@ -1,129 +0,0 @@ -use crate::{Store, StoreInner}; -use std::rc::Weak; - -/// Used by hosts to limit resource consumption of instances. -/// -/// A [`Store`] can be created with a resource limiter so that hosts can take into account -/// non-WebAssembly resource usage to determine if a linear memory or table should grow. -pub trait ResourceLimiter { - /// Notifies the resource limiter that an instance's linear memory has been requested to grow. - /// - /// * `current` is the current size of the linear memory in WebAssembly page units. - /// * `desired` is the desired size of the linear memory in WebAssembly page units. - /// * `maximum` is either the linear memory's maximum or a maximum from an instance allocator, - /// also in WebAssembly page units. A value of `None` indicates that the linear memory is - /// unbounded. - /// - /// This function should return `true` to indicate that the growing operation is permitted or - /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no - /// effect as the linear memory will not grow. - fn memory_growing( - &self, - store: &Store, - current: u32, - desired: u32, - maximum: Option, - ) -> bool; - - /// Notifies the resource limiter that an instance's table has been requested to grow. - /// - /// * `current` is the current number of elements in the table. - /// * `desired` is the desired number of elements in the table. - /// * `maximum` is either the table's maximum or a maximum from an instance allocator, - /// A value of `None` indicates that the table is unbounded. - /// - /// This function should return `true` to indicate that the growing operation is permitted or - /// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no - /// effect as the table will not grow. - fn table_growing( - &self, - store: &Store, - current: u32, - desired: u32, - maximum: Option, - ) -> bool; -} - -pub(crate) struct ResourceLimiterProxy { - store: Weak, - limiter: Box, -} - -impl ResourceLimiterProxy { - pub(crate) fn new(store: &Store, limiter: impl ResourceLimiter + 'static) -> Self { - Self { - store: store.weak(), - limiter: Box::new(limiter), - } - } -} - -impl wasmtime_runtime::ResourceLimiter for ResourceLimiterProxy { - fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { - self.limiter.memory_growing( - &Store::upgrade(&self.store).unwrap(), - current, - desired, - maximum, - ) - } - - fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { - self.limiter.table_growing( - &Store::upgrade(&self.store).unwrap(), - current, - desired, - maximum, - ) - } -} - -/// A resource limiter that statically limits how much memories and tables can grow. -pub struct StaticResourceLimiter { - memory_limit: Option, - table_limit: Option, -} - -impl StaticResourceLimiter { - /// Creates a new [`StaticResourceLimiter`]. - /// - /// The `memory_limit` parameter is the number of WebAssembly pages a linear memory can grow to. - /// If `None`, the limiter will not limit linear memory growth. - /// - /// The `table_limit` parameter is the number of elements a table can grow to. - /// If `None`, the limiter will not limit table growth. - pub fn new(memory_limit: Option, table_limit: Option) -> Self { - Self { - memory_limit, - table_limit, - } - } -} - -impl ResourceLimiter for StaticResourceLimiter { - fn memory_growing( - &self, - _store: &Store, - _current: u32, - desired: u32, - _maximum: Option, - ) -> bool { - match self.memory_limit { - Some(limit) if desired > limit => false, - _ => true, - } - } - - fn table_growing( - &self, - _store: &Store, - _current: u32, - desired: u32, - _maximum: Option, - ) -> bool { - match self.table_limit { - Some(limit) if desired > limit => false, - _ => true, - } - } -} diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs new file mode 100644 index 000000000000..0406f6bbbc23 --- /dev/null +++ b/crates/wasmtime/src/limits.rs @@ -0,0 +1,256 @@ +use crate::{Store, StoreInner}; +use std::rc::Weak; + +pub(crate) const DEFAULT_INSTANCE_LIMIT: usize = 10000; +pub(crate) const DEFAULT_TABLE_LIMIT: usize = 10000; +pub(crate) const DEFAULT_MEMORY_LIMIT: usize = 10000; + +/// Used by hosts to limit resource consumption of instances at runtime. +/// +/// A [`Store`] can be created with a resource limiter so that hosts can take into account +/// non-WebAssembly resource usage to determine if a linear memory or table should grow. +pub trait ResourceLimiter { + /// Notifies the resource limiter that an instance's linear memory has been requested to grow. + /// + /// * `current` is the current size of the linear memory in WebAssembly page units. + /// * `desired` is the desired size of the linear memory in WebAssembly page units. + /// * `maximum` is either the linear memory's maximum or a maximum from an instance allocator, + /// also in WebAssembly page units. A value of `None` indicates that the linear memory is + /// unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. + /// + /// Note that this function will be called even when the desired count exceeds the given maximum. + /// + /// Returning `true` when a maximum has been exceeded will have no effect as the linear memory + /// will not be grown. + fn memory_growing( + &self, + store: &Store, + current: u32, + desired: u32, + maximum: Option, + ) -> bool; + + /// Notifies the resource limiter that an instance's table has been requested to grow. + /// + /// * `current` is the current number of elements in the table. + /// * `desired` is the desired number of elements in the table. + /// * `maximum` is either the table's maximum or a maximum from an instance allocator, + /// A value of `None` indicates that the table is unbounded. + /// + /// This function should return `true` to indicate that the growing operation is permitted or + /// `false` if not permitted. + /// + /// Note that this function will be called even when the desired count exceeds the given maximum. + /// + /// Returning `true` when a maximum has been exceeded will have no effect as the table will + /// not be grown. + fn table_growing( + &self, + store: &Store, + current: u32, + desired: u32, + maximum: Option, + ) -> bool; + + /// The maximum number of instances that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn instances(&self) -> usize { + DEFAULT_INSTANCE_LIMIT + } + + /// The maximum number of tables that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn tables(&self) -> usize { + DEFAULT_TABLE_LIMIT + } + + /// The maximum number of linear memories that can be created for a `Store`. + /// + /// Instantiation will fail with an error if this limit is exceeded. + /// + /// This value defaults to 10,000. + fn memories(&self) -> usize { + DEFAULT_MEMORY_LIMIT + } +} + +pub(crate) struct ResourceLimiterProxy { + store: Weak, + limiter: T, +} + +impl ResourceLimiterProxy { + pub(crate) fn new(store: &Store, limiter: T) -> Self { + Self { + store: store.weak(), + limiter, + } + } +} + +impl wasmtime_runtime::ResourceLimiter for ResourceLimiterProxy { + fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { + self.limiter.memory_growing( + &Store::upgrade(&self.store).unwrap(), + current, + desired, + maximum, + ) + } + + fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { + self.limiter.table_growing( + &Store::upgrade(&self.store).unwrap(), + current, + desired, + maximum, + ) + } + + fn instances(&self) -> usize { + self.limiter.instances() + } + + fn tables(&self) -> usize { + self.limiter.tables() + } + + fn memories(&self) -> usize { + self.limiter.memories() + } +} + +/// Used to build [`StoreLimits`]. +pub struct StoreLimitsBuilder(StoreLimits); + +impl StoreLimitsBuilder { + /// Creates a new [`StoreLimitsBuilder`]. + pub fn new() -> Self { + Self(StoreLimits::default()) + } + + /// The maximum number of WebAssembly pages a linear memory can grow to. + /// + /// Growing a linear memory beyond this limit will fail. + /// + /// By default, linear memory pages will not be limited. + pub fn memory_pages(mut self, limit: u32) -> Self { + self.0.memory_pages = Some(limit); + self + } + + /// The maximum number of elements in a table. + /// + /// Growing a table beyond this limit will fail. + /// + /// By default, table elements will not be limited. + pub fn table_elements(mut self, limit: u32) -> Self { + self.0.table_elements = Some(limit); + self + } + + /// The maximum number of instances that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + pub fn instances(mut self, limit: usize) -> Self { + self.0.instances = limit; + self + } + + /// The maximum number of tables that can be created for a `Store`. + /// + /// Module instantiation will fail if this limit is exceeded. + /// + /// This value defaults to 10,000. + pub fn tables(mut self, tables: usize) -> Self { + self.0.tables = tables; + self + } + + /// The maximum number of linear memories that can be created for a `Store`. + /// + /// Instantiation will fail with an error if this limit is exceeded. + /// + /// This value defaults to 10,000. + pub fn memories(mut self, memories: usize) -> Self { + self.0.memories = memories; + self + } + + /// Consumes this builder and returns the [`StoreLimits`]. + pub fn build(self) -> StoreLimits { + self.0 + } +} + +/// Provides limits for a [`Store`]. +pub struct StoreLimits { + memory_pages: Option, + table_elements: Option, + instances: usize, + tables: usize, + memories: usize, +} + +impl Default for StoreLimits { + fn default() -> Self { + Self { + memory_pages: None, + table_elements: None, + instances: DEFAULT_INSTANCE_LIMIT, + tables: DEFAULT_TABLE_LIMIT, + memories: DEFAULT_MEMORY_LIMIT, + } + } +} + +impl ResourceLimiter for StoreLimits { + fn memory_growing( + &self, + _store: &Store, + _current: u32, + desired: u32, + _maximum: Option, + ) -> bool { + match self.memory_pages { + Some(limit) if desired > limit => false, + _ => true, + } + } + + fn table_growing( + &self, + _store: &Store, + _current: u32, + desired: u32, + _maximum: Option, + ) -> bool { + match self.table_elements { + Some(limit) if desired > limit => false, + _ => true, + } + } + + fn instances(&self) -> usize { + self.instances + } + + fn tables(&self) -> usize { + self.tables + } + + fn memories(&self) -> usize { + self.memories + } +} diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 63714fc0b9a3..89ff3ee28bfe 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -1,9 +1,9 @@ use crate::{ module::ModuleRegistry, signatures::SignatureCollection, trampoline::StoreInstanceHandle, - Engine, Func, Module, ResourceLimiter, ResourceLimiterProxy, Trap, + Engine, Func, Module, ResourceLimiter, ResourceLimiterProxy, Trap, DEFAULT_INSTANCE_LIMIT, + DEFAULT_MEMORY_LIMIT, DEFAULT_TABLE_LIMIT, }; use anyhow::{bail, Result}; -use std::any::{Any, TypeId}; use std::cell::{Cell, RefCell}; use std::collections::{hash_map::Entry, HashMap}; use std::convert::TryFrom; @@ -15,6 +15,10 @@ use std::ptr; use std::rc::{Rc, Weak}; use std::sync::Arc; use std::task::{Context, Poll}; +use std::{ + any::{Any, TypeId}, + cell::Ref, +}; use wasmtime_runtime::{ InstanceAllocator, InstanceHandle, ModuleInfo, OnDemandInstanceAllocator, SignalHandler, TrapInfo, VMCallerCheckedAnyfunc, VMContext, VMExternRef, VMExternRefActivationsTable, @@ -155,9 +159,17 @@ impl Store { } } - /// Creates a new store to be associated with the given [`Engine`] and using the given - /// resource limiter for any instances created in the store. - pub fn new_with_limiter(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { + /// Creates a new store to be associated with the given [`Engine`] and using the supplied + /// resource limiter. + /// + /// # Example + /// + /// ```rust + /// # use wasmtime::{Engine, Store, StoreLimitsBuilder}; + /// let engine = Engine::default(); + /// let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().instances(10).build()); + /// ``` + pub fn new_with_limits(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { let store = Store::new(engine); { @@ -217,8 +229,8 @@ impl Store { } } - pub(crate) fn limiter(&self) -> Option> { - self.inner.limiter.borrow().clone() + pub(crate) fn limiter(&self) -> Ref>> { + self.inner.limiter.borrow() } pub(crate) fn signatures(&self) -> &RefCell { @@ -250,8 +262,6 @@ impl Store { } pub(crate) fn bump_resource_counts(&self, module: &Module) -> Result<()> { - let config = self.engine().config(); - fn bump(slot: &Cell, max: usize, amt: usize, desc: &str) -> Result<()> { let new = slot.get().saturating_add(amt); if new > max { @@ -269,19 +279,11 @@ impl Store { 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")?; + let (max_instances, max_memories, max_tables) = self.limits(); + + bump(&self.inner.instance_count, max_instances, 1, "instance")?; + bump(&self.inner.memory_count, max_memories, memories, "memory")?; + bump(&self.inner.table_count, max_tables, tables, "table")?; Ok(()) } @@ -866,6 +868,19 @@ impl Store { Err(trap) => unsafe { wasmtime_runtime::raise_user_trap(trap.into()) }, } } + + fn limits(&self) -> (usize, usize, usize) { + let limiter = self.inner.limiter.borrow(); + + limiter + .as_ref() + .map(|l| (l.instances(), l.memories(), l.tables())) + .unwrap_or(( + DEFAULT_INSTANCE_LIMIT, + DEFAULT_MEMORY_LIMIT, + DEFAULT_TABLE_LIMIT, + )) + } } unsafe impl TrapInfo for Store { diff --git a/crates/wasmtime/src/trampoline.rs b/crates/wasmtime/src/trampoline.rs index 2ab6d6b88666..5ebc436eba13 100644 --- a/crates/wasmtime/src/trampoline.rs +++ b/crates/wasmtime/src/trampoline.rs @@ -78,7 +78,7 @@ fn create_handle( as *const VMExternRefActivationsTable as *mut _, module_info_lookup: Some(store.module_info_lookup()), - limiter: None, + limiter: store.limiter().as_ref(), }, )?; diff --git a/tests/all/limiter.rs b/tests/all/limits.rs similarity index 91% rename from tests/all/limiter.rs rename to tests/all/limits.rs index 27956fad093d..2ce2d781faf0 100644 --- a/tests/all/limiter.rs +++ b/tests/all/limits.rs @@ -4,14 +4,20 @@ use std::rc::Rc; use wasmtime::*; #[test] -fn test_static_limiter() -> Result<()> { +fn test_limits() -> Result<()> { let engine = Engine::default(); let module = Module::new( &engine, r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, )?; - let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(Some(10), Some(5))); + let store = Store::new_with_limits( + &engine, + StoreLimitsBuilder::new() + .memory_pages(10) + .table_elements(5) + .build(), + ); let instance = Instance::new(&store, &module, &[])?; @@ -44,14 +50,14 @@ fn test_static_limiter() -> Result<()> { } #[test] -fn test_static_limiter_memory_only() -> Result<()> { +fn test_limits_memory_only() -> Result<()> { let engine = Engine::default(); let module = Module::new( &engine, r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, )?; - let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(Some(10), None)); + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memory_pages(10).build()); let instance = Instance::new(&store, &module, &[])?; @@ -77,14 +83,15 @@ fn test_static_limiter_memory_only() -> Result<()> { } #[test] -fn test_static_limiter_table_only() -> Result<()> { +fn test_limits_table_only() -> Result<()> { let engine = Engine::default(); let module = Module::new( &engine, r#"(module (memory (export "m") 0) (table (export "t") 0 anyfunc))"#, )?; - let store = Store::new_with_limiter(&engine, StaticResourceLimiter::new(None, Some(5))); + let store = + Store::new_with_limits(&engine, StoreLimitsBuilder::new().table_elements(5).build()); let instance = Instance::new(&store, &module, &[])?; @@ -198,7 +205,7 @@ fn test_custom_limiter() -> Result<()> { )?; let dropped = Rc::new(Cell::new(false)); - let store = Store::new_with_limiter(&engine, HostMemoryLimiter(dropped.clone())); + let store = Store::new_with_limits(&engine, HostMemoryLimiter(dropped.clone())); assert!(store .set(Rc::new(RefCell::new(MemoryContext { diff --git a/tests/all/main.rs b/tests/all/main.rs index 172d6b9fd951..89c686119e42 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -13,7 +13,7 @@ mod import_calling_export; mod import_indexes; mod instance; mod invoke_func_via_table; -mod limiter; +mod limits; mod linker; mod memory_creator; mod module; diff --git a/tests/all/module_linking.rs b/tests/all/module_linking.rs index 101655eff318..357caa982f39 100644 --- a/tests/all/module_linking.rs +++ b/tests/all/module_linking.rs @@ -190,7 +190,6 @@ fn imports_exports() -> Result<()> { fn limit_instances() -> Result<()> { let mut config = Config::new(); config.wasm_module_linking(true); - config.max_instances(10); let engine = Engine::new(&config)?; let module = Module::new( &engine, @@ -216,7 +215,7 @@ fn limit_instances() -> Result<()> { ) "#, )?; - let store = Store::new(&engine); + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().instances(10).build()); let err = Instance::new(&store, &module, &[]).err().unwrap(); assert!( err.to_string().contains("resource limit exceeded"), @@ -231,7 +230,6 @@ 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, @@ -252,7 +250,7 @@ fn limit_memories() -> Result<()> { ) "#, )?; - let store = Store::new(&engine); + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memories(10).build()); let err = Instance::new(&store, &module, &[]).err().unwrap(); assert!( err.to_string().contains("resource limit exceeded"), @@ -266,7 +264,6 @@ fn limit_memories() -> Result<()> { 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, @@ -287,7 +284,7 @@ fn limit_tables() -> Result<()> { ) "#, )?; - let store = Store::new(&engine); + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().tables(10).build()); let err = Instance::new(&store, &module, &[]).err().unwrap(); assert!( err.to_string().contains("resource limit exceeded"), From a82f722797c9c3408942d7cb54351f49ffa684d8 Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Thu, 8 Apr 2021 18:52:48 -0700 Subject: [PATCH 3/4] Code review feedback and bug fix. * `Memory::new` now returns `Result` so that an error can be returned if the initial requested memory exceeds any limits placed on the store. * Changed an `Arc` to `Rc` as the `Arc` wasn't necessary. * Removed `Store` from the `ResourceLimiter` callbacks. Custom resource limiter implementations are free to capture any context they want, so no need to unnecessarily store a weak reference to `Store` from the proxy type. * Fixed a bug in the pooling instance allocator where an instance would be leaked from the pool. Previously, this would only have happened if the OS was unable to make the necessary linear memory available for the instance. With these changes, however, the instance might not be created due to limits placed on the store. We now properly deallocate the instance on error. * Added more tests, including one that covers the fix mentioned above. --- RELEASES.md | 13 + crates/c-api/src/memory.rs | 8 +- crates/fuzzing/src/oracles/dummy.rs | 2 +- crates/runtime/src/instance/allocator.rs | 17 +- .../runtime/src/instance/allocator/pooling.rs | 85 +++-- crates/runtime/src/memory.rs | 87 +++-- crates/runtime/src/table.rs | 77 ++-- crates/wasmtime/src/limits.rs | 85 +---- crates/wasmtime/src/memory.rs | 13 +- crates/wasmtime/src/store.rs | 69 ++-- crates/wast/src/spectest.rs | 2 +- examples/memory.rs | 2 +- tests/all/externals.rs | 4 +- tests/all/limits.rs | 334 +++++++++++------- tests/all/linker.rs | 4 +- 15 files changed, 450 insertions(+), 352 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 73ebf304c465..acf115f1c5e3 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -4,6 +4,19 @@ ## Unreleased +### Added + +* Added `Store::with_limits`, `StoreLimits`, and `ResourceLimiter` to the + Wasmtime API to help with enforcing resource limits at runtime. The + `ResourceLimiter` trait can be implemented by custom resource limiters to + decide if linear memories or tables can be grown. + +### Changed + +* Breaking: `Memory::new` has been changed to return `Result` as creating a + host memory object is now a fallible operation when the initial size of + the memory exceeds the store limits. + ## 0.26.0 Released 2021-04-05. diff --git a/crates/c-api/src/memory.rs b/crates/c-api/src/memory.rs index 55a001deed7d..54d3936849fb 100644 --- a/crates/c-api/src/memory.rs +++ b/crates/c-api/src/memory.rs @@ -31,13 +31,13 @@ impl wasm_memory_t { pub extern "C" fn wasm_memory_new( store: &wasm_store_t, mt: &wasm_memorytype_t, -) -> Box { - let memory = Memory::new(&store.store, mt.ty().ty.clone()); - Box::new(wasm_memory_t { +) -> Option> { + let memory = Memory::new(&store.store, mt.ty().ty.clone()).ok()?; + Some(Box::new(wasm_memory_t { ext: wasm_extern_t { which: memory.into(), }, - }) + })) } #[no_mangle] diff --git a/crates/fuzzing/src/oracles/dummy.rs b/crates/fuzzing/src/oracles/dummy.rs index f03bbfa06706..99b5be736bee 100644 --- a/crates/fuzzing/src/oracles/dummy.rs +++ b/crates/fuzzing/src/oracles/dummy.rs @@ -87,7 +87,7 @@ pub fn dummy_table(store: &Store, ty: TableType) -> Table { /// Construct a dummy memory for the given memory type. pub fn dummy_memory(store: &Store, ty: MemoryType) -> Memory { - Memory::new(store, ty) + Memory::new(store, ty).unwrap() } /// Construct a dummy instance for the given instance type. diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index c4825874a019..1e4d8008774d 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -15,6 +15,7 @@ use std::any::Any; use std::cell::RefCell; use std::convert::TryFrom; use std::ptr::{self, NonNull}; +use std::rc::Rc; use std::slice; use std::sync::Arc; use thiserror::Error; @@ -61,7 +62,7 @@ pub struct InstanceAllocationRequest<'a> { pub module_info_lookup: Option<*const dyn ModuleInfoLookup>, /// The resource limiter to use for the instance. - pub limiter: Option<&'a Arc>, + pub limiter: Option<&'a Rc>, } /// An link error while instantiating a module. @@ -595,21 +596,21 @@ impl OnDemandInstanceAllocator { fn create_tables( module: &Module, - limiter: &Option<&Arc>, - ) -> PrimaryMap { + limiter: Option<&Rc>, + ) -> Result, InstantiationError> { let num_imports = module.num_imported_tables; let mut tables: PrimaryMap = PrimaryMap::with_capacity(module.table_plans.len() - num_imports); for table in &module.table_plans.values().as_slice()[num_imports..] { - tables.push(Table::new_dynamic(table, limiter)); + tables.push(Table::new_dynamic(table, limiter).map_err(InstantiationError::Resource)?); } - tables + Ok(tables) } fn create_memories( &self, module: &Module, - limiter: &Option<&Arc>, + limiter: Option<&Rc>, ) -> Result, InstantiationError> { let creator = self .mem_creator @@ -642,8 +643,8 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator { &self, mut req: InstanceAllocationRequest, ) -> Result { - let memories = self.create_memories(&req.module, &req.limiter)?; - let tables = Self::create_tables(&req.module, &req.limiter); + let memories = self.create_memories(&req.module, req.limiter)?; + let tables = Self::create_tables(&req.module, req.limiter)?; let host_state = std::mem::replace(&mut req.host_state, Box::new(())); diff --git a/crates/runtime/src/instance/allocator/pooling.rs b/crates/runtime/src/instance/allocator/pooling.rs index d9bb374ba50c..7a17b2b143df 100644 --- a/crates/runtime/src/instance/allocator/pooling.rs +++ b/crates/runtime/src/instance/allocator/pooling.rs @@ -18,6 +18,7 @@ use std::cell::RefCell; use std::cmp::min; use std::convert::TryFrom; use std::mem; +use std::rc::Rc; use std::sync::{Arc, Mutex}; use wasmtime_environ::{ entity::{EntitySet, PrimaryMap}, @@ -376,10 +377,45 @@ impl InstancePool { } } + unsafe fn setup_instance( + &self, + index: usize, + mut req: InstanceAllocationRequest, + ) -> Result { + let instance = self.instance(index); + + instance.module = req.module.clone(); + instance.offsets = VMOffsets::new( + std::mem::size_of::<*const u8>() as u8, + instance.module.as_ref(), + ); + instance.host_state = std::mem::replace(&mut req.host_state, Box::new(())); + + Self::set_instance_memories( + instance, + self.memories.get(index), + self.memories.max_wasm_pages, + req.limiter, + )?; + + Self::set_instance_tables( + instance, + self.tables.get(index), + self.tables.max_elements, + req.limiter, + )?; + + initialize_vmcontext(instance, req); + + Ok(InstanceHandle { + instance: instance as _, + }) + } + fn allocate( &self, strategy: PoolingAllocationStrategy, - mut req: InstanceAllocationRequest, + req: InstanceAllocationRequest, ) -> Result { let index = { let mut free_list = self.free_list.lock().unwrap(); @@ -390,36 +426,14 @@ impl InstancePool { free_list.swap_remove(free_index) }; - let host_state = std::mem::replace(&mut req.host_state, Box::new(())); - unsafe { - let instance = self.instance(index); - - instance.module = req.module.clone(); - instance.offsets = VMOffsets::new( - std::mem::size_of::<*const u8>() as u8, - instance.module.as_ref(), - ); - instance.host_state = host_state; - - Self::set_instance_memories( - instance, - self.memories.get(index), - self.memories.max_wasm_pages, - &req.limiter, - )?; - - Self::set_instance_tables( - instance, - self.tables.get(index), - self.tables.max_elements, - &req.limiter, - )?; - - initialize_vmcontext(instance, req); - - Ok(InstanceHandle { - instance: instance as _, + self.setup_instance(index, req).or_else(|e| { + // Deallocate the allocated instance on error + let instance = self.instance(index); + self.deallocate(&InstanceHandle { + instance: instance as _, + }); + Err(e) }) } } @@ -482,7 +496,7 @@ impl InstancePool { instance: &mut Instance, mut memories: impl Iterator, max_pages: u32, - limiter: &Option<&Arc>, + limiter: Option<&Rc>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -514,7 +528,7 @@ impl InstancePool { instance: &mut Instance, mut tables: impl Iterator, max_elements: u32, - limiter: &Option<&Arc>, + limiter: Option<&Rc>, ) -> Result<(), InstantiationError> { let module = instance.module.as_ref(); @@ -526,9 +540,10 @@ impl InstancePool { commit_table_pages(base, max_elements as usize * mem::size_of::<*mut u8>()) .map_err(InstantiationError::Resource)?; - instance - .tables - .push(Table::new_static(plan, base as _, max_elements, limiter)); + instance.tables.push( + Table::new_static(plan, base as _, max_elements, limiter) + .map_err(InstantiationError::Resource)?, + ); } let mut dropped_elements = instance.dropped_elements.borrow_mut(); diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index a8dea35c62e5..973016224d52 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -5,13 +5,13 @@ use crate::mmap::Mmap; use crate::vmcontext::VMMemoryDefinition; use crate::ResourceLimiter; -use anyhow::Result; +use anyhow::{bail, Result}; use more_asserts::{assert_ge, assert_le}; use std::cell::{Cell, RefCell}; use std::cmp::min; use std::convert::TryFrom; use std::ptr; -use std::sync::Arc; +use std::rc::Rc; use wasmtime_environ::{MemoryPlan, MemoryStyle, WASM_MAX_PAGES, WASM_PAGE_SIZE}; /// A memory allocator @@ -203,7 +203,7 @@ enum MemoryStorage { /// Represents an instantiation of a WebAssembly memory. pub struct Memory { storage: MemoryStorage, - limiter: Option>, + limiter: Option>, } impl Memory { @@ -211,12 +211,13 @@ impl Memory { pub fn new_dynamic( plan: &MemoryPlan, creator: &dyn RuntimeMemoryCreator, - limiter: &Option<&Arc>, + limiter: Option<&Rc>, ) -> Result { - Ok(Self { - storage: MemoryStorage::Dynamic(creator.new_memory(plan)?), - limiter: limiter.cloned(), - }) + Self::new( + plan, + MemoryStorage::Dynamic(creator.new_memory(plan)?), + limiter, + ) } /// Create a new static (immovable) memory instance for the specified plan. @@ -225,21 +226,50 @@ impl Memory { base: *mut u8, maximum: u32, make_accessible: fn(*mut u8, usize) -> Result<()>, - limiter: &Option<&Arc>, + limiter: Option<&Rc>, + ) -> Result { + let storage = MemoryStorage::Static { + base, + size: Cell::new(plan.memory.minimum), + maximum: min(plan.memory.maximum.unwrap_or(maximum), maximum), + make_accessible, + #[cfg(all(feature = "uffd", target_os = "linux"))] + guard_page_faults: RefCell::new(Vec::new()), + }; + + Self::new(plan, storage, limiter) + } + + fn new( + plan: &MemoryPlan, + storage: MemoryStorage, + limiter: Option<&Rc>, ) -> Result { - if plan.memory.minimum > 0 { - make_accessible(base, plan.memory.minimum as usize * WASM_PAGE_SIZE as usize)?; + if let Some(limiter) = limiter { + if !limiter.memory_growing(0, plan.memory.minimum, plan.memory.maximum) { + bail!( + "memory minimum size of {} pages exceeds memory limits", + plan.memory.minimum + ); + } + } + + if let MemoryStorage::Static { + base, + make_accessible, + .. + } = &storage + { + if plan.memory.minimum > 0 { + make_accessible( + *base, + plan.memory.minimum as usize * WASM_PAGE_SIZE as usize, + )?; + } } Ok(Self { - storage: MemoryStorage::Static { - base, - size: Cell::new(plan.memory.minimum), - maximum: min(plan.memory.maximum.unwrap_or(maximum), maximum), - make_accessible, - #[cfg(all(feature = "uffd", target_os = "linux"))] - guard_page_faults: RefCell::new(Vec::new()), - }, + storage, limiter: limiter.cloned(), }) } @@ -288,11 +318,15 @@ impl Memory { /// Generally, prefer using `InstanceHandle::memory_grow`, which encapsulates /// this unsafety. pub unsafe fn grow(&self, delta: u32) -> Option { - if let Some(limiter) = &self.limiter { - let current = self.size(); - let desired = current.checked_add(delta)?; + let old_size = self.size(); + if delta == 0 { + return Some(old_size); + } + + let new_size = old_size.checked_add(delta)?; - if !limiter.memory_growing(current, desired, self.maximum()) { + if let Some(limiter) = &self.limiter { + if !limiter.memory_growing(old_size, new_size, self.maximum()) { return None; } } @@ -309,13 +343,6 @@ impl Memory { #[cfg(all(feature = "uffd", target_os = "linux"))] self.reset_guard_pages().ok()?; - let old_size = size.get(); - if delta == 0 { - return Some(old_size); - } - - let new_size = old_size.checked_add(delta)?; - if new_size > *maximum || new_size >= WASM_MAX_PAGES { return None; } diff --git a/crates/runtime/src/table.rs b/crates/runtime/src/table.rs index 7be92eb1d133..34cd654da24b 100644 --- a/crates/runtime/src/table.rs +++ b/crates/runtime/src/table.rs @@ -4,12 +4,13 @@ use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition}; use crate::{ResourceLimiter, Trap, VMExternRef}; +use anyhow::{bail, Result}; use std::cell::{Cell, RefCell}; use std::cmp::min; use std::convert::TryInto; use std::ops::Range; use std::ptr; -use std::sync::Arc; +use std::rc::Rc; use wasmtime_environ::wasm::TableElementType; use wasmtime_environ::{ir, TablePlan}; @@ -110,23 +111,26 @@ enum TableStorage { /// Represents an instance's table. pub struct Table { storage: TableStorage, - limiter: Option>, + limiter: Option>, } impl Table { /// Create a new dynamic (movable) table instance for the specified table plan. - pub fn new_dynamic(plan: &TablePlan, limiter: &Option<&Arc>) -> Self { + pub fn new_dynamic( + plan: &TablePlan, + limiter: Option<&Rc>, + ) -> Result { let elements = RefCell::new(vec![ptr::null_mut(); plan.table.minimum as usize]); let ty = plan.table.ty.clone(); let maximum = plan.table.maximum; - Self { - storage: TableStorage::Dynamic { - elements, - ty, - maximum, - }, - limiter: limiter.cloned(), - } + + let storage = TableStorage::Dynamic { + elements, + ty, + maximum, + }; + + Self::new(plan, storage, limiter) } /// Create a new static (immovable) table instance for the specified table plan. @@ -134,20 +138,40 @@ impl Table { plan: &TablePlan, data: *mut *mut u8, maximum: u32, - limiter: &Option<&Arc>, - ) -> Self { + limiter: Option<&Rc>, + ) -> Result { let size = Cell::new(plan.table.minimum); let ty = plan.table.ty.clone(); let maximum = min(plan.table.maximum.unwrap_or(maximum), maximum); - Self { - storage: TableStorage::Static { - data, - size, - ty, - maximum, - }, - limiter: limiter.cloned(), + + let storage = TableStorage::Static { + data, + size, + ty, + maximum, + }; + + Self::new(plan, storage, limiter) + } + + fn new( + plan: &TablePlan, + storage: TableStorage, + limiter: Option<&Rc>, + ) -> Result { + if let Some(limiter) = limiter { + if !limiter.table_growing(0, plan.table.minimum, plan.table.maximum) { + bail!( + "table minimum size of {} elements exceeds table limits", + plan.table.minimum + ); + } } + + Ok(Self { + storage, + limiter: limiter.cloned(), + }) } /// Returns the type of the elements in this table. @@ -235,18 +259,15 @@ impl Table { /// Generally, prefer using `InstanceHandle::table_grow`, which encapsulates /// this unsafety. pub unsafe fn grow(&self, delta: u32, init_value: TableElement) -> Option { - if let Some(limiter) = &self.limiter { - let current = self.size(); - let desired = current.checked_add(delta)?; + let old_size = self.size(); + let new_size = old_size.checked_add(delta)?; - if !limiter.table_growing(current, desired, self.maximum()) { + if let Some(limiter) = &self.limiter { + if !limiter.table_growing(old_size, new_size, self.maximum()) { return None; } } - let old_size = self.size(); - - let new_size = old_size.checked_add(delta)?; if let Some(max) = self.maximum() { if new_size > max { return None; diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index 0406f6bbbc23..169630a058d4 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -1,13 +1,10 @@ -use crate::{Store, StoreInner}; -use std::rc::Weak; - pub(crate) const DEFAULT_INSTANCE_LIMIT: usize = 10000; pub(crate) const DEFAULT_TABLE_LIMIT: usize = 10000; pub(crate) const DEFAULT_MEMORY_LIMIT: usize = 10000; /// Used by hosts to limit resource consumption of instances at runtime. /// -/// A [`Store`] can be created with a resource limiter so that hosts can take into account +/// A [`Store`](crate::Store) can be created with a resource limiter so that hosts can take into account /// non-WebAssembly resource usage to determine if a linear memory or table should grow. pub trait ResourceLimiter { /// Notifies the resource limiter that an instance's linear memory has been requested to grow. @@ -25,13 +22,7 @@ pub trait ResourceLimiter { /// /// Returning `true` when a maximum has been exceeded will have no effect as the linear memory /// will not be grown. - fn memory_growing( - &self, - store: &Store, - current: u32, - desired: u32, - maximum: Option, - ) -> bool; + fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool; /// Notifies the resource limiter that an instance's table has been requested to grow. /// @@ -47,15 +38,9 @@ pub trait ResourceLimiter { /// /// Returning `true` when a maximum has been exceeded will have no effect as the table will /// not be grown. - fn table_growing( - &self, - store: &Store, - current: u32, - desired: u32, - maximum: Option, - ) -> bool; + fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool; - /// The maximum number of instances that can be created for a `Store`. + /// The maximum number of instances that can be created for a [`Store`](crate::Store). /// /// Module instantiation will fail if this limit is exceeded. /// @@ -64,7 +49,7 @@ pub trait ResourceLimiter { DEFAULT_INSTANCE_LIMIT } - /// The maximum number of tables that can be created for a `Store`. + /// The maximum number of tables that can be created for a [`Store`](crate::Store). /// /// Module instantiation will fail if this limit is exceeded. /// @@ -73,7 +58,7 @@ pub trait ResourceLimiter { DEFAULT_TABLE_LIMIT } - /// The maximum number of linear memories that can be created for a `Store`. + /// The maximum number of linear memories that can be created for a [`Store`](crate::Store). /// /// Instantiation will fail with an error if this limit is exceeded. /// @@ -83,49 +68,27 @@ pub trait ResourceLimiter { } } -pub(crate) struct ResourceLimiterProxy { - store: Weak, - limiter: T, -} - -impl ResourceLimiterProxy { - pub(crate) fn new(store: &Store, limiter: T) -> Self { - Self { - store: store.weak(), - limiter, - } - } -} +pub(crate) struct ResourceLimiterProxy(pub T); impl wasmtime_runtime::ResourceLimiter for ResourceLimiterProxy { fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { - self.limiter.memory_growing( - &Store::upgrade(&self.store).unwrap(), - current, - desired, - maximum, - ) + self.0.memory_growing(current, desired, maximum) } fn table_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { - self.limiter.table_growing( - &Store::upgrade(&self.store).unwrap(), - current, - desired, - maximum, - ) + self.0.table_growing(current, desired, maximum) } fn instances(&self) -> usize { - self.limiter.instances() + self.0.instances() } fn tables(&self) -> usize { - self.limiter.tables() + self.0.tables() } fn memories(&self) -> usize { - self.limiter.memories() + self.0.memories() } } @@ -158,7 +121,7 @@ impl StoreLimitsBuilder { self } - /// The maximum number of instances that can be created for a `Store`. + /// The maximum number of instances that can be created for a [`Store`](crate::Store). /// /// Module instantiation will fail if this limit is exceeded. /// @@ -168,7 +131,7 @@ impl StoreLimitsBuilder { self } - /// The maximum number of tables that can be created for a `Store`. + /// The maximum number of tables that can be created for a [`Store`](crate::Store). /// /// Module instantiation will fail if this limit is exceeded. /// @@ -178,7 +141,7 @@ impl StoreLimitsBuilder { self } - /// The maximum number of linear memories that can be created for a `Store`. + /// The maximum number of linear memories that can be created for a [`Store`](crate::Store). /// /// Instantiation will fail with an error if this limit is exceeded. /// @@ -194,7 +157,7 @@ impl StoreLimitsBuilder { } } -/// Provides limits for a [`Store`]. +/// Provides limits for a [`Store`](crate::Store). pub struct StoreLimits { memory_pages: Option, table_elements: Option, @@ -216,26 +179,14 @@ impl Default for StoreLimits { } impl ResourceLimiter for StoreLimits { - fn memory_growing( - &self, - _store: &Store, - _current: u32, - desired: u32, - _maximum: Option, - ) -> bool { + fn memory_growing(&self, _current: u32, desired: u32, _maximum: Option) -> bool { match self.memory_pages { Some(limit) if desired > limit => false, _ => true, } } - fn table_growing( - &self, - _store: &Store, - _current: u32, - desired: u32, - _maximum: Option, - ) -> bool { + fn table_growing(&self, _current: u32, desired: u32, _maximum: Option) -> bool { match self.table_elements { Some(limit) if desired > limit => false, _ => true, diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 4ceaed7f56ae..57aee031ffad 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -262,7 +262,7 @@ impl Memory { /// let store = Store::new(&engine); /// /// let memory_ty = MemoryType::new(Limits::new(1, None)); - /// let memory = Memory::new(&store, memory_ty); + /// let memory = Memory::new(&store, memory_ty)?; /// /// let module = Module::new(&engine, "(module (memory (import \"\" \"\") 1))")?; /// let instance = Instance::new(&store, &module, &[memory.into()])?; @@ -270,13 +270,12 @@ impl Memory { /// # Ok(()) /// # } /// ``` - pub fn new(store: &Store, ty: MemoryType) -> Memory { - let (instance, wasmtime_export) = - generate_memory_export(store, &ty).expect("generated memory"); - Memory { + pub fn new(store: &Store, ty: MemoryType) -> Result { + let (instance, wasmtime_export) = generate_memory_export(store, &ty)?; + Ok(Memory { instance, wasmtime_export, - } + }) } /// Returns the underlying type of this memory. @@ -572,7 +571,7 @@ mod tests { .dynamic_memory_guard_size(0); let store = Store::new(&Engine::new(&cfg).unwrap()); let ty = MemoryType::new(Limits::new(1, None)); - let mem = Memory::new(&store, ty); + let mem = Memory::new(&store, ty).unwrap(); assert_eq!(mem.wasmtime_export.memory.offset_guard_size, 0); match mem.wasmtime_export.memory.style { wasmtime_environ::MemoryStyle::Dynamic => {} diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index 89ff3ee28bfe..c0182a90ea94 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -4,6 +4,7 @@ use crate::{ DEFAULT_MEMORY_LIMIT, DEFAULT_TABLE_LIMIT, }; use anyhow::{bail, Result}; +use std::any::{Any, TypeId}; use std::cell::{Cell, RefCell}; use std::collections::{hash_map::Entry, HashMap}; use std::convert::TryFrom; @@ -12,13 +13,9 @@ use std::future::Future; use std::hash::{Hash, Hasher}; use std::pin::Pin; use std::ptr; -use std::rc::{Rc, Weak}; +use std::rc::Rc; use std::sync::Arc; use std::task::{Context, Poll}; -use std::{ - any::{Any, TypeId}, - cell::Ref, -}; use wasmtime_runtime::{ InstanceAllocator, InstanceHandle, ModuleInfo, OnDemandInstanceAllocator, SignalHandler, TrapInfo, VMCallerCheckedAnyfunc, VMContext, VMExternRef, VMExternRefActivationsTable, @@ -93,7 +90,7 @@ pub(crate) struct StoreInner { current_poll_cx: Cell<*mut Context<'static>>, out_of_gas_behavior: Cell, context_values: RefCell>>, - limiter: RefCell>>, + limiter: Option>, } #[derive(Copy, Clone)] @@ -127,6 +124,24 @@ impl Hash for HostInfoKey { impl Store { /// Creates a new store to be associated with the given [`Engine`]. pub fn new(engine: &Engine) -> Self { + Self::new_(engine, None) + } + + /// Creates a new store to be associated with the given [`Engine`] and using the supplied + /// resource limiter. + /// + /// # Example + /// + /// ```rust + /// # use wasmtime::{Engine, Store, StoreLimitsBuilder}; + /// let engine = Engine::default(); + /// let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().instances(10).build()); + /// ``` + pub fn new_with_limits(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { + Self::new_(engine, Some(Rc::new(ResourceLimiterProxy(limiter)))) + } + + fn new_(engine: &Engine, limiter: Option>) -> Self { // Ensure that wasmtime_runtime's signal handlers are configured. Note // that at the `Store` level it means we should perform this // once-per-thread. Platforms like Unix, however, only require this @@ -154,32 +169,11 @@ impl Store { current_poll_cx: Cell::new(ptr::null_mut()), out_of_gas_behavior: Cell::new(OutOfGas::Trap), context_values: RefCell::new(HashMap::new()), - limiter: RefCell::new(None), + limiter, }), } } - /// Creates a new store to be associated with the given [`Engine`] and using the supplied - /// resource limiter. - /// - /// # Example - /// - /// ```rust - /// # use wasmtime::{Engine, Store, StoreLimitsBuilder}; - /// let engine = Engine::default(); - /// let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().instances(10).build()); - /// ``` - pub fn new_with_limits(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { - let store = Store::new(engine); - - { - let mut l = store.inner.limiter.borrow_mut(); - *l = Some(Arc::new(ResourceLimiterProxy::new(&store, limiter))); - } - - store - } - /// Gets a host function from the [`Config`](crate::Config) associated with this [`Store`]. /// /// Returns `None` if the given host function is not defined. @@ -229,8 +223,8 @@ impl Store { } } - pub(crate) fn limiter(&self) -> Ref>> { - self.inner.limiter.borrow() + pub(crate) fn limiter(&self) -> &Option> { + &self.inner.limiter } pub(crate) fn signatures(&self) -> &RefCell { @@ -278,7 +272,6 @@ impl Store { 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; - let (max_instances, max_memories, max_tables) = self.limits(); bump(&self.inner.instance_count, max_instances, 1, "instance")?; @@ -322,15 +315,6 @@ impl Store { self.existing_instance_handle(InstanceHandle::from_vmctx(cx)) } - pub(crate) fn weak(&self) -> Weak { - Rc::downgrade(&self.inner) - } - - pub(crate) fn upgrade(weak: &Weak) -> Option { - let inner = weak.upgrade()?; - Some(Self { inner }) - } - #[cfg_attr(not(target_os = "linux"), allow(dead_code))] // not used on all platforms pub(crate) fn set_signal_handler(&self, handler: Option>>) { *self.inner.signal_handler.borrow_mut() = handler; @@ -870,9 +854,8 @@ impl Store { } fn limits(&self) -> (usize, usize, usize) { - let limiter = self.inner.limiter.borrow(); - - limiter + self.inner + .limiter .as_ref() .map(|l| (l.instances(), l.memories(), l.tables())) .unwrap_or(( diff --git a/crates/wast/src/spectest.rs b/crates/wast/src/spectest.rs index ef744fa42d50..cd8b7bc9d1aa 100644 --- a/crates/wast/src/spectest.rs +++ b/crates/wast/src/spectest.rs @@ -39,7 +39,7 @@ pub fn link_spectest(linker: &mut Linker) -> Result<()> { linker.define("spectest", "table", table)?; let ty = MemoryType::new(Limits::new(1, Some(2))); - let memory = Memory::new(linker.store(), ty); + let memory = Memory::new(linker.store(), ty)?; linker.define("spectest", "memory", memory)?; Ok(()) diff --git a/examples/memory.rs b/examples/memory.rs index 70e1b724a947..e47c249e5fc9 100644 --- a/examples/memory.rs +++ b/examples/memory.rs @@ -75,7 +75,7 @@ fn main() -> Result<()> { println!("Creating stand-alone memory..."); let memorytype = MemoryType::new(Limits::new(5, Some(5))); - let memory2 = Memory::new(&wasmtime_store, memorytype); + let memory2 = Memory::new(&wasmtime_store, memorytype)?; assert_eq!(memory2.size(), 5); assert!(memory2.grow(1).is_err()); assert!(memory2.grow(0).is_ok()); diff --git a/tests/all/externals.rs b/tests/all/externals.rs index 9bffa2c08d61..0e26166f0715 100644 --- a/tests/all/externals.rs +++ b/tests/all/externals.rs @@ -67,7 +67,7 @@ fn cross_store() -> anyhow::Result<()> { let ty = GlobalType::new(ValType::I32, Mutability::Const); let global = Global::new(&store2, ty, Val::I32(0))?; let ty = MemoryType::new(Limits::new(1, None)); - let memory = Memory::new(&store2, ty); + let memory = Memory::new(&store2, ty)?; let ty = TableType::new(ValType::FuncRef, Limits::new(1, None)); let table = Table::new(&store2, ty, Val::FuncRef(None))?; @@ -356,7 +356,7 @@ fn read_write_memory_via_api() { let cfg = Config::new(); let store = Store::new(&Engine::new(&cfg).unwrap()); let ty = MemoryType::new(Limits::new(1, None)); - let mem = Memory::new(&store, ty); + let mem = Memory::new(&store, ty).unwrap(); mem.grow(1).unwrap(); let value = b"hello wasm"; diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 2ce2d781faf0..3d603b647c19 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::rc::Rc; use wasmtime::*; @@ -21,30 +21,42 @@ fn test_limits() -> Result<()> { let instance = Instance::new(&store, &module, &[])?; - let memory = instance.get_memory("m").unwrap(); - - memory.grow(3)?; - memory.grow(5)?; - memory.grow(2)?; - - assert_eq!( - memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), - "failed to grow memory by `1`" - ); - - let table = instance.get_table("t").unwrap(); - - table.grow(2, Val::FuncRef(None))?; - table.grow(1, Val::FuncRef(None))?; - table.grow(2, Val::FuncRef(None))?; + // Test instance exports and host objects hitting the limit + for memory in std::array::IntoIter::new([ + instance.get_memory("m").unwrap(), + Memory::new(&store, MemoryType::new(Limits::new(0, None)))?, + ]) { + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + + assert_eq!( + memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), + "failed to grow memory by `1`" + ); + } - assert_eq!( - table - .grow(1, Val::FuncRef(None)) - .map_err(|e| e.to_string()) - .unwrap_err(), - "failed to grow table by `1`" - ); + // Test instance exports and host objects hitting the limit + for table in std::array::IntoIter::new([ + instance.get_table("t").unwrap(), + Table::new( + &store, + TableType::new(ValType::FuncRef, Limits::new(0, None)), + Val::FuncRef(None), + )?, + ]) { + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + + assert_eq!( + table + .grow(1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + } Ok(()) } @@ -61,23 +73,61 @@ fn test_limits_memory_only() -> Result<()> { let instance = Instance::new(&store, &module, &[])?; - let memory = instance.get_memory("m").unwrap(); + // Test instance exports and host objects hitting the limit + for memory in std::array::IntoIter::new([ + instance.get_memory("m").unwrap(), + Memory::new(&store, MemoryType::new(Limits::new(0, None)))?, + ]) { + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + + assert_eq!( + memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), + "failed to grow memory by `1`" + ); + } - memory.grow(3)?; - memory.grow(5)?; - memory.grow(2)?; + // Test instance exports and host objects *not* hitting the limit + for table in std::array::IntoIter::new([ + instance.get_table("t").unwrap(), + Table::new( + &store, + TableType::new(ValType::FuncRef, Limits::new(0, None)), + Val::FuncRef(None), + )?, + ]) { + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + } - assert_eq!( - memory.grow(1).map_err(|e| e.to_string()).unwrap_err(), - "failed to grow memory by `1`" - ); + Ok(()) +} - let table = instance.get_table("t").unwrap(); +#[test] +fn test_initial_memory_limits_exceeded() -> Result<()> { + let engine = Engine::default(); + let module = Module::new(&engine, r#"(module (memory (export "m") 11))"#)?; - table.grow(2, Val::FuncRef(None))?; - table.grow(1, Val::FuncRef(None))?; - table.grow(2, Val::FuncRef(None))?; - table.grow(1, Val::FuncRef(None))?; + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memory_pages(10).build()); + + match Instance::new(&store, &module, &[]) { + Ok(_) => unreachable!(), + Err(e) => assert_eq!( + e.to_string(), + "Insufficient resources: memory minimum size of 11 pages exceeds memory limits" + ), + } + + match Memory::new(&store, MemoryType::new(Limits::new(25, None))) { + Ok(_) => unreachable!(), + Err(e) => assert_eq!( + e.to_string(), + "Insufficient resources: memory minimum size of 25 pages exceeds memory limits" + ), + } Ok(()) } @@ -95,26 +145,102 @@ fn test_limits_table_only() -> Result<()> { let instance = Instance::new(&store, &module, &[])?; - let memory = instance.get_memory("m").unwrap(); + // Test instance exports and host objects *not* hitting the limit + for memory in std::array::IntoIter::new([ + instance.get_memory("m").unwrap(), + Memory::new(&store, MemoryType::new(Limits::new(0, None)))?, + ]) { + memory.grow(3)?; + memory.grow(5)?; + memory.grow(2)?; + memory.grow(1)?; + } - memory.grow(3)?; - memory.grow(5)?; - memory.grow(2)?; - memory.grow(1)?; + // Test instance exports and host objects hitting the limit + for table in std::array::IntoIter::new([ + instance.get_table("t").unwrap(), + Table::new( + &store, + TableType::new(ValType::FuncRef, Limits::new(0, None)), + Val::FuncRef(None), + )?, + ]) { + table.grow(2, Val::FuncRef(None))?; + table.grow(1, Val::FuncRef(None))?; + table.grow(2, Val::FuncRef(None))?; + + assert_eq!( + table + .grow(1, Val::FuncRef(None)) + .map_err(|e| e.to_string()) + .unwrap_err(), + "failed to grow table by `1`" + ); + } - let table = instance.get_table("t").unwrap(); + Ok(()) +} - table.grow(2, Val::FuncRef(None))?; - table.grow(1, Val::FuncRef(None))?; - table.grow(2, Val::FuncRef(None))?; +#[test] +fn test_initial_table_limits_exceeded() -> Result<()> { + let engine = Engine::default(); + let module = Module::new(&engine, r#"(module (table (export "t") 23 anyfunc))"#)?; - assert_eq!( - table - .grow(1, Val::FuncRef(None)) - .map_err(|e| e.to_string()) - .unwrap_err(), - "failed to grow table by `1`" - ); + let store = + Store::new_with_limits(&engine, StoreLimitsBuilder::new().table_elements(4).build()); + + match Instance::new(&store, &module, &[]) { + Ok(_) => unreachable!(), + Err(e) => assert_eq!( + e.to_string(), + "Insufficient resources: table minimum size of 23 elements exceeds table limits" + ), + } + + match Table::new( + &store, + TableType::new(ValType::FuncRef, Limits::new(99, None)), + Val::FuncRef(None), + ) { + Ok(_) => unreachable!(), + Err(e) => assert_eq!( + e.to_string(), + "Insufficient resources: table minimum size of 99 elements exceeds table limits" + ), + } + + Ok(()) +} + +#[test] +fn test_pooling_allocator_initial_limits_exceeded() -> Result<()> { + let mut config = Config::new(); + config.allocation_strategy(InstanceAllocationStrategy::Pooling { + strategy: PoolingAllocationStrategy::NextAvailable, + module_limits: ModuleLimits::default(), + instance_limits: InstanceLimits { + count: 1, + ..Default::default() + }, + }); + + let engine = Engine::new(&config)?; + let module = Module::new(&engine, r#"(module (memory (export "m") 5))"#)?; + + let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memory_pages(3).build()); + + match Instance::new(&store, &module, &[]) { + Ok(_) => unreachable!(), + Err(e) => assert_eq!( + e.to_string(), + "Insufficient resources: memory minimum size of 5 pages exceeds memory limits" + ), + } + + // An instance should still be able to be created after the failure above + let module = Module::new(&engine, r#"(module (memory (export "m") 2))"#)?; + + Instance::new(&store, &module, &[])?; Ok(()) } @@ -124,55 +250,41 @@ struct MemoryContext { wasm_memory_used: usize, memory_limit: usize, limit_exceeded: bool, + limiter_dropped: bool, } -struct HostMemoryLimiter(Rc>); +struct HostMemoryLimiter(Rc>); impl ResourceLimiter for HostMemoryLimiter { - fn memory_growing( - &self, - store: &Store, - current: u32, - desired: u32, - maximum: Option, - ) -> bool { - if let Some(ctx) = store.get::>>() { - let mut ctx = ctx.borrow_mut(); - - // Check if the desired exceeds a maximum (either from Wasm or from the host) - if desired > maximum.unwrap_or(u32::MAX) { - ctx.limit_exceeded = true; - return false; - } + fn memory_growing(&self, current: u32, desired: u32, maximum: Option) -> bool { + let mut ctx = self.0.borrow_mut(); - assert_eq!(current as usize * 0x10000, ctx.wasm_memory_used); - let desired = desired as usize * 0x10000; + // Check if the desired exceeds a maximum (either from Wasm or from the host) + if desired > maximum.unwrap_or(u32::MAX) { + ctx.limit_exceeded = true; + return false; + } - if desired + ctx.host_memory_used > ctx.memory_limit { - ctx.limit_exceeded = true; - return false; - } + assert_eq!(current as usize * 0x10000, ctx.wasm_memory_used); + let desired = desired as usize * 0x10000; - ctx.wasm_memory_used = desired; + if desired + ctx.host_memory_used > ctx.memory_limit { + ctx.limit_exceeded = true; + return false; } + ctx.wasm_memory_used = desired; true } - fn table_growing( - &self, - _store: &Store, - _current: u32, - _desired: u32, - _maximum: Option, - ) -> bool { + fn table_growing(&self, _current: u32, _desired: u32, _maximum: Option) -> bool { true } } impl Drop for HostMemoryLimiter { fn drop(&mut self) { - self.0.set(true); + self.0.borrow_mut().limiter_dropped = true; } } @@ -204,17 +316,17 @@ fn test_custom_limiter() -> Result<()> { r#"(module (import "" "alloc" (func $alloc (param i32) (result i32))) (memory (export "m") 0) (func (export "f") (param i32) (result i32) local.get 0 call $alloc))"#, )?; - let dropped = Rc::new(Cell::new(false)); - let store = Store::new_with_limits(&engine, HostMemoryLimiter(dropped.clone())); + let context = Rc::new(RefCell::new(MemoryContext { + host_memory_used: 0, + wasm_memory_used: 0, + memory_limit: 1 << 20, // 16 wasm pages is the limit for both wasm + host memory + limit_exceeded: false, + limiter_dropped: false, + })); + + let store = Store::new_with_limits(&engine, HostMemoryLimiter(context.clone())); - assert!(store - .set(Rc::new(RefCell::new(MemoryContext { - host_memory_used: 0, - wasm_memory_used: 0, - memory_limit: 1 << 20, // 16 wasm pages is the limit for both wasm + host memory - limit_exceeded: false - }))) - .is_ok()); + assert!(store.set(context.clone()).is_ok()); let linker = Linker::new(&store); let instance = linker.instantiate(&module)?; @@ -225,13 +337,7 @@ fn test_custom_limiter() -> Result<()> { memory.grow(5)?; memory.grow(2)?; - assert!( - !store - .get::>>() - .unwrap() - .borrow() - .limit_exceeded - ); + assert!(!context.borrow().limit_exceeded); // Grow the host "memory" by 384 KiB let f = instance.get_typed_func::("f")?; @@ -241,13 +347,7 @@ fn test_custom_limiter() -> Result<()> { assert_eq!(f.call(2 * 0x10000).unwrap(), 1); // Memory is at the maximum, but the limit hasn't been exceeded - assert!( - !store - .get::>>() - .unwrap() - .borrow() - .limit_exceeded - ); + assert!(!context.borrow().limit_exceeded); // Try to grow the memory again assert_eq!( @@ -255,24 +355,12 @@ fn test_custom_limiter() -> Result<()> { "failed to grow memory by `1`" ); - assert!( - store - .get::>>() - .unwrap() - .borrow() - .limit_exceeded - ); + assert!(context.borrow().limit_exceeded); // Try to grow the host "memory" again assert_eq!(f.call(1).unwrap(), 0); - assert!( - store - .get::>>() - .unwrap() - .borrow() - .limit_exceeded - ); + assert!(context.borrow().limit_exceeded); drop(f); drop(memory); @@ -280,7 +368,7 @@ fn test_custom_limiter() -> Result<()> { drop(linker); drop(store); - assert!(dropped.get()); + assert!(context.borrow().limiter_dropped); Ok(()) } diff --git a/tests/all/linker.rs b/tests/all/linker.rs index e3d21e96bb2d..2f1707dd7178 100644 --- a/tests/all/linker.rs +++ b/tests/all/linker.rs @@ -51,11 +51,11 @@ fn link_twice_bad() -> Result<()> { // memories let ty = MemoryType::new(Limits::new(1, None)); - let memory = Memory::new(&store, ty); + let memory = Memory::new(&store, ty)?; linker.define("m", "", memory.clone())?; assert!(linker.define("m", "", memory.clone()).is_err()); let ty = MemoryType::new(Limits::new(2, None)); - let memory = Memory::new(&store, ty); + let memory = Memory::new(&store, ty)?; assert!(linker.define("m", "", memory.clone()).is_err()); // tables From 17ec96eee1bcc502cabe7a7a76b000cb4289e0cb Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Fri, 16 Apr 2021 15:36:22 -0700 Subject: [PATCH 4/4] Code review feedback. * Add another memory to `test_pooling_allocator_initial_limits_exceeded` to ensure a partially created instance is successfully deallocated. * Update some doc comments for better documentation of `Store` and `ResourceLimiter`. --- crates/wasmtime/src/limits.rs | 5 +++-- crates/wasmtime/src/store.rs | 23 ++++++++++++++++++++--- tests/all/limits.rs | 11 +++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/crates/wasmtime/src/limits.rs b/crates/wasmtime/src/limits.rs index 169630a058d4..fc65aa2e90f6 100644 --- a/crates/wasmtime/src/limits.rs +++ b/crates/wasmtime/src/limits.rs @@ -4,8 +4,9 @@ pub(crate) const DEFAULT_MEMORY_LIMIT: usize = 10000; /// Used by hosts to limit resource consumption of instances at runtime. /// -/// A [`Store`](crate::Store) can be created with a resource limiter so that hosts can take into account -/// non-WebAssembly resource usage to determine if a linear memory or table should grow. +/// [`Store::new_with_limits`](crate::Store::new_with_limits) can be used +/// with a resource limiter to take into account non-WebAssembly resource +/// usage to determine if a linear memory or table should be grown. pub trait ResourceLimiter { /// Notifies the resource limiter that an instance's linear memory has been requested to grow. /// diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs index c0182a90ea94..fb60ded7d009 100644 --- a/crates/wasmtime/src/store.rs +++ b/crates/wasmtime/src/store.rs @@ -122,20 +122,37 @@ impl Hash for HostInfoKey { } impl Store { - /// Creates a new store to be associated with the given [`Engine`]. + /// Creates a new [`Store`] to be associated with the given [`Engine`]. + /// + /// The created [`Store`] will place no additional limits on the size of linear + /// memories or tables at runtime. Linear memories and tables will be allowed to + /// grow to any upper limit specified in their definitions. + /// + /// The store will limit the number of instances, linear memories, and tables created to 10,000. + /// + /// Use [`Store::new_with_limits`] with a [`StoreLimitsBuilder`](crate::StoreLimitsBuilder) to + /// specify different limits for the store. pub fn new(engine: &Engine) -> Self { Self::new_(engine, None) } - /// Creates a new store to be associated with the given [`Engine`] and using the supplied + /// Creates a new [`Store`] to be associated with the given [`Engine`] and using the supplied /// resource limiter. /// + /// A [`ResourceLimiter`] can be implemented by hosts to control the size of WebAssembly + /// linear memories and tables when a request is made to grow them. + /// + /// [`StoreLimitsBuilder`](crate::StoreLimitsBuilder) can be used to create a + /// [`StoreLimits`](crate::StoreLimits) that implements [`ResourceLimiter`] using + /// static limit values. + /// /// # Example /// /// ```rust /// # use wasmtime::{Engine, Store, StoreLimitsBuilder}; + /// // Place a limit on linear memories so they cannot grow beyond 1 MiB /// let engine = Engine::default(); - /// let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().instances(10).build()); + /// let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memory_pages(16).build()); /// ``` pub fn new_with_limits(engine: &Engine, limiter: impl ResourceLimiter + 'static) -> Self { Self::new_(engine, Some(Rc::new(ResourceLimiterProxy(limiter)))) diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 3d603b647c19..f4c74be61228 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -215,9 +215,13 @@ fn test_initial_table_limits_exceeded() -> Result<()> { #[test] fn test_pooling_allocator_initial_limits_exceeded() -> Result<()> { let mut config = Config::new(); + config.wasm_multi_memory(true); config.allocation_strategy(InstanceAllocationStrategy::Pooling { strategy: PoolingAllocationStrategy::NextAvailable, - module_limits: ModuleLimits::default(), + module_limits: ModuleLimits { + memories: 2, + ..Default::default() + }, instance_limits: InstanceLimits { count: 1, ..Default::default() @@ -225,7 +229,10 @@ fn test_pooling_allocator_initial_limits_exceeded() -> Result<()> { }); let engine = Engine::new(&config)?; - let module = Module::new(&engine, r#"(module (memory (export "m") 5))"#)?; + let module = Module::new( + &engine, + r#"(module (memory (export "m1") 2) (memory (export "m2") 5))"#, + )?; let store = Store::new_with_limits(&engine, StoreLimitsBuilder::new().memory_pages(3).build());