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 d784f603f5ee..bedf1f8d9189 100644 --- a/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs +++ b/crates/wasmtime/src/runtime/vm/stack_switching/stack/unix.rs @@ -258,14 +258,21 @@ impl VMContinuationStack { 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. + // within the usable stack space. For Mmap allocations, + // self.len includes the guard page, which is not writable. + // Without subtracting the guard page, a high-arity function + // type could pass this check but write into the guard page, + // causing a segfault (see #13703). + let page_size = rustix::param::page_size(); + let usable_len = match self.allocator { + Allocator::Mmap => self.len.saturating_sub(page_size), + Allocator::Custom => self.len, + }; ensure!( - total_control_size <= self.len, + total_control_size <= usable_len, "continuation function type requires {total_control_size} bytes \ - of stack control data, which exceeds the {}-byte stack allocation", - self.len, + of stack control data, which exceeds the {usable_len}-byte \ + usable stack allocation", ); let args_data_ptr = if args_capacity == 0 { ptr::null_mut() diff --git a/tests/all/tags.rs b/tests/all/tags.rs index 120d7f27c516..0db6793f1868 100644 --- a/tests/all/tags.rs +++ b/tests/all/tags.rs @@ -95,8 +95,8 @@ fn stack_switching_cont_new_high_arity_rejected() -> Result<()> { // 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. + // guard page, so total mmap = 12288 but usable = 8192. + // 800 params * 16 bytes + 64 byte header = 12864 > 8192. config.async_stack_size(8192); config.max_wasm_stack(4096); @@ -108,7 +108,7 @@ fn stack_switching_cont_new_high_arity_rejected() -> Result<()> { // 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. + // but exceeds the 8192-byte usable stack space. let n_params = 800; let params: String = (0..n_params).map(|_| " i32").collect(); let wat = format!( @@ -134,6 +134,56 @@ fn stack_switching_cont_new_high_arity_rejected() -> Result<()> { return Ok(()); } +// Regression test for #13703: with async_stack_size=8192 and 600 params, the +// control data (600 * 16 + 64 = 9664 bytes) fits within the total mmap +// allocation (12288 = 8192 + 4096 guard) but exceeds the usable stack space +// (8192). Before the fix, the bounds check compared against self.len (which +// includes the guard page), so this case passed the check and then wrote +// into the guard page, causing a segfault. +#[test] +#[cfg_attr(miri, ignore)] +fn stack_switching_cont_new_guard_page_arity_rejected() -> Result<()> { + let mut config = Config::new(); + config.wasm_stack_switching(true); + config.wasm_exceptions(true); + config.wasm_function_references(true); + 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(()); + }; + + // 600 params: control data = 600 * 16 + 64 = 9664 bytes. + // This exceeds the 8192-byte usable stack but fits within the + // 12288-byte total allocation (including guard page). + let n_params = 600; + 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.