Closed Bug 1472638 Opened 6 years ago Closed 6 years ago

Crash [@ __memset_sse2] or Hit MOZ_CRASH(Tried to access a protected region!) with OOM

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: decoder, Assigned: nbp)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 23885c14f025 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe): oomTest(function() { eval(` var BIG_INDEX = 4294967290; var arr = Array(BIG_INDEX); arr[BIG_INDEX - 1] = 'a'; arr.length = BIG_INDEX - 5000; status = inSection(1); `); }); Backtrace: received signal SIGSEGV, Segmentation fault. __memset_sse2 () at ../sysdeps/x86_64/multiarch/../memset.S:65 #0 __memset_sse2 () at ../sysdeps/x86_64/multiarch/../memset.S:65 #1 0x0000000000536f7b in memset (__len=40, __ch=206, __dest=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/string3.h:90 #2 js::detail::BumpChunk::setBump (this=this@entry=0x7fffec3e2000, newBump=newBump@entry=0x7fffec3e3018 '\315' <repeats 199 times>, <incomplete sequence \315>...) at js/src/ds/LifoAlloc.h:335 #3 0x00000000005591d9 in js::detail::BumpChunk::tryAlloc (n=40, this=0x7fffec3e2000) at js/src/ds/LifoAlloc.h:461 #4 js::LifoAlloc::allocImpl (n=40, this=0x7ffff49fd578) at js/src/ds/LifoAlloc.h:590 #5 js::LifoAlloc::alloc (this=0x7ffff49fd578, n=40) at js/src/ds/LifoAlloc.h:663 #6 0x0000000000d3a6ed in js::LifoAlloc::new_<js::ObjectGroup::Property, jsid&> (this=<optimized out>) at js/src/ds/LifoAlloc.h:887 #7 js::ObjectGroup::getProperty (this=this@entry=0x7ffff2b60a90, sweep=..., cx=cx@entry=0x7ffff5f17000, obj=obj@entry=0x7ffff4d36a30, id=id@entry=...) at js/src/vm/TypeInference-inl.h:1142 #8 0x0000000000d04fd3 in js::ObjectGroup::markPropertyNonData (this=0x7ffff2b60a90, cx=cx@entry=0x7ffff5f17000, obj=obj@entry=0x7ffff4d36a30, id=..., id@entry=...) at js/src/vm/TypeInference.cpp:2885 #9 0x00000000004db4ef in js::MarkTypePropertyNonData (id=..., obj=0x7ffff4d36a30, cx=0x7ffff5f17000) at js/src/vm/TypeInference-inl.h:471 #10 js::DeleteProperty (result=..., id=..., obj=..., cx=0x7ffff5f17000) at js/src/vm/JSObject-inl.h:256 #11 js::DeleteElement (cx=0x7ffff5f17000, obj=..., obj@entry=..., index=index@entry=4294967237, result=...) at js/src/vm/JSObject-inl.h:268 #12 0x00000000004c812e in js::ArraySetLength (cx=<optimized out>, cx@entry=0x7ffff5f17000, arr=arr@entry=..., id=..., id@entry=..., attrs=attrs@entry=4, value=..., value@entry=..., result=...) at js/src/builtin/Array.cpp:834 #13 0x00000000004c906b in array_length_setter (cx=cx@entry=0x7ffff5f17000, obj=..., id=..., v=..., result=...) at js/src/builtin/Array.cpp:645 #14 0x0000000000c168c9 in js::CallJSSetterOp (result=..., v=..., id=..., obj=..., op=<optimized out>, cx=0x7ffff5f17000) at js/src/vm/JSContext-inl.h:301 #15 NativeSetExistingDataProperty (result=..., v=..., shape=..., obj=..., cx=0x7ffff5f17000) at js/src/vm/NativeObject.cpp:2535 #16 SetExistingProperty (cx=<optimized out>, id=..., id@entry=..., v=v@entry=..., receiver=..., receiver@entry=..., pobj=..., pobj@entry=..., prop=..., prop@entry=..., result=...) at js/src/vm/NativeObject.cpp:2745 #17 0x0000000000c3a5d5 in js::NativeSetProperty<(js::QualifiedBool)1> (cx=<optimized out>, cx@entry=0x7ffff5f17000, obj=..., id=id@entry=..., v=..., v@entry=..., receiver=..., receiver@entry=..., result=...) at js/src/vm/NativeObject.cpp:2788 #18 0x00000000005b786c in js::SetProperty (cx=0x7ffff5f17000, obj=..., id=..., v=..., receiver=..., result=...) at js/src/vm/NativeObject.h:1705 #19 0x00000000005a345e in SetPropertyOperation (rval=..., id=..., lval=..., op=<optimized out>, cx=0x7ffff5f17000) at js/src/vm/Interpreter.cpp:268 #20 Interpret (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:2990 #21 0x00000000005ae5f6 in js::RunScript (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:423 #22 0x00000000005b195d in js::ExecuteKernel (cx=<optimized out>, cx@entry=0x7ffff5f17000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=<optimized out>) at js/src/vm/Interpreter.cpp:771 #23 0x00000000005ea3b0 in EvalKernel (cx=0x7ffff5f17000, v=..., v@entry=..., evalType=evalType@entry=DIRECT_EVAL, caller=..., env=env@entry=..., pc=<optimized out>, vp=...) at js/src/builtin/Eval.cpp:319 #24 0x00000000005eabbe in js::DirectEval (cx=<optimized out>, v=..., vp=...) at js/src/builtin/Eval.cpp:427 #25 0x000000000069a683 in js::jit::DoCallFallback (cx=<optimized out>, frame=0x7fffffffbf28, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffffbed8, res=...) at js/src/jit/BaselineIC.cpp:2641 #26 0x00002ade6844028c in ?? () #27 0x00007fffffffbea0 in ?? () [...] #58 0x0000000000000000 in ?? () rax 0x7fffec3e2ff0 140737156886512 rbx 0x7fffec3e2000 140737156882432 rcx 0x7fffec3e2ff0 140737156886512 rdx 0x28 40 rsi 0xce 206 rdi 0x7fffec3e2ff0 140737156886512 rbp 0x7fffffffa240 140737488331328 rsp 0x7fffffffa158 140737488331096 r8 0x5dee488d 1575897229 r9 0x9e3779b8 2654435768 r10 0x0 0 r11 0x7ffff487b400 140737295922176 r12 0x0 0 r13 0x7fffec3e3018 140737156886552 r14 0x28 40 r15 0x7fffec3e3018 140737156886552 rip 0x7ffff6bc026c <__memset_sse2+44> => 0x7ffff6bc026c <__memset_sse2+44>: movdqu %xmm0,-0x10(%rdi,%rdx,1) 0x7ffff6bc0272 <__memset_sse2+50>: ja 0x7ffff6bc0280 <__memset_sse2+64> The crash looks like a stack space exhaustion but outside of GDB I also get the MOZ_CRASH, so I suspect this might be taking a different control flow route depending on how much stack space is available. Marking s-s until investigated further.
Flags: needinfo?(nicolas.b.pierron)
This definitely looks like what is being reported to us by crash-stat, i-e some write on the bottom of protected buffers during a LifoAlloc allocation. (see also Bug 1263794 comment 34)
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
This particular problem happens on OOM under getOrCreateChunk, when we cannot allocate a new chunk. The first step of getOrCreateChunk is to flag the last chunk as reserved, but in case of OOM it will remain in the last chunk position. In this particular case we attempt to allocate a 64k hash set, fail, and resume under ObjectGroup::markPropertyNonData as if nothing happend. Thus the last chunk is being protected while remaining the last chunk. This is an instrumentation bug and might not be the issue we are looking for. I will make a quick fix to handle this false-positive. Thread 1 hit Breakpoint 3, js::detail::BumpChunk::setRWUntil (this=0xb90ce20, loc=js::detail::BumpChunk::Loc::Reserved) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:76 76 if (!protect_) (rr) bt #0 js::detail::BumpChunk::setRWUntil (this=0xb90ce20, loc=js::detail::BumpChunk::Loc::Reserved) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:76 #1 0x000000000148c254 in js::LifoAlloc::getOrCreateChunk (this=0x2fbe8c8, n=65544) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:210 #2 0x00000000006120d2 in js::LifoAlloc::allocImpl (this=0x2fbe8c8, n=65544) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:593 #3 0x000000000061204d in js::LifoAlloc::alloc (this=0x2fbe8c8, n=65544) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:663 #4 0x000000000120feb8 in js::LifoAlloc::newArrayUninitialized<js::ObjectGroup::Property*> (this=0x2fbe8c8, count=8193) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:763 #5 0x000000000120fabd in js::LifoAlloc::newArray<js::ObjectGroup::Property*> (this=0x2fbe8c8, count=8193) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:753 #6 0x000000000120fced in js::TypeHashSet::InsertTry<jsid, js::ObjectGroup::Property, js::ObjectGroup::Property> ( alloc=..., values=@0x7f61a0c68f08: 0xb8fae28, count=@0x7ffd252e571c: 2048, key=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:778 #7 0x0000000001201db5 in js::TypeHashSet::Insert<jsid, js::ObjectGroup::Property, js::ObjectGroup::Property> ( alloc=..., values=@0x7f61a0c68f08: 0xb8fae28, count=@0x7ffd252e571c: 2048, key=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:852 #8 0x00000000011ff04a in js::ObjectGroup::getProperty (this=0x7f61a0c68ee0, sweep=..., cx=0x2f8a160, obj=0x7f61a0e38930, id=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:1149 #9 0x00000000011bad48 in js::ObjectGroup::markPropertyNonData (this=0x7f61a0c68ee0, cx=0x2f8a160, obj=0x7f61a0e38930, id=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference.cpp:2885
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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/44a69a4ebc09 user: Nicolas B. Pierron date: Wed Feb 14 17:12:00 2018 +0000 summary: Bug 1437600 - Use mprotect to prevent mutations of inaccessible regions. r=luke This iteration took 318.517 seconds to run.
The problem is that we were protecting the last chunk, even in case of some allocation failures that we recover from. Which should explain the cases reported on TI. This patch adds a lambda to protect the last chunk. I did not used a ScopeExit because last() returns an iterator, and even if it would be safe in the current implementation, it would be unwise to alias it and use it after doing some append() to the same container.
Attachment #8989156 - Flags: review?(tcampbell)
Calling it sec-other because it sounds like a false-positive (comment 2). If I'm interpreting that correctly please unhide the bug and remove sec-other. Otherwise please rate appropriately
Blocks: 1437600
Flags: needinfo?(nicolas.b.pierron)
Keywords: sec-other
Comment on attachment 8989156 [details] [diff] [review] Only protect the last chunk when we append a new chunk. Review of attachment 8989156 [details] [diff] [review]: ----------------------------------------------------------------- Seems reasonable. Other approach would be to add your anonymous function to the BumpChunk class instead, but what you have may be simpler for now.
Attachment #8989156 - Flags: review?(tcampbell) → review+
Group: javascript-core-security
Flags: needinfo?(nicolas.b.pierron)
Keywords: sec-other
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b0328b70458a Only protect the last chunk when we append a new chunk. r=tcampbell
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment on attachment 8989156 [details] [diff] [review] Only protect the last chunk when we append a new chunk. Approval Request Comment [Feature/Bug causing the regression]: Bug 1437600 [User impact if declined]: false-positive crash reports [Is this code covered by automated tests?]: tested by fuzzers. [Has the fix been verified in Nightly?]: yes. (removed ~1 crash per day from nightly) [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: It removes wrong & extra memory protection from pages in case of OOM during a large allocation. [String changes made/needed]: N/A
Attachment #8989156 - Flags: approval-mozilla-beta?
This is marked disabled for 62 but looks like it would still affect dev edition.
Comment on attachment 8989156 [details] [diff] [review] Only protect the last chunk when we append a new chunk. Fix for false positive asserts, had good results in nightly, so let's uplift for beta 8.
Attachment #8989156 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: