842300 - IonMonkey: Crash [@ js::GetObjectClass] with invalid read

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 0acbd06d48a9 (run with --ion-eager):


evaluate("\
  var x = function g() {\
    return g.toString();\
  };\
  x('y')\
", { noScriptRval : true });
evaluate("\
  x('y')\
", { noScriptRval : true });

Comment 1

[Christian Holler (:decoder)]

Debug trace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004050bb in js::GetObjectClass (obj=(js::RawObject) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6) at ../../jsfriendapi.h:366
366         return reinterpret_cast<const shadow::Object*>(obj)->type->clasp;
(gdb) bt 8
#0  0x00000000004050bb in js::GetObjectClass (obj=(js::RawObject) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6) at ../../jsfriendapi.h:366
#1  0x0000000000405337 in js::IsProxy (obj=(js::RawObject) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6) at ../../jsproxy.h:239
#2  0x0000000000409c4e in JSObject::isProxy (this=(const JSObject * const) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6) at ../../jsobjinlines.h:1296
#3  0x0000000000c69b7f in IsCacheableListBase (obj=(JSObject *) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6) at /srv/repos/mozilla-central/js/src/ion/IonCaches.cpp:89
#4  0x0000000000c6f029 in TryAttachNativeGetPropStub (cx=0x13bae90, ion=0x1449e10, cache=..., obj=(JSObject * const) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6, name="toString", safepointIndex=
    0x1449fd0, returnAddr=0x7ffff7fed873, isCacheable=0x7fffffff510f) at /srv/repos/mozilla-central/js/src/ion/IonCaches.cpp:855
#5  0x0000000000c6fbf7 in js::ion::GetPropertyCache (cx=0x13bae90, cacheIndex=0, obj=(JSObject * const) 0x7ffff602d381 Cannot access memory at address 0x5000007ffff603b6, vp=JSVAL_VOID)
    at /srv/repos/mozilla-central/js/src/ion/IonCaches.cpp:964
#6  0x00007ffff7fe9e84 in ?? ()
#7  0x00007fffffff51b0 in ?? ()
(More stack frames follow...)
(gdb) x /i $pc
=> 0x4050bb <js::GetObjectClass(JSObject*)+16>: mov    (%rax),%rax
(gdb) info reg rax
rax            0x13d76  81270
(gdb) l
363     inline js::Class *
364     GetObjectClass(RawObject obj)
365     {
366         return reinterpret_cast<const shadow::Object*>(obj)->type->clasp;
367     }


Corrupted obj pointer, marking sec-critical.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   121830:507316db9c59
user:        David Anderson
date:        Wed Feb 13 17:24:50 2013 -0800
summary:     Implement JSOP_CALLEE in JM (bug 794427, r=bhackett).

This iteration took 123.461 seconds to run.

Comment 3

[Christian Holler (:decoder)]

Needinfo from dvander based on comment 2 :)

Comment 4

[Christian Holler (:decoder)]

Simpler test for the testsuite:

var x = function g() {
  return g.toString();
};
for ( var i = 0; i < 3; i++ ) 
  x(i)

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

This was actually a pre-existing bug in IonMonkey, exposed much easier now that JM can compile JSOP_CALLEE.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Easy fix, when inlining make sure we read the innermost callee instead of the outermost.

Comment 7

[Andrew McCreight [:mccr8]]

Updating tracking flags based on comment 5 (that this is a preexisting Ionmonkey bug, so 17 and b2g are unaffected).

Comment 8

[Daniel Veditz [:dveditz]]

Would be nice to land in Beta too if the patch is low risk.

Comment 9

[Nicolas B. Pierron [:nbp — off until 29-09]]

Comment on attachment 718684 [details] [diff] [review]
fix

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

sounds good to me.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 718684 [details] [diff] [review]
fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not easily.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No.

Which older supported branches are affected by this flaw?

19 through 22.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

This patch should apply.

How likely is this patch to cause regressions; how much testing does it need?

Unlikely; tbpl+awfy is fine.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

This needs to land and then get nominated for uplift before Tues Mar 12th when we go to build on beta 5.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

This doesn't apply cleanly to inbound. Please rebase.

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4db85af71a2c

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4db85af71a2c

Comment 15

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 16

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 718684 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): IonMonkey
User impact if declined: potential sg:crit
Testing completed (on m-c, etc.): Yes
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 17

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/releases/mozilla-aurora/rev/333a93611b4a
https://hg.mozilla.org/releases/mozilla-beta/rev/fc49b18945ae