bytecodealliance / lucet

Lucet, the Sandboxing WebAssembly Compiler.
Apache License 2.0
4.06k stars 165 forks source link

☢️ Remove UB from `extern "C"` ABI on annotated hostcalls #584

Closed acfoltzer closed 4 years ago

acfoltzer commented 4 years ago

This was a programming error introduced in the transition between the lucet_hostcalls! macro and the #[lucet_hostcall] attribute.

The expansion of the code in the attribute called the annotated function from within a catch_unwind without modifying its ABI. Since it is UB to unwind from a non-Rust ABI function, release mode optimizations would elide the code which would otherwise handle an unwind from the function. This caused the process to abort if the function ever did unwind, since the system unwinding runtime would reach the end of the guest stack without finding any frames willing to handle the exception.

I believe this behavior was very difficult, if not impossible, to observe in practice with the current state of the Lucet codebase. The only unwinding that these functions could directly induce are the assertions in vmctx::instance_from_vmctx that should never be violated except in the case of a Lucet programming error, or a dynamic linking problem.

However, in #583 we are adding an option to instances that would cause lucet_vmctx_grow_memory to potentially terminate the instance via the lucet_hostcall_terminate!() mechanism that is implemented via unwinding. We discovered this problem during testing on that branch, and so we need to land this fix before it can proceed.

This patch contains a fix to the immediate problem by removing the erroneous extern "C" specifiers. It also prevents the problem recurring in the future by generating a compile-time error from #[lucet_hostcall] if there is an ABI specifier present on the function it annotates.