Closed Bug 756615 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: nbp)

References

Details

(Keywords: crash, sec-moderate, testcase)

Crash Data

Attachments

(1 file)

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
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
Group: core-security
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.
Duplicate of this bug: 756619
Duplicate of this bug: 756629
Duplicate of this bug: 756630
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
guessing sec-moderate for now, that the stack values could further other attacks.
Keywords: sec-moderate
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.
Attachment #626657 - Flags: review?(jdemooij)
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.
Attachment #626657 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/003e7b33fd8c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.