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

VERIFIED FIXED in Firefox 44, Firefox OS master

Status

()

--
critical
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks: 2 bugs, 5 keywords)

Trunk
mozilla46
x86_64
Mac OS X
assertion, crash, regression, sec-high, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 wontfix, firefox44+ fixed, firefox45+ fixed, firefox46+ fixed, firefox-esr3844+ fixed, firefox-esr45 fixed, b2g-v2.0 wontfix, b2g-v2.0M wontfix, b2g-v2.1 wontfix, b2g-v2.1S wontfix, b2g-v2.2 affected, b2g-v2.5 affected, b2g-v2.2r affected, b2g-master fixed)

Details

(Whiteboard: [jsbugmon:update][adv-main44+][adv-esr38.6+], crash signature)

Attachments

(4 attachments)

(Reporter)

Description

3 years ago
// 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.
(Reporter)

Comment 1

3 years ago
Created attachment 8682880 [details]
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)
(Reporter)

Updated

3 years ago
Group: javascript-core-security
(Reporter)

Comment 2

3 years ago
Created attachment 8682881 [details]
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)
(Reporter)

Comment 3

3 years ago
Setting s-s because this seems to be accessing 0xe5e5e5e5e5e5e5e5 in opt builds.
Crash Signature: [@ js::jit::ExecutableAllocator::releasePoolPages]
Keywords: crash
Summary: Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h → Crash [@ js::jit::ExecutableAllocator::releasePoolPages] or Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h
(Reporter)

Comment 4

3 years ago
// 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
Summary: Crash [@ js::jit::ExecutableAllocator::releasePoolPages] or Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h → Crash [@ js::jit::ExecutableAllocator::releasePoolPages] or Assertion failure: m_refCount == 1, at jit/ExecutableAllocator.h or Assertion failure: jrt_->mutatingBackedgeList_, at jit/JitCompartment.h
(Reporter)

Comment 5

3 years ago
Created attachment 8682883 [details]
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)
(Reporter)

Comment 6

3 years ago
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?
Blocks: 1132564
Flags: needinfo?(jdemooij)
(Assignee)

Comment 7

3 years ago
Created attachment 8684204 [details] [diff] [review]
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8684204 - Flags: review?(bhackett1024)
Attachment #8684204 - Flags: review?(bhackett1024) → review+
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.
Flags: needinfo?(jdemooij)
Keywords: sec-high
(Assignee)

Comment 9

3 years ago
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.
Flags: needinfo?(jdemooij)
Attachment #8684204 - Flags: sec-approval?
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?
Attachment #8684204 - Flags: sec-approval? → sec-approval+
status-firefox43: --- → wontfix
status-firefox44: --- → affected
status-firefox46: --- → affected
status-firefox-esr38: --- → affected
tracking-firefox44: --- → +
tracking-firefox45: --- → +
tracking-firefox46: --- → +
tracking-firefox-esr38: --- → 44+
https://hg.mozilla.org/mozilla-central/rev/350fbdbad784
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Jan, could you please nominate patch for uplift to Beta, Aurora, esr38? Thanks!
Flags: needinfo?(jdemooij)
status-b2g-v2.0: --- → wontfix
status-b2g-v2.0M: --- → wontfix
status-b2g-v2.1: --- → wontfix
status-b2g-v2.1S: --- → wontfix
status-b2g-v2.2: --- → affected
status-b2g-v2.2r: --- → affected
status-b2g-v2.5: --- → affected
status-b2g-master: --- → fixed
status-firefox-esr45: --- → affected
Target Milestone: --- → mozilla46
(Assignee)

Comment 14

3 years ago
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.
Flags: needinfo?(jdemooij)
Attachment #8684204 - Flags: approval-mozilla-esr38?
Attachment #8684204 - Flags: approval-mozilla-beta?
Attachment #8684204 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1189343
Attachment #8684204 - Flags: approval-mozilla-esr38?
Attachment #8684204 - Flags: approval-mozilla-esr38+
Attachment #8684204 - Flags: approval-mozilla-beta?
Attachment #8684204 - Flags: approval-mozilla-beta+
Attachment #8684204 - Flags: approval-mozilla-aurora?
Attachment #8684204 - Flags: approval-mozilla-aurora+
This doesn't apply cleanly to esr38. Can we get a rebased patch if this needs to go there?
Flags: needinfo?(jdemooij)
Group: javascript-core-security → core-security-release
(Assignee)

Comment 19

3 years ago
https://hg.mozilla.org/releases/mozilla-esr38/rev/4444e94a99cb
status-firefox-esr38: affected → fixed
Flags: needinfo?(jdemooij)
Gary, can you verify fixed? Thanks.
Flags: needinfo?(gary)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main44+][adv-esr38.6+]
(Reporter)

Updated

3 years ago
Flags: needinfo?(gary)
(Reporter)

Comment 21

3 years ago
Waiting on JSBugMon's result.
Flags: needinfo?(gary)
(Reporter)

Comment 22

3 years ago
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.
Status: RESOLVED → VERIFIED
Flags: needinfo?(gary)
status-firefox-esr45: affected → fixed
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.