Closed
Bug 1254122
Opened 8 years ago
Closed 8 years ago
Assertion failure: returnAddr > method_->raw(), at js/src/jit/BaselineJIT.cpp:731 with OOM and TypedObject
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla48
People
(Reporter: decoder, Assigned: efaust)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [jsbugmon:update][adv-main46+][adv-esr45.1+])
Attachments
(1 file)
2.67 KB,
patch
|
jandem
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
Sylvestre
:
approval-mozilla-esr45+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision b6acf4d4fc20 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2): for (lfLocal in this) {} function foo() { var T = TypedObject; var ObjectStruct = new T.StructType({ f: T.Object }); var o = new ObjectStruct(); function writeObject(o, v, expected) { o.f = v; } for (var i = 0; i < 5; i++) writeObject(o, {}, "helo"); for (var i = 0; i < 5; i++) writeObject(o, null, "null"); writeObject(o, "three", "three"); for (var i = 0; oomAfterAllocations(1); i++) writeObject(o, 4.5, "4.5"); writeObject(o, undefined, ""); for (var i = 0; (class get {}.get++) < 5; i++) writeString(s, 4.5, "4.5");var lfcode = new Array(); } foo(); Backtrace: Program received signal SIGSEGV, Segmentation fault. 0x00000000005f94cc in js::jit::BaselineScript::icEntryFromReturnAddress (this=<optimized out>, returnAddr=<optimized out>) at js/src/jit/BaselineJIT.cpp:731 #0 0x00000000005f94cc in js::jit::BaselineScript::icEntryFromReturnAddress (this=<optimized out>, returnAddr=<optimized out>) at js/src/jit/BaselineJIT.cpp:731 #1 0x00000000006bc0fd in js::jit::JitFrameIterator::baselineScriptAndPc (this=this@entry=0x7fffffffae50, scriptRes=scriptRes@entry=0x0, pcRes=pcRes@entry=0x7fffffffad90) at js/src/jit/JitFrames.cpp:238 #2 0x00000000006e329d in js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:834 #3 0x00007ffff7fe8608 in ?? () #4 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7fffffffae50 140737488334416 rcx 0x7ffff6ca588d 140737333844109 rdx 0x0 0 rsi 0x7ffff6f7a9d0 140737336814032 rdi 0x7ffff6f791c0 140737336807872 rbp 0x7fffffffaca0 140737488333984 rsp 0x7fffffffaca0 140737488333984 r8 0x7ffff7fdf7c0 140737354004416 r9 0x6372732f736a2f6c 7165916604736876396 r10 0x7fffffffaa60 140737488333408 r11 0x7ffff6c27ee0 140737333329632 r12 0x7ffff7ea1090 140737352700048 r13 0x7fffffffad90 140737488334224 r14 0x0 0 r15 0x7fffffffae50 140737488334416 rip 0x5f94cc <js::jit::BaselineScript::icEntryFromReturnAddress(unsigned char*)+60> => 0x5f94cc <js::jit::BaselineScript::icEntryFromReturnAddress(unsigned char*)+60>: movl $0x2db,0x0 0x5f94d7 <js::jit::BaselineScript::icEntryFromReturnAddress(unsigned char*)+71>: callq 0x4a6f30 <abort()> Marking this s-s because the assertion doesn't look safe to me. If it depends on TypedObject, it might be Nightly-only.
Updated•8 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•8 years ago
|
||
JSBugMon: Bisection requested, result: === Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20151009120049" and the hash "2c91f257f53d9c2d1e0df1578ccc0fcf9c740bf3". The "bad" changeset has the timestamp "20151009120613" and the hash "250cd0bf3ce07627f08057cc7a38bc2d67174f9f". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2c91f257f53d9c2d1e0df1578ccc0fcf9c740bf3&tochange=250cd0bf3ce07627f08057cc7a38bc2d67174f9f
Updated•8 years ago
|
Group: core-security → javascript-core-security
Comment 2•8 years ago
|
||
ni? Eric due to possible regressor bug 1105463
Blocks: 1105463
Flags: needinfo?(efaustbmo)
Assignee | ||
Comment 3•8 years ago
|
||
bug 1105463 is almost certainly not a regressor, here. It just made |class get{}.get++| not a syntax error. Jan has graciously agreed to help me look into this anyway.
Assignee | ||
Comment 4•8 years ago
|
||
OK, I've tracked this down. We incorrectly set up the stack for callTypeUpdateIC() in several SetProp stubs including TypedObject and Unboxed, which leads to a bogus value in ICTailCallReg, consulted in HandleException after an oom-related failure in DoTypeUpdateFallback. Jan, do you think we should land tests with this? It seems like it's reasonably easy to get this to happen, and if it does, we'll get the wrong stub there. It seems like the repercussions could be quite bad. It also seems like this should go to Beta, since ICSetProp_Unboxed is older and not Nightly-only.
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Flags: needinfo?(efaustbmo)
Attachment #8733249 -
Flags: review?(jdemooij)
Comment 5•8 years ago
|
||
Comment on attachment 8733249 [details] [diff] [review] Fix Review of attachment 8733249 [details] [diff] [review]: ----------------------------------------------------------------- Looks great. I'd land the test a few weeks after this has made it into a stable release.
Attachment #8733249 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8733249 [details] [diff] [review] Fix [Security approval request comment] How easily could an exploit be constructed based on the patch? Since we're not gonna land the test immediately, you'd have to be already familiar, and then get pretty lucky. 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? 41+. This is headed for Aurora47 Beta46 and esr45. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This should apply everywhere. How likely is this patch to cause regressions; how much testing does it need? quite unlikely. Tested locally, and we can let it sit on central for a day or two before uplifting.
Attachment #8733249 -
Flags: sec-approval?
Comment 7•8 years ago
|
||
sec-approval+. Please nominate for branches as well.
status-firefox45:
--- → wontfix
status-firefox46:
--- → affected
status-firefox48:
--- → affected
status-firefox-esr38:
--- → unaffected
status-firefox-esr45:
--- → affected
tracking-firefox46:
--- → +
tracking-firefox47:
--- → +
tracking-firefox48:
--- → +
tracking-firefox-esr45:
--- → ?
Updated•8 years ago
|
Attachment #8733249 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a5e93a96d8
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e0a5e93a96d8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 10•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8733249 [details] [diff] [review] Fix [Approval Request Comment] User impact if declined: Potential possibly exploitable crash with OOM. Incorrect JITCode. Fix Landed on Version: Landed cleanly with no visible regressions on 48. Risk to taking this patch (and alternatives if risky): Changing jitcode always introduces the possibiliy of regressions, but this one is quite small, safe, and intuitive. The failure is quite localized, so I am quite confident this will not provide new risk. String or UUID changes made by this patch: None. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8733249 -
Flags: approval-mozilla-esr45?
Attachment #8733249 -
Flags: approval-mozilla-beta?
Attachment #8733249 -
Flags: approval-mozilla-aurora?
Comment 12•8 years ago
|
||
Comment on attachment 8733249 [details] [diff] [review] Fix Fix for (potential) crash, let's uplift to aurora and beta.
Attachment #8733249 -
Flags: approval-mozilla-beta?
Attachment #8733249 -
Flags: approval-mozilla-beta+
Attachment #8733249 -
Flags: approval-mozilla-aurora?
Attachment #8733249 -
Flags: approval-mozilla-aurora+
Comment 13•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/5629a22932f7
Flags: in-testsuite?
Updated•8 years ago
|
Group: javascript-core-security → core-security-release
Updated•8 years ago
|
Comment 15•8 years ago
|
||
Comment on attachment 8733249 [details] [diff] [review] Fix sec-high, taking it. Should be in 45.1.0
Attachment #8733249 -
Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
Updated•8 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main46+][adv-esr45.1+]
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
•