Closed
Bug 557946
Opened 15 years ago
Closed 15 years ago
TM: Crash with malformed typemap in nested trees or "Assertion failure: *(JSObject**)slot == NULL, at ../jstracer.cpp"
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
blocking1.9.2 | --- | .4+ |
status1.9.2 | --- | .4-fixed |
status1.9.1 | --- | unaffected |
People
(Reporter: dvander, Assigned: dvander)
References
Details
(5 keywords, Whiteboard: [sg:critical][ccbr] fixed-in-tracemonkey)
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dmandelin
:
review+
beltzner
:
approval1.9.2.4+
|
Details | Diff | Splinter Review |
<mwu> dvander: http://developer.android.com/reference/java/util/concurrent/Future.html and then click on Exchanger<V> in the lower left frame (in classes)
I can reproduce this, but the trace is fairly complicated so I'm bisecting.
Updated•15 years ago
|
Assignee: general → dvander
Whiteboard: [sg:crit]
Updated•15 years ago
|
Whiteboard: [sg:crit] → [sg:crit][ccbr]
Updated•15 years ago
|
Whiteboard: [sg:crit][ccbr] → [sg:critical][ccbr]
Assignee | ||
Comment 2•15 years ago
|
||
Gah. I just tried reproducing this and can't get it anymore. mwu, do you still see this at all?
Comment 3•15 years ago
|
||
I can still reproduce with those steps. I updated my build 5 days ago from m-c.
Assignee | ||
Comment 4•15 years ago
|
||
Okay, thanks for confirming, I can indeed still get this (I think I was clicking the wrong link or something).
This bug is actually really bad. Test case forthcoming.
Comment 5•15 years ago
|
||
This is security critical and exploitable and affects 1.9.2 and 1.9.3. We should get it into the next dot releases if we can.
Severity: normal → critical
blocking1.9.2: --- → ?
blocking2.0: --- → ?
tracking-fennec: --- → ?
Priority: -- → P2
Comment 6•15 years ago
|
||
Andreas, the bisect pointed to bug 536564 which we haven't taken on mozilla-1.9.2 yet (only landed on mozilla-central on Feb 2, 2010) - is that bisect no longer thought to be the cause? Are we sure that this affects mozilla-1.9.2?
Assignee | ||
Comment 7•15 years ago
|
||
There's an outer function with a variable |m| of type T1. This function has a loop which calls into an inner function that closes over |m|. The closure sets |m| to type T2. There is now a disparity between the FrameInfo view of |m|, which was captured at the point of the call as T1, and SideExits captured later with T2.
Normally that's fine because most trace exits are "shallow" - the exit occurs in the outermost trace, and thus the exit contains the most recent types. The problem here is that the frames are "deep". In this case, we expect that inner functions could not have changed the types in earlier frames, and thus we reconstruct the stack from the FrameInfo.
With closures that's not true. The stack's types don't match and we're tagging values wrong.
Note: The callee guard on function calls makes it slightly harder to test case this. The trigger must occur in the same activation that the tracer was recorded.
The hard fix would be changing LeaveTree to use the innermost nested exit per tree (rather than just the innermost tree). The easy fix is just detecting this mutation and aborting the trace. After talking to Andreas it seems like the easy fix is best, since this is extremely rare in the wild.
Assignee | ||
Comment 8•15 years ago
|
||
(In reply to comment #6)
Correct, the bisect was just noise. This affects 1.9.2 and trunk (but not 1.9.1).
Comment 9•15 years ago
|
||
(In reply to comment #8)
> (In reply to comment #6)
> Correct, the bisect was just noise. This affects 1.9.2 and trunk (but not
> 1.9.1).
Using the testcase in comment 7, it asserts at Assertion failure: *(JSObject**)slot == NULL, at ../jstracer.cpp
autoBisect shows this is probably related to bug 507449:
The first bad revision is:
changeset: 31041:42338f16ac10
user: David Mandelin
date: Tue Aug 04 11:01:13 2009 -0700
summary: Bug 507449: Trace JSOP_GETXPROP, r=gal
Blocks: 507449
OS: Linux → All
Hardware: x86 → All
Summary: TM: crash with malformed typemap in nested trees → TM: Crash with malformed typemap in nested trees or "Assertion failure: *(JSObject**)slot == NULL, at ../jstracer.cpp"
Version: unspecified → Trunk
Comment 10•15 years ago
|
||
Marking "needed" right now; we're going to do a second build for Firefox 3.6.4, and if a patch becomes available and bakes we'll take it for sure, but I'm not sure we'd hold that respin for this.
Andreas: if you think this *must* make it out in the next support release (probably ship mid-May) as opposed to the one following (mid-June), please let me know why.
blocking1.9.2: ? → needed
Assignee | ||
Comment 11•15 years ago
|
||
I'm going to vote that this is not a *must* take, since it's rare.
Comment 12•15 years ago
|
||
Yeah, not about whether or not we'll take it (we will!) more about whether or not we hold out of process plugins (Firefox 3.6.4) until we've got a fix for this. Are you guys working on a fix now, and have a good sense of how long before it might be baked and ready to land on mozilla-1.9.2?
Comment 13•15 years ago
|
||
Mike: My main concern is that we are about to land a fix so we can bake this. Even with a non-obvious commit message, and even if we withhold the test case until we ship, people can probably easily tell its a security fix, since the corresponding bugzilla bug is closed and the patch goes onto branch. Its still not trivial to extract a test case from the fix, so I don't think the world is going to end if we skip 3.6.4, but I do think we would open up a window of opportunity to get zero-day-ed based on the information leak inherent in our work flow.
PS: We will try to land a fix tonight.
Updated•15 years ago
|
blocking1.9.2: needed → .4+
Assignee | ||
Comment 14•15 years ago
|
||
passes trace-tests, STR no longer crash
separate 1.9.2 patch coming, will have dmandelin review that one for a second set of eyes
Attachment #440422 -
Flags: review?(gal)
Updated•15 years ago
|
Attachment #440422 -
Flags: review?(gal) → review+
Assignee | ||
Comment 15•15 years ago
|
||
Whiteboard: [sg:critical][ccbr] → [sg:critical][ccbr] fixed-in-tracemonkey
Assignee | ||
Comment 16•15 years ago
|
||
Attachment #440431 -
Flags: review?(dmandelin)
Assignee | ||
Comment 17•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Attachment #440431 -
Flags: review?(dmandelin) → review+
Assignee | ||
Updated•15 years ago
|
Attachment #440431 -
Flags: approval1.9.2.4?
Comment 18•15 years ago
|
||
Comment on attachment 440431 [details] [diff] [review]
branch fix
a=beltzner, please land on mozilla-1.9.2 as well as GECKO1924_20100413_RELBRANCH
Attachment #440431 -
Flags: approval1.9.2.4? → approval1.9.2.4+
Assignee | ||
Comment 19•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/dd01cd5b746c
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7394bc6de281
Hope I did this right, haven't pushed to a Mercurial branch before.
status1.9.2:
--- → .4-fixed
Comment 20•15 years ago
|
||
(In reply to comment #13)
> since the corresponding bugzilla bug is closed
This can be mitigated by using a cover bug, fwiw - e.g. bug 549349.
Comment 21•15 years ago
|
||
I cannot reproduce this bug on 1.9.2 pre-fix using the original link. I don't get a crash or an assert in my debug builds.
Assignee | ||
Comment 22•15 years ago
|
||
(In reply to comment #21)
Al, it is likely we don't trace that particular site on 1.9.2 (which is why my original bisect was misleading). That is, an old bug being accidentally triggered by a new feature.
Comment 23•15 years ago
|
||
So this is just defense in depth in case there is a way to expose this on 1.9.2?
Assignee | ||
Comment 24•15 years ago
|
||
The attached test case will definitely crash on 1.9.2.
Comment 25•15 years ago
|
||
You're right. Doh.
It crashes the 1.9.2 debug build I made at the build 1 time and it fails to crash the build I built this morning (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100422 Namoroka/3.6.4 (.NET CLR 3.5.30729)).
Verified for 1.9.2.
Keywords: verified1.9.2
Updated•15 years ago
|
status1.9.1:
--- → unaffected
Updated•14 years ago
|
Group: core-security
Updated•14 years ago
|
blocking2.0: ? → betaN+
Comment 27•12 years ago
|
||
Automatically extracted testcase for this bug was committed:
https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
Updated•11 years ago
|
tracking-fennec: ? → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•