User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/10.10 (maverick) Firefox/3.6.12 Build Identifier: The following minimized testcase (tested with JS from mozilla-2.0b8 and tip) causes an assertion in the shell: gczeal(2); test(); function test() { function g(){ test(); yield; } for (i in g()) { } } Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit - 4096, &stackDummy), at jsgc.cpp:2558 Reproducible: Always

FWIW, another test case, same assert: --- gczeal(2) function x() { [null].some(x) } x(); ---

I tried a bisect and found that in older revisions, the testcase here causes a different assertion, namely: Assertion failure: !JSID_IS_VOID(lastProp->id), at ../jsscope.h:678 This was introduced here: Changeset 54707: bad The first bad revision is: changeset: 54707 parent: 54627 user: Gregor Wagner <removed> date: Fri Sep 24 10:54:39 2010 -0700 summary: Bug 558861 - Compartmental GC (r=gal) I did another bisect then for when the assertion changed to the one described in this bug: changeset: 55509 user: Gregor Wagner <removed> date: Tue Oct 05 10:09:50 2010 -0700 summary: Bug 600310 - TM: don't perform GC outside of stack quota r=igor

Bug 600310 suggests that this is security relevant. I ran the testcase again with debugging but without abort() on assert and I get this: Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy), at ../jsgc.cpp:2680 Assertion failure: !JSID_IS_VOID(lastProp->id), at ../jsscope.h:678 Assertion failure: nslots <= dslots[-1].toPrivateUint32(), at ../jsobj.cpp:3810 /scratch/holler/LangFuzz/mozroot.ff4.dbg/work/assertCheckStackSize/min.js:10: out of memory Assertion failure: addr % sizeof(FreeCell) == 0, at ../../jsgc.h:311 Program received signal SIGSEGV, Segmentation fault. 0x0000000000544ff6 in GetGCThingTraceKind (thing=0xdadadadadadadada) at ../jsgc.h:504 504 return GetFinalizableTraceKind(cell->arena()->header()->thingKind); (gdb) bt #0 0x0000000000544ff6 in GetGCThingTraceKind (thing=0xdadadadadadadada) at ../jsgc.h:504 #1 0x000000000054960a in Mark<JSObject> (trc=0x7ffffff82990, thing=0xdadadadadadadada) at ../jsgcinlines.h:139 #2 0x00000000005493e9 in js::Shape::trace (this=0xa66030, trc=0x7ffffff82990) at ../jsscope.cpp:1371 #3 0x00000000004e1cba in JSObject::trace (this=0x7ffff690f5f0, trc=0x7ffffff82990) at ../jsscopeinlines.h:137 #4 0x00000000004de57d in js_TraceObject (trc=0x7ffffff82990, obj=0x7ffff690f5f0) at ../jsobj.cpp:6214 #5 0x000000000049e116 in MarkChildren (trc=0x7ffffff82990, obj=0x7ffff690f5f0) at ../jsgcinlines.h:204 #6 0x00000000004a8b0d in js::gc::Arena<JSObject>::markDelayedChildren (this=0x7ffff690f000, trc=0x7ffffff82990) at ../jsgc.cpp:1432 #7 0x00000000004a08a3 in js::GCMarker::markDelayedChildren (this=0x7ffffff82990) at ../jsgc.cpp:1451 #8 0x00000000004a1dfe in MarkAndSweep (cx=0xa63000, gckind=GC_NORMAL) at ../jsgc.cpp:2289 #9 0x00000000004a229b in GCUntilDone (cx=0xa63000, gckind=GC_NORMAL) at ../jsgc.cpp:2639 #10 0x00000000004a2412 in js_GC (cx=0xa63000, gckind=GC_NORMAL) at ../jsgc.cpp:2702 #11 0x00000000004a7faf in RefillFinalizableFreeList<JSString> (cx=0xa63000, thingKind=4) at ../jsgc.cpp:1230 #12 0x00000000004349cb in NewFinalizableGCThing<JSString> (cx=0xa63000, thingKind=4) at ../jsgcinlines.h:83 #13 0x000000000055cc28 in js_NewGCString (cx=0xa63000) at ../jsgcinlines.h:100 #14 0x000000000055842e in js_NewString (cx=0xa63000, chars=0xa713a0, length=18) at ../jsstr.cpp:3420 #15 0x000000000042c673 in JS_NewStringCopyZ (cx=0xa63000, s=0xa7c5c0 "too much recursion") at ../jsapi.cpp:5037 #16 0x000000000048fe80 in js_ErrorToException (cx=0xa63000, message=0xa7c5c0 "too much recursion", reportp=0x7ffffff82e30, callback=0x458f22 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0) at ../jsexn.cpp:1188 #17 0x0000000000457a4c in ReportError (cx=0xa63000, message=0xa7c5c0 "too much recursion", reportp=0x7ffffff82e30, callback=0x458f22 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0) at ../jscntxt.cpp:1330 #18 0x00000000004588e5 in js_ReportErrorNumberVA (cx=0xa63000, flags=0, callback=0x458f22 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=26, charArgs=1, ap=0x7ffffff82ee0) at ../jscntxt.cpp:1683 #19 0x000000000042d218 in JS_ReportErrorNumber (cx=0xa63000, errorCallback=0x458f22 <js_GetErrorMessage(void*, char const*, unsigned int)>, userRef=0x0, errorNumber=26) at ../jsapi.cpp:5377 #20 0x0000000000457d76 in js_ReportOverRecursed (cx=0xa63000) at ../jscntxt.cpp:1420 #21 0x0000000000683fe5 in js::Interpret (cx=0xa63000, entryFrame=0x7ffff6ac4030, inlineCallCount=0, interpFlags=0) at ../jsinterp.cpp:2203 #22 0x00000000004b7542 in js::RunScript (cx=0xa63000, script=0xa82a80, fp=0x7ffff6ac4030) at ../jsinterp.cpp:642 I cut the backtrace here because it's quite long but it doesn't look too healthy ;)

I think gczeal is a shell-only function. Is there a variant of these test cases that can trip the stack assert in the browser?

(In reply to comment #5) > I think gczeal is a shell-only function. Is there a variant of these test cases > that can trip the stack assert in the browser? Or, equally seriously, does the underlying problem affect the browser already, and the gczeal is needed only to trip the assert rather than silently overflowing? I.e., is this a browser security threat or not?

(In reply to comment #5) > I think gczeal is a shell-only function. Is there a variant of these test cases > that can trip the stack assert in the browser? I think the gczeal here is only required so the gc actually tries to collect immediately. If this is the case, then it's possible to trigger this in browser even under normal conditions (by causing a GC through something else, like allocating a lot of different things). I just talked to Jesse and he also said that gczeal at a high level will only cause the GC to happen everytime a GC "could" happen. So it should be relevant for browser.

You can enable gczeal in the browser (javascript.options.gczeal?). Web pages can't, but assume a web page can cause GC to happen at the "right" moment by allocating exactly the correct number of objects before triggering the buggy code. If a bug can be exploited with gczeal, it can probably be exploited without gczeal.

This seems to be just another version of bug 605011, where we end up using up even more stack space trying to generate the exception and starting GC. 8192 proved not to be enough: it still crashes about 1/4 the time with that setting.

Comment on attachment 500293 [details] [diff] [review] Patch >diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp > # if JS_STACK_GROWTH_DIRECTION > 0 > /* cx->stackLimit is set to jsuword(-1) by default. */ > JS_ASSERT_IF(cx->stackLimit != jsuword(-1), >- JS_CHECK_STACK_SIZE(cx->stackLimit + 4096, &stackDummy)); >+ JS_CHECK_STACK_SIZE(cx->stackLimit + (1 << 14), &stackDummy)); > # else > /* -4k because it is possible to perform a GC during an overrecursion report. */ >- JS_ASSERT_IF(cx->stackLimit, JS_CHECK_STACK_SIZE(cx->stackLimit - 4096, &stackDummy)); >+ JS_ASSERT_IF(cx->stackLimit, JS_CHECK_STACK_SIZE(cx->stackLimit - (1 << 14), &stackDummy)); > # endif > #endif Nit: update the comment to reflect the new constant.

All you changed here is the assert condition? Is there no real problem here and it was a false-alarm assert? That seems to be how bug 605011 was judged.

How could only the assertion be wrong here if the code crashes when continuing through the assertions?

(In reply to comment #12) > All you changed here is the assert condition? Is there no real problem here and > it was a false-alarm assert? That seems to be how bug 605011 was judged. IIUC, what is happening is that we get near the stack limit due to infinite recursion, so we start to throw an error. Then, during processing the error, we call a bunch of functions, growing the stack more, and then do the check again, this time asserting because we are over the limit. But the error generation process allocates a fixed additional amount of stack, so it won't walk all over everything. I think Igor knows more about this. (In reply to comment #13) > How could only the assertion be wrong here if the code crashes when continuing > through the assertions? On the test case in comment 0, I get no crash: $ shell/js /c/sources/scratch/b.js c:/sources/scratch/b.js:4: InternalError: too much recursion

(In reply to comment #14) > On the test case in comment 0, I get no crash: > > $ shell/js /c/sources/scratch/b.js > c:/sources/scratch/b.js:4: InternalError: too much recursion When running the debug version with gdb and using "handle SIGABRT nopass", you'll get a crash when continuing (see my backtrace above). Either this is due to the assertions being executed, or it's coincidence that the optimized build doesnt crash. I'll try valgrind as well...

decoder: how much memory do you have? dmandelin: how much memory do you have? > I'll try valgrind as well... That's a good idea here.

(In reply to comment #16) > decoder: how much memory do you have? The trace was produced on an x86-64 compute server with 10 GB of memory. I just tried to reproduce exactly the same again and it didn't work (I got the same message that dmandelin got). I updated the mc trunk though in the mean time, I guess some code change(s) cause the test to behave differently now. I really hate GC related bugs^^ I'll try again to reproduce the crash from above somehow. > > > I'll try valgrind as well... > > That's a good idea here. didn't show up anything unfortunately..

I reproduced the crash. It seems that I mistakenly took revision 55509 (the revision introducing the assertion) for performing that test. So it could be that this crash is due to another bug that was fixed later on. I could try a bisect there and see where it changes. Anyways, the test doesn't crash anymore on tip/mc.

marking sg:nse per comment 9. let me know if I've misjudged.