Closed Bug 1057598 Opened 5 years ago Closed 5 years ago

Crash [@ js::jit::JitFrameIterator::operator++]

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla35
Tracking Status
firefox31 --- unaffected
firefox32 --- unaffected
firefox33 + fixed
firefox34 + fixed
firefox35 + verified
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: crash, sec-moderate, testcase, Whiteboard: [jsbugmon:][b2g-adv-main2.2-])

Crash Data

Attachments

(3 files)

The following testcase crashes on mozilla-central revision cd2acc7ab2f8 (run with --no-threads --fuzzing-safe):


setObjectMetadataCallback(function( r, ... d)  {});
setJitCompilerOption("ion.usecount.trigger", 20);
var uceFault = function (i) {
    if (i > 98)
        uceFault = function (i) { return true; };
}
var uceFault_str_split = eval(uneval(uceFault).replace('uceFault', 'uceFault_str_split'))
function rstr_split(i) {
    var x = "str01234567899876543210rts".split("" + i);
    if (uceFault_str_split(i) || uceFault_str_split(i)) {
    }
}
for (i = 0; i < 100; i++) {
    rstr_split(i);
}
Needinfo from :nbp because he fixed the last bug with that signature.
Flags: needinfo?(nicolas.b.pierron)
Whiteboard: [jsbugmon:update,bisect]
Jan, maybe you can also take a look here, since nbp is still away? Thanks!
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Suppress the object metadata callback for RStringSplit too.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8482170 - Flags: review?(nicolas.b.pierron)
Flags: needinfo?(jdemooij)
Flags: needinfo?(nicolas.b.pierron)
Attachment #8482170 - Flags: review?(nicolas.b.pierron) → review+
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision bc7deafdac4b).
Whiteboard: [jsbugmon:update,bisect,ignore] → [jsbugmon:bisectfix]
Attachment #8487775 - Flags: review+
Comment on attachment 8487775 [details] [diff] [review]
Patch without testcase

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Not easily but also not super hard.

> 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 similar to comments elsewhere in the file.

> Which older supported branches are affected by this flaw?
33+.

> If not all supported branches, which bug introduced the flaw?
Bug 1028675.

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

> How likely is this patch to cause regressions; how much testing does it need?
Unlikely.
Attachment #8487775 - Flags: sec-approval?
Comment on attachment 8487775 [details] [diff] [review]
Patch without testcase

The object metadata callback is not exposed to content so this isn't sec-critical.
Attachment #8487775 - Flags: sec-approval?
Whiteboard: [jsbugmon:bisectfix] → [jsbugmon:]
JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/64203c2e785d
user:        Nicolas B. Pierron
date:        Wed Sep 10 19:11:20 2014 +0200
summary:     Bug 1063816 - Rename useCount to warmUpCounter. r=h4writer

This iteration took 0.656 seconds to run.
(In reply to Christian Holler (:decoder) from comment #10)
> JSBugMon: Fix Bisection requested, result:
> autoBisect shows this is probably related to the following changeset:
> 
> The first good revision is:
> changeset:   https://hg.mozilla.org/mozilla-central/rev/64203c2e785d
> user:        Nicolas B. Pierron
> date:        Wed Sep 10 19:11:20 2014 +0200
> summary:     Bug 1063816 - Rename useCount to warmUpCounter. r=h4writer
> 
> This iteration took 0.656 seconds to run.

Oh, crap I did not thought of that while doing the renaming, this appears to be fix because "usecount" is replaced by "warmup". I guess …

The following testcase crashes on mozilla-central revision 64203c2e785d (run with --no-threads --fuzzing-safe):


setObjectMetadataCallback(function( r, ... d)  {});
setJitCompilerOption("ion.warmup.trigger", 20);
var uceFault = function (i) {
    if (i > 98)
        uceFault = function (i) { return true; };
}
var uceFault_str_split = eval(uneval(uceFault).replace('uceFault', 'uceFault_str_split'))
function rstr_split(i) {
    var x = "str01234567899876543210rts".split("" + i);
    if (uceFault_str_split(i) || uceFault_str_split(i)) {
    }
}
for (i = 0; i < 100; i++) {
    rstr_split(i);
}
https://hg.mozilla.org/mozilla-central/rev/b6c66d55c46e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Jan, can we have an uplift request for aurora & beta? thanks
Flags: needinfo?(jdemooij)
Comment on attachment 8482170 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1028675.
[User impact if declined]: Crashes when using devtools.
[Describe test coverage new/current, TBPL]: Tested on TBPL.
[Risks and why]: Very low risk.
[String/UUID change made/needed]: None.
Attachment #8482170 - Flags: approval-mozilla-beta?
Attachment #8482170 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jdemooij)
Attachment #8482170 - Flags: approval-mozilla-beta?
Attachment #8482170 - Flags: approval-mozilla-beta+
Attachment #8482170 - Flags: approval-mozilla-aurora?
Attachment #8482170 - Flags: approval-mozilla-aurora+
Group: core-security
Whiteboard: [jsbugmon:] → [jsbugmon:][b2g-adv-main2.2-]
You need to log in before you can comment on or make changes to this bug.