1263862 - Assertion failure: ssi_.type() == StaticScopeIter<CanGC>::Block, at js/src/vm/ScopeObject.cpp:1506 with OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 29d5a4175c8b (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --ion-eager):

function loadFile(lfVarx) oomTest(function() eval(lfVarx))
for (var i = 0; i < 10; ++i) {
  loadFile(`
    "use strict"
    const s = () => s
  `);
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000af3dea in js::ScopeIter::settle (this=this@entry=0x7fffffff9c40) at js/src/vm/ScopeObject.cpp:1506
#0  0x0000000000af3dea in js::ScopeIter::settle (this=this@entry=0x7fffffff9c40) at js/src/vm/ScopeObject.cpp:1506
#1  0x0000000000af4124 in js::ScopeIter::ScopeIter(JSContext*, js::AbstractFramePtr, unsigned char*, mozilla::detail::GuardObjectNotifier&&) (this=0x7fffffff9c40, cx=0x7ffff6908800, frame=..., pc=<optimized out>, _notifier=<unknown type in /home/decoder/LangFuzz/work/remote/builds/debug64/dist/bin/js, CU 0x3f376e9, DIE 0x4146be8>) at js/src/vm/ScopeObject.cpp:1457
#2  0x000000000080fe0e in js::jit::DebugEpilogue (cx=cx@entry=0x7ffff6908800, frame=frame@entry=0x7fffffffa398, pc=0x7ffff69a53cc <incomplete sequence \306>, ok=ok@entry=false) at js/src/jit/VMFunctions.cpp:698
#3  0x00000000006b9f6c in js::jit::OnLeaveBaselineFrame (cx=cx@entry=0x7ffff6908800, frame=..., pc=<optimized out>, rfe=rfe@entry=0x7fffffffa330, frameOk=frameOk@entry=false) at js/src/jit/JitFrames.cpp:463
#4  0x00000000006febbe in HandleExceptionBaseline (pc=0x7ffff69a53cc <incomplete sequence \306>, rfe=<optimized out>, frame=..., cx=0x7ffff6908800) at js/src/jit/JitFrames.cpp:696
#5  js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:837
#6  0x00007ffff7fe8608 in ?? ()
[...]
#22 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffffff9c40	140737488329792
rcx	0x7ffff6ca588d	140737333844109
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffff9ba0	140737488329632
rsp	0x7fffffff9b80	140737488329600
r8	0x7ffff7fdf7c0	140737354004416
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffff9940	140737488329024
r11	0x7ffff6c27ee0	140737333329632
r12	0x7fffffff9c78	140737488329848
r13	0x7fffffff9bc0	140737488329664
r14	0x7fffffff9c30	140737488329776
r15	0x7ffff6908830	140737330055216
rip	0xaf3dea <js::ScopeIter::settle()+1306>
=> 0xaf3dea <js::ScopeIter::settle()+1306>:	movl   $0x5e2,0x0
   0xaf3df5 <js::ScopeIter::settle()+1317>:	callq  0x4ab6f0 <abort()>


Marking s-s for now because I remember this assert to be security-related in some cases.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20151013053056" and the hash "8d9c20c241be7d7b3cfa90a3368a77db42172781".
The "bad" changeset has the timestamp "20151013054956" and the hash "d80f9d6921f8209ef01aa730be9a97ab727704d1".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8d9c20c241be7d7b3cfa90a3368a77db42172781&tochange=d80f9d6921f8209ef01aa730be9a97ab727704d1

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: OOM_VERBOSE=1 stack from m-c rev d53b301a14e1

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

JIT assembler stuff is on the OOM_VERBOSE=1 stack, setting needinfo? from Jan/Hannes as a start.

Comment 4

[Jan de Mooij [:jandem]]

Slightly simpler test below, fails with --baseline-eager.

Shu, we fail to create a strict eval CallObject for a BaselineFrame. Then the exception handler calls OnLeaveBaselineFrame -> jit::DebugEpilogue, where we create a ScopeIter. Then we assert in ScopeIter::settle because ssi_.type() is Eval instead of Block.

  function loadFile(lfVarx) {
      oomTest(() => eval(lfVarx));
  }
  for (var i = 0; i < 10; ++i) {
      loadFile(`"use strict"; const s = () => s;`);
  }

Comment 5

[Andrew McCreight [:mccr8]]

How bad of a security issue is this? Should we just mark it sec-high or is it maybe okay?

Comment 6

[Shu-yu Guo [:shu]]

Why is this s-s to begin with, actually?

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Jan de Mooij [:jandem] from comment #4)
> Slightly simpler test below, fails with --baseline-eager.
> 
> Shu, we fail to create a strict eval CallObject for a BaselineFrame. Then
> the exception handler calls OnLeaveBaselineFrame -> jit::DebugEpilogue,
> where we create a ScopeIter. Then we assert in ScopeIter::settle because
> ssi_.type() is Eval instead of Block.
> 

I kinda think now that safe OOM handling is a pipe dream. I don't see any value in dealing with corner cases like this.

Comment 8

[Shu-yu Guo [:shu]]

Attachment: Fix OOM case in ScopeIter::settle.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8743107 [details] [diff] [review]
Fix OOM case in ScopeIter::settle.

Review of attachment 8743107 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.

Comment 10

[Daniel Veditz [:dveditz]]

(In reply to Shu-yu Guo [:shu] from comment #7)
> I kinda think now that safe OOM handling is a pipe dream. I don't see any
> value in dealing with corner cases like this.

Safe recovering from OOM is a pipe dream, but turning allocations failed due to OOM into exploits is reality. It's fine to throw up our hands and suicide in corner cases, but not ignore them.

Comment 11

[Shu-yu Guo [:shu]]

(In reply to Daniel Veditz [:dveditz] from comment #10)
> (In reply to Shu-yu Guo [:shu] from comment #7)
> > I kinda think now that safe OOM handling is a pipe dream. I don't see any
> > value in dealing with corner cases like this.
> 
> Safe recovering from OOM is a pipe dream, but turning allocations failed due
> to OOM into exploits is reality. It's fine to throw up our hands and suicide
> in corner cases, but not ignore them.

FWIW this bug isn't a security issue.

Comment 12

[Shu-yu Guo [:shu]]

Setting checkin-needed until my SSH key is updated.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0580b9b6ad16

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70bfcf9f09d5

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Pushed a follow-up: bail out if oomTest is not defined: https://hg.mozilla.org/integration/mozilla-inbound/rev/70bfcf9f09d5

Shu, are you okay with that?

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0580b9b6ad16
https://hg.mozilla.org/mozilla-central/rev/70bfcf9f09d5

Comment 17

[Shu-yu Guo [:shu]]

(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #15)
> Pushed a follow-up: bail out if oomTest is not defined:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/70bfcf9f09d5
> 
> Shu, are you okay with that?

Yep, thanks!