Closed Bug 1064346 Opened 5 years ago Closed 5 years ago
Crash [@ Is
Inside Nursery] with use-after-free (likely OOM)
The attached testcase crashes on mozilla-central revision 2255d7d187b2 (run with --no-threads --fuzzing-safe).
This looks like a memory-sensitive issue (likely OOM) and it only reproduced for me in an optimized build (--disable-debug --enable-optimize --enable-valgrind --enable-gczeal). Backtrace: Program received signal SIGSEGV, Segmentation fault. IsInsideNursery (cell=0x4b4b4b4b4b4b4b4b) at ../../dist/include/js/HeapAPI.h:264 264 return location & ChunkLocationAnyNursery; #0 IsInsideNursery (cell=0x4b4b4b4b4b4b4b4b) at ../../dist/include/js/HeapAPI.h:264 #1 MarkInternal<JSAtom> (trc=0x1650518, thingp=0x7ffff535f0b8) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:285 #2 0x000000000078016d in trace (trc=0x1650518, this=0x7ffff535f080) at /srv/repos/mozilla-central/js/src/jsfun.cpp:720 #3 fun_trace (trc=0x1650518, obj=0x7ffff535f080) at /srv/repos/mozilla-central/js/src/jsfun.cpp:757 #4 0x00000000004fe8e5 in js::GCMarker::processMarkStackTop (this=0x1650518, budget=...) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:1748 #5 0x00000000004d6cad in js::GCMarker::drainMarkStack (this=0x1650518, budget=...) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:1810 #6 0x000000000079c272 in drainMarkStack (phase=js::gcstats::PHASE_MARK, sliceBudget=..., this=0x164fc18) at /srv/repos/mozilla-central/js/src/jsgc.cpp:4817 #7 js::gc::GCRuntime::incrementalCollectSlice (this=0x164fc18, budget=<optimized out>, reason=<optimized out>) at /srv/repos/mozilla-central/js/src/jsgc.cpp:5351 rax 0x4b4fffe8 5425512962856058856 rbx 0x1650518 23397656 rcx 0x20000 131072 rdx 0xa95db6 11099574 rsi 0xf535f0b8 140737307340984 rdi 0x4b4b4b4b 5425512962855750475 rbp 0x1650518 23397656 rsp 0xffffca00 140737488341504 r8 0xc011 49169 r9 0x21080 135296 r10 0x100 256 r11 0x0 8796093022208 r12 0xf5342158 140737307222360 r13 0xf5360088 140737307345032 r14 0x2 2 r15 0xffffcb40 140737488341824 rip 0x4c1d3c <MarkInternal<JSAtom>(JSTracer*, JSAtom**)+76> => 0x4c1d3c <MarkInternal<JSAtom>(JSTracer*, JSAtom**)+76>: mov (%rax),%eax Looks dangerous to me, marking s-s. Ccing Jonco and sfink because it looks related to GGC.
Can you look at this, Terrence? It looks like Jon is on PTO.
Assignee: nobody → terrence
This is yet another path where JSFunction's quirky C-struct overlay on object slots is biting us. In this case, we're failing a JSFunction allocation at setSingletonType after Object::create and before NewFunction. This leaves a JSObject with JSFunction::class's trace hook in the heap without having initialized it's memory in a way that the trace hook can understand it. In this case, shape->slotSpan() is hiding the slots and leaving them uninitialized; it really doesn't matter though. Whether the values we leave are uninitialized memory, poisoning, or even correct-for-an-object UndefinedValues, when JSFunction::trace looks at atom_, it's going to try to mark gibberish and crash. We've already hacked up a bunch of paths at various places in the object and gc guts to manually check for JSFunction, so I think the right solution here is just to extend that and make JSObject::create aware that it needs to give JSFunctions C-style POD memory initialization.
Attachment #8487552 - Flags: review?(wmccloskey)
Attachment #8487552 - Flags: review?(wmccloskey) → review+
Comment on attachment 8487552 [details] [diff] [review] bug_1064346-v0.diff [Security approval request comment] How easily could an exploit be constructed based on the patch? Pretty difficult. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Yes. Which older supported branches are affected by this flaw? All of them. If not all supported branches, which bug introduced the flaw? Nak. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should be easy to backport. How likely is this patch to cause regressions; how much testing does it need?
Attachment #8487552 - Flags: sec-approval?
How likely is this to cause regressions?
(In reply to Al Billings [:abillings] from comment #6) > How likely is this to cause regressions? Very, very unlikely.
Comment on attachment 8487552 [details] [diff] [review] bug_1064346-v0.diff sec-approval+ for checkin on 9/23 or later. We should have Aurora, Beta, and ESR31 patches for then too.
Attachment #8487552 - Flags: sec-approval? → sec-approval+
Terrence, can we have the uplift request? Thanks
Whiteboard: [checkin on 9/23]
Comment on attachment 8487552 [details] [diff] [review] bug_1064346-v0.diff Approval Request Comment [Feature/regressing bug #]: None - it's an ancient problem. [User impact if declined]: A potentially exploitable crash. [Describe test coverage new/current, TBPL]: The fuzzer testcase is too large (and potentially sensitive?) to be useful as a test. [Risks and why]: Low. This simply adds initialization for JSFunction. [String/UUID change made/needed]: None.
Comment on attachment 8487552 [details] [diff] [review] bug_1064346-v0.diff Giving approvals (except for release branch, which I cleared).
Attachment #8487552 - Flags: approval-mozilla-release?
Attachment #8487552 - Flags: approval-mozilla-esr31?
Attachment #8487552 - Flags: approval-mozilla-esr31+
Attachment #8487552 - Flags: approval-mozilla-beta?
Attachment #8487552 - Flags: approval-mozilla-beta+
Attachment #8487552 - Flags: approval-mozilla-aurora?
Attachment #8487552 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/6232dcb73aa5 https://hg.mozilla.org/releases/mozilla-beta/rev/fd4720dd6a46 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/41046048a8f7 https://hg.mozilla.org/releases/mozilla-esr31/rev/611e355a3543 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ae553c9bedd1
Reproduced the original issue using the information from comment #0 and comment #1 using revision 2255d7d187b2 fx 35 [revision: b85c260821ab] - Passed fx 34 [revision: ee3cd82e3acb] - Passed fx 33 [revision: d79568d581e6] - Passed esr31 [revision: 8c431dcec0ff] - Passed
You need to log in before you can comment on or make changes to this bug.