Last Comment Bug 766018 - Use-after-free during GC on shutdown
: Use-after-free during GC on shutdown
Status: RESOLVED FIXED
[advisory-tracking+][qa-]
: crash, sec-moderate
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla16
Assigned To: Bobby Holley (PTO through June 13)
:
Mentors:
Depends on:
Blocks: 755255
  Show dependency treegraph
 
Reported: 2012-06-18 21:17 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-14 08:13 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
+
fixed
wontfix


Attachments
Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1 (2.31 KB, patch)
2012-06-19 04:36 PDT, Bobby Holley (PTO through June 13)
peterv: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-18 21:17:04 PDT
STR
 (1) Make b2g "desktop" build: https://wiki.mozilla.org/Gaia/Hacking#Building_B2G
 (2) Start up b2g
 (3) Shut it down

I crash during shutdown 100% of the time.  valgrind tells me the problem is here

==24266== Invalid read of size 4
==24266==    at 0x94970C2: nsBaseHashtable<nsDepCharHashKey, JSObject*, JSObject*>::IsInitialized() const (nsBaseHashtable.h:84)
==24266==    by 0x949694B: XPCWrappedNativeScope::TraceDOMPrototypes(JSTracer*) (XPCWrappedNativeScope.cpp:777)
==24266==    by 0x9420F23: TraceXPCGlobal(JSTracer*, JSObject*) (nsXPConnect.cpp:1047)
==24266==    by 0xA7B1F7F: js::GCMarker::processMarkStackTop(js::SliceBudget&) (Marking.cpp:1181)
==24266==    by 0xA7B2162: js::GCMarker::drainMarkStack(js::SliceBudget&) (Marking.cpp:1225)
==24266==    by 0xA5A4F00: MarkGrayAndWeak(JSRuntime*) (jsgc.cpp:3130)
==24266==    by 0xA5A4FEC: EndMarkPhase(JSRuntime*) (jsgc.cpp:3146)
==24266==    by 0xA5A6199: NonIncrementalMark(JSRuntime*, js::JSGCInvocationKind) (jsgc.cpp:3435)
==24266==    by 0xA5A7135: GCCycle(JSRuntime*, bool, long, js::JSGCInvocationKind) (jsgc.cpp:3779)
==24266==    by 0xA5A762D: Collect(JSRuntime*, bool, long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:3886)
==24266==    by 0xA5A7771: js::GC(JSRuntime*, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:3910)
==24266==    by 0xA566EC6: js::DestroyContext(JSContext*, js::DestroyContextMode) (jscntxt.cpp:316)
==24266==    by 0xA521A44: JS_DestroyContext (jsapi.cpp:1164)
==24266==    by 0x941E696: nsXPConnect::~nsXPConnect() (nsXPConnect.cpp:114)
==24266==    by 0x941E7AD: nsXPConnect::~nsXPConnect() (nsXPConnect.cpp:128)
==24266==    by 0x941E1BF: nsXPConnect::Release() (in /home/cjones/mozilla/gaia-dbg/toolkit/library/libxul.so)
==24266==    by 0x941EBA2: nsXPConnect::ReleaseXPConnectSingleton() (nsXPConnect.cpp:221)
==24266==    by 0x94693A7: xpcModuleDtor() (XPCModule.cpp:25)
==24266==    by 0x862735F: LayoutModuleDtor() (nsLayoutModule.cpp:1202)
==24266==    by 0x9D777BC: nsComponentManagerImpl::KnownModule::~KnownModule() (nsComponentManager.h:175)
==24266==    by 0x9D80576: nsAutoPtr<nsComponentManagerImpl::KnownModule>::~nsAutoPtr() (nsAutoPtr.h:71)
==24266==    by 0x9D801C8: nsTArrayElementTraits<nsAutoPtr<nsComponentManagerImpl::KnownModule> >::Destruct(nsAutoPtr<nsComponentManagerImpl::KnownModule>*) (nsTArray.h:348)
==24266==    by 0x9D7F888: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) (nsTArray.h:1211)
==24266==    by 0x9D7E89F: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) (nsTArray.h:931)
==24266==    by 0x9D7D670: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::Clear() (nsTArray.h:942)
==24266==    by 0x9D79CBD: nsComponentManagerImpl::Shutdown() (nsComponentManager.cpp:737)
==24266==    by 0x9D24EDF: mozilla::ShutdownXPCOM(nsIServiceManager*) (nsXPComInit.cpp:680)
==24266==    by 0x9D2490F: NS_ShutdownXPCOM_P (nsXPComInit.cpp:542)
==24266==    by 0x8331AF5: ScopedXPCOMStartup::~ScopedXPCOMStartup() (nsAppRunner.cpp:1103)
==24266==    by 0x833B394: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:3875)
==24266==  Address 0x26c842c4 is 132 bytes inside a block of size 168 free'd
==24266==    at 0x4C2A403: free (vg_replace_malloc.c:427)
==24266==    by 0x687DFA1: moz_free (mozalloc.cpp:48)
==24266==    by 0x949588B: XPCWrappedNativeScope::~XPCWrappedNativeScope() (mozalloc.h:224)
==24266==    by 0x9496110: XPCWrappedNativeScope::KillDyingScopes() (XPCWrappedNativeScope.cpp:521)
==24266==    by 0x9496328: XPCWrappedNativeScope::SystemIsBeingShutDown() (XPCWrappedNativeScope.cpp:604)
==24266==    by 0x941E67A: nsXPConnect::~nsXPConnect() (nsXPConnect.cpp:111)
==24266==    by 0x941E7AD: nsXPConnect::~nsXPConnect() (nsXPConnect.cpp:128)
==24266==    by 0x941E1BF: nsXPConnect::Release() (in /home/cjones/mozilla/gaia-dbg/toolkit/library/libxul.so)
==24266==    by 0x941EBA2: nsXPConnect::ReleaseXPConnectSingleton() (nsXPConnect.cpp:221)
==24266==    by 0x94693A7: xpcModuleDtor() (XPCModule.cpp:25)
==24266==    by 0x862735F: LayoutModuleDtor() (nsLayoutModule.cpp:1202)
==24266==    by 0x9D777BC: nsComponentManagerImpl::KnownModule::~KnownModule() (nsComponentManager.h:175)
==24266==    by 0x9D80576: nsAutoPtr<nsComponentManagerImpl::KnownModule>::~nsAutoPtr() (nsAutoPtr.h:71)
==24266==    by 0x9D801C8: nsTArrayElementTraits<nsAutoPtr<nsComponentManagerImpl::KnownModule> >::Destruct(nsAutoPtr<nsComponentManagerImpl::KnownModule>*) (nsTArray.h:348)
==24266==    by 0x9D7F888: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::DestructRange(unsigned int, unsigned int) (nsTArray.h:1211)
==24266==    by 0x9D7E89F: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) (nsTArray.h:931)
==24266==    by 0x9D7D670: nsTArray<nsAutoPtr<nsComponentManagerImpl::KnownModule>, nsTArrayDefaultAllocator>::Clear() (nsTArray.h:942)
==24266==    by 0x9D79CBD: nsComponentManagerImpl::Shutdown() (nsComponentManager.cpp:737)
==24266==    by 0x9D24EDF: mozilla::ShutdownXPCOM(nsIServiceManager*) (nsXPComInit.cpp:680)
==24266==    by 0x9D2490F: NS_ShutdownXPCOM_P (nsXPComInit.cpp:542)
==24266==    by 0x8331AF5: ScopedXPCOMStartup::~ScopedXPCOMStartup() (nsAppRunner.cpp:1103)
==24266==    by 0x833B394: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:3875)
==24266==    by 0x833B568: XRE_main (nsAppRunner.cpp:3929)
==24266==    by 0x401EDD: do_main(int, char**) (nsBrowserApp.cpp:153)
==24266==    by 0x402133: main (nsBrowserApp.cpp:236)
==24266== 

Marking ss because I have no idea what the extent of the underlying issue is here.  If I had to guess, I would be it's some XPCOM thing staying alive too long in b2g, but I dunno.
Comment 1 Bobby Holley (PTO through June 13) 2012-06-19 04:36:14 PDT
Ahah! I ran into this over in bug 755255, but misdiagnosed it. Patch coming right up.
Comment 2 Bobby Holley (PTO through June 13) 2012-06-19 04:36:49 PDT
Created attachment 634350 [details] [diff] [review]
Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1
Comment 3 Daniel Veditz [:dveditz] 2012-06-20 13:10:04 PDT
The feasibility of exploiting this seems low, the attackers script would have to reallocate and prep that chunk of memory after it was freed earlier in shutdown and before it's referenced. If they could, though, it would be remote code execution.
Comment 4 Bobby Holley (PTO through June 13) 2012-06-21 07:16:10 PDT
Pushed to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/da7795985d8f
Comment 5 Ed Morley [:emorley] 2012-06-22 03:53:50 PDT
https://hg.mozilla.org/mozilla-central/rev/da7795985d8f
Comment 6 Daniel Veditz [:dveditz] 2012-06-22 11:39:44 PDT
Is this fix needed on previous releases (aurora/beta/esr)?
Comment 7 Bobby Holley (PTO through June 13) 2012-06-24 00:30:59 PDT
(In reply to Daniel Veditz [:dveditz] from comment #6)
> Is this fix needed on previous releases (aurora/beta/esr)?

Eh. It theoretically affects them, but it's not clear to me that it's ever possible to trigger this hazard on those branches. If it was, we'd have seen crashes.
Comment 8 Alex Keybl [:akeybl] 2012-06-24 12:43:45 PDT
Bobby - you mentioned in bug 759402 that it may be fixed by this bug. What's the risk profile here?
Comment 9 Bobby Holley (PTO through June 13) 2012-06-24 15:49:06 PDT
(In reply to Bobby Holley (:bholley) from comment #7)

> it's not clear to me that it's ever
> possible to trigger this hazard on those branches. If it was, we'd have seen
> crashes.

(In reply to Alex Keybl [:akeybl] from comment #8)
> Bobby - you mentioned in bug 759402 that it may be fixed by this bug. What's
> the risk profile here?

Oh right - I stand corrected. In that case we should probably take this anywhere we want to fix that crash. It's a low-risk fix IMO.
Comment 10 Alex Keybl [:akeybl] 2012-07-02 17:08:19 PDT
(In reply to Bobby Holley (:bholley) from comment #9)
> (In reply to Alex Keybl [:akeybl] from comment #8)
> > Bobby - you mentioned in bug 759402 that it may be fixed by this bug. What's
> > the risk profile here?
> 
> Oh right - I stand corrected. In that case we should probably take this
> anywhere we want to fix that crash. It's a low-risk fix IMO.

Yep - let's get this into Aurora 15 and Beta 14 in that case. Can you nominate/land tomorrow before EOD?
Comment 11 Bobby Holley (PTO through June 13) 2012-07-03 02:28:09 PDT
Comment on attachment 634350 [details] [diff] [review]
Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1

Nominating per comment 10. Low-risk crash fix.
Comment 12 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-03 11:47:02 PDT
Comment on attachment 634350 [details] [diff] [review]
Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1

approving - please land immediately to aurora/beta branches as we need the builds to complete before giving today's go to build for beta.
Comment 14 Bobby Holley (PTO through June 13) 2012-07-03 15:21:30 PDT
Thanks Andrew!
Comment 15 Alex Keybl [:akeybl] 2012-07-05 16:36:40 PDT
(In reply to Bobby Holley (:bholley) from comment #7)
> Eh. It theoretically affects them, but it's not clear to me that it's ever
> possible to trigger this hazard on those branches. If it was, we'd have seen
> crashes.

Given that, marking as wontfix.

Note You need to log in before you can comment on or make changes to this bug.