The default bug view has changed. See this FAQ.

Use-after-free during GC on shutdown

RESOLVED FIXED in Firefox 14

Status

()

Core
XPConnect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: bholley)

Tracking

({crash, sec-moderate})

Trunk
mozilla16
x86_64
Linux
crash, sec-moderate
Points:
---

Firefox Tracking Flags

(firefox14+ fixed, firefox15+ fixed, firefox16+ fixed, firefox-esr10 wontfix)

Details

(Whiteboard: [advisory-tracking+][qa-])

Attachments

(1 attachment)

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.
Ahah! I ran into this over in bug 755255, but misdiagnosed it. Patch coming right up.
Created attachment 634350 [details] [diff] [review]
Null out the XPCWrappedNativeScope slot when scopes are deleted on shutdown. v1
Attachment #634350 - Flags: review?(peterv)
Blocks: 755255
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.
Keywords: crash, sec-moderate
Attachment #634350 - Flags: review?(peterv) → review+
Pushed to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/da7795985d8f
Assignee: nobody → bobbyholley+bmo
https://hg.mozilla.org/mozilla-central/rev/da7795985d8f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox16: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Is this fix needed on previous releases (aurora/beta/esr)?
(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

5 years ago
Bobby - you mentioned in bug 759402 that it may be fixed by this bug. What's the risk profile here?
(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.
(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?
tracking-firefox15: --- → +
tracking-firefox16: --- → +
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.
Attachment #634350 - Flags: approval-mozilla-beta?
Attachment #634350 - Flags: approval-mozilla-aurora?
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.
Attachment #634350 - Flags: approval-mozilla-beta?
Attachment #634350 - Flags: approval-mozilla-beta+
Attachment #634350 - Flags: approval-mozilla-aurora?
Attachment #634350 - Flags: approval-mozilla-aurora+
status-firefox14: --- → affected
status-firefox15: --- → affected
tracking-firefox14: --- → +
https://hg.mozilla.org/releases/mozilla-beta/rev/84241548fe12
https://hg.mozilla.org/releases/mozilla-aurora/rev/a52296e14e00
status-firefox14: affected → fixed
status-firefox15: affected → fixed
Thanks Andrew!
(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.
status-firefox-esr10: --- → wontfix
Whiteboard: [advisory-tracking+]
Group: core-security

Updated

5 years ago
Whiteboard: [advisory-tracking+] → [advisory-tracking+][qa-]
You need to log in before you can comment on or make changes to this bug.