GC related crash/asserts: isFunctionFrame() / obj->numSlots() >= first + count

VERIFIED FIXED in mozilla2.0b9

Status

()

defect
--
critical
VERIFIED FIXED
9 years ago
5 years ago

People

(Reporter: decoder, Assigned: Waldo)

Tracking

(Blocks 1 bug, {regression, testcase})

Trunk
mozilla2.0b9
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+, status1.9.2 unaffected, status1.9.1 unaffected)

Details

(Whiteboard: [sg:high?] fixed-in-tracemonkey)

Attachments

(3 attachments)

The attached two testcases trigger two (likely related) assertions with the shell:

Assertion failure: isFunctionFrame(), at ../jsinterp.h:312
Assertion failure: obj->numSlots() >= first + count, at jsfun.cpp:1370

The second assertion isn't always triggered, sometimes also the first one shows up instead. For a reliable run, try running in valgrind also.

Both versions will crash when continuing after the assertions.

The bug might be related to 617139 because these examples also involve infinite recursion and GC triggering. The assertions are different though.
Whiteboard: [sg:critical?]
Assignee: general → jimb
blocking2.0: --- → betaN+
Keywords: testcase
Taking, I've been working with code involving these assertions, so it's probably quickest for me to fix this...
Assignee: jimb → jwalden+bmo
Posted patch PatchSplinter Review
What happens:

  * we call a heavyweight function, which recurs until just short of
    over-recursion, and then...
  * we create a Call object for the (very nested) frame
  * we set the Call object's private to that stack frame
  * we call js::Interpret, which immediately errors out from over-recursion
  * ...and ScriptEpilogue is never called, so js_PutCallObject never unsets
    that Call object's stack frame pointer!
  * now the Call object's stack frame pointer dangles, or points into newer
    frames, or whatever

This doesn't look to be a problem in 192.  See the putActivationObjects call that happens regardless whether js_Interpret succeeded or failed:

http://hg.mozilla.org/releases/mozilla-1.9.2/file/c5b805f21466/js/src/jsinterp.cpp#l1386

It's only a recent problem likely introduced when we added ScriptEpilogue.
Attachment #501570 - Flags: review?(dvander)
Also not a problem in 191, which unconditionally called js_PutCallObject after calling js_Interpret.
Comment on attachment 501570 [details] [diff] [review]
Patch

Nice find.
Attachment #501570 - Flags: review?(dvander) → review+
http://hg.mozilla.org/tracemonkey/rev/b33fddc6066e
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b9
http://hg.mozilla.org/mozilla-central/rev/b33fddc6066e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Keywords: regression
How likely would a web attacker be able to pull off the conditions in comment 4? Is the impact here really "critical" or more "moderate"?
It's easy to pull off those conditions.  What's harder is getting the now-dead call object to be traced.  The recursion error happens early enough that the call object pointer never escapes into script, so either you need to keep it on the stack in a way that it gets reused, you have to "guess" what it is and get it on the stack somehow (easier said than done), or something.  I don't know an easy way to do that, but I don't know there isn't an easy way.  Given that the testcase here was consistently reproducible, I suspect there probably is an easy way.  But that's only halfway: you still need to leverage the failed assertion further.  It might or might not be possible to do that.  But I suspect, given how many more slots you'd be tracing than would be okay, it's probably possible.

I'd say this leans more to critical than moderate, but I could be mistaken.  There's too much complexity of behavior here, none of it in "normal" semantics (well-defined by the relevant function interfaces, that is), to say without investigating a fair bit, I think.
Given the uncertainty bumping the severity down a notch to "high".
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:high?] fixed-in-tracemonkey
Blocks: 676763
Fixed for a long time and not affecting old branches, opening this.
Group: core-security
Status: RESOLVED → VERIFIED
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.