Closed Bug 1075546 Opened 10 years ago Closed 10 years ago

Assertion failure: entry_ == makeIndex(clasp, key, kind), at vm/Runtime.h


(Core :: JavaScript: GC, defect)

Not set



Tracking Status
firefox32 --- wontfix
firefox33 --- wontfix
firefox34 + verified
firefox35 --- verified
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- disabled
b2g-v2.0M --- disabled
b2g-v2.1 --- disabled
b2g-v2.2 --- fixed


(Reporter: gkw, Assigned: jonco)



(4 keywords, Whiteboard: [jsbugmon:][adv-main34+])


(1 file)

for (var i = 0; i < 200; ++i) {
    Object.getOwnPropertyNames(undefined + "");
function p(s) {
    for (var i = 0; i < s.length; i++) {
function m(f) {
    var a = [];
    for (var j = 0; j < 700; ++j) {
        try {
        } catch (e) {
f = Function("\
    function f() {\
for (var x = 0; x < 99; x++) {

asserts the following js debug shell on m-c changeset 6c824fbb73e8 with --no-asmjs --no-baseline --no-ion --no-threads at Assertion failure: entry_ == makeIndex(clasp, key, kind), at vm/Runtime.h.

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140930050920" and the hash "1de7e7a7ad8d".
The "bad" changeset has the timestamp "20140930052624" and the hash "f33f7dc50aa9".

Likely regression window:

Jon, is bug 1073577 a possible regressor? (Setting this as s-s because bug 1073577 is s-s.)
Flags: needinfo?(jcoppeard)
There are no nice stacks available for js shells from tinderbox-builds.
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
This does not reproduce for locally compiled builds but seems to reproduce readily with tinderbox-builds ones.
(In reply to Gary Kwong [:gkw] [:nth10sd] GMT-4 Sep 29 - Oct 3 from comment #0)
Bug 1073577 added the assertion, but this may be a pre-existing problem.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attached patch bug1075546Splinter Review
Repros fine for me with the attached testcase.

The problem was the fix for bug 1073577 was incomplete.  When recording the current GC number, it's important to record it *before* a GC can happen :)

With this patch, the gc number is recorded only if the cache lookup fails.  If it succeeds, there are two cases - either we create a new object successfully and return, or we fail and GC.  In the latter case we just leave gcNumber initialized to zero, which means the later test for GC will always fail.
Attachment #8499493 - Flags: review?(terrence)
Attachment #8499493 - Flags: review?(terrence) → review+
Comment on attachment 8499493 [details] [diff] [review]

[Security approval request comment]

This is a followup to fix an issue with the fix in bug 1073577, so the details here are mostly the same.

How easily could an exploit be constructed based on the patch?


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

The contents of the patch indicate an issue with GCs occurring in specific places, but I don't think it's easy to take advantage of that.

Which older supported branches are affected by this flaw?

As per bug 1073577, everything back to FF 32.

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

Generational GC enabled on desktop in bug 619558.

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

Backporting should be combined with bug 1073577, the patch is easy to create and should apply without problems.

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

Should be low risk, again it would be a good idea to let it sit on central for a week first.
Attachment #8499493 - Flags: sec-approval?
Comment on attachment 8499493 [details] [diff] [review]

This is too late to make it into 33. We're building soon for the release build. We should get this on Aurora though. Can you make and nominate an Aurora patch?
What is the security severity on this? bug 1073577 is a sec-audit, though I don't know if that's actually correct.
(In reply to Al Billings [:abillings] from comment #8)
I've nominated the bug 1073577 for aurora/beta uplift.  The patch here will apply cleanly after that one.  I'm waiting on sec-approval to land this though (my guess at a security rating is for the original bug is sec-high).
Attachment #8499493 - Flags: sec-approval? → sec-approval+
Keywords: sec-high
Jon, please go ahead and do the Aurora approval request when you push this to inbound so we don't wait another day to cycle around.
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8499493 [details] [diff] [review]

Approval Request Comment
[Feature/regressing bug #]: Bug 619558 (fix in bug 1073577 is incomplete)
[User impact if declined]: Potential security vulnerability
[Describe test coverage new/current, TBPL]: Test case in bug
[Risks and why]: Low
[String/UUID change made/needed]: None
Attachment #8499493 - Flags: approval-mozilla-aurora?
Attachment #8499493 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced the original issue using the steps from comment #0:

* Build Used:
** Assertion: Assertion failure: entry_ == makeIndex(clasp, key, kind), at /builds/slave/m-cen-osx64-d-0000000000000000/build/js/src/vm/Runtime.h:334

Verified using the following builds:

* Build Used:
** ran the poc from comment #0 several times (10x) without receiving the original assertion

* Build Used:
** ran the poc from comment #0 several times (10x) without receiving the original assertion

* Build Used:
** ran the poc from comment #0 several times (10x) without receiving the original assertion
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main34+]
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.