1644554 - Permafailing tier 2 js\src\jit-test\tests\wasm\simd\ad-hack.js | Unknown (code 3221225477, args "") [0.1 s]

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P3)

Description

[Treeherder Bug Filer]

Filed by: ncsoregi [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=305225872&repo=mozilla-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/CtqVRmAzSluLJywSG7pOBg/runs/0/artifacts/public/logs/live_backing.log

---

[task 2020-06-05T15:54:27.823Z] TEST-PASS | js\src\jit-test\tests\wasm\regress\no-directives\debugger-no-script.js | Success (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [0.1 s]
[task 2020-06-05T15:54:27.880Z] TEST-PASS | js\src\jit-test\tests\wasm\regress\no-directives\debugger-no-script.js | Success (code 3, args "--baseline-eager") [0.1 s]
[task 2020-06-05T15:54:27.935Z] TEST-PASS | js\src\jit-test\tests\wasm\regress\no-directives\debugger-no-script.js | Success (code 3, args "--no-blinterp --no-baseline --no-ion --more-compartments") [0.1 s]
[task 2020-06-05T15:54:27.993Z] TEST-PASS | js\src\jit-test\tests\wasm\regress\no-directives\debugger-no-script.js | Success (code 3, args "--blinterp-eager") [0.1 s]
[task 2020-06-05T15:54:28.142Z] Exit code: 3221225477
[task 2020-06-05T15:54:28.142Z] FAIL - wasm\simd\ad-hack.js
[task 2020-06-05T15:54:28.142Z] TEST-UNEXPECTED-FAIL | js\src\jit-test\tests\wasm\simd\ad-hack.js | Unknown (code 3221225477, args "") [0.1 s]
[task 2020-06-05T15:54:28.143Z] INFO exit-status : 3221225477
[task 2020-06-05T15:54:28.143Z] INFO timed-out : False
[task 2020-06-05T15:54:28.522Z] Exit code: 3221225477
[task 2020-06-05T15:54:28.523Z] FAIL - wasm\simd\ad-hack.js
[task 2020-06-05T15:54:28.523Z] TEST-UNEXPECTED-FAIL | js\src\jit-test\tests\wasm\simd\ad-hack.js | Unknown (code 3221225477, args "--ion-eager --ion-offthread-compile=off --more-compartments") [0.4 s]
[task 2020-06-05T15:54:28.523Z] INFO exit-status : 3221225477
[task 2020-06-05T15:54:28.523Z] INFO timed-out : False
[task 2020-06-05T15:54:28.586Z] TEST-PASS | js\src\jit-test\tests\wasm\simd\ad-hack.js | Success (code 59, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [0.1 s]
[task 2020-06-05T15:54:28.737Z] Exit code: 3221225477
[task 2020-06-05T15:54:28.737Z] FAIL - wasm\simd\ad-hack.js

Comment 1

[Natalia Csoregi [:nataliaCs]]

:lth can you please take a look at this?
Thank you.

Comment 2

[Lars T Hansen [:lth]]

Sure, will look into it first thing in the morning.

Comment 3

[Lars T Hansen [:lth]]

3221225477 is 0xc0000005 which is access violation. The SIMD test cases pass only on runs that disable SSE with either --no-wasm-sse or --no-sse3.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1637332

Comment 5

[Lars T Hansen [:lth]]

Looking at this now but it may take some time because, Windows. It should be a Nightly-only problem.

Comment 6

[Lars T Hansen [:lth]]

OK, I can repro locally, happy times...

Comment 7

[Lars T Hansen [:lth]]

This basically crashes and/or hangs and/or overrecurses in the call to run()

var badImporter = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
  (module
    (import "" "worker" (func $worker (param v128) (result v128)))
    (func (export "run")
      (drop (call $worker (v128.const i32x4 0 1 2 3)))))`)),
             {"":{worker: function(a) { print('here'); return a; }}});

print("calling")
badImporter.exports.run();
print("returned")

Comment 8

[Lars T Hansen [:lth]]

Disassembly of the run function:

00000000  56                        push %rsi
00000001  55                        push %rbp
00000002  8b ec                     mov %esp, %ebp
00000004  66 0f 1f 44 00 00         nopw %ax, (%rax,%rax,1)
0000000A  66 0f 1f 44 00 00         nopw %ax, (%rax,%rax,1)
00000010  83 f9 01                  cmp $0x01, %ecx
00000013  0f 84 0b 00 00 00         jz 0x0000000000000024
00000019  0f 0b                     ud2
0000001B  0f 1f 44 00 00            nopl %eax, (%rax,%rax,1)
00000020  56                        push %rsi
00000021  55                        push %rbp
00000022  8b ec                     mov %esp, %ebp
00000024  83 ec 14                  sub $0x14, %esp
00000027  39 66 18                  cmpl %esp, 0x18(%rsi)
0000002A  0f 82 02 00 00 00         jb 0x0000000000000032
00000030  0f 0b                     ud2
00000032  66 0f 6f 05 f0 00 a3 24   movdqax 0x0000000024A3012A, %xmm0
0000003A  f3 0f 7f 04 24            movdqux %xmm0, (%rsp)
0000003F  f7 c4 0f 00 00 00         test $0x0F, %esp
00000045  0f 84 01 00 00 00         jz 0x000000000000004C
0000004B  cc                        int3
0000004C  8b 46 30                  movl 0x30(%rsi), %eax
0000004F  8b 5e 38                  movl 0x38(%rsi), %ebx
00000052  8b 4e 10                  movl 0x10(%rsi), %ecx
00000055  89 59 4c                  movl %ebx, 0x4C(%rcx)
00000058  8b 76 34                  movl 0x34(%rsi), %esi
0000005B  ff d0                     call %rax
0000005D  8b 74 24 18               movl 0x18(%rsp), %esi
00000061  8b 4e 10                  movl 0x10(%rsi), %ecx
00000064  8b 56 0c                  movl 0x0C(%rsi), %edx
00000067  89 51 4c                  movl %edx, 0x4C(%rcx)
0000006A  83 c4 14                  add $0x14, %esp
0000006D  5d                        pop %rbp
0000006E  5e                        pop %rsi
0000006F  c3                        ret

Comment 9

[Lars T Hansen [:lth]]

Crashes in WasmFrameIter::popFrame with this stack (from Firefox):

>	xul.dll!js::wasm::WasmFrameIter::popFrame() Line 199	C++
 	xul.dll!js::wasm::WasmFrameIter::WasmFrameIter(js::jit::JitActivation * activation, js::wasm::Frame * fp) Line 79	C++
 	[Inline Frame] xul.dll!mozilla::MaybeOneOf<js::jit::JSJitFrameIter,js::wasm::WasmFrameIter>::construct(js::jit::JitActivation * & aArgs) Line 105	C++
 	[Inline Frame] xul.dll!js::JitFrameIter::JitFrameIter(js::jit::JitActivation * act, bool mustUnwindActivation) Line 92	C++
 	xul.dll!js::FrameIter::settleOnActivation() Line 301	C++
 	[Inline Frame] xul.dll!js::FrameIter::FrameIter(JSContext * cx, js::FrameIter::DebuggerEvalOption debuggerEvalOption, JSPrincipals * principals) Line 363	C++
 	xul.dll!js::NonBuiltinFrameIter::NonBuiltinFrameIter(JSContext * cx, JSPrincipals * principals) Line 480	C++
 	xul.dll!PopulateReportBlame(JSContext * cx, JSErrorReport * report) Line 182	C++
 	xul.dll!js::ReportErrorNumberVA(JSContext * cx, js::IsWarning isWarning, const JSErrorFormatString *(*)(void *, const unsigned int) callback, void * userRef, const unsigned int errorNumber, js::ErrorArgumentsType argumentsType, char * ap) Line 472	C++
 	[Inline Frame] xul.dll!JS_ReportErrorNumberUTF8VA(JSContext * cx, const JSErrorFormatString *(*)(void *, const unsigned int) errorCallback, void * userRef, const unsigned int errorNumber, char * ap) Line 4822	C++
 	xul.dll!JS_ReportErrorNumberUTF8(JSContext * cx, const JSErrorFormatString *(*)(void *, const unsigned int) errorCallback, void * userRef, const unsigned int errorNumber, ...) Line 4812	C++
 	xul.dll!js::wasm::Instance::callImport_v128(js::wasm::Instance * instance, int funcImportIndex, int argc, unsigned __int64 * argv) Line 602	C++
 	[External Code]	
 	[Frames below may be incorrect and/or missing]	Unknown

because in this line (line 199 in today's code) fp_ is 0x02, ie uninitialized.

 MOZ_ASSERT(code_ == &fp_->tls->instance->code());

This suggests a corrupted stack or stack layout.

Comment 10

[Lars T Hansen [:lth]]

The problem here is that the InterpExit stub clobbers the incoming stack args and corrupts the instance pointer. This is platform-independent, we were possibly just lucky to catch it on Win32 because the data are packed more tightly on the stack.

It works like this: We make a call from wasm, this is routed to the InterpExit stub. This stub executes the usual prologue setting up the exitFP, then it copies the stack arguments from the incoming frame to the outgoing frame. Stack arguments are copied using the StackCopy routine, which just does a bit-by-bit copy. So far so good. However, the interp stub has only set aside 8 bytes for each argument as per the Jit ABI, and StackCopy copies 16 bytes for the V128. When the V128 is the uppermost outgoing arg in memory and exactly abuts the incoming data (return address and instance pointer) then the copy overwrites those data. Since the instance pointer is copied after the stack arguments, garbage is moved over.

The bug came into existence when I factored the StackCopy routine without thinking about this problem. The non-copying case (where we box as values) handles the situation correctly, just storing a double(0) instead. After all, passing a v128 to JS is illegal and an exception will be thrown by the callImport code.

The fix is really in FillArgumentArrayForExit. When setting up arguments for the Jit ABI it must not copy v128 data and should not call StackCopy.

On the subject of the callImport code: callImport_v128() throws an error immediately but I think it should not do this. callImport() itself contains the proper check and it would be better for callImport_v128() to defer to it to throw the appropriate exception. I'll make that a separate non-security bug.

Comment 11

[Lars T Hansen [:lth]]

Attachment: Bug 1644554 - Block V128 in the Jit ABI

Comment 12

[Lars T Hansen [:lth]]

Attachment: Bug 1644554 - Block V128 in the Jit ABI

Comment 13

[Lars T Hansen [:lth]]

This does not need a test case as the test case is already in the test suite.

Comment 14

[Lars T Hansen [:lth]]

Since this is nightly-only i'll just flip the appropriate bits and land it...

Comment 15

[Lars T Hansen [:lth]]

Seems difficult to construct a sensible attack here, one could perhaps think about faking the instance pointer with a carefully constructed value and maybe find some code that writes to a field the instance, but that code would have to be found in the call chain that populates the exception report, so this is not terribly likely.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/af354efba63a