Closed Bug 1515816 Opened 5 years ago Closed 5 years ago

Crash [@ JSObject::hasLazyGroup] with ReadableStream and OOM

Categories

(Core :: JavaScript Engine, defect, P1)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: decoder, Assigned: jorendorff)

References

Details

(4 keywords, Whiteboard: [jsbugmon:])

Crash Data

Attachments

(2 files)

The following testcase crashes on mozilla-central revision d62e952be812 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

See attachment.


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x567da915 in JSObject::hasLazyGroup (this=0x0) at js/src/vm/JSObject.h:164
#1  JSObject::group (this=0x0) at js/src/vm/JSObject.h:146
#2  0x56b3f4db in js::NativeObject::createWithTemplate (templateObject=..., heap=js::gc::DefaultHeap, cx=0xf6e2e800) at js/src/vm/NativeObject-inl.h:560
#3  ReadableStreamCreateReadResult (cx=<optimized out>, value=..., done=true, forAuthorCode=js::ForAuthorCodeBool::Yes) at js/src/builtin/Stream.cpp:1538
#4  0x56b3fb12 in ReadableStreamCloseInternal (cx=<optimized out>, unwrappedStream=...) at js/src/builtin/Stream.cpp:1482
#5  0x56b407f1 in ReadableStreamCancel (cx=<optimized out>, unwrappedStream=..., reason=...) at js/src/builtin/Stream.cpp:1409
#6  0x56b4241b in ReadableStreamReaderGenericCancel (reason=..., unwrappedReader=..., cx=<optimized out>) at js/src/builtin/Stream.cpp:2001
#7  ReadableStreamDefaultReader_cancel (cx=<optimized out>, argc=0, vp=0xf639d830) at js/src/builtin/Stream.cpp:1876
#8  0x567e3c60 in CallJSNative (cx=0xf6e2e800, native=0x56b422d0 <ReadableStreamDefaultReader_cancel(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
#9  0x567d632d in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:535
#10 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#11 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#12 0x569d248e in js::fun_call (cx=0xf6e2e800, argc=1, vp=0xf639dce8) at js/src/vm/JSFunction.cpp:1192
#13 0x567e3c60 in CallJSNative (cx=0xf6e2e800, native=0x569d21c0 <js::fun_call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
#14 0x567d632d in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:535
#15 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#16 0x567d6b0f in js::CallFromStack (cx=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:594
#17 0x56f174c5 in js::jit::DoCallFallback (cx=<optimized out>, frame=0xf639dd28, stub=0xf6757b80, argc=1, vp=0xf639dce8, res=...) at js/src/jit/BaselineIC.cpp:3909
#18 0x35cb93fa in ?? ()
#19 0xf6757b80 in ?? ()
#20 0x35ccb66b in ?? ()
#21 0x35caeb7c in ?? ()
#22 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#23 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#24 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#25 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#26 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#27 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#28 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#29 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, cx@entry=0xf6e2e800, resultPromise=resultPromise@entry=..., generatorVal=..., generatorVal@entry=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
#30 0x568a2107 in AsyncFunctionStart (generatorVal=..., resultPromise=..., cx=0xf6e2e800) at js/src/vm/AsyncFunction.cpp:218
#31 WrappedAsyncFunction (cx=0xf6e2e800, argc=0, vp=0xf639e5a8) at js/src/vm/AsyncFunction.cpp:94
#32 0x35cbde08 in ?? ()
#33 0xf6756220 in ?? ()
#34 0x35ccb66b in ?? ()
#35 0x35caeb7c in ?? ()
[...] (Entering/Leaving JIT repeatedly)
#106 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#107 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#108 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#109 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#110 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#111 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#112 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#113 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, cx@entry=0xf6e2e800, resultPromise=resultPromise@entry=..., generatorVal=..., generatorVal@entry=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
#114 0x568a2107 in AsyncFunctionStart (generatorVal=..., resultPromise=..., cx=0xf6e2e800) at js/src/vm/AsyncFunction.cpp:218
#115 WrappedAsyncFunction (cx=0xf6e2e800, argc=0, vp=0xf63a1a58) at js/src/vm/AsyncFunction.cpp:94
#116 0x35cbde08 in ?? ()
#117 0xf6756220 in ?? ()
#118 0x35ccb66b in ?? ()
#119 0x35caeb7c in ?? ()
#120 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#121 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#122 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#123 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#124 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#125 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#126 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#127 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, resultPromise=..., generatorVal=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
eax	0x0	0
ebx	0x57c1cff4	1472319476
ecx	0xf639d308	-163982584
edx	0xf639d3cc	-163982388
esi	0x57c1cff4	1472319476
edi	0xf639d320	-163982560
ebp	0xf639d2c8	4130984648
esp	0xf639d2c0	4130984640
eip	0x567da915 <JSObject::group() const+21>
=> 0x567da915 <JSObject::group() const+21>:	mov    (%eax),%eax
   0x567da917 <JSObject::group() const+23>:	mov    0xc(%eax),%edx


This crash seems very sensitive, making it hard to reduce the testcase further. It should reproduce reliably though on a 32-bit linux debug build. Marking s-s because of that, until investigated.

Given the stack, it could be some sort of stack space exhaustion causing an early return somewhere.
Attached file Testcase
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
This involves streams. Is this something you can investigate?
Flags: needinfo?(jorendorff)
I configured and built the engine with these options. Unfortunately the crash doesn't reproduce in my Linux VM.

Asking Jeff for help since he's got Linux installed on bare metal. If this doesn't work, I guess the next step is to get access to a fuzzer instance.
Flags: needinfo?(jorendorff) → needinfo?(jwalden)

Testcase didn't fail for me on trunk or at the reported rev with clang r345952, or at the reported rev with gcc 8.0.1. Sounds like time for that fuzzer instance...

Flags: needinfo?(jwalden)

Christian, we're having trouble reproducing this. Can we get access to a fuzzer instance to investigate further?

Flags: needinfo?(choller)

I found the problem: The testcase has two remaining load() calls that I overlooked.

Waldo, can you just edit the testcase, search for the definition of "libdir" and change it to point to your copy of mozilla-central/js/src/jit-test/lib/ ?

Flags: needinfo?(choller) → needinfo?(jwalden)

I have reproduced this. Interestingly, an abort() in ReportOverRecursed does not hit. I didn't have time today to investigate further.

Flags: needinfo?(jorendorff)

Very boring OOM crash. oom_recovery_tco++;

Assignee: nobody → jorendorff
Flags: needinfo?(jorendorff)

Not sec-anything. Crashes at NULL. Patch coming.

Group: javascript-core-security
Priority: -- → P1
Summary: Crash [@ JSObject::hasLazyGroup] with ReadableStream → Crash [@ JSObject::hasLazyGroup] with ReadableStream and OOM
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/847d1877feda
Fix missing OOM check in ReadableStreamCreateReadResult. r=arai
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Is this something we should consider uplifting to Beta for Fx65?

Blocks: 1503324
Flags: needinfo?(jwalden)
Flags: needinfo?(jorendorff)
Flags: in-testsuite+

Yes, the code change is trivial and worth uplifting.

If the new test gives you any trouble, just delete it. It shouldn't, but the test case is more temperamental than I'd like. Or ping me on IRC; it's no trouble to take a second and look at any failures.

Flags: needinfo?(jorendorff)

Comment on attachment 9036045 [details]
Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r?arai

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505122

User impact if declined: possible crash after OOM that we would otherwise handle

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Tiny, well-understood patch adding an error check.

String changes made/needed: none

Attachment #9036045 - Flags: approval-mozilla-beta?

Comment on attachment 9036045 [details]
Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r?arai

[Triage Comment]
Fixes an OOM crash. Covered by automated tests. Approved for 65.0b12.

Attachment #9036045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.