Skip to content

Commit

Permalink
Print an error on async stack overflow (#9304)
Browse files Browse the repository at this point in the history
* Print an error on async stack overflow

This commit updates Wasmtime's handling of traps on Unix platforms to
print an error message on stack overflow when the guard page is hit.
This is distinct from stack overflow in WebAssembly which raises a
normal trap and can be caught. This is instead to be used on
misconfigured hosts where the async stack is too small or wasm was
allowed to take up too much of the async stack. Currently no error
message is printed and the program simply aborts with a core dump which
can be difficult to debug.

This instead registers the range of the async guard page with the trap
handling infrastructure to test the faulting address and if it lies
within this range. If so then a small message is printed and then the
program is aborted with `libc::abort()`.

This does not impact the safety of any prior embedding or fix any
issues. It's instead intended purely as a diagnostic tool to help users
more quickly understand that stack size configuration settings are the
likely culprit.

* Fix build of c-api and tests

prtest:full

* Fix build on Windows

* Fix a warning on Windows

* Fix dead code on miri
  • Loading branch information
alexcrichton authored Sep 26, 2024
1 parent 719d673 commit 110e70f
Show file tree
Hide file tree
Showing 13 changed files with 235 additions and 38 deletions.
3 changes: 3 additions & 0 deletions crates/c-api/src/async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ unsafe impl StackMemory for CHostStackMemory {
let base = unsafe { top.sub(len) as usize };
base..base + len
}
fn guard_range(&self) -> Range<*mut u8> {
std::ptr::null_mut()..std::ptr::null_mut()
}
}

pub type wasmtime_new_stack_memory_callback_t = extern "C" fn(
Expand Down
24 changes: 19 additions & 5 deletions crates/fiber/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,26 @@ impl FiberStack {
}

/// Creates a new fiber stack with the given pointer to the bottom of the
/// stack plus the byte length of the stack.
/// stack plus how large the guard size and stack size are.
///
/// The `bottom` pointer should be addressable for `len` bytes. The page
/// beneath `bottom` should be unmapped as a guard page.
/// The bytes from `bottom` to `bottom.add(guard_size)` should all be
/// guaranteed to be unmapped. The bytes from `bottom.add(guard_size)` to
/// `bottom.add(guard_size + len)` should be addressable.
///
/// # Safety
///
/// This is unsafe because there is no validation of the given pointer.
///
/// The caller must properly allocate the stack space with a guard page and
/// make the pages accessible for correct behavior.
pub unsafe fn from_raw_parts(bottom: *mut u8, len: usize) -> io::Result<Self> {
Ok(Self(imp::FiberStack::from_raw_parts(bottom, len)?))
pub unsafe fn from_raw_parts(
bottom: *mut u8,
guard_size: usize,
len: usize,
) -> io::Result<Self> {
Ok(Self(imp::FiberStack::from_raw_parts(
bottom, guard_size, len,
)?))
}

/// Gets the top of the stack.
Expand All @@ -67,6 +74,11 @@ impl FiberStack {
pub fn is_from_raw_parts(&self) -> bool {
self.0.is_from_raw_parts()
}

/// Returns the range of memory that the guard page(s) reside in.
pub fn guard_range(&self) -> Option<Range<*mut u8>> {
self.0.guard_range()
}
}

/// A creator of RuntimeFiberStacks.
Expand All @@ -85,6 +97,8 @@ pub unsafe trait RuntimeFiberStack: Send + Sync {
fn top(&self) -> *mut u8;
/// The valid range of the stack without guard pages.
fn range(&self) -> Range<usize>;
/// The range of the guard page(s)
fn guard_range(&self) -> Range<*mut u8>;
}

pub struct Fiber<'a, Resume, Yield, Return> {
Expand Down
21 changes: 16 additions & 5 deletions crates/fiber/src/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub struct FiberStack {

enum FiberStackStorage {
Mmap(#[allow(dead_code)] MmapFiberStack),
Unmanaged,
Unmanaged(usize),
Custom(#[allow(dead_code)] Box<dyn RuntimeFiberStack>),
}

Expand All @@ -70,21 +70,21 @@ impl FiberStack {
})
}

pub unsafe fn from_raw_parts(base: *mut u8, len: usize) -> io::Result<Self> {
pub unsafe fn from_raw_parts(base: *mut u8, guard_size: usize, len: usize) -> io::Result<Self> {
// See comments in `mod asan` below for why asan has a different stack
// allocation strategy.
if cfg!(asan) {
return Self::from_custom(asan::new_fiber_stack(len)?);
}
Ok(FiberStack {
base,
base: base.add(guard_size),
len,
storage: FiberStackStorage::Unmanaged,
storage: FiberStackStorage::Unmanaged(guard_size),
})
}

pub fn is_from_raw_parts(&self) -> bool {
matches!(self.storage, FiberStackStorage::Unmanaged)
matches!(self.storage, FiberStackStorage::Unmanaged(_))
}

pub fn from_custom(custom: Box<dyn RuntimeFiberStack>) -> io::Result<Self> {
Expand Down Expand Up @@ -115,6 +115,17 @@ impl FiberStack {
let base = self.base as usize;
Some(base..base + self.len)
}

pub fn guard_range(&self) -> Option<Range<*mut u8>> {
match &self.storage {
FiberStackStorage::Unmanaged(guard_size) => unsafe {
let start = self.base.sub(*guard_size);
Some(start..self.base)
},
FiberStackStorage::Mmap(mmap) => Some(mmap.mapping_base..self.base),
FiberStackStorage::Custom(custom) => Some(custom.guard_range()),
}
}
}

struct MmapFiberStack {
Expand Down
10 changes: 9 additions & 1 deletion crates/fiber/src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ impl FiberStack {
Ok(Self(size))
}

pub unsafe fn from_raw_parts(_base: *mut u8, _len: usize) -> io::Result<Self> {
pub unsafe fn from_raw_parts(
_base: *mut u8,
_guard_size: usize,
_len: usize,
) -> io::Result<Self> {
Err(io::Error::from_raw_os_error(ERROR_NOT_SUPPORTED as i32))
}

Expand All @@ -34,6 +38,10 @@ impl FiberStack {
pub fn range(&self) -> Option<Range<usize>> {
None
}

pub fn guard_range(&self) -> Option<Range<*mut u8>> {
None
}
}

pub struct Fiber {
Expand Down
1 change: 1 addition & 0 deletions crates/wasmtime/src/runtime/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,7 @@ pub(crate) fn invoke_wasm_and_catch_traps<T>(
store.0.signal_handler(),
store.0.engine().config().wasm_backtrace,
store.0.engine().config().coredump_on_trap,
store.0.async_guard_range(),
store.0.default_caller(),
closure,
);
Expand Down
6 changes: 6 additions & 0 deletions crates/wasmtime/src/runtime/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub unsafe trait StackMemory: Send + Sync {
fn top(&self) -> *mut u8;
/// The range of where this stack resides in memory, excluding guard pages.
fn range(&self) -> Range<usize>;
/// The range of memory where the guard region of this stack resides.
fn guard_range(&self) -> Range<*mut u8>;
}

pub(crate) struct FiberStackProxy(pub Box<dyn StackMemory>);
Expand All @@ -69,4 +71,8 @@ unsafe impl RuntimeFiberStack for FiberStackProxy {
fn range(&self) -> Range<usize> {
self.0.range()
}

fn guard_range(&self) -> Range<*mut u8> {
self.0.guard_range()
}
}
58 changes: 50 additions & 8 deletions crates/wasmtime/src/runtime/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ use core::future::Future;
use core::marker;
use core::mem::{self, ManuallyDrop};
use core::num::NonZeroU64;
use core::ops::{Deref, DerefMut};
use core::ops::{Deref, DerefMut, Range};
use core::pin::Pin;
use core::ptr;
use core::task::{Context, Poll};
Expand Down Expand Up @@ -391,7 +391,26 @@ pub struct StoreOpaque {
#[cfg(feature = "async")]
struct AsyncState {
current_suspend: UnsafeCell<*mut wasmtime_fiber::Suspend<Result<()>, (), Result<()>>>,
current_poll_cx: UnsafeCell<*mut Context<'static>>,
current_poll_cx: UnsafeCell<PollContext>,
}

#[cfg(feature = "async")]
#[derive(Clone, Copy)]
struct PollContext {
future_context: *mut Context<'static>,
guard_range_start: *mut u8,
guard_range_end: *mut u8,
}

#[cfg(feature = "async")]
impl Default for PollContext {
fn default() -> PollContext {
PollContext {
future_context: core::ptr::null_mut(),
guard_range_start: core::ptr::null_mut(),
guard_range_end: core::ptr::null_mut(),
}
}
}

// Lots of pesky unsafe cells and pointers in this structure. This means we need
Expand Down Expand Up @@ -536,7 +555,7 @@ impl<T> Store<T> {
#[cfg(feature = "async")]
async_state: AsyncState {
current_suspend: UnsafeCell::new(ptr::null_mut()),
current_poll_cx: UnsafeCell::new(ptr::null_mut()),
current_poll_cx: UnsafeCell::new(PollContext::default()),
},
fuel_reserve: 0,
fuel_yield_interval: None,
Expand Down Expand Up @@ -1779,13 +1798,13 @@ impl StoreOpaque {
}

let poll_cx_inner_ptr = unsafe { *poll_cx_box_ptr };
if poll_cx_inner_ptr.is_null() {
if poll_cx_inner_ptr.future_context.is_null() {
return None;
}

Some(AsyncCx {
current_suspend: self.async_state.current_suspend.get(),
current_poll_cx: poll_cx_box_ptr,
current_poll_cx: unsafe { core::ptr::addr_of_mut!((*poll_cx_box_ptr).future_context) },
track_pkey_context_switch: self.pkey.is_some(),
})
}
Expand Down Expand Up @@ -2053,6 +2072,18 @@ at https://bytecodealliance.org/security.

self.num_component_instances += 1;
}

pub(crate) fn async_guard_range(&self) -> Range<*mut u8> {
#[cfg(feature = "async")]
unsafe {
let ptr = self.async_state.current_poll_cx.get();
(*ptr).guard_range_start..(*ptr).guard_range_end
}
#[cfg(not(feature = "async"))]
{
core::ptr::null_mut()..core::ptr::null_mut()
}
}
}

impl<T> StoreContextMut<'_, T> {
Expand Down Expand Up @@ -2123,7 +2154,7 @@ impl<T> StoreContextMut<'_, T> {

struct FiberFuture<'a> {
fiber: Option<wasmtime_fiber::Fiber<'a, Result<()>, (), Result<()>>>,
current_poll_cx: *mut *mut Context<'static>,
current_poll_cx: *mut PollContext,
engine: Engine,
// See comments in `FiberFuture::resume` for this
state: Option<crate::runtime::vm::AsyncWasmCallState>,
Expand Down Expand Up @@ -2259,10 +2290,21 @@ impl<T> StoreContextMut<'_, T> {
// On exit from this function, though, we reset the polling
// context back to what it was to signify that `Store` no longer
// has access to this pointer.
let guard = self
.fiber()
.stack()
.guard_range()
.unwrap_or(core::ptr::null_mut()..core::ptr::null_mut());
unsafe {
let _reset = Reset(self.current_poll_cx, *self.current_poll_cx);
*self.current_poll_cx =
core::mem::transmute::<&mut Context<'_>, *mut Context<'static>>(cx);
*self.current_poll_cx = PollContext {
future_context: core::mem::transmute::<
&mut Context<'_>,
*mut Context<'static>,
>(cx),
guard_range_start: guard.start,
guard_range_end: guard.end,
};

// After that's set up we resume execution of the fiber, which
// may also start the fiber for the first time. This either
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,16 @@ impl StackPool {
let bottom_of_stack = self
.mapping
.as_ptr()
.add((index * self.stack_size) + self.page_size)
.add(index * self.stack_size)
.cast_mut();

commit_pages(bottom_of_stack, size_without_guard)?;

let stack =
wasmtime_fiber::FiberStack::from_raw_parts(bottom_of_stack, size_without_guard)?;
let stack = wasmtime_fiber::FiberStack::from_raw_parts(
bottom_of_stack,
self.page_size,
size_without_guard,
)?;
Ok(stack)
}
}
Expand Down
49 changes: 48 additions & 1 deletion crates/wasmtime/src/runtime/vm/sys/unix/machports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ use mach2::port::*;
use mach2::thread_act::*;
use mach2::thread_status::*;
use mach2::traps::*;
use std::mem;
use std::io;
use std::mem::{self, MaybeUninit};
use std::ptr::addr_of_mut;
use std::thread;
use wasmtime_environ::Trap;
Expand All @@ -61,6 +62,8 @@ pub struct TrapHandler {
thread: Option<thread::JoinHandle<()>>,
}

static mut PREV_SIGBUS: MaybeUninit<libc::sigaction> = MaybeUninit::uninit();

impl TrapHandler {
pub unsafe fn new() -> TrapHandler {
// Mach ports do not currently work across forks, so configure Wasmtime to
Expand All @@ -86,6 +89,21 @@ impl TrapHandler {
// we're not very interested in so it's detached here.
let thread = thread::spawn(|| handler_thread());

// Setup a SIGBUS handler which is used for printing that the stack was
// overflowed when a host overflows its fiber stack.
unsafe {
let mut handler: libc::sigaction = mem::zeroed();
handler.sa_flags = libc::SA_SIGINFO | libc::SA_ONSTACK;
handler.sa_sigaction = sigbus_handler as usize;
libc::sigemptyset(&mut handler.sa_mask);
if libc::sigaction(libc::SIGBUS, &handler, PREV_SIGBUS.as_mut_ptr()) != 0 {
panic!(
"unable to install signal handler: {}",
io::Error::last_os_error(),
);
}
}

TrapHandler {
thread: Some(thread),
}
Expand All @@ -102,6 +120,35 @@ impl Drop for TrapHandler {
}
}

unsafe extern "C" fn sigbus_handler(
signum: libc::c_int,
siginfo: *mut libc::siginfo_t,
context: *mut libc::c_void,
) {
// If this is a faulting address within the async guard range listed within
// our tls storage then print a helpful message about it and abort.
// Otherwise forward to the previous SIGBUS handler, if any.
tls::with(|info| {
let info = match info {
Some(info) => info,
None => return,
};
let faulting_addr = (*siginfo).si_addr() as usize;
let start = info.async_guard_range.start;
let end = info.async_guard_range.end;
if start as usize <= faulting_addr && faulting_addr < end as usize {
super::signals::abort_stack_overflow();
}
});

super::signals::delegate_signal_to_previous_handler(
PREV_SIGBUS.as_ptr(),
signum,
siginfo,
context,
)
}

// Note that this is copied from Gecko at
//
// https://searchfox.org/mozilla-central/rev/ed93119be4818da1509bbcb7b28e245853eeedd5/js/src/wasm/WasmSignalHandlers.cpp#583-601
Expand Down
Loading

0 comments on commit 110e70f

Please sign in to comment.