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

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
critical
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: decoder, Assigned: jorendorff)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla66
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 fixed, firefox66 fixed)

Details

(Whiteboard: [jsbugmon:], crash signature)

Attachments

(2 attachments)

Reporter

Description

6 months ago
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.
Reporter

Comment 1

6 months ago
Posted file Testcase

Updated

6 months ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]

Comment 2

6 months ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Updated

6 months ago
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
This involves streams. Is this something you can investigate?
Flags: needinfo?(jorendorff)
Assignee

Comment 4

6 months ago
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)
Assignee

Comment 6

6 months ago

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

Flags: needinfo?(choller)
Reporter

Comment 7

6 months ago

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)
Assignee

Comment 8

6 months ago

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

Assignee

Updated

6 months ago
Flags: needinfo?(jorendorff)
Assignee

Comment 9

6 months ago

Very boring OOM crash. oom_recovery_tco++;

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

Comment 10

6 months ago

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

Comment 14

5 months ago
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/847d1877feda
Fix missing OOM check in ReadableStreamCreateReadResult. r=arai

Comment 15

5 months ago
bugherder
Status: NEW → RESOLVED
Closed: 5 months 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+
Assignee

Comment 17

5 months ago

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)
Assignee

Comment 18

5 months ago

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.