bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

Assertion failure: ptr.found() && &*ptr == &e.front(), at vm/ObjectGroup.cpp

VERIFIED FIXED in Firefox 38, Firefox OS master



JavaScript Engine
3 years ago
2 years ago


(Reporter: gkw, Assigned: jonco)


(Blocks: 1 bug, 4 keywords)

Mac OS X
assertion, regression, sec-high, testcase
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox37 unaffected, firefox38+ verified, firefox39+ verified, firefox-esr31 unaffected, b2g-v1.4 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.1S unaffected, b2g-v2.2 unaffected, b2g-master fixed)


(Whiteboard: [jsbugmon:update])


(2 attachments)



3 years ago
// Randomly chosen test: js/src/jit-test/tests/debug/bug1108159.js
startgc(0, "shrinking");
var g = newGlobal();
g.offThreadCompileScript('debugger;', {});

asserts js debug shell on m-c changeset 0a8b3b67715a with --fuzzing-safe --ion-offthread-compile=off --ion-eager at Assertion failure: ptr.found() && &*ptr == &e.front(), at vm/ObjectGroup.cpp.

Configure options:

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

python -u ~/fuzzing/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build -R ~/trees/mozilla-central" -r 0a8b3b67715a

autoBisect is running.
Flags: needinfo?

Comment 1

3 years ago
Created attachment 8570011 [details]

(lldb) bt 5
* thread #1: tid = 0x289ce, 0x0000000100248bb4 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::ObjectGroupCompartment::checkNewTableAfterMovingGC(this=<unavailable>, table=<unavailable>) + 964 at ObjectGroup.cpp:1544, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100248bb4 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::ObjectGroupCompartment::checkNewTableAfterMovingGC(this=<unavailable>, table=<unavailable>) + 964 at ObjectGroup.cpp:1544
    frame #1: 0x00000001007e6c21 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::CheckHashTablesAfterMovingGC(JSRuntime*) [inlined] js::CompartmentsIterT<js::ZonesIter>::operator->(this=0x0000000101d2be00, this=0x00000001028a48b8) const + 15 at ObjectGroup.h:678
    frame #2: 0x00000001007e6c12 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::CheckHashTablesAfterMovingGC(rt=0x000000010201f800) + 114 at jsgc.cpp:7021
    frame #3: 0x00000001007e656d js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::GCRuntime::compactPhase(this=0x000000010201fb30, reason=<unavailable>) + 749 at jsgc.cpp:5525
    frame #4: 0x00000001007e83a2 js-dbg-64-dm-nsprBuild-darwin-0a8b3b67715a`js::gc::GCRuntime::incrementalCollectSlice(this=0x000000010201fb30, budget=<unavailable>, reason=API) + 1138 at jsgc.cpp:5941
Flags: needinfo?

Comment 2

3 years ago
Something related to Debugger and/or compacting GC might be at play here, setting s-s to be safe since GC is all over the stack.
Group: core-security

Comment 3

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/26d8f946a53b
user:        Jon Coppeard
date:        Fri Jan 16 14:34:32 2015 +0000
summary:     Bug 650161 - Enable compacting GC on GC_SHRINK collections r=terrence r=glandium

Jon, is compacting GC a likely regressor?
Flags: needinfo?(jcoppeard)

Comment 4

3 years ago
Created attachment 8570427 [details] [diff] [review]

What's happening here is that GlobalHelperThreadState::finishParseTask() temporarily creates cross-compartment pointers for ObjectGroup prototypes which become same-compartment pointers when MergeCompartments() happens.  However if a GC tries to move the prototype objects before this point then it won't know about the cross-compartment pointers and it will not update them.

The fix is to finish any ongoing GC activity before creating these pointers and assert that we don't trigger any more before calling MergeCompartments().
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attachment #8570427 - Flags: review?(terrence)
status-firefox37: --- → unaffected
status-firefox38: --- → affected
status-firefox-esr31: --- → unaffected
Keywords: sec-high
Comment on attachment 8570427 [details] [diff] [review]

Review of attachment 8570427 [details] [diff] [review]:

Attachment #8570427 - Flags: review?(terrence) → review+

Comment 6

3 years ago
Test testcase is hanging on opt builds due to bug 1138390.
Depends on: 1138390

Comment 7

3 years ago
Comment on attachment 8570427 [details] [diff] [review]

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Hard, since it relies on the timing of GC slices.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?


Which older supported branches are affected by this flaw?

Aurora is affected.

If not all supported branches, which bug introduced the flaw?

Bug 650161.

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

This patch should apply cleanly.

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

Attachment #8570427 - Flags: sec-approval?
Comment on attachment 8570427 [details] [diff] [review]

sec-approval+ for trunk.

We should get a patch nominated and approved for Aurora as well.
Attachment #8570427 - Flags: sec-approval? → sec-approval+
tracking-firefox38: --- → +
tracking-firefox39: --- → +
Last Resolved: 3 years ago
status-firefox39: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
status-firefox39: fixed → verified
JSBugMon: This bug has been automatically verified fixed.

Comment 12

3 years ago
Needinfo'ing myself to request aurora uplift in a week.
Flags: needinfo?(jcoppeard)

Comment 13

3 years ago
Comment on attachment 8570427 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Bug 650161
[User impact if declined]: Possible security issue.
[Describe test coverage new/current, TreeHerder]: On central for a week.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.
Flags: needinfo?(jcoppeard)
Attachment #8570427 - Flags: approval-mozilla-aurora?
Attachment #8570427 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Backed out for jit bustage in https://hg.mozilla.org/releases/mozilla-aurora/rev/ae81a10f22b6


Do we need to get bug 1138390 uplifted to aurora first?
status-firefox38: fixed → affected
Flags: needinfo?(jcoppeard)

Comment 16

3 years ago
(In reply to Wes Kocher (:KWierso) from comment #15)

> Do we need to get bug 1138390 uplifted to aurora first?

Yes, we do.  I requested uplift for both and didn't expect one to get uplifted without the other :)
Flags: needinfo?(jcoppeard)
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → unaffected
status-b2g-v2.1S: --- → unaffected
status-b2g-v2.2: --- → unaffected
status-b2g-master: --- → fixed
status-firefox38: affected → fixed
status-firefox38: fixed → verified
JSBugMon: This bug has been automatically verified fixed on Fx38


3 years ago
Group: 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.