766018 - Use-after-free during GC on shutdown

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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 (:bholley)]

Ahah! I ran into this over in bug 755255, but misdiagnosed it. Patch coming right up.

Comment 2

[Bobby Holley (:bholley)]

Attachment: Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1

Comment 3

[Daniel Veditz [:dveditz]]

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 (:bholley)]

Pushed to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/da7795985d8f

Comment 5

[Ed Morley [:emorley]]

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

Comment 6

[Daniel Veditz [:dveditz]]

Is this fix needed on previous releases (aurora/beta/esr)?

Comment 7

[Bobby Holley (:bholley)]

(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]]

Bobby - you mentioned in bug 759402 that it may be fixed by this bug. What's the risk profile here?

Comment 9

[Bobby Holley (:bholley)]

(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]]

(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 (:bholley)]

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]

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 13

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/releases/mozilla-beta/rev/84241548fe12
https://hg.mozilla.org/releases/mozilla-aurora/rev/a52296e14e00

Comment 14

[Bobby Holley (:bholley)]

Thanks Andrew!

Comment 15

[Alex Keybl [:akeybl]]

(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.