1488386 - Hit MOZ_CRASH(mprotect(PROT_READ) failed) at js/src/gc/Memory.cpp:913 via [@ DecompileExpressionFromStack]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision b75561ff5ffe (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

See attachment.


Backtrace:

received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe8bff700 (LWP 550)]
0x0000000000fc464d in js::gc::MakePagesReadOnly (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:913
#0  0x0000000000fc464d in js::gc::MakePagesReadOnly (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:913
#1  0x0000000000d3214e in js::detail::BumpChunk::setRWUntil (this=<optimized out>, loc=loc@entry=js::detail::BumpChunk::Loc::Reserved) at js/src/ds/LifoAlloc.cpp:108
#2  0x0000000000d3426e in js::detail::BumpChunk::setRWUntil (loc=js::detail::BumpChunk::Loc::Reserved, this=<optimized out>) at js/src/ds/LifoAlloc.cpp:67
#3  js::LifoAlloc::<lambda()>::operator() (__closure=<synthetic pointer>) at js/src/ds/LifoAlloc.cpp:228
#4  js::LifoAlloc::getOrCreateChunk (this=this@entry=0x7fffe90a6988, n=n@entry=17080) at js/src/ds/LifoAlloc.cpp:265
#5  0x0000000000561feb in js::LifoAlloc::allocImpl (n=17080, this=0x7fffe90a6988) at js/src/ds/LifoAlloc.h:609
#6  js::LifoAlloc::alloc (this=0x7fffe90a6988, n=17080) at js/src/ds/LifoAlloc.h:679
#7  0x0000000000b13198 in js::LifoAlloc::newArrayUninitialized<OffsetAndDefIndex> (count=2135, this=<optimized out>) at js/src/ds/LifoAlloc.h:779
#8  js::LifoAlloc::newArray<OffsetAndDefIndex> (count=2135, this=<optimized out>) at js/src/ds/LifoAlloc.h:769
#9  (anonymous namespace)::BytecodeParser::Bytecode::captureOffsetStack (this=<optimized out>, this=<optimized out>, depth=2135, stack=0x7fffe3d33508, alloc=...) at js/src/vm/BytecodeUtil.cpp:377
#10 (anonymous namespace)::BytecodeParser::recordBytecode (this=0x7fffe8bfc120, offset=<optimized out>, offsetStack=0x7fffe3d33508, stackDepth=2135) at js/src/vm/BytecodeUtil.cpp:768
#11 0x0000000000b13a7d in (anonymous namespace)::BytecodeParser::parse (this=this@entry=0x7fffe8bfc120) at js/src/vm/BytecodeUtil.cpp:967
#12 0x0000000000b18cca in DecompileExpressionFromStack (cx=<optimized out>, cx@entry=0x7fffe90a6000, spindex=spindex@entry=-2192, skipStackHits=skipStackHits@entry=0, v=..., v@entry=..., res=res@entry=0x7fffe8bfc698) at js/src/vm/BytecodeUtil.cpp:2285
#13 0x0000000000b190b7 in js::DecompileValueGenerator (cx=cx@entry=0x7fffe90a6000, spindex=-2192, v=..., fallbackArg=..., skipStackHits=skipStackHits@entry=0) at js/src/vm/BytecodeUtil.cpp:2311
#14 0x0000000000c05506 in js::ReportValueErrorFlags (cx=0x7fffe90a6000, flags=flags@entry=0, errorNumber=9, spindex=<optimized out>, v=..., fallback=..., fallback@entry=..., arg1=0x0, arg2=0x0) at js/src/vm/JSContext.cpp:1010
#15 0x00000000005c5f13 in js::ReportValueError (arg2=0x0, arg1=0x0, fallback=..., v=..., spindex=<optimized out>, errorNumber=<optimized out>, cx=<optimized out>) at js/src/vm/JSContext.h:1132
#16 js::ReportIsNotFunction (cx=<optimized out>, v=..., numToSkip=<optimized out>, construct=<optimized out>) at js/src/vm/Interpreter.cpp:327
#17 0x00000000005df49e in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7fffe90a6000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:509
#18 0x00000000005dfa8d in InternalCall (cx=0x7fffe90a6000, args=...) at js/src/vm/Interpreter.cpp:588
#19 0x00000000005d232f in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:594
#20 Interpret (cx=0x7fffe90a6000, state=...) at js/src/vm/Interpreter.cpp:3266
#21 0x00000000005def86 in js::RunScript (cx=0x7fffe90a6000, state=...) at js/src/vm/Interpreter.cpp:429
#22 0x00000000005e230d in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7fffe8bfde18) at js/src/vm/Interpreter.cpp:777
#23 0x00000000005e2709 in js::Execute (cx=<optimized out>, cx@entry=0x7fffe90a6000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7fffe8bfde18) at js/src/vm/Interpreter.cpp:810
#24 0x0000000000a97eb3 in ExecuteScript (cx=cx@entry=0x7fffe90a6000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7fffe8bfde18) at js/src/jsapi.cpp:4669
#25 0x0000000000aab683 in ExecuteScript (cx=0x7fffe90a6000, envChain=..., scriptArg=..., rval=0x7fffe8bfde18) at js/src/jsapi.cpp:4688
#26 0x0000000000aab72a in JS_ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., scriptArg@entry=..., rval=..., rval@entry=...) at js/src/jsapi.cpp:4709
#27 0x000000000046c237 in Evaluate (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:2053
#28 0x00000000005ead01 in CallJSNative (cx=0x7fffe90a6000, native=0x46b6e0 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:449
#29 0x00000000005df467 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7fffe90a6000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:537
#30 0x00000000005dfa8d in InternalCall (cx=0x7fffe90a6000, args=...) at js/src/vm/Interpreter.cpp:588
#31 0x00000000005dfbda in js::CallFromStack (cx=<optimized out>, args=...) at js/src/vm/Interpreter.cpp:594
#32 0x0000000000750c33 in js::jit::DoCallFallback (cx=<optimized out>, frame=0x7fffe8bfde68, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffe8bfde18, res=...) at js/src/jit/BaselineIC.cpp:3606
#33 0x00000121ac7c8e9c in ?? ()
[...]
#59 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffe90a6988	140737103161736
rcx	0x7ffff6c3a0c9	140737333403849
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffe8bfbf30	140737098268464
rsp	0x7fffe8bfbf30	140737098268464
r8	0x7ffff6eeb770	140737336227696
r9	0x7fffe8bff700	140737098282752
r10	0x0	0
r11	0x206	518
r12	0x8000	32768
r13	0x7fffe8bfbf50	140737098268496
r14	0x7fffe90a6988	140737103161736
r15	0x7fffc57c62d8	140736506651352
rip	0xfc464d <js::gc::MakePagesReadOnly(void*, unsigned long)+189>
=> 0xfc464d <js::gc::MakePagesReadOnly(void*, unsigned long)+189>:	movl   $0x0,0x0
   0xfc4658 <js::gc::MakePagesReadOnly(void*, unsigned long)+200>:	ud2


The attached testcase is packaged only, not reduced and still contains references to "/srv/repos/mozilla-central/js/src/jit-test/lib/" pointing to the jit-test lib directory. You might have to adjust that path and see if you can still reproduce the issue. The testcase is particularly fragile and I'm trying to reduce it, but it already failed once and I ended up with a non-reproducing testcase. I suggest we start working on this using the packaged version as long as it still reproduces.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Fuzzing Team]

JSBugMon: Cannot process bug: Error: Failed to isolate test from comment

Comment 3

[Fuzzing Team]

JSBugMon: Bisection requested, failed due to error: Error: Failed to isolate test from comment

Comment 4

[Ted Campbell [:tcampbell]]

Is this not just due to /proc/sys/vm/max_map_count limit? Bug 1437600 causes a lot of mappings to split and we may exceed this limit.

Comment 5

[Nicolas B. Pierron [:nbp]]

Attachment: Only keep memory protection for IonMonkey lifo alloc chunks going off-thread.

This patch removes the logic to distinguish subset of the pages and to flag
subparts of pages as non-accessible. It only keep the code which is used to mark
LifoAlloc chunks as read-only / read-write used when IonMonkey's compilation is
moved from the main thread to the helper threads.

Comment 6

[Nicolas B. Pierron [:nbp]]

(In reply to Ted Campbell [:tcampbell] from comment #4)
> Is this not just due to /proc/sys/vm/max_map_count limit? Bug 1437600 causes
> a lot of mappings to split and we may exceed this limit.

Yes, it is. While I was not able to reproduce this particular issue locally, the way MacOS/X crashes (within mprotect call) makes me think this approach is only good for local testing.

Comment 7

[Ted Campbell [:tcampbell]]

Comment on attachment 9006550 [details] [diff] [review]
Only keep memory protection for IonMonkey lifo alloc chunks going off-thread.

Review of attachment 9006550 [details] [diff] [review]:
-----------------------------------------------------------------

r=me to remove diagnostics that hit OS limits.

::: js/src/ds/LifoAlloc.cpp
@@ +61,5 @@
>      return (uint8_t*) uptr;
>  }
>  
>  void
> +BumpChunk::setReadOnly() const

Remove const and the const_cast below. Mappings change only when bump pointer does so I think we want compiler to complain if someone calls setReadOnly on a const object.

Comment 8

[Nicolas B. Pierron [:nbp]]

This issue is no a security issue. This is a debug-only instrumentation which stress the system reserved space for the page table mappings by causing a lot of fragmentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d552195f097f
Only keep memory protection for IonMonkey lifo alloc chunks going off-thread. r=tcampbell

Comment 10

[Dorel Luca [:dluca]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Sounds like this can ride the trains, but feel free to nominate for Beta uplift if you feel strongly otherwise.