diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 08a6ff2c38..f69b17a8ae 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -12,6 +12,7 @@ use crate::vmcontext::{ VMInterrupts, VMMemoryDefinition, VMMemoryImport, VMTableDefinition, VMTableImport, }; use crate::{ExportFunction, ExportGlobal, ExportMemory, ExportTable, Store}; +use anyhow::Error; use memoffset::offset_of; use more_asserts::assert_lt; use std::alloc::Layout; @@ -64,6 +65,14 @@ pub trait ResourceLimiter { /// this method. fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option) -> bool; + /// Notifies the resource limiter that growing a linear memory, permitted by + /// the `memory_growing` method, has failed. + /// + /// Reasons for failure include: the growth exceeds the `maximum` passed to + /// `memory_growing`, or the operating system failed to allocate additional + /// memory. In that case, `error` might be downcastable to a `std::io::Error`. + fn memory_grow_failed(&mut self, _error: &Error) {} + /// Notifies the resource limiter that an instance's table has been requested to grow. /// /// * `current` is the current number of elements in the table. diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index d8a48dd390..0f25c7a1d8 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -5,7 +5,7 @@ use crate::mmap::Mmap; use crate::vmcontext::VMMemoryDefinition; use crate::ResourceLimiter; -use anyhow::{bail, format_err, Result}; +use anyhow::{bail, format_err, Error, Result}; use more_asserts::{assert_ge, assert_le}; use std::convert::TryFrom; use wasmtime_environ::{MemoryPlan, MemoryStyle, WASM32_MAX_PAGES, WASM64_MAX_PAGES}; @@ -50,9 +50,9 @@ pub trait RuntimeLinearMemory: Send + Sync { /// Grow memory to the specified amount of bytes. /// - /// Returns `None` if memory can't be grown by the specified amount + /// Returns an error if memory can't be grown by the specified amount /// of bytes. - fn grow_to(&mut self, size: usize) -> Option<()>; + fn grow_to(&mut self, size: usize) -> Result<()>; /// Return a `VMMemoryDefinition` for exposing the memory to compiled wasm /// code. @@ -137,7 +137,7 @@ impl RuntimeLinearMemory for MmapMemory { self.maximum } - fn grow_to(&mut self, new_size: usize) -> Option<()> { + fn grow_to(&mut self, new_size: usize) -> Result<()> { if new_size > self.mmap.len() - self.offset_guard_size - self.pre_guard_size { // If the new size of this heap exceeds the current size of the // allocation we have, then this must be a dynamic heap. Use @@ -145,14 +145,13 @@ impl RuntimeLinearMemory for MmapMemory { // and then copy over the memory from before. let request_bytes = self .pre_guard_size - .checked_add(new_size)? - .checked_add(self.extra_to_reserve_on_growth)? - .checked_add(self.offset_guard_size)?; + .checked_add(new_size) + .and_then(|s| s.checked_add(self.extra_to_reserve_on_growth)) + .and_then(|s| s.checked_add(self.offset_guard_size)) + .ok_or_else(|| format_err!("overflow calculating size of memory allocation"))?; - let mut new_mmap = Mmap::accessible_reserved(0, request_bytes).ok()?; - new_mmap - .make_accessible(self.pre_guard_size, new_size) - .ok()?; + let mut new_mmap = Mmap::accessible_reserved(0, request_bytes)?; + new_mmap.make_accessible(self.pre_guard_size, new_size)?; new_mmap.as_mut_slice()[self.pre_guard_size..][..self.accessible] .copy_from_slice(&self.mmap.as_slice()[self.pre_guard_size..][..self.accessible]); @@ -166,17 +165,15 @@ impl RuntimeLinearMemory for MmapMemory { // initial allocation to grow into before the heap is moved in // memory. assert!(new_size > self.accessible); - self.mmap - .make_accessible( - self.pre_guard_size + self.accessible, - new_size - self.accessible, - ) - .ok()?; + self.mmap.make_accessible( + self.pre_guard_size + self.accessible, + new_size - self.accessible, + )?; } self.accessible = new_size; - Some(()) + Ok(()) } fn vmmemory(&self) -> VMMemoryDefinition { @@ -215,6 +212,25 @@ pub enum Memory { Dynamic(Box), } +fn memory_growing( + limiter: &mut Option<&mut dyn ResourceLimiter>, + current: usize, + desired: usize, + maximum: Option, +) -> bool { + match limiter { + Some(ref mut l) => l.memory_growing(current, desired, maximum), + None => true, + } +} + +fn memory_grow_failed(limiter: &mut Option<&mut dyn ResourceLimiter>, error: &Error) { + match limiter { + Some(l) => l.memory_grow_failed(error), + None => {} + } +} + impl Memory { /// Create a new dynamic (movable) memory instance for the specified plan. pub fn new_dynamic( @@ -260,7 +276,7 @@ impl Memory { /// bytes. fn limit_new( plan: &MemoryPlan, - limiter: Option<&mut dyn ResourceLimiter>, + mut limiter: Option<&mut dyn ResourceLimiter>, ) -> Result<(usize, Option)> { // Sanity-check what should already be true from wasm module validation. let absolute_max = if plan.memory.memory64 { @@ -322,13 +338,11 @@ impl Memory { // calculation overflowed. This means that the `minimum` we're informing // the limiter is lossy and may not be 100% accurate, but for now the // expected uses of `limiter` means that's ok. - if let Some(limiter) = limiter { - if !limiter.memory_growing(0, minimum.unwrap_or(absolute_max), maximum) { - bail!( - "memory minimum size of {} pages exceeds memory limits", - plan.memory.minimum - ); - } + if !memory_growing(&mut limiter, 0, minimum.unwrap_or(absolute_max), maximum) { + bail!( + "memory minimum size of {} pages exceeds memory limits", + plan.memory.minimum + ); } // At this point we need to actually handle overflows, so bail out with @@ -389,26 +403,40 @@ impl Memory { pub unsafe fn grow( &mut self, delta_pages: u64, - limiter: Option<&mut dyn ResourceLimiter>, + mut limiter: Option<&mut dyn ResourceLimiter>, ) -> Option { let old_byte_size = self.byte_size(); + // Wasm spec: when growing by 0 pages, always return the current size. if delta_pages == 0 { return Some(old_byte_size); } + // largest wasm-page-aligned region of memory it is possible to + // represent in a usize. This will be impossible for the system to + // actually allocate. + let absolute_max = 0usize.wrapping_sub(WASM_PAGE_SIZE); + // calculate byte size of the new allocation. Let it overflow up to + // usize::MAX, then clamp it down to absolute_max. let new_byte_size = usize::try_from(delta_pages) - .ok()? - .checked_mul(WASM_PAGE_SIZE)? - .checked_add(old_byte_size)?; - let maximum = self.maximum_byte_size(); + .unwrap_or(usize::MAX) + .saturating_mul(WASM_PAGE_SIZE) + .saturating_add(old_byte_size); + let new_byte_size = if new_byte_size > absolute_max { + absolute_max + } else { + new_byte_size + }; + let maximum = self.maximum_byte_size(); + // Limiter gets first chance to reject memory_growing. + if !memory_growing(&mut limiter, old_byte_size, new_byte_size, maximum) { + return None; + } + + // Never exceed maximum, even if limiter permitted it. if let Some(max) = maximum { if new_byte_size > max { - return None; - } - } - if let Some(limiter) = limiter { - if !limiter.memory_growing(old_byte_size, new_byte_size, maximum) { + memory_grow_failed(&mut limiter, &format_err!("Memory maximum size exceeded")); return None; } } @@ -428,19 +456,25 @@ impl Memory { make_accessible, .. } => { + // Never exceed static memory size if new_byte_size > base.len() { + memory_grow_failed(&mut limiter, &format_err!("static memory size exceeded")); return None; } - make_accessible( + // Operating system can fail to make memory accessible + let r = make_accessible( base.as_mut_ptr().add(old_byte_size), new_byte_size - old_byte_size, - ) - .ok()?; + ); + r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?; *size = new_byte_size; } - Memory::Dynamic(mem) => mem.grow_to(new_byte_size)?, + Memory::Dynamic(mem) => { + let r = mem.grow_to(new_byte_size); + r.map_err(|e| memory_grow_failed(&mut limiter, &e)).ok()?; + } } Some(old_byte_size) } diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 8cfd900645..645ecaaa15 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -524,9 +524,10 @@ pub unsafe trait LinearMemory: Send + Sync + 'static { /// Grows this memory to have the `new_size`, in bytes, specified. /// - /// Returns `None` if memory can't be grown by the specified amount - /// of bytes. Returns `Some` if memory was grown successfully. - fn grow_to(&mut self, new_size: usize) -> Option<()>; + /// Returns `Err` if memory can't be grown by the specified amount + /// of bytes. The error may be downcastable to `std::io::Error`. + /// Returns `Ok` if memory was grown successfully. + fn grow_to(&mut self, new_size: usize) -> Result<()>; /// Return the allocated memory as a mutable pointer to u8. fn as_ptr(&self) -> *mut u8; diff --git a/crates/wasmtime/src/trampoline/memory.rs b/crates/wasmtime/src/trampoline/memory.rs index d3507504ec..942cb6bd6f 100644 --- a/crates/wasmtime/src/trampoline/memory.rs +++ b/crates/wasmtime/src/trampoline/memory.rs @@ -36,7 +36,7 @@ impl RuntimeLinearMemory for LinearMemoryProxy { self.mem.maximum_byte_size() } - fn grow_to(&mut self, new_size: usize) -> Option<()> { + fn grow_to(&mut self, new_size: usize) -> Result<()> { self.mem.grow_to(new_size) } diff --git a/tests/all/limits.rs b/tests/all/limits.rs index 502b55954f..3e276d8c81 100644 --- a/tests/all/limits.rs +++ b/tests/all/limits.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{bail, Result}; use wasmtime::*; const WASM_PAGE_SIZE: usize = wasmtime_environ::WASM_PAGE_SIZE as usize; @@ -385,3 +385,161 @@ fn test_custom_limiter() -> Result<()> { Ok(()) } + +#[derive(Default)] +struct MemoryGrowFailureDetector { + /// Arguments of most recent call to memory_growing + current: usize, + desired: usize, + /// Display impl of most recent call to memory_grow_failed + error: Option, +} + +impl ResourceLimiter for MemoryGrowFailureDetector { + fn memory_growing(&mut self, current: usize, desired: usize, _maximum: Option) -> bool { + self.current = current; + self.desired = desired; + true + } + + fn memory_grow_failed(&mut self, err: &anyhow::Error) { + self.error = Some(err.to_string()); + } + + fn table_growing(&mut self, _current: u32, _desired: u32, _maximum: Option) -> bool { + true + } +} + +#[test] +fn custom_limiter_detect_grow_failure() -> Result<()> { + let mut config = Config::new(); + config.allocation_strategy(InstanceAllocationStrategy::Pooling { + strategy: PoolingAllocationStrategy::NextAvailable, + module_limits: ModuleLimits { + memory_pages: 10, + ..Default::default() + }, + instance_limits: InstanceLimits { + ..Default::default() + }, + }); + let engine = Engine::new(&config).unwrap(); + let linker = Linker::new(&engine); + + let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#)?; + + let context = MemoryGrowFailureDetector::default(); + + let mut store = Store::new(&engine, context); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module)?; + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Grow the memory by 640 KiB (10 pages) + memory.grow(&mut store, 10)?; + + assert!(store.data().error.is_none()); + assert_eq!(store.data().current, 0); + assert_eq!(store.data().desired, 10 * 64 * 1024); + + // Grow past the static limit set by ModuleLimits. + // The ResourcLimiter will permit this, but the grow will fail. + assert_eq!( + memory.grow(&mut store, 1).unwrap_err().to_string(), + "failed to grow memory by `1`" + ); + + assert_eq!(store.data().current, 10 * 64 * 1024); + assert_eq!(store.data().desired, 11 * 64 * 1024); + assert_eq!( + store.data().error.as_ref().unwrap(), + "Memory maximum size exceeded" + ); + + drop(store); + + Ok(()) +} + +#[cfg(target_os = "linux")] +#[test] +fn custom_limiter_detect_os_oom_failure() -> Result<()> { + let pid = unsafe { libc::fork() }; + if pid == 0 { + // Child process + let r = std::panic::catch_unwind(|| { + // Ask Linux to limit this process to 128MiB of memory + let process_max_memory: usize = 128 * 1024 * 1024; + unsafe { + // limit process to 128MiB memory + let rlimit = libc::rlimit { + rlim_cur: 0, + rlim_max: process_max_memory as u64, + }; + let res = libc::setrlimit(libc::RLIMIT_DATA, &rlimit); + assert_eq!(res, 0, "setrlimit failed: {}", res); + }; + + // Default behavior of on-demand memory allocation so that a + // memory grow will hit Linux for a larger mmap. + let engine = Engine::default(); + let linker = Linker::new(&engine); + let module = Module::new(&engine, r#"(module (memory (export "m") 0))"#).unwrap(); + + let context = MemoryGrowFailureDetector::default(); + + let mut store = Store::new(&engine, context); + store.limiter(|s| s as &mut dyn ResourceLimiter); + let instance = linker.instantiate(&mut store, &module).unwrap(); + let memory = instance.get_memory(&mut store, "m").unwrap(); + + // Small (640KiB) grow should succeed + memory.grow(&mut store, 10).unwrap(); + assert!(store.data().error.is_none()); + assert_eq!(store.data().current, 0); + assert_eq!(store.data().desired, 10 * 64 * 1024); + + // Try to grow past the process's memory limit. + // This should fail. + let pages_exceeding_limit = process_max_memory / (64 * 1024); + let err_msg = memory + .grow(&mut store, pages_exceeding_limit as u64) + .unwrap_err() + .to_string(); + assert!( + err_msg.starts_with("failed to grow memory"), + "unexpected error: {}", + err_msg + ); + + assert_eq!(store.data().current, 10 * 64 * 1024); + assert_eq!( + store.data().desired, + (pages_exceeding_limit + 10) * 64 * 1024 + ); + // The memory_grow_failed hook should show Linux gave OOM: + let err_msg = store.data().error.as_ref().unwrap(); + assert!( + err_msg.starts_with("System call failed: Cannot allocate memory"), + "unexpected error: {}", + err_msg + ); + }); + // on assertion failure, exit 1 so parent process can fail test. + std::process::exit(if r.is_err() { 1 } else { 0 }); + } else { + // Parent process + let mut wstatus: libc::c_int = 0; + unsafe { libc::wait(&mut wstatus) }; + if libc::WIFEXITED(wstatus) { + if libc::WEXITSTATUS(wstatus) == 0 { + Ok(()) + } else { + bail!("child exited with failure"); + } + } else { + bail!("child didnt exit??") + } + } +} diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs index 96722b3198..7ba8ed0078 100644 --- a/tests/all/memory_creator.rs +++ b/tests/all/memory_creator.rs @@ -62,7 +62,7 @@ mod not_for_windows { Some(self.size - self.guard_size) } - fn grow_to(&mut self, new_size: usize) -> Option<()> { + fn grow_to(&mut self, new_size: usize) -> Result<(), anyhow::Error> { println!("grow to {:x}", new_size); let delta = new_size - self.used_wasm_bytes; unsafe { @@ -73,7 +73,7 @@ mod not_for_windows { *self.glob_bytes_counter.lock().unwrap() += delta; self.used_wasm_bytes = new_size; - Some(()) + Ok(()) } fn as_ptr(&self) -> *mut u8 {