Closed
Bug 1252707
Opened 8 years ago
Closed 8 years ago
Crash [@ IsInsideNursery] with OOM and use-after-free
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: decoder, Assigned: bhackett1024)
References
(Blocks 1 open bug)
Details
(7 keywords, Whiteboard: [jsbugmon:update][adv-main46+][adv-esr45.1+][adv-esr38.8+])
Crash Data
Attachments
(2 files)
913 bytes,
patch
|
terrence
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
Sylvestre
:
approval-mozilla-esr38+
Sylvestre
:
approval-mozilla-esr45+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
964 bytes,
patch
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision e15383656900 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-extra-checks --ion-offthread-compile=off): STATUS = "STATUS " function printStatus(msg) { lines = msg.split(); print(STATUS + lines[0]); } Object.defineProperty(Object.prototype, 'e', {}); Object.prototype.apply = summary = 'GC without recursion'; Object.prototype[0] = /a/ printStatus(summary); oomAfterAllocations(10); printStatus(summary)({}.__proto__.watch('x', print)); Backtrace: Program received signal SIGSEGV, Segmentation fault. IsInsideNursery (cell=0xfffc4d4d4d4d4d4d) at js/src/debug64/dist/include/js/HeapAPI.h:333 #0 IsInsideNursery (cell=0xfffc4d4d4d4d4d4d) at js/src/debug64/dist/include/js/HeapAPI.h:333 #1 isTenured (this=0xfffc4d4d4d4d4d4d) at js/src/gc/Heap.h:222 #2 js::gc::TenuredCell::arenaHeader (this=0xfffc4d4d4d4d4d4d) at js/src/gc/Heap.h:1411 #3 0x0000000000979f3f in isMarked (color=0, this=<optimized out>) at js/src/gc/Heap.h:1380 #4 js::Shape::sweep (this=this@entry=0x7ffff7e8ce70) at js/src/jspropertytree.cpp:206 #5 0x00000000008ff9a4 in SweepThing (shape=0x7ffff7e8ce70) at js/src/jsgc.cpp:5463 #6 SweepArenaList<js::Shape> (sliceBudget=..., arenasToSweep=<optimized out>) at js/src/jsgc.cpp:5484 #7 js::gc::GCRuntime::sweepPhase (this=this@entry=0x7ffff695d430, sliceBudget=...) at js/src/jsgc.cpp:5585 #8 0x0000000000908113 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff695d430, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6192 #9 0x00000000009092e0 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff695d430, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6415 #10 0x0000000000909821 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff695d430, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6521 #11 0x0000000000909a53 in js::gc::GCRuntime::gc (this=this@entry=0x7ffff695d430, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6579 #12 0x00000000008bb40c in js::DestroyContext (cx=0x7ffff6907800, mode=js::DCM_FORCE_GC) at js/src/jscntxt.cpp:181 #13 0x00000000008bb65e in JS_DestroyContext (cx=<optimized out>) at js/src/jsapi.cpp:581 #14 0x000000000048076f in DestroyContext (withGC=true, cx=0x7ffff6907800) at js/src/shell/js.cpp:6351 #15 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7253 rax 0xfffc4d4d4d4d4d4d -1040905502110387 rbx 0xfffc4d4d4d4d4d4d -1040905502110387 rcx 0xfffc4d4d4d400000 -1040905502982144 rdx 0xfffc4d4d4d4fffe8 -1040905501933592 rsi 0x0 0 rdi 0xfffc4d4d4d4d4d4d -1040905502110387 rbp 0x7fffffffd030 140737488343088 rsp 0x7fffffffd030 140737488343088 r8 0x0 0 r9 0x7ffff6a00368 140737331069800 r10 0x7ffff6930420 140737330218016 r11 0x7ffff6a00379 140737331069817 r12 0x7ffff7e8ce70 140737352617584 r13 0x7ffff46ac000 140737294024704 r14 0x7fffffffd550 140737488344400 r15 0x7ffff7e8ce70 140737352617584 rip 0x4dc950 <js::gc::TenuredCell::arenaHeader() const+32> => 0x4dc950 <js::gc::TenuredCell::arenaHeader() const+32>: mov (%rdx),%edx 0x4dc952 <js::gc::TenuredCell::arenaHeader() const+34>: test %edx,%edx This looks like a use-after-free crash or some other memory corruption given the memory patterns in the crashing register. Marking s-s.
Comment 1•8 years ago
|
||
Yeah this doesn't look too healthy. NI? terrence but feel free to forward if it's somewhere else.
Flags: needinfo?(terrence)
Updated•8 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 2•8 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/f82a7f0db599 user: Tooru Fujisawa date: Wed Mar 18 18:22:05 2015 +0900 summary: Bug 1079919 - Part 0: Add RegExp ClassSpec. r=bholley This iteration took 192.276 seconds to run.
Pending more analysis by Terrence, not sure if bug 1079919 is the culprit here since OOM is related.
Comment 4•8 years ago
|
||
We're going off the rails right here: https://dxr.mozilla.org/mozilla-central/source/js/src/vm/Shape.cpp#403 We've allocated a fresh Shape, but we return before initializing it because the allocation in growSlots fails. If I move the initialization up above the setSlotSpan, other assertions fail. I think we need some way to initialize the shape's pointers to something safe before exiting. It looks like this was regressed by db8e6de6f68f in bug 693221. Passing ni? to Brian.
Flags: needinfo?(terrence) → needinfo?(bhackett1024)
Assignee | ||
Comment 5•8 years ago
|
||
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8726754 -
Flags: review?(terrence)
Updated•8 years ago
|
status-firefox44:
--- → wontfix
status-firefox45:
--- → wontfix
status-firefox46:
--- → affected
status-firefox-esr38:
--- → affected
status-firefox-esr45:
--- → affected
Comment 6•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch Review of attachment 8726754 [details] [diff] [review]: ----------------------------------------------------------------- Awesome!
Attachment #8726754 -
Flags: review?(terrence) → review+
Updated•8 years ago
|
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Probably very hard --- an OOM at a specific point causes the GC to access uninitialized GC thing memory. 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? All. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Trivial. How likely is this patch to cause regressions; how much testing does it need? Not at all. Approval Request Comment [Feature/regressing bug #]: bug 693221 [User impact if declined]: potential crash [Describe test coverage new/current, TreeHerder]: none [Risks and why]: none
Attachment #8726754 -
Flags: sec-approval?
Attachment #8726754 -
Flags: approval-mozilla-esr45?
Attachment #8726754 -
Flags: approval-mozilla-beta?
Attachment #8726754 -
Flags: approval-mozilla-aurora?
Comment 8•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch This was a bit last minute before the merge and I am about to start the beta 1 build. We can uplift this for beta 2 though. Just letting you know I see it.
Comment 9•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch sec-approval+
Attachment #8726754 -
Flags: sec-approval? → sec-approval+
Comment 10•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch Crash fix, sec-high, approved for beta and aurora.
Attachment #8726754 -
Flags: approval-mozilla-beta?
Attachment #8726754 -
Flags: approval-mozilla-beta+
Attachment #8726754 -
Flags: approval-mozilla-aurora?
Attachment #8726754 -
Flags: approval-mozilla-aurora+
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6daac63d773
status-firefox48:
--- → affected
Updated•8 years ago
|
Attachment #8726754 -
Flags: approval-mozilla-esr38?
Comment 12•8 years ago
|
||
*sigh* I could have sworn I'd fixed the author information on this before pushing. In the future, attaching patches with proper commit information would have avoided that problem... https://hg.mozilla.org/mozilla-central/rev/b6daac63d773
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
Comment 13•8 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•8 years ago
|
Group: javascript-core-security → core-security-release
https://hg.mozilla.org/releases/mozilla-aurora/rev/351882d470a7 https://hg.mozilla.org/releases/mozilla-beta/rev/59f372c35b24
Comment 15•8 years ago
|
||
Comment on attachment 8726754 [details] [diff] [review] patch Fix a sec-high, taking it. Should be in 45.1.0 & 38.8.0
Attachment #8726754 -
Flags: approval-mozilla-esr45?
Attachment #8726754 -
Flags: approval-mozilla-esr45+
Attachment #8726754 -
Flags: approval-mozilla-esr38?
Attachment #8726754 -
Flags: approval-mozilla-esr38+
This doesn't apply cleanly to esr38.
Flags: needinfo?(bhackett1024)
Assignee | ||
Comment 18•8 years ago
|
||
This patch should apply cleanly to esr38.
Flags: needinfo?(bhackett1024)
Updated•8 years ago
|
tracking-firefox46:
--- → +
tracking-firefox47:
--- → +
tracking-firefox48:
--- → +
tracking-firefox-esr38:
--- → 46+
tracking-firefox-esr45:
--- → 46+
Comment 19•8 years ago
|
||
This needs ESR38 checkin.
Keywords: checkin-needed
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main46+][adv-esr45.1+]
Comment 20•8 years ago
|
||
has problems to uplift to esr38: Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 59f372c35b24 grafting 335694:59f372c35b24 "Bug 1252707. r=terrence a=lizzard" merging js/src/vm/Shape.cpp warning: conflicts while merging js/src/vm/Shape.cpp! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue)
Flags: needinfo?(bhackett1024)
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #20) > has problems to uplift to esr38: > > Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 59f372c35b24 > grafting 335694:59f372c35b24 "Bug 1252707. r=terrence a=lizzard" > merging js/src/vm/Shape.cpp > warning: conflicts while merging js/src/vm/Shape.cpp! (edit, then use 'hg > resolve --mark') > abort: unresolved conflicts, can't continue > (use hg resolve and hg graft --continue) See comment 17/18
Flags: needinfo?(bhackett1024)
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Updated•8 years ago
|
Flags: needinfo?(cbook)
Comment 22•8 years ago
|
||
sorry brian, missed this patch landed in https://hg.mozilla.org/releases/mozilla-esr38/rev/3fdd280fa099
Flags: needinfo?(cbook)
Updated•8 years ago
|
Keywords: checkin-needed
Updated•8 years ago
|
Whiteboard: [jsbugmon:update][adv-main46+][adv-esr45.1+] → [jsbugmon:update][adv-main46+][adv-esr45.1+][adv-esr38.8+]
Updated•8 years ago
|
Group: core-security-release
Updated•7 years ago
|
Keywords: csectype-uaf
You need to log in
before you can comment on or make changes to this bug.
Description
•