Closed Bug 1425387 Opened 2 years ago Closed 2 years ago

Assertion failure: iter.cur() == iter.maybeSkipAutomaticInstructions(), at js/src/jit/arm/MacroAssembler-arm.cpp:359 with wasm and Debugger

Categories

(Core :: JavaScript Engine, defect, P1, critical)

ARM
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: decoder, Assigned: sstangl)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 8062887ff0d9 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --arm-asm-nop-fill=1 --ion-offthread-compile=off):

var g = newGlobal();
var dbg = new Debugger(g);
g.eval(`
  o = new WebAssembly.parse(
    new WebAssembly.Module(wasmTextToBinary('(module (func) (export "" 0))'))
  );
`);


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0853a5ae in js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator> (imm32=..., dest=..., c=js::jit::Assembler::Always, rs=js::jit::Assembler::L_MOVWT, iter=...) at js/src/jit/arm/MacroAssembler-arm.cpp:359
#1  0x08915046 in js::jit::MacroAssembler::patchAdd32ToPtr (imm=..., offset=..., this=0xf6dfe3b0) at js/src/jit/arm/MacroAssembler-arm-inl.h:376
#2  js::wasm::BaseStackFrame::patchAllocStack (this=0xf6dfe27c) at js/src/wasm/WasmBaselineCompile.cpp:1191
#3  js::wasm::BaseCompiler::endFunction (this=0xf6dfdec0) at js/src/wasm/WasmBaselineCompile.cpp:3009
#4  js::wasm::BaseCompiler::emitFunction (this=0xf6dfdec0) at js/src/wasm/WasmBaselineCompile.cpp:9212
#5  js::wasm::BaselineCompileFunctions (env=..., lifo=..., inputs=..., code=0xf6e9d1b8, error=0xf6dfeaf4) at js/src/wasm/WasmBaselineCompile.cpp:9361
#6  0x08997533 in ExecuteCompileTask (task=0xf6e9d058, error=error@entry=0xf6dfeaf4) at js/src/wasm/WasmGenerator.cpp:626
#7  0x08997e40 in js::wasm::ExecuteCompileTaskFromHelperThread (task=0xf6e9d058) at js/src/wasm/WasmGenerator.cpp:644
#8  0x087a5681 in js::HelperThread::handleWasmWorkload (this=0xf6e3d400, locked=..., mode=js::wasm::CompileMode::Tier1) at js/src/vm/HelperThreads.cpp:1831
#9  0x087b3ff3 in js::HelperThread::threadLoop (this=0xf6e3d400) at js/src/vm/HelperThreads.cpp:2299
#10 0x087b4298 in js::HelperThread::ThreadMain (arg=0xf6e3d400) at js/src/vm/HelperThreads.cpp:1816
#11 0x087b482b in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0u> (this=0xf6e0e058) at js/src/threading/Thread.h:242
#12 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0xf6e0e058) at js/src/threading/Thread.h:235
#13 0xf7fb226a in start_thread (arg=0xf6dffb40) at pthread_create.c:333
#14 0xf7cd33fe in clone () from /lib32/libc.so.6
eax	0x0	0
ebx	0xf6dfdd80	-153100928
ecx	0xf7d9f864	-136710044
edx	0x0	0
esi	0xf6ea0078	-152436616
edi	0x8d98ff4	148475892
ebp	0xf6dfdd68	4141866344
esp	0xf6dfdd40	4141866304
eip	0x853a5ae <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+366>
=> 0x853a5ae <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+366>:	movl   $0x0,0x0
   0x853a5b8 <js::jit::MacroAssemblerARM::ma_mov_patch<js::jit::BufferInstructionIterator>(js::jit::Imm32, js::jit::Register, js::jit::Assembler::Condition, js::jit::Assembler::RelocStyle, js::jit::BufferInstructionIterator)+376>:	ud2
This is an assertion added during a sec-audit protected bug, so it should be protected too.
Group: core-security, javascript-core-security
Flags: needinfo?(sstangl)
Group: core-security
Priority: -- → P1
Assignee: nobody → sstangl
This isn't a security problem. The issue is that I added an assertion that ma_mov_patch() is only called on "intentionally-inserted" instructions. When the flag --arm-asm-nop-fill=1 is passed, NOPs are automatically inserted in places where they could never be in normal execution, and that causes the asserted condition to no longer be true.

The assertion is valuable to have, but I suppose it's not true in all conditions due to this argument.
Flags: needinfo?(sstangl)
This patch will skip automatically-inserted NOPs when patching instructions.

These NOPs can't be present there in normal execution. They are inserted only by --arm-asm-nop-fill=1.
Attachment #8937835 - Flags: review?(bbouvier)
Attachment #8937835 - Flags: review?(bbouvier) → review+
Keywords: checkin-needed
Keywords: sec-other
Should we close this now?
Flags: needinfo?(sstangl)
Yes, it was landed.
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(sstangl)
Resolution: --- → FIXED
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.