1221385 - Crash [@ js::jit::ExecutableAllocator::releasePoolPages] or Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h or Assertion failure: jrt_->mutatingBackedgeList_, at jit/JitCompartment.h

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

// jsfunfuzz-generated
setJitCompilerOption('baseline.enable', 1);
// Adapted from randomly chosen test: js/src/jit-test/tests/basic/testBug756919.js
gcparam("maxBytes", gcparam("gcBytes") + 1);
function f() {}
f();

asserts js debug shell on m-c changeset 451a18579143 with --fuzzing-safe --no-threads --no-ion --no-baseline at Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h

Configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --disable-threadsafe --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r 451a18579143

autoBisect is running.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x731767, 0x00000001002598a7 js-dbg-64-dm-darwin-451a18579143`js::jit::ExecutablePool::release(this=<unavailable>, willDestroy=<unavailable>) + 183 at ExecutableAllocator.h:105, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001002598a7 js-dbg-64-dm-darwin-451a18579143`js::jit::ExecutablePool::release(this=<unavailable>, willDestroy=<unavailable>) + 183 at ExecutableAllocator.h:105
    frame #1: 0x0000000100259778 js-dbg-64-dm-darwin-451a18579143`js::jit::ExecutableAllocator::~ExecutableAllocator(this=0x0000000102d90c40) + 56 at ExecutableAllocator.h:195
    frame #2: 0x00000001004cb554 js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(JSContext*) [inlined] void js_delete<js::jit::JitRuntime>(p=0x0000000102d90c40) + 13 at Utility.h:370
    frame #3: 0x00000001004cb547 js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(this=0x0000000102d78000, cx=0x0000000102d69400) + 199 at jscompartment.cpp:174
    frame #4: 0x00000001008e9b50 js-dbg-64-dm-darwin-451a18579143`JS::Zone::createJitZone(JSContext*) [inlined] JSRuntime::getJitRuntime(this=<unavailable>, cx=0x0000000102d69400) + 18 at Runtime.h:940
(lldb)

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Opt stack

(lldb) bt 5
* thread #1: tid = 0x7346cb, 0x000000010011e3c1 js-64-dm-darwin-451a18579143`js::jit::ExecutableAllocator::releasePoolPages(this=0x0000000101f94230, pool=0x0000000104270240) + 33 at ExecutableAllocator.h:239, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
  * frame #0: 0x000000010011e3c1 js-64-dm-darwin-451a18579143`js::jit::ExecutableAllocator::releasePoolPages(this=0x0000000101f94230, pool=0x0000000104270240) + 33 at ExecutableAllocator.h:239
    frame #1: 0x000000010012107a js-64-dm-darwin-451a18579143`js::jit::JitCode::finalize(js::FreeOp*) [inlined] void js_delete<js::jit::ExecutablePool>(p=<unavailable>) + 8 at Utility.h:370
    frame #2: 0x0000000100121072 js-64-dm-darwin-451a18579143`js::jit::JitCode::finalize(js::FreeOp*) [inlined] js::jit::ExecutablePool::release(this=<unavailable>, willDestroy=false) + 6 at ExecutableAllocator.h:107
    frame #3: 0x000000010012106c js-64-dm-darwin-451a18579143`js::jit::JitCode::finalize(js::FreeOp*) [inlined] js::jit::ExecutablePool::release(this=<unavailable>, n=<unavailable>, kind=<unavailable>) + 43 at ExecutableAllocator.h:132
    frame #4: 0x0000000100121041 js-64-dm-darwin-451a18579143`js::jit::JitCode::finalize(this=0x000000010407afd0, fop=<unavailable>) + 161 at Ion.cpp:864
(lldb)

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Setting s-s because this seems to be accessing 0xe5e5e5e5e5e5e5e5 in opt builds.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

// jsfunfuzz-generated
setJitCompilerOption('baseline.enable', 1);
// Adapted from randomly chosen test: js/src/jit-test/tests/basic/testBug756919.js
gcparam("maxBytes", gcparam("gcBytes"));
function f() {}
f();

This variant asserts debug builds on m-c rev 451a18579143 when run with --fuzzing-safe --no-threads --no-ion --no-baseline at Assertion failure: jrt_->mutatingBackedgeList_, at jit/JitCompartment.h

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack for Assertion failure: jrt_->mutatingBackedgeList_

(lldb) bt 5
* thread #1: tid = 0x73a63f, 0x00000001004cb68c js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(JSContext*) [inlined] js::jit::JitRuntime::AutoMutateBackedges::~AutoMutateBackedges() + 52 at JitCompartment.h:215, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001004cb68c js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(JSContext*) [inlined] js::jit::JitRuntime::AutoMutateBackedges::~AutoMutateBackedges() + 52 at JitCompartment.h:215
    frame #1: 0x00000001004cb658 js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(JSContext*) [inlined] js::jit::JitRuntime::AutoMutateBackedges::~AutoMutateBackedges() at JitCompartment.h:214
    frame #2: 0x00000001004cb658 js-dbg-64-dm-darwin-451a18579143`JSRuntime::createJitRuntime(this=<unavailable>, cx=<unavailable>) + 472 at jscompartment.cpp:187
    frame #3: 0x00000001008e9b50 js-dbg-64-dm-darwin-451a18579143`JS::Zone::createJitZone(JSContext*) [inlined] JSRuntime::getJitRuntime(this=<unavailable>, cx=0x0000000102e69400) + 18 at Runtime.h:940
    frame #4: 0x00000001008e9b3e js-dbg-64-dm-darwin-451a18579143`JS::Zone::createJitZone(this=0x0000000102eab000, cx=0x0000000102e69400) + 30 at Zone.cpp:280
(lldb)

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/4d4b564c9d84
user:        Jan de Mooij
date:        Sat Feb 14 10:37:44 2015 +0100
summary:     Bug 1132564 part 2 - Move ExecutableAllocator into JitRuntime. r=luke

Jan, is bug 1132564 a likely regressor?

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Patch

This patch makes us crash when JitRuntime initialization fails. A bit unfortunate because JitRuntime::initialize allocates a bunch of trampolines etc, but I think the only reasonable/non-horrible alternative is moving the ExecutableAllocator to JSRuntime, which is also a bit unfortunate.

Comment 8

[Al Billings [:abillings - ex-MoCo]]

This patch has been sitting a month. Can we move this forward and get it in?

We've made this a sec-high. It will need sec-approval+ to go in.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8684204 [details] [diff] [review]
Patch

[Security approval request comment]
* How easily could an exploit be constructed based on the patch?
Not easily; you'd have to create a worker and trigger OOM at the right moment.

* Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No; there's a comment but it's still very hard to exploit.

* Which older supported branches are affected by this flaw?
All.

* Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Easy to backport.

* How likely is this patch to cause regressions; how much testing does it need?
Unlikely.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8684204 [details] [diff] [review]
Patch

sec-approval+ for trunk.
Can we get Aurora, Beta, and ESR38 (if possible) patches made and nominated?

Comment 11

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/350fbdbad784

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/350fbdbad784

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jan, could you please nominate patch for uplift to Beta, Aurora, esr38? Thanks!

Comment 14

[Jan de Mooij [:jandem]]

Comment on attachment 8684204 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1132564.
[User impact if declined]: Crashes, sec issues.
[Describe test coverage new/current, TreeHerder]: Fixes the reported failure.
[Risks and why]: Low risk. We now crash when creating the JitRuntime fails, but (without e10s) we only do that when we start the process or create a worker.
[String/UUID change made/needed]: None.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a6bb8cbba8cb

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d5aa8940675e

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This doesn't apply cleanly to esr38. Can we get a rebased patch if this needs to go there?

Comment 19

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/4444e94a99cb

Comment 20

[Matt Wobensmith [:mwobensmith][:matt:]]

Gary, can you verify fixed? Thanks.

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Waiting on JSBugMon's result.

Comment 22

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/350fbdbad784
user:        Jan de Mooij
date:        Wed Dec 30 13:27:09 2015 +0100
summary:     Bug 1221385 - Handle OOM during JitRuntime initialization a bit better. r=bhackett

Nevermind, I verified that this has been fixed on mozilla-central.