bytecodealliance / lucet

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

Ensure a requested amount of stack is avaiable before hostcalls #567

Closed iximeow closed 4 years ago

iximeow commented 4 years ago

This changes lucetc's generation of hostcalls to, instead of directly calling hostcalls, call a synthetic trampoline to do some safety checks. That trampoline then will call the original hostcall, if it passes the required checks.

In our case, the only check is that the guest's stack use is below some threshold that the instance has set, ensuring that an amount of space is available for hostcalls. My arbitrary selection for a hostcall stack space guarantee is 16kb. If the guest fails the stack availability check, it terminates with a stack overflow - this mitigates the issues if a stack overflow in host code by making it an explicit stack overflow in the guest when reasonable. Hostcall stack overflows may still occur, and are still fatal, but we can now guarantee a certain amount of space for hostcall authors. This means hostcall implementors have the same kind of constraints as writing any other function - a stack overflow in any other host code would be fatal in a way beyond our control, just the same.

This adds a notable testing complication: since lucetc inserts non-wasm-derived details now, the mock modules we use in lucet-runtime tests are a little further from being real modules. Mock modules don't have trampolines between mocked "guest" code and hostcalls they would make, and I don't think we can insert trampolines if we wanted (without a worrying maintenance burden and risk of worse mock/real skew). My conclusion is that testing details related to hostcall stack overflows must be done using real wasm modules through lucetc, and tests not involving hostcall stack overflows (so, the ones that exist already), can continue being against mock modules.

acfoltzer commented 4 years ago

Also, if you're up for it, adding a page describing this to /docs/src would be greatly appreciated. There is hopefully material that exists already about the high-level design that you can adapt, but also a description of the low-level implementation via Cranelift IR would be :100:

iximeow commented 4 years ago

Docs and tests (and cleanup) have been added! The two additional tests I've added are pulled from stack and position the stack near but not quite overflowing, testing behavior with a default, and then custom (very shallow) hostcall reservation.

Docs were good because I had to go through a minor exercise justifying why the trampoline stack check's condition was correct; Cranelift's operand order for ifcmp_sp is backwards from what you might think, if you haven't read the documentation recently!

iximeow commented 4 years ago

Wrote only 9728 of 10240 bytes tar: home/circleci/project/target/release/deps/lucet_wasi-8e6fe6c44d08f0fe.d: Cannot write: No space left on device okay, CI