Skip to content

Commit

Permalink
Fix access to VMMemoryDefinition::current_length on big-endian (#3013)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
uweigand authored Jun 23, 2021
1 parent b8c0ac7 commit 83007b7
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 16 deletions.
8 changes: 4 additions & 4 deletions crates/runtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand Down
7 changes: 4 additions & 3 deletions crates/runtime/src/instance/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
_ => {
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 18 additions & 2 deletions crates/runtime/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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;
Expand All @@ -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(),
}
Expand Down
2 changes: 1 addition & 1 deletion crates/runtime/src/vmcontext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
6 changes: 3 additions & 3 deletions crates/wasmtime/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion crates/wasmtime/src/trampoline/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<InstanceId> {
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/all/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(())
}
Expand Down

0 comments on commit 83007b7

Please sign in to comment.