#1605 WASM crash on 32-bits with 64-bit casts

Open
opened 9 months ago by wolfbeast · 3 comments
wolfbeast commented 9 months ago (Migrated from github.com)

As part of investigating hCaptcha crashes reported on the forum in Basilisk, it’s come to light that implicit casts to 64-bit in compiled WASM code can cause a deliberate breakpoint crash (in lieu of having a potentially vulnerable type confusion situation).
A ticket has been opened for this with hCaptcha and they will be patching the issue on their end, but we should also look into a graceful failure exit from this situation in the WASM platform code so it doesn’t crash our users on buggy WASM content but simply returns a failure.

Relevant code:
0955b34e67/js/src/wasm/WasmBaselineCompile.cpp (L1523-L1547)

Call stack:

>	xul.dll!js::wasm::BaseCompiler::popI64(js::wasm::BaseCompiler::Stk & v, js::wasm::BaseCompiler::RegI64 r) Line 1545	C++
 	xul.dll!js::wasm::BaseCompiler::popI64() Line 1556	C++
 	xul.dll!js::wasm::BaseCompiler::pop2xI64(js::wasm::BaseCompiler::RegI64 * r0, js::wasm::BaseCompiler::RegI64 * r1) Line 3541	C++
 	xul.dll!js::wasm::BaseCompiler::emitSelect() Line 6225	C++
 	xul.dll!js::wasm::BaseCompiler::emitBody() Line 6648	C++
 	xul.dll!js::wasm::BaseCompiler::emitFunction() Line 7216	C++
 	xul.dll!js::wasm::BaselineCompileFunction(js::wasm::IonCompileTask * task) Line 7470	C++
 	xul.dll!js::wasm::CompileFunction(js::wasm::IonCompileTask * task) Line 3805	C++
 	xul.dll!js::HelperThread::handleWasmWorkload(js::AutoLockHelperThreadState & locked) Line 1425	C++
 	xul.dll!js::HelperThread::threadLoop() Line 1877	C++
 	xul.dll!js::detail::ThreadTrampoline<void (__cdecl&)(void *),js::HelperThread *>::Start(void * aPack) Line 228	C++
As part of investigating hCaptcha crashes [reported on the forum](https://forum.palemoon.org/viewtopic.php?f=61&t=24663) in Basilisk, it's come to light that implicit casts to 64-bit in compiled WASM code can cause a deliberate breakpoint crash (in lieu of having a potentially vulnerable type confusion situation). A ticket has been opened for this with hCaptcha and they will be patching the issue on their end, but we should also look into a graceful failure exit from this situation in the WASM platform code so it doesn't crash our users on buggy WASM content but simply returns a failure. Relevant code: https://github.com/MoonchildProductions/UXP/blob/0955b34e67d3eceb0c5bed9eae26f2456dfb005f/js/src/wasm/WasmBaselineCompile.cpp#L1523-L1547 Call stack: ``` > xul.dll!js::wasm::BaseCompiler::popI64(js::wasm::BaseCompiler::Stk & v, js::wasm::BaseCompiler::RegI64 r) Line 1545 C++ xul.dll!js::wasm::BaseCompiler::popI64() Line 1556 C++ xul.dll!js::wasm::BaseCompiler::pop2xI64(js::wasm::BaseCompiler::RegI64 * r0, js::wasm::BaseCompiler::RegI64 * r1) Line 3541 C++ xul.dll!js::wasm::BaseCompiler::emitSelect() Line 6225 C++ xul.dll!js::wasm::BaseCompiler::emitBody() Line 6648 C++ xul.dll!js::wasm::BaseCompiler::emitFunction() Line 7216 C++ xul.dll!js::wasm::BaselineCompileFunction(js::wasm::IonCompileTask * task) Line 7470 C++ xul.dll!js::wasm::CompileFunction(js::wasm::IonCompileTask * task) Line 3805 C++ xul.dll!js::HelperThread::handleWasmWorkload(js::AutoLockHelperThreadState & locked) Line 1425 C++ xul.dll!js::HelperThread::threadLoop() Line 1877 C++ xul.dll!js::detail::ThreadTrampoline<void (__cdecl&)(void *),js::HelperThread *>::Start(void * aPack) Line 228 C++ ```
wolfbeast commented 9 months ago (Migrated from github.com)
Owner

I’ve looked into possible options but this is pretty deep into the baseline compiler which is complex assembler calls that don’t all have easy ways to check types passed in.

Short term workaround is to disable the WASM baselinejit compiler on 32-bit because it simply looks like our x86 support for jit compilation is lacking here when it encounters 64-bit native code. Thankfully that’s easy because we have a separate pref for this already ready. Of course this means we’ll lose performance on x86 but so be it.

I've looked into possible options but this is pretty deep into the baseline compiler which is complex assembler calls that don't all have easy ways to check types passed in. Short term workaround is to disable the WASM baselinejit compiler on 32-bit because it simply looks like our x86 support for jit compilation is lacking here when it encounters 64-bit native code. Thankfully that's easy because we have a separate pref for this already ready. Of course this means we'll lose performance on x86 but so be it.
wolfbeast commented 9 months ago (Migrated from github.com)
Owner

Considering the very favourable response from the hCaptcha people who have fixed and tested a workaround for the problematic 64-bit integer cast (will be deployed over the day), I’m putting this on hold for now as the only web compat issue that we know of has been taken care of and I’d prefer not to unnecessarily hamper performance.

Considering the very favourable response from the hCaptcha people who have fixed and tested a workaround for the problematic 64-bit integer cast (will be deployed over the day), I'm putting this on hold for now as the only web compat issue that we know of has been taken care of and I'd prefer not to unnecessarily hamper performance.
wolfbeast commented 9 months ago (Migrated from github.com)
Owner

This will be on hold perpetually until such time as there’s a reduced repro available that we can use to examine the crash.
hCaptcha unfortunately doesn’t have such available for us.

This will be on hold perpetually until such time as there's a reduced repro available that we can use to examine the crash. hCaptcha unfortunately doesn't have such available for us.
Sign in to join this conversation.
No Milestone
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.