Closed
Bug 1075546
Opened 10 years ago
Closed 10 years ago
Assertion failure: entry_ == makeIndex(clasp, key, kind), at vm/Runtime.h
Categories
(Core :: JavaScript: GC, defect)
Tracking
()
VERIFIED
FIXED
mozilla35
People
(Reporter: gkw, Assigned: jonco)
References
Details
(4 keywords, Whiteboard: [jsbugmon:][adv-main34+])
Attachments
(1 file)
(deleted),
patch
|
terrence
:
review+
abillings
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
for (var i = 0; i < 200; ++i) {
Object.getOwnPropertyNames(undefined + "");
}
function p(s) {
for (var i = 0; i < s.length; i++) {
s.charCodeAt(i);
}
}
function m(f) {
var a = [];
for (var j = 0; j < 700; ++j) {
try {
f()
} catch (e) {
a.push(e.toString());
}
}
p(uneval(a));
}
f = Function("\
function f() {\
functionf\n{}\
}\
m(f);\
");
f();
f();
for (var x = 0; x < 99; x++) {
newGlobal()
}
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.
https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-01-mozilla-central-debug/jsshell-mac64.zip
=== 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: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1de7e7a7ad8d&tochange=f33f7dc50aa9
Jon, is bug 1073577 a possible regressor? (Setting this as s-s because bug 1073577 is s-s.)
Flags: needinfo?(jcoppeard)
Reporter | ||
Comment 1•10 years ago
|
||
There are no nice stacks available for js shells from tinderbox-builds.
Updated•10 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Comment 2•10 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Reporter | ||
Comment 3•10 years ago
|
||
This does not reproduce for locally compiled builds but seems to reproduce readily with tinderbox-builds ones.
Assignee | ||
Comment 4•10 years ago
|
||
(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)
Assignee | ||
Comment 5•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8499493 -
Flags: review?(terrence) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8499493 [details] [diff] [review]
bug1075546
[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?
Difficult.
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?
Updated•10 years ago
|
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → disabled
status-b2g-v2.0M:
--- → disabled
status-b2g-v2.1:
--- → disabled
status-b2g-v2.2:
--- → fixed
status-firefox32:
--- → wontfix
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox-esr31:
--- → unaffected
Comment 7•10 years ago
|
||
Comment on attachment 8499493 [details] [diff] [review]
bug1075546
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?
Updated•10 years ago
|
tracking-firefox34:
--- → +
Comment 8•10 years ago
|
||
What is the security severity on this? bug 1073577 is a sec-audit, though I don't know if that's actually correct.
Assignee | ||
Comment 9•10 years ago
|
||
(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).
Updated•10 years ago
|
Attachment #8499493 -
Flags: sec-approval? → sec-approval+
Comment 10•10 years ago
|
||
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.
Assignee | ||
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8499493 [details] [diff] [review]
bug1075546
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?
Updated•10 years ago
|
Attachment #8499493 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•10 years ago
|
Keywords: checkin-needed
Comment 14•10 years ago
|
||
Comment 16•10 years ago
|
||
Reproduced the original issue using the steps from comment #0:
* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/10/2014-10-01-mozilla-central-debug/
** 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: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-central-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion
* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-aurora-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion
* Build Used: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-21-mozilla-beta-debug/
** ran the poc from comment #0 several times (10x) without receiving the original assertion
Updated•10 years ago
|
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main34+]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•