From cbcb2c4535dca1f2b01b607ed0381cf2005cf824 Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Tue, 22 Jun 2021 13:50:16 +0200 Subject: [PATCH] Fix access to VMMemoryDefinition::current_length on big-endian The current_length member is defined as "usize" in Rust code, but generated wasm code refers to it as if it were "u32". While this happens to mostly work on little-endian machines (as long as the length is < 4GB), it will always fail on big-endian machines. Fixed by making current_length "u32" in Rust as well, and ensuring the actual memory size is always less than 4GB. --- crates/runtime/src/instance.rs | 8 ++++---- crates/runtime/src/instance/allocator.rs | 7 ++++--- crates/runtime/src/memory.rs | 20 ++++++++++++++++++-- crates/runtime/src/vmcontext.rs | 2 +- crates/wasmtime/src/memory.rs | 6 +++--- crates/wasmtime/src/trampoline/memory.rs | 4 +++- tests/all/instance.rs | 4 ++-- 7 files changed, 35 insertions(+), 16 deletions(-) diff --git a/crates/runtime/src/instance.rs b/crates/runtime/src/instance.rs index 5dc34f546413..65c7efb6e21d 100644 --- a/crates/runtime/src/instance.rs +++ b/crates/runtime/src/instance.rs @@ -703,10 +703,10 @@ impl Instance { if src .checked_add(len) - .map_or(true, |n| n as usize > src_mem.current_length) + .map_or(true, |n| n > src_mem.current_length) || dst .checked_add(len) - .map_or(true, |m| m as usize > dst_mem.current_length) + .map_or(true, |m| m > dst_mem.current_length) { return Err(Trap::wasm(ir::TrapCode::HeapOutOfBounds)); } @@ -741,7 +741,7 @@ impl Instance { if dst .checked_add(len) - .map_or(true, |m| m as usize > memory.current_length) + .map_or(true, |m| m > memory.current_length) { return Err(Trap::wasm(ir::TrapCode::HeapOutOfBounds)); } @@ -825,7 +825,7 @@ impl Instance { .map_or(true, |n| n as usize > data.len()) || dst .checked_add(len) - .map_or(true, |m| m as usize > memory.current_length) + .map_or(true, |m| m > memory.current_length) { return Err(Trap::wasm(ir::TrapCode::HeapOutOfBounds)); } diff --git a/crates/runtime/src/instance/allocator.rs b/crates/runtime/src/instance/allocator.rs index 7c935288fbe0..c1a6bea496a5 100644 --- a/crates/runtime/src/instance/allocator.rs +++ b/crates/runtime/src/instance/allocator.rs @@ -309,7 +309,7 @@ fn check_memory_init_bounds( let end = start.checked_add(init.data.len()); match end { - Some(end) if end <= memory.current_length => { + Some(end) if end <= memory.current_length as usize => { // Initializer is in bounds } _ => { @@ -382,8 +382,9 @@ fn initialize_instance( MemoryInitialization::Paged { map, out_of_bounds } => { for (index, pages) in map { let memory = instance.memory(index); - let slice = - unsafe { slice::from_raw_parts_mut(memory.base, memory.current_length) }; + let slice = unsafe { + slice::from_raw_parts_mut(memory.base, memory.current_length as usize) + }; for (page_index, page) in pages.iter().enumerate() { if let Some(data) = page { diff --git a/crates/runtime/src/memory.rs b/crates/runtime/src/memory.rs index e69dd056ee96..5d5dbdc28378 100644 --- a/crates/runtime/src/memory.rs +++ b/crates/runtime/src/memory.rs @@ -155,6 +155,10 @@ impl RuntimeLinearMemory for MmapMemory { // Linear memory size would exceed the index range. return None; } + // FIXME: https://github.com/bytecodealliance/wasmtime/issues/3022 + if new_pages == WASM_MAX_PAGES { + return None; + } let delta_bytes = usize::try_from(delta).unwrap() * WASM_PAGE_SIZE as usize; let prev_bytes = usize::try_from(prev_pages).unwrap() * WASM_PAGE_SIZE as usize; @@ -194,7 +198,8 @@ impl RuntimeLinearMemory for MmapMemory { fn vmmemory(&self) -> VMMemoryDefinition { VMMemoryDefinition { base: unsafe { self.mmap.alloc.as_mut_ptr().add(self.pre_guard_size) }, - current_length: self.mmap.size as usize * WASM_PAGE_SIZE as usize, + current_length: u32::try_from(self.mmap.size as usize * WASM_PAGE_SIZE as usize) + .unwrap(), } } } @@ -271,6 +276,13 @@ impl Memory { } fn limit_new(plan: &MemoryPlan, limiter: Option<&mut dyn ResourceLimiter>) -> Result<()> { + // FIXME: https://github.com/bytecodealliance/wasmtime/issues/3022 + if plan.memory.minimum == WASM_MAX_PAGES { + bail!( + "memory minimum size of {} pages exceeds memory limits", + plan.memory.minimum + ); + } if let Some(limiter) = limiter { if !limiter.memory_growing(0, plan.memory.minimum, plan.memory.maximum) { bail!( @@ -362,6 +374,10 @@ impl Memory { if new_size > maximum.unwrap_or(WASM_MAX_PAGES) { return None; } + // FIXME: https://github.com/bytecodealliance/wasmtime/issues/3022 + if new_size == WASM_MAX_PAGES { + return None; + } 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; @@ -381,7 +397,7 @@ impl Memory { match self { Memory::Static { base, size, .. } => VMMemoryDefinition { base: base.as_ptr() as *mut _, - current_length: *size as usize * WASM_PAGE_SIZE as usize, + current_length: u32::try_from(*size as usize * WASM_PAGE_SIZE as usize).unwrap(), }, Memory::Dynamic(mem) => mem.vmmemory(), } diff --git a/crates/runtime/src/vmcontext.rs b/crates/runtime/src/vmcontext.rs index 980c87e2a646..89498e8f4aae 100644 --- a/crates/runtime/src/vmcontext.rs +++ b/crates/runtime/src/vmcontext.rs @@ -203,7 +203,7 @@ pub struct VMMemoryDefinition { pub base: *mut u8, /// The current logical size of this linear memory in bytes. - pub current_length: usize, + pub current_length: u32, } #[cfg(test)] diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs index 7ea2c96a533a..a65b8d29f2d6 100644 --- a/crates/wasmtime/src/memory.rs +++ b/crates/wasmtime/src/memory.rs @@ -318,7 +318,7 @@ impl Memory { unsafe { let store = store.into(); let definition = *store[self.0].definition; - slice::from_raw_parts(definition.base, definition.current_length) + slice::from_raw_parts(definition.base, definition.current_length as usize) } } @@ -334,7 +334,7 @@ impl Memory { unsafe { let store = store.into(); let definition = *store[self.0].definition; - slice::from_raw_parts_mut(definition.base, definition.current_length) + slice::from_raw_parts_mut(definition.base, definition.current_length as usize) } } @@ -395,7 +395,7 @@ impl Memory { /// /// Panics if this memory doesn't belong to `store`. pub fn data_size(&self, store: impl AsContext) -> usize { - unsafe { (*store.as_context()[self.0].definition).current_length } + unsafe { (*store.as_context()[self.0].definition).current_length as usize } } /// Returns the size, in WebAssembly pages, of this wasm memory. diff --git a/crates/wasmtime/src/trampoline/memory.rs b/crates/wasmtime/src/trampoline/memory.rs index 25ee7c2462e6..e5e03db211a5 100644 --- a/crates/wasmtime/src/trampoline/memory.rs +++ b/crates/wasmtime/src/trampoline/memory.rs @@ -7,6 +7,7 @@ use wasmtime_environ::entity::PrimaryMap; use wasmtime_environ::{wasm, MemoryPlan, MemoryStyle, Module, WASM_PAGE_SIZE}; use wasmtime_runtime::{RuntimeLinearMemory, RuntimeMemoryCreator, VMMemoryDefinition}; +use std::convert::TryFrom; use std::sync::Arc; pub fn create_memory(store: &mut StoreOpaque<'_>, memory: &MemoryType) -> Result { @@ -48,7 +49,8 @@ impl RuntimeLinearMemory for LinearMemoryProxy { fn vmmemory(&self) -> VMMemoryDefinition { VMMemoryDefinition { base: self.mem.as_ptr(), - current_length: self.mem.size() as usize * WASM_PAGE_SIZE as usize, + current_length: u32::try_from(self.mem.size() as usize * WASM_PAGE_SIZE as usize) + .unwrap(), } } } diff --git a/tests/all/instance.rs b/tests/all/instance.rs index 84d2f97392b1..1def40354d1c 100644 --- a/tests/all/instance.rs +++ b/tests/all/instance.rs @@ -55,7 +55,7 @@ fn linear_memory_limits() -> Result<()> { fn test(engine: &Engine) -> Result<()> { let wat = r#" (module - (memory 65535) + (memory 65534) (func (export "foo") (result i32) i32.const 1 @@ -68,7 +68,7 @@ fn linear_memory_limits() -> Result<()> { let instance = Instance::new(&mut store, &module, &[])?; let foo = instance.get_typed_func::<(), i32, _>(&mut store, "foo")?; - assert_eq!(foo.call(&mut store, ())?, 65535); + assert_eq!(foo.call(&mut store, ())?, 65534); assert_eq!(foo.call(&mut store, ())?, -1); Ok(()) }