756615 - IonMonkey: OOM Testing: Crash [@ JSFunction::kind]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following command crashes on ionmonkey revision 8c54899dae82 (dbg build):

js  -e 'const libdir = "js/src/jit-test/lib/";' -A 11456 -f js/src/jit-test/tests/debug/Script-gc-03.js

Comment 1

[Christian Holler (:decoder)]

Still seeing this crash, but in a different test now (rev 88ea2e529609):

js -e 'const libdir = "js/src/jit-test/lib/";' -A 7752 -f js/src/jit-test/tests/pic/self8.js

Comment 2

[Jan de Mooij [:jandem]]

This triggers an OOM in ConvertFrames, under

    BailoutClosure *br = cx->new_<BailoutClosure>();

Problem is that js_ReportOutOfMemory calls PopulateReportBlame and this function uses ScriptFrameIter. This fails because we have not yet restored the stack frames...

Marking s-s for now - I haven't looked into this much, but an attacker may be able to use this to access arbitrary stack values.

Comment 6

[Daniel Veditz [:dveditz]]

guessing sec-moderate for now, that the stack values could further other attacks.

Comment 7

[Nicolas B. Pierron [:nbp]]

Attachment: Fix Stack iteration in unsafe locations.

Use a different allocation which does not attempt to iterate over the stack which is currently modified.  Remove pointers to the stack for security reasons and to
ease tracking similar bugs with a SEGV.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 626657 [details] [diff] [review]
Fix Stack iteration in unsafe locations.

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

r=me with nits addressed.

::: js/src/ion/Bailouts.cpp
@@ +222,5 @@
>  
>      // Forbid OSR in the future: bailouts are now expected.
>      it.ionScript()->forbidOsr();
>  
> +    BailoutClosure *br = OffTheBooks::new_<BailoutClosure>();

Please add a small comment here explaining why we use OffTheBooks instead of cx->new_ (in case somebody decides to change it back).

You also have to bump the "OffTheBooks::" count in js/src/Makefile.in to avoid "make check" failures.

Comment 9

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/projects/ionmonkey/rev/003e7b33fd8c