Closed Bug 746103 Opened 12 years ago Closed 12 years ago

Crash [@ js::types::TypeSet::add] with valgrind errors indicating use-after-free

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla15
Tracking Status
firefox13 --- wontfix
firefox14 + fixed
firefox15 + fixed
firefox-esr10 14+ fixed

People

(Reporter: decoder, Assigned: bhackett1024)

Details

(5 keywords, Whiteboard: [sg:critical][js:p1:fx16][advisory-tracking+][qa?])

Crash Data

Attachments

(3 files)

The following test crashes on mozilla-central revision c61e7c3a232a (options -m -n -a):


gczeal(2);
function testCallProtoMethod() {
    function X() { 
        this.valueOf = new testCallProtoMethod( "return this.value" );
    } 
    X.prototype.getName = function () { return "X"; }
    var a = [new X, new X, new getName, new new this, new Y];
}
testCallProtoMethod();


Although this particular test crashes with a null pointer deref, it seems to be rather a memory corruption (original test crashed with non-null pointer). Also Valgrind shows the following errors:

==5904== Invalid read of size 8
==5904==    at 0x77B4B3: SetPropCompiler::updateMonitoredTypes() (PolyIC.cpp:480)
==5904==    by 0x77BB7E: SetPropCompiler::update() (PolyIC.cpp:613)
==5904==    by 0x7819FE: js::mjit::ic::SetProp(js::VMFrame&, js::mjit::ic::PICInfo*) (PolyIC.cpp:1911)
==5904==    by 0x6D1361: ??? (in /srv/repos/mozilla-central/js/src/debug64/shell/js)
==5904==    by 0x6D28C6: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1052)
[...]
==5904==  Address 0x720c1a8 is 1,704 bytes inside a block of size 1,736 free'd
==5904==    at 0x4C282ED: free (vg_replace_malloc.c:366)
==5904==    by 0x4039DE: js_free (Utility.h:201)
[...]
==5904==    by 0x6D3565: js::mjit::JITScript::destroyChunk(js::FreeOp*, unsigned int, bool) (MethodJIT.cpp:1332)
==5904==    by 0x6D34A0: js::mjit::JITScript::destroy(js::FreeOp*) (MethodJIT.cpp:1319)
==5904==    by 0x6D3A72: JSScript::ReleaseCode(js::FreeOp*, JSScript::JITScriptHandle*) (MethodJIT.cpp:1456)
==5904==    by 0x4846B9: js::mjit::ReleaseScriptCode(js::FreeOp*, JSScript*) (MethodJIT.h:908)
==5904==    by 0x486E5C: JSCompartment::discardJitCode(js::FreeOp*) (jscompartment.cpp:486)
==5904==    by 0x487156: JSCompartment::sweep(js::FreeOp*, bool) (jscompartment.cpp:530)
==5904==
==5904== Invalid read of size 8
==5904==    at 0x4E40E7: js::types::TypeSet::add(JSContext*, js::types::TypeConstraint*, bool) (jsinfer.cpp:396)
==5904==    by 0x4E471A: js::types::TypeSet::addSubset(JSContext*, js::types::TypeSet*) (jsinfer.cpp:534)
==5904==    by 0x77B4C5: SetPropCompiler::updateMonitoredTypes() (PolyIC.cpp:480)
==5904==    by 0x77BB7E: SetPropCompiler::update() (PolyIC.cpp:613)
==5904==    by 0x7819FE: js::mjit::ic::SetProp(js::VMFrame&, js::mjit::ic::PICInfo*) (PolyIC.cpp:1911)
[...]
==5904==    by 0x6D2B56: CheckStackAndEnterMethodJIT(JSContext*, js::StackFrame*, void*, bool) (MethodJIT.cpp:1111)
==5904==  Address 0x71d0568 is 29,000 bytes inside a block of size 131,072 free'd
==5904==    at 0x4C282ED: free (vg_replace_malloc.c:366)
[...]
==5904==    by 0x4C0E31: js::gc::RunDebugGC(JSContext*) (jsgc.cpp:3929)


Marking s-s as this looks like a GC use-after-free.
Whiteboard: js-triage-needed → [sg:critical] js-triage-needed
Does this affect the ESR-10 branch, or is it a more recent regression?
Assignee: general → dmandelin
Christian, what build config are you using? I tried this on OSX 10.6 with -m -n -a on a debug build and got no crash on rev c61e7c3a232a and tip.
(In reply to David Mandelin from comment #2)
> Christian, what build config are you using? I tried this on OSX 10.6 with -m
> -n -a on a debug build and got no crash on rev c61e7c3a232a and tip.

Should be --enable-debug --disable-optimize --enable-valgrind.

Have you tried Valgrind on it? It might not crash on every machine but the Valgrind errors might still be visible.
I reproduced this with m-c changeset 64187d60fae7:

valgrind --smc-check=all-non-file --dsymutil=yes ./js -m -n -a testcase.js

on Mac OS X 10.7 32-bit debug js shell.
Can you attach a full stack for the valgrind logs?
Whiteboard: [sg:critical] js-triage-needed → [sg:critical][js:p1:fx16]
Attached file Valgrind stack
Brian, hope this Valgrind stack helps.
Hi, I'm trying to see what triggers the GC, which requires a deeper stack.  Can you increase the stack depth that valgrind reports, with --num-callers ?
Looks like all the stacks end in generated code

==60901==    by 0x2F64FDF: ???

which presumably doesn't help Brian much.  One thing you could try,
assuming you're on Linux, is to run with valgrind's built in GDB server
enabled.  Then, when you get to the error in question, you can ask GDB
"where", and maybe it can produce a better stack.  GDB has better last-ditch
unwinding heuristics than V does.

To enable the gdb server, give --vgdb-error=0 to V and follow the directions it
prints.  You need to be on Linux and using a recent V (3.7.0, or better the trunk)
Attached patch patchSplinter Review
No, this is enough stack information (thanks).  The problem is that getType() can trigger a GC and jitcode / analysis discarding when constructing the lazy type object for singletons.
Assignee: dmandelin → bhackett1024
Attachment #629161 - Flags: review?(dvander)
Attachment #629161 - Flags: review?(dvander) → review+
Comment on attachment 629161 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): TI
User impact if declined: Potential extremely difficult to trigger security issue.
Risk to taking this patch (and alternatives if risky): None.
Attachment #629161 - Flags: approval-mozilla-beta?
Attachment #629161 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/907a48923ab4
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Because this is TI, assuming ESR is affected.
Comment on attachment 629161 [details] [diff] [review]
patch

[Triage Comment]
Approved for Beta 14 (post-merge) since this is a low risk sg:crit fix.
Attachment #629161 - Flags: approval-mozilla-beta?
Attachment #629161 - Flags: approval-mozilla-beta+
Attachment #629161 - Flags: approval-mozilla-aurora?
Comment on attachment 629161 [details] [diff] [review]
patch

[Approval Request Comment]
Risk to taking this patch (and alternatives if risky):  None
Attachment #629161 - Flags: approval-mozilla-esr10?
Comment on attachment 629161 [details] [diff] [review]
patch

please go ahead and land to ESR.
Attachment #629161 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Whiteboard: [sg:critical][js:p1:fx16] → [sg:critical][js:p1:fx16][advisory-tracking+]
Christian, is there somewhere I can get builds to test this fix for Firefox 14, 15, and ESR 10.0.6? if not, how can I make some?
Whiteboard: [sg:critical][js:p1:fx16][advisory-tracking+] → [sg:critical][js:p1:fx16][advisory-tracking+][qa?]
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #21)
> Christian, is there somewhere I can get builds to test this fix for Firefox
> 14, 15, and ESR 10.0.6? if not, how can I make some?

Right now, we cannot get beta, release or ESR builds because I cannot push these to try.

You could build them manually but then you would require the toolchain to do so on Linux/Mac (install Clang/LLVM). If that's what you want, then I can provide you with a manual.
What about Aurora? I'm not sure I can get the builds built and verified in time, given the current time window, whether I had a manual or not. That said, it would be helpful to have a manual so I can set up an environment on my own and make verification of these bugs simpler in the future.
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #23)
> What about Aurora?

You're lucky :D, I already have nightly aurora builds for ASan at http://people.mozilla.org/~choller/firefox/asan/ :)

> it would be helpful to have a manual so I can set up an environment on
> my own and make verification of these bugs simpler in the future.

Okay. There's a wikipage but it's outdated right now. I have the update ready already but I won't be able to put it there until tomorrow. I'll ping you on IRC tomorrow with the link. If I forget it, please bug me on IRC^^
(In reply to Christian Holler (:decoder) from comment #24)
> (In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #23)
> > What about Aurora?
> 
> You're lucky :D, I already have nightly aurora builds for ASan at
> http://people.mozilla.org/~choller/firefox/asan/ :)

Yeah, I knew about those builds, and I've been using them for some of these ASan builds; thanks for that. I think the problem from where I stand is getting an ASan build which reproduces the crash so that I can trust my verification.
Group: core-security
Not adding the test since it's too unreliable.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.