Closed Bug 1308048 Opened 4 years ago Closed 4 years ago

Crash [@ js::CompartmentChecker::fail] or Assertion failure: arena()->allocated(), at js/src/gc/Heap.h:1211

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla52
Tracking Status
firefox49 --- wontfix
firefox-esr45 50+ fixed
firefox50 + verified
firefox51 + verified
firefox52 + verified

People

(Reporter: gkw, Assigned: jonco)

References

Details

(6 keywords, Whiteboard: [jsbugmon:update][adv-main50+][adv-esr45.5+])

Attachments

(3 files)

The following testcase crashes on mozilla-central revision 42c95d88aaaa (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

// jsfunfuzz-generated
m = 'x';
for (var i = 0; i < 99; i++) {
    try {
        m += m;
    } catch (e) {}
}
offThreadCompileScript("", ({
    elementAttributeName: m
}));
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/Debugger-findScripts-06.js
var n = newGlobal();
// jsfunfuzz-generated
n.runOffThreadScript();


Backtrace:

0   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x00000001012ca031 js::gc::TenuredCell::isMarked(unsigned int) const + 241 (Heap.h:1211)
1   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x000000010128c6b0 js::GlobalHelperThreadState::finishParseTask(JSContext*, js::ParseTaskKind, void*) + 368 (jscntxtinlines.h:122)
2   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x000000010128d241 js::GlobalHelperThreadState::finishScriptParseTask(JSContext*, void*) + 17 (HelperThreads.cpp:1278)
3   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x0000000100b16497 runOffThreadScript(JSContext*, unsigned int, JS::Value*) + 167 (RootingAPI.h:783)
4   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x00000001012a38cd js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 93 (jscntxtinlines.h:240)
5   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x00000001012a3648 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 696 (Interpreter.cpp:446)
6   js-dbg-64-dm-clang-darwin-42c95d88aaaa	0x00000001012a3e7e js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) + 46 (Interpreter.cpp:522)
/snip

For detailed crash information, see attachment.

This also crashes js opt builds [@ js::CompartmentChecker::fail]. Setting s-s because GC is on the stack and that js opt builds show "Compartment mismatch":

*** Compartment mismatch 0x7fb0d05086d0 vs. 0x4b4b4b4b4b4b4b4b
Segmentation fault: 11

Seems to go back prior to m-c rev dc4b163f7db7 (early Nov 2014) so unable to get a bisection window.
Jon, do you know what might be the issue here, since GC is on the stack?
Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
The problem is that ParseTask::finish can GC, and the parse task is no longer traced after it's removed from finished parse task list.  I think just rooting the script is sufficient.
Attachment #8798339 - Flags: review?(jdemooij)
Attachment #8798339 - Flags: review?(jdemooij) → review+
Comment on attachment 8798339 [details] [diff] [review]
bug1308048-root-parsed-script

[Security approval request comment]

How easily could an exploit be constructed based on the patch? Certainly difficult.  I'm not sure this is even possible.

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

Which older supported branches are affected by this flaw? Everything back to FF33.

If not all supported branches, which bug introduced the flaw? Bug 1031636 seems to have introduce the possibility for GC here.

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

How likely is this patch to cause regressions; how much testing does it need? Unlikely.
Attachment #8798339 - Flags: sec-approval?
sec-approval+ for trunk. I think we should take this on at least Aurora and Beta as well if the patch applies.
Attachment #8798339 - Flags: sec-approval? → sec-approval+
https://hg.mozilla.org/mozilla-central/rev/bb5040568cfb
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8798339 [details] [diff] [review]
bug1308048-root-parsed-script

Approval Request Comment
[Feature/regressing bug #]: Bug 1031636.
[User impact if declined]: Possible crash / security vulnerability.
[Describe test coverage new/current, TreeHerder]: On m-c since 8th October.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Attachment #8798339 - Flags: approval-mozilla-beta?
Attachment #8798339 - Flags: approval-mozilla-aurora?
Blocks: 1031636
Comment on attachment 8798339 [details] [diff] [review]
bug1308048-root-parsed-script

Sec-high, Aurora51+, Beta50+

Hi Jon, we will also need a patch for uplift to ESR45.
Flags: needinfo?(jcoppeard)
Attachment #8798339 - Flags: approval-mozilla-beta?
Attachment #8798339 - Flags: approval-mozilla-beta+
Attachment #8798339 - Flags: approval-mozilla-aurora?
Attachment #8798339 - Flags: approval-mozilla-aurora+
Attached patch bug1308048-esr45Splinter Review
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high bug.
User impact if declined: Possible crash / security vulnerability.
Fix Landed on Version: FF33.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(jcoppeard)
Attachment #8799676 - Flags: review+
Attachment #8799676 - Flags: approval-mozilla-esr45?
Group: javascript-core-security → core-security-release
Comment on attachment 8799676 [details] [diff] [review]
bug1308048-esr45

sec-high, taking it on esr 45
Attachment #8799676 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx50
JSBugMon: This bug has been automatically verified fixed on Fx51
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main50+][adv-esr45.5+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.