diff --git a/crates/wasmtime/src/runtime/vm/stack_switching.rs b/crates/wasmtime/src/runtime/vm/stack_switching.rs index 614134b9b89c..60b4ea38a481 100644 --- a/crates/wasmtime/src/runtime/vm/stack_switching.rs +++ b/crates/wasmtime/src/runtime/vm/stack_switching.rs @@ -327,7 +327,7 @@ pub fn cont_new( contref_args_ptr, param_count, result_count, - ); + )?; // Now that the initial stack pointer was set by the initialization // function, use it to determine stack limit. diff --git a/crates/wasmtime/src/runtime/vm/stack_switching/stack.rs b/crates/wasmtime/src/runtime/vm/stack_switching/stack.rs index 9138aca7359f..ac75c2454650 100644 --- a/crates/wasmtime/src/runtime/vm/stack_switching/stack.rs +++ b/crates/wasmtime/src/runtime/vm/stack_switching/stack.rs @@ -105,7 +105,7 @@ impl VMContinuationStack { args: *mut VMHostArray, parameter_count: u32, return_value_count: u32, - ) { + ) -> Result<()> { self.0.initialize( func_ref, caller_vmctx, diff --git a/crates/wasmtime/src/runtime/vm/stack_switching/stack/dummy.rs b/crates/wasmtime/src/runtime/vm/stack_switching/stack/dummy.rs index 5d364da7fd3a..fdff6186987f 100644 --- a/crates/wasmtime/src/runtime/vm/stack_switching/stack/dummy.rs +++ b/crates/wasmtime/src/runtime/vm/stack_switching/stack/dummy.rs @@ -62,6 +62,7 @@ impl VMContinuationStack { _args: *mut VMHostArray, _parameter_count: u32, _return_value_count: u32, - ) { + ) -> Result<()> { + Ok(()) } } diff --git a/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs b/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs index a2c17126080e..d784f603f5ee 100644 --- a/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs +++ b/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs @@ -63,6 +63,7 @@ use std::io; use std::ops::Range; use std::ptr; +use crate::prelude::*; use crate::runtime::vm::stack_switching::VMHostArray; use crate::runtime::vm::{VMContext, VMFuncRef, ValRaw}; @@ -230,7 +231,7 @@ impl VMContinuationStack { args: *mut VMHostArray, parameter_count: u32, return_value_count: u32, - ) { + ) -> Result<()> { let tos = self.top; unsafe { @@ -245,8 +246,27 @@ impl VMContinuationStack { debug_assert_eq!(args_ref.capacity, 0); debug_assert_eq!(args_ref.length, 0); - let args_data_size = - usize::try_from(args_capacity).unwrap() * std::mem::size_of::(); + let total_control_size = usize::try_from(args_capacity)? + .checked_mul(std::mem::size_of::()) + .and_then(|s| s.checked_add(0x40)) + .ok_or_else(|| { + format_err!( + "continuation function type with {args_capacity} args \ + overflows stack control data size calculation" + ) + })?; + let args_data_size = total_control_size - 0x40; + + // Ensure the control data (fixed header + args buffer) fits + // within the stack allocation. Without this check, a + // high-arity function type could write past the guard page + // into adjacent memory. + ensure!( + total_control_size <= self.len, + "continuation function type requires {total_control_size} bytes \ + of stack control data, which exceeds the {}-byte stack allocation", + self.len, + ); let args_data_ptr = if args_capacity == 0 { ptr::null_mut() } else { @@ -260,22 +280,21 @@ impl VMContinuationStack { // Data near top of stack: (0x08, wasmtime_continuation_start_address().addr()), (0x10, tos.sub(0x10).addr()), - (0x18, tos.sub(0x40 + args_data_size).addr()), - (0x20, usize::try_from(args_capacity).unwrap()), + (0x18, tos.sub(total_control_size).addr()), + (0x20, usize::try_from(args_capacity)?), // Data after the args buffer: (0x28 + args_data_size, func_ref.addr()), (0x30 + args_data_size, caller_vmctx.addr()), (0x38 + args_data_size, args.addr()), - ( - 0x40 + args_data_size, - usize::try_from(return_value_count).unwrap(), - ), + (0x40 + args_data_size, usize::try_from(return_value_count)?), ]; for (offset, data) in to_store { store(offset, data); } } + + Ok(()) } } diff --git a/tests/all/tags.rs b/tests/all/tags.rs index 896772d3fc33..120d7f27c516 100644 --- a/tests/all/tags.rs +++ b/tests/all/tags.rs @@ -80,6 +80,60 @@ fn wasm_import_tags() -> Result<()> { return Ok(()); } +// Tests that `cont.new` with a function type whose arity exceeds the +// continuation stack size produces an error rather than writing past the +// stack allocation. +#[test] +#[cfg_attr(miri, ignore)] +fn stack_switching_cont_new_high_arity_rejected() -> Result<()> { + let mut config = Config::new(); + config.wasm_stack_switching(true); + config.wasm_exceptions(true); + config.wasm_function_references(true); + + // Use a small continuation stack so we can overflow it with fewer + // than 1000 params (the wasmparser limit for function params). + // With async_stack_size = 8192: + // VMContinuationStack::new rounds to page size (8192), adds a + // guard page, so self.len = 8192 + 4096 = 12288. + // 800 params * 16 bytes + 64 byte header = 12864 > 12288. + config.async_stack_size(8192); + config.max_wasm_stack(4096); + + let Ok(engine) = Engine::new(&config) else { + // Stack switching is not supported on all platforms; skip gracefully. + assert!(!(cfg!(target_arch = "x86_64") && cfg!(unix))); + return Ok(()); + }; + + // Build a WAT module with a high-arity function type. + // 800 params stays under wasmparser's MAX_WASM_FUNCTION_PARAMS (1000) + // but exceeds the 12288-byte stack allocation. + let n_params = 800; + let params: String = (0..n_params).map(|_| " i32").collect(); + let wat = format!( + r#"(module + (type $ft (func (param{params}))) + (type $ct (cont $ft)) + (func $target (type $ft)) + (elem declare func $target) + (func (export "run") + (drop (cont.new $ct (ref.func $target))) + ) + )"# + ); + + let module = Module::new(&engine, &wat)?; + let mut store = Store::new(&engine, ()); + let instance = Instance::new(&mut store, &module, &[])?; + let run = instance.get_typed_func::<(), ()>(&mut store, "run")?; + + let err = run.call(&mut store, ()).unwrap_err(); + err.assert_contains("exceeds"); + + return Ok(()); +} + // Tests that enabling inlining with stack switching, for now, returns an error. // If the support in Cranelift is fixed to the point that this is fine to // enable, then delete this test and the check in `config.rs` as well.