686107 - Crash [@ JSC::MacroAssemblerCodePtr::executableAddress()]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test crashes on mozilla-central revision d078623f7875 (options -m -n -a), tested on 64 bit:


function testKeyTransitions() {
  var i, key, result, message;
  var array = [];
  for (i = 0; i != 10; i++) {
    key = (i < 3) ? 'pop' : (/\u009e\u0029/g );
    array[key](i);
  }
}
testKeyTransitions();


Looks like a null-pointer deref, but s-s to be sure:

==39098== Invalid read of size 8
==39098==    at 0x4423CC: JSC::MacroAssemblerCodePtr::executableAddress() const (MacroAssemblerCodeRef.h:150)
==39098==    by 0x76FAF1: JSC::RepatchBuffer::RepatchBuffer(JSC::MacroAssemblerCodeRef const&) (RepatchBuffer.h:54)
==39098==    by 0x76FBD6: js::mjit::ic::Repatcher::Repatcher(js::mjit::JITScript*) (ICRepatcher.h:64)
==39098==    by 0x77806C: js::mjit::ic::BaseIC::disable(JSContext*, char const*, void*) (PolyIC.cpp:2233)
==39098==    by 0x77830A: js::mjit::ic::GetElementIC::disable(JSContext*, char const*) (PolyIC.cpp:2290)
==39098==    by 0x77AF86: js::mjit::ic::GetElementIC::update(js::VMFrame&, JSContext*, JSObject*, js::Value const&, jsid, js::Value*) (PolyIC.cpp:2757)
==39098==    by 0x77B131: js::mjit::ic::CallElement(js::VMFrame&, js::mjit::ic::GetElementIC*) (PolyIC.cpp:2788)
==39098==    by 0x6DF341: ??? (MethodJIT.cpp:153)
==39098==    by 0x6DF5DB: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, js::Value*, bool) (MethodJIT.cpp:884)
==39098==    by 0x6DF800: CheckStackAndEnterMethodJIT(JSContext*, js::StackFrame*, void*, bool) (MethodJIT.cpp:945)
==39098==    by 0x6DF950: js::mjit::JaegerShotAtSafePoint(JSContext*, void*, bool) (MethodJIT.cpp:972)
==39098==    by 0x502554: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2206)
==39098==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Comment 1

[David Mandelin [:dmandelin]]

Brian, can you take a look at this to see if it's a vulnerability?

Comment 2

[Christian Holler (:decoder)]

Here's a bisect for the regressing changeset (possibly inaccurate):

The first bad revision is:
changeset:   76087:d763fda00eb9
user:        Brian Hackett
date:        Mon Aug 01 09:09:39 2011 -0700
summary:     [INFER] Only allow expansion of all inline frames in a compartment, bug 675251.


However, this test no longer reproduces on mozilla-central tip. Here's a bisect for a possible fix:

The first good revision is:
changeset:   77145:b961a248e94d
user:        Brian Hackett
date:        Sat Sep 17 19:31:33 2011 -0700
summary:     [INFER] Reapply f1c585415dd4 7c89b0ff453d 19794de530f1 (bug 686000).


Not sure if this is really fixed though, could also be that the second changeset just breaks the test. I guess Brian can figure that out :)


I also tested the latest aurora JS shell and cannot reproduce there.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: patch

Sorry, missed this bug when it was first filed.

Recompilation bug under ic::CallElement, if js_InternNonIntElementId triggers recompilation then we would still try to disable the IC afterwards.  ic::GetElement already does the right thing, I don't know why I didn't fix this when I fixed that one.

Comment 5

[Christian Holler (:decoder)]

I made this sg:critical? because bug 692065 is a duplicate of this and shows use-after-free with invalid reads/writes so this could be exploitable.

Comment 6

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7277c1bddd22

Comment 7

[Asa Dotzler [:asa]]

Trigger happy. I approved this and it hasn't hit m-c yet. Please hold off on landing this until it's made a cycle or three on m-c with no fallout.

Comment 8

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/mozilla-central/rev/7277c1bddd22

Comment 9

[Daniel Veditz [:dveditz]]

I'm assuming this doesn't affect Firefox 8?

Comment 10

[Brian Hackett [Laid off!]]

(In reply to Daniel Veditz from comment #9)
> I'm assuming this doesn't affect Firefox 8?

No, this doesn't.

Comment 11

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9c3b0542705c

Comment 12

[Brian Hackett [Laid off!]]

Oops, not sure how I reset those bits.

Comment 13

[u279076]

Christian, can you please verify if this is fixed?

Comment 14

[Christian Holler (:decoder)]

Verified fixed on Firefox 9 and 10.

Comment 15

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929