740654 - "Assertion failure: enumerators == cx->enumerators,"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

mjitChunkLimit(31)
o = {}
o.valueOf = function() {
    for (var p in undefined) {
        a = new Function;
    }
    +o;
};
+o;

asserts 32-bit js debug shell *intermittently* on m-c changeset 92fe907ddac8 with -m and -n at Assertion failure: enumerators == cx->enumerators,

May be related to bug 736012. s-s because that bug is s-s.

(gdb) bt 10
#0  0xf776a430 in __kernel_vsyscall ()
#1  0xf773bebe in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42
#2  0x083ade82 in MOZ_Crash () at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:79
#3  0x083adede in MOZ_Assert (s=0x83ed73c "enumerators == cx->enumerators", file=0x83ed5cc "/home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp", ln=453) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/mfbt/Assertions.cpp:88
#4  0x08138a52 in js::CheckStackBalance::~CheckStackBalance (this=0xff864190, __in_chrg=<optimized out>) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:453
#5  0x0813896c in js::RunScript (cx=0x9008d38, script=0xf6c06128, fp=0xf6e60c38) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:455
#6  0x08138d92 in js::InvokeKernel (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:528
#7  0x080930c3 in js::Invoke (cx=0x9008d38, args=..., construct=js::NO_CONSTRUCT) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.h:172
#8  0x08138f55 in js::Invoke (cx=0x9008d38, thisv=..., fval=..., argc=0, argv=0x0, rval=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsinterp.cpp:560
#9  0x08175aab in js::MaybeCallMethod (cx=0x9008d38, obj=0xf6c0c3d0, id=..., vp=0xff8643c8) at /home/fuzz2lin/Desktop/jsfunfuzz-dbg-32-mc-90608-92fe907ddac8/compilePath/js/src/jsobj.cpp:5553
(More stack frames follow...)

(2000+ stacktrace lines follow)

Comment 1

[Luke Wagner [:luke]]

Ah, so the basic issue is that we rejoin the interpreter and a JSOP_ENDITER and then immediately hit the C stack recursion limit.  This jumps to error handling which assumes that, if we are parked on JSOP_ENDITER, we have already executed CloseIterator, so it doesn't unwind the iterator, leaving cx->enumerators invalid.  I'm thinking of what is the least awful fix...

Comment 2

[Luke Wagner [:luke]]

Attachment: fix

After talking with bhackett, we decided that the real culprit here is js::Interpret executing 'error' before jumping to *regs.pc: the assumption in interpreter and mjit stack-unwinding is that, if you throw at pc, you at least started to execute *pc.  That means it is correct for unwinding to not execute CloseIterator when *pc == JSOP_ENDITER: if we started executing JSOP_ENDITER we must have already called CloseIterator.

Comment 3

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3a185f034768

Comment 4

[Daniel Veditz [:dveditz]]

Can content influence the mjitChunkLimit? Is that setting necessary to this bug? Trying to guess whether this is an exploitable bug we need to worry about, and whether we need to back-port to the ESR.

Comment 5

[Christian Holler (:decoder)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> Can content influence the mjitChunkLimit? Is that setting necessary to this
> bug?

Content cannot influence the chunk limit, it is a fixed value there. However, I don't think it's necessary to modify the value, you just need to make the code larger to you hit the boundary in the right place with the default value.

Comment 6

[Luke Wagner [:luke]]

Hitting this bug is extremely difficult.  One, you have to hit the chunk limit in just the right way, but you also have to exhaust the C stack at exactly the right place from the right op, etc.

Comment 7

[Jesse Ruderman]

I wouldn't count on it being hard to exhaust the C stack at exactly the right place.

Comment 8

[Luke Wagner [:luke]]

I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10 only on windows...

Comment 9

[Luke Wagner [:luke]]

Oh, I thought this was a question about backporting to aurora, but comment 4 is asking about ESR.  I don't think bug 732744 is on ESR.

Comment 10

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

(In reply to Luke Wagner [:luke] from comment #8)
> I dunno, it took the fuzzers 6 weeks to find a test-case that repros 1 in 10
> only on windows...

To clarify, this repros only in 32-bit Linux and Windows. I don't think I've managed to reproduce in 64-bit Linux or Mac (both architectures).

Time taken to find a testcase is not a good gauge of how easy / hard it is to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer* to hit the bug.

Comment 11

[Luke Wagner [:luke]]

(In reply to Gary Kwong [:gkw, :nth10sd] from comment #10)
> to hit the bug, it's just a gauge of how easy / hard it is *for the fuzzer*
> to hit the bug.

Sure, in general, but this is a fuzzer-only type of bug.  Given that the bug isn't on a long-term release, it seems fine to let the fix ride the train out.

Comment 12

[Daniel Veditz [:dveditz]]

I think comment 9 is implying this is a regression from bug 732744 which is why we don't need it on ESR.

Taking a SWAG at sg:moderate given comment 6 explaining you'd need to reliably hit two conditions at the same time to exploit.

Comment 13

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

This landed on mozilla-central:

https://hg.mozilla.org/mozilla-central/rev/3a185f034768

Comment 14

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 15

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 17

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397