937582 - nsPrefetchNode use-after-free

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Jesse Ruderman]

Attachment: ASan stacks (A)

About a third of the time, just starting Firefox (pointing at Bugzilla?) and shutting it down triggers a use-after-free of an nsPrefetchNode.

Comment 1

[Jesse Ruderman]

Attachment: ASan stacks (B)

Comment 2

[Jesse Ruderman]

nsPrefetchService::mQueueHead is a raw pointer, with lots of manual addref and release calls in 
http://mxr.mozilla.org/mozilla-central/source/uriloader/prefetch/nsPrefetchService.cpp

Comment 3

[Honza Bambas (:mayhemer)]

Adding Steve, just in case this could be some ODA off main tread triggered issue.  nsPrefetchService is a very old and AFAIK unmaintained code.

Jesse, I presume you are able to reproduce this.  Probably just replacing manual addrefs/releases with refptrs will fix this.  It's IMO easy to do, but harder to test.

Comment 4

[Jesse Ruderman]

Sure, I can test a patch by launching and quitting Firefox 10 times.

Comment 5

[Patrick McManus [:mcmanus]]

honza, will you write the patch?

Comment 6

[(no longer active)]

This code is completely bogus.  nsPrefetchService::DequeueNode pretends that it addrefs, but it doesn't, and its callers seem to use getter_AddRefs which means that as soon as DequeueNode is called once, the reference for the node starts to be unbalanced (it's decremented once too much).  And of course mQueueHead and mQueueTail are raw pointers as Jesse mentions, which basically means that any time that we don't get a UAF here we're just getting lucky.  :(

Comment 7

[Honza Bambas (:mayhemer)]

(In reply to Patrick McManus [:mcmanus] from comment #5)
> honza, will you write the patch?

Yes, I think I can take care of this.

Comment 8

[Steve Workman [:sworkman] (INACTIVE)]

Took a look at the stack traces - it's possible that the ODA off main thread stuff could have exposed this, but there's nothing in the traces to point to that directly. If using nsRefPtrs doesn't fix it, I can take a look.

Comment 9

[Honza Bambas (:mayhemer)]

Attachment: v1

- all members made nsrefptr
- checking main-thread only access with assertions (I suspect there could be a threading problem according the release thread stack traces)

(no try push made)

Jesse, please re-test with this patch.  Thanks.

Comment 10

[Honza Bambas (:mayhemer)]

See comment 9

Comment 11

[Al Billings [:abillings - ex-MoCo]]

How far back are we affected by this?

Comment 12

[Jesse Ruderman]

This patch did NOT fix the bug for me.  I never hit the assertion (tried debug and debug+asan), but I still hit the ASan complaint (in both asan and debug+asan).

Comment 13

[Jesse Ruderman]

Attachment: ASan stacks (A')

Comment 14

[Patrick McManus [:mcmanus]]

Comment on attachment 8334860 [details] [diff] [review]
v1

I'm clearing the r? flag here based on comment 12.. but honza if you think this should go forward as a partial fix (or just as a different fix that turns out to be unrelated to the test case) then let me know and I'll look at it asap.. I just figure it might change.

Comment 15

[Honza Bambas (:mayhemer)]

I was a bit afraid of this.  Since when writing this patch and examining the code it looked like it's correct (regardless the ADDREF/RELEASE used directly).

I'll get to this next week again.

(In reply to Al Billings [:abillings] from comment #11)
> How far back are we affected by this?

The manual ADDREF/RELEASE were introduced at bug 372969 (mozilla 1.9 - Fx3).  However, this crash can be there from other reason.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

(In reply to Honza Bambas (:mayhemer) from comment #15)
 
> The manual ADDREF/RELEASE were introduced at bug 372969 (mozilla 1.9 - Fx3).
> However, this crash can be there from other reason.

Lacking other data points, I'm going to assume, "the dawn of time."

Comment 17

[Honza Bambas (:mayhemer)]

For info: 
- I'm on win (7)
- I'm now testing with a fresh profile
- to repro the same crash stack, I've set the proxy setting to "none"

Result is either a crash [1] on a completely different address, so this seems more like a general heap corruption then a problem directly with the prefetch service... or I crashed on releasing a prefetch node (same heap error) for which I was logging the number of addrefs and releases - they were identical, so the node referencing was correct.


[1] HEAP: Free Heap block e19a230 modified at e19a290 after it was freed
Windows has triggered a breakpoint in firefox.exe.
>	ntdll.dll!_RtlpBreakPointHeap@4()  + 0x23 bytes	
 	ntdll.dll!@RtlpAllocateHeap@24()  + 0x577a5 bytes	
 	ntdll.dll!_RtlAllocateHeap@12()  + 0x5cb8 bytes	
 	ntdll.dll!_RtlDebugAllocateHeap@12()  + 0xb5 bytes	
 	ntdll.dll!@RtlpAllocateHeap@24()  + 0x57600 bytes	
 	ntdll.dll!_RtlAllocateHeap@12()  + 0x5cb8 bytes	
 	msvcr100d.dll!_heap_alloc_base(unsigned int size=100)  Line 55	C
 	msvcr100d.dll!_heap_alloc_dbg_impl(unsigned int nSize=64, int nBlockUse=1, const char * szFileName=0x00000000, int nLine=0, int * errno_tmp=0x0045ca54)  Line 431 + 0x9 bytes	C++
 	msvcr100d.dll!_nh_malloc_dbg_impl(unsigned int nSize=64, int nhFlag=0, int nBlockUse=1, const char * szFileName=0x00000000, int nLine=0, int * errno_tmp=0x0045ca54)  Line 239 + 0x19 bytes	C++
 	msvcr100d.dll!_nh_malloc_dbg(unsigned int nSize=64, int nhFlag=0, int nBlockUse=1, const char * szFileName=0x00000000, int nLine=0)  Line 302 + 0x1d bytes	C++
 	msvcr100d.dll!malloc(unsigned int nSize=64)  Line 56 + 0x15 bytes	C++
 	mozjs.dll!js_malloc(unsigned int bytes=64)  Line 144 + 0xa bytes	C++
 	mozjs.dll!js::MallocProvider<js::ThreadSafeContext>::malloc_(unsigned int bytes=64)  Line 590 + 0x9 bytes	C++
 	mozjs.dll!js::MallocProvider<js::ThreadSafeContext>::pod_malloc<js::HeapSlot>(unsigned int numElems=8)  Line 643	C++
 	mozjs.dll!AllocateSlots(js::ThreadSafeContext * cx=0x02cfa1b0, JSObject * obj=0x0f324d60, unsigned int nslots=8)  Line 2483	C++
 	mozjs.dll!JSObject::growSlots(js::ThreadSafeContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, unsigned int oldCount=0, unsigned int newCount=8)  Line 2544 + 0x18 bytes	C++
 	mozjs.dll!JSObject::updateSlotsForSpan(js::ThreadSafeContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, unsigned int oldSpan=0, unsigned int newSpan=1)  Line 2415 + 0x1d bytes	C++
 	mozjs.dll!JSObject::setLastProperty(js::ThreadSafeContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<js::Shape *> shape={...})  Line 2451 + 0x15 bytes	C++
 	mozjs.dll!JSObject::getChildProperty(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<js::Shape *> parent={...}, js::StackShape & child={...})  Line 405 + 0x1b bytes	C++
 	mozjs.dll!JSObject::getOrLookupChildProperty<0>(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<js::Shape *> parent={...}, js::StackShape & child={...})  Line 859 + 0x1a bytes	C++
 	mozjs.dll!JSObject::addPropertyInternal<0>(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x00000000, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x00000000, unsigned int slot=16777215, unsigned int attrs=1, unsigned int flags=0, int shortid=0, js::Shape * * spp=0x00000000, bool allowDictionary=true)  Line 636 + 0x22 bytes	C++
 	mozjs.dll!JSObject::putProperty<0>(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x00000000, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x00000000, unsigned int slot=16777215, unsigned int attrs=1, unsigned int flags=0, int shortid=0)  Line 819 + 0x2f bytes	C++
 	mozjs.dll!DefinePropertyOrElement<0>(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x53256cfc, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x532421bc, unsigned int attrs=1, unsigned int flags=0, int shortid=0, JS::Handle<JS::Value> value={...}, bool callSetterAfterwards=false, bool setterIsStrict=false)  Line 3541 + 0x36 bytes	C++
 	mozjs.dll!js::DefineNativeProperty(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, JS::Handle<JS::Value> value={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x53256cfc, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x532421bc, unsigned int attrs=1, unsigned int flags=0, int shortid=0, unsigned int defineHow=0)  Line 3651 + 0x2d bytes	C++
 	mozjs.dll!js::baseops::DefineGeneric(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, JS::Handle<JS::Value> value={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x53256cfc, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x532421bc, unsigned int attrs=1)  Line 3278 + 0x27 bytes	C++
 	mozjs.dll!JSObject::defineGeneric(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, JS::Handle<jsid> id={...}, JS::Handle<JS::Value> value={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x53256cfc, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x532421bc, unsigned int attrs=1)  Line 3293 + 0x21 bytes	C++
 	mozjs.dll!JSObject::defineProperty(js::ExclusiveContext * cx=0x02cfa1b0, JS::Handle<JSObject *> obj={...}, js::PropertyName * name=0x03612d00, JS::Handle<JS::Value> value={...}, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)* getter=0x53256cfc, bool (JSContext *, JS::Handle<JSObject *>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>)* setter=0x532421bc, unsigned int attrs=1)  Line 3302 + 0x2b bytes	C++
 	mozjs.dll!js::CreateItrResultObject(JSContext * cx=0x02cfa1b0, JS::Handle<JS::Value> value={...}, bool done=true)  Line 712 + 0x3c bytes	C++
 	mozjs.dll!`anonymous namespace'::MapIteratorObject::next_impl(JSContext * cx=0x02cfa1b0, JS::CallArgs args={...})  Line 987 + 0x28 bytes	C++
 	mozjs.dll!JS::CallNonGenericMethod(JSContext * cx=0x02cfa1b0, bool (JS::Handle<JS::Value>)* Test=0x5324fb64, bool (JSContext *, JS::CallArgs)* Impl=0x5325a76c, JS::CallArgs args={...})  Line 110 + 0x1d bytes	C++
 	mozjs.dll!`anonymous namespace'::MapIteratorObject::next(JSContext * cx=0x02cfa1b0, unsigned int argc=1, JS::Value * vp=0x0384edf8)  Line 999 + 0x26 bytes	C++
 	mozjs.dll!js::CallJSNative(JSContext * cx=0x02cfa1b0, bool (JSContext *, unsigned int, JS::Value *)* native=0x532671c9, const JS::CallArgs & args={...})  Line 220 + 0x19 bytes	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x02cfa1b0, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT)  Line 463 + 0x16 bytes	C++
 	mozjs.dll!Interpret(JSContext * cx=0x02cfa1b0, js::RunState & state={...})  Line 2505 + 0x2a bytes	C++
 	mozjs.dll!js::RunScript(JSContext * cx=0x02cfa1b0, js::RunState & state={...})  Line 420 + 0xd bytes	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x02cfa1b0, JS::CallArgs args={...}, js::MaybeConstruct construct=NO_CONSTRUCT)  Line 482 + 0xd bytes	C++
 	mozjs.dll!js::Invoke(JSContext * cx=0x02cfa1b0, const JS::Value & thisv={...}, const JS::Value & fval={...}, unsigned int argc=3, JS::Value * argv=0x0045eb94, JS::MutableHandle<JS::Value> rval={...})  Line 513 + 0x4e bytes	C++
 	mozjs.dll!JS_CallFunctionValue(JSContext * cx=0x02cfa1b0, JSObject * objArg=0x0730f730, JS::Value fval={...}, unsigned int argc=3, JS::Value * argv=0x0045eb94, JS::Value * rval=0x0045eb3c)  Line 5006 + 0x3e bytes	C++
 	xul.dll!nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS * wrapper=0x0df2b1b8, unsigned short methodIndex=3, const XPTMethodDescriptor * info_=0x005c09a0, nsXPTCMiniVariant * nativeParams=0x0045ec50)  Line 1425 + 0x45 bytes	C++
 	xul.dll!nsXPCWrappedJS::CallMethod(unsigned short methodIndex=3, const XPTMethodDescriptor * info=0x005c09a0, nsXPTCMiniVariant * params=0x0045ec50)  Line 506	C++
 	xul.dll!PrepareAndDispatch(nsXPTCStubBase * self=0x0df2b230, unsigned int methodIndex=3, unsigned int * args=0x0045ed18, unsigned int * stackBytesToPop=0x0045ed08)  Line 85 + 0x21 bytes	C++
 	xul.dll!SharedStub()  Line 113	C++
 	xul.dll!nsTimerImpl::Fire()  Line 567	C++
 	xul.dll!nsTimerEvent::Run()  Line 637	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x0045eefb)  Line 612 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x00547990, bool mayWait=true)  Line 263 + 0x17 bytes	C++
 	xul.dll!nsThread::Shutdown()  Line 465 + 0xb bytes	C++
 	xul.dll!nsRunnableMethodImpl<enum tag_nsresult (__stdcall nsIThread::*)(void),void,1>::Run()  Line 384	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x0045efff)  Line 612 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x00547990, bool mayWait=true)  Line 263 + 0x17 bytes	C++
 	xul.dll!nsThread::Shutdown()  Line 465 + 0xb bytes	C++
 	xul.dll!mozilla::LazyIdleThread::ShutdownThread()  Line 288 + 0x28 bytes	C++
 	xul.dll!mozilla::LazyIdleThread::Notify(nsITimer * aTimer=0x0da70d38)  Line 480 + 0xb bytes	C++
 	xul.dll!nsTimerImpl::Fire()  Line 555	C++
 	xul.dll!nsTimerEvent::Run()  Line 637	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=true, bool * result=0x0045f317)  Line 612 + 0x19 bytes	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * thread=0x00547990, bool mayWait=true)  Line 263 + 0x17 bytes	C++
 	xul.dll!nsHttpConnectionMgr::Shutdown()  Line 156 + 0xd bytes	C++
 	xul.dll!nsHttpHandler::Observe(nsISupports * subject=0x00000000, const char * topic=0x147c4e48, const wchar_t * data=0x147c6cb0)  Line 1777	C++
 	xul.dll!nsObserverList::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x147c4e48, const wchar_t * someData=0x147c6cb0)  Line 97	C++
 	xul.dll!nsObserverService::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x147c4e48, const wchar_t * someData=0x147c6cb0)  Line 337	C++
 	xul.dll!nsXREDirProvider::DoShutdown()  Line 855	C++
 	xul.dll!ScopedXPCOMStartup::~ScopedXPCOMStartup()  Line 1129	C++
 	xul.dll!ScopedXPCOMStartup::`scalar deleting destructor'()  + 0xf bytes	C++
 	xul.dll!XREMain::XRE_main(int argc=4, char * * argv=0x00524bd0, const nsXREAppData * aAppData=0x0045f650)  Line 4067 + 0x1f bytes	C++
 	xul.dll!XRE_main(int argc=4, char * * argv=0x00524bd0, const nsXREAppData * aAppData=0x0045f650, unsigned int aFlags=0)  Line 4244 + 0x17 bytes	C++
 	firefox.exe!do_main(int argc=4, char * * argv=0x00524bd0, nsIFile * xreDirectory=0x0052d148)  Line 280 + 0x19 bytes	C++
 	firefox.exe!NS_internal_main(int argc=4, char * * argv=0x00524bd0)  Line 647 + 0x11 bytes	C++
 	firefox.exe!wmain(int argc=4, wchar_t * * argv=0x00682f80)  Line 105 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 552 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 371	C

Comment 18

[Honza Bambas (:mayhemer)]

Attachment: v2 (one-liner)

As often, the more a problem is obvious the more difficult to spot it :)

First, we are breaking the AsyncOpen contract: when the proxy service is down, which is after xpcom shutdown, and with proxy setting 'none', we call OnStart/OnStop before we exit from AsyncOpen:

	xul.dll!mozilla::net::HttpBaseChannel::DoNotifyListener() 
	xul.dll!mozilla::net::nsHttpChannel::OnProxyAvailable() 
	xul.dll!nsAsyncResolveRequest::DoCallback() 
	xul.dll!nsAsyncResolveRequest::Run() 
	xul.dll!nsProtocolProxyService::AsyncResolveInternal() 
	xul.dll!nsProtocolProxyService::AsyncResolve2() 
	xul.dll!mozilla::net::nsHttpChannel::ResolveProxy() 
	xul.dll!mozilla::net::nsHttpChannel::AsyncOpen() 

The heap corruption is not coming from bad addref/release but from writing to a released memory at nsPrefetchNode::OpenChannel:

    rv = mChannel->AsyncOpen(this, nullptr);
    NS_ENSURE_SUCCESS(rv, rv);

    mState = nsIDOMLoadStatus::REQUESTED; //*

...where on the //* line |this| can be already freed.  ASAN build nor VS debugger has caught this bad access and rather screamed on later access to the broken block.

Comment 19

[Patrick McManus [:mcmanus]]

honza - awesome!

it sounds like this might trip up some other httpchannel caller too.. could we also fix it by having nsHttpChannel::OnProxyAvailable post an event when it needs to run the NS_FAILED {cancel, donotifylistener()} path? I think that would fix the contract problem without introducing a scheduler round trip to the normal fast path.

maybe we want both patches - the one you suggest is obviously trivial to backport.

Comment 20

[Patrick McManus [:mcmanus]]

I just now saw that you filed a different bug 931439 for a change to httpchannel. great; we'll split it.

Comment 21

[Honza Bambas (:mayhemer)]

Comment on attachment 8338106 [details] [diff] [review]
v2 (one-liner)

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
- no clue, hard for me to evaluate (I should educate my self in this area) ; to sum: we early after freeing a block write to it at a block+few bytes address - this may lead to an (un?)predictably broken heap

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
- no (I plan an empty checkin message)

Which older supported branches are affected by this flaw?
- all

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
- this can be very easily backported

How likely is this patch to cause regressions; how much testing does it need?
- very low chance, just keeps a reference locally during call on an (at the moment valid) object, STR is easy and reproducible at 100% cases for me

Comment 22

[Al Billings [:abillings - ex-MoCo]]

I need release management to weigh in here. We're probably too late in the cycle to take this though you say the risk is low. It may make sense just to take this on Aurora and Central but we may wish to wait a few weeks until we've shipped 26.

Comment 23

[Daniel Veditz [:dveditz]]

This patch is trivially safe, there's no reason not to take it now even on Beta. If we don't take it on beta let's at least land it everywhere else since it's a fuzzblocker.

Comment 24

[Jesse Ruderman]

(In reply to Patrick McManus [:mcmanus] from comment #20)
> I just now saw that you filed a different bug 931439 for a change to
> httpchannel. great; we'll split it.

Bug 943149

Comment 25

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8338106 [details] [diff] [review]
v2 (one-liner)

Sec-approval+ for trunk and giving approval for Aurora and Beta based on Dan's comments.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/044c28763a8d
https://hg.mozilla.org/releases/mozilla-aurora/rev/72560d160a16
https://hg.mozilla.org/releases/mozilla-beta/rev/17195f059d65

I landed it sans-commit message per the approval request, but I'll point out that doing so pretty much screams "HEY LOOK AT ME!" to anybody who understands our processes.

Also, can we get an esr24 approval request?

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/044c28763a8d

Comment 28

[Matt Wobensmith [:mwobensmith][:matt:]]

Jesse, as per comment 12, would you mind trying this again to verify that the patch really works for you this time? Thanks.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/17195f059d65

Comment 30

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8338106 [details] [diff] [review]
v2 (one-liner)

Check this into ESR24, please.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/fe6a13755fa3

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/c714e1bd607f

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/c714e1bd607f

Comment 34

[Jesse Ruderman]

(In reply to Matt Wobensmith from comment #28)
> Jesse, as per comment 12, would you mind trying this again to verify that
> the patch really works for you this time? Thanks.

I tested a mozilla-central build after the patch landed. The problem seems to be fixed :)

Comment 35

[Jesse Ruderman]

Honza, can you split the patch in comment 9 (moving to nsRefPtr) into a new bug?  I like reducing the amount of unsafe code.

Comment 36

[Honza Bambas (:mayhemer)]

(In reply to Jesse Ruderman from comment #35)
> Honza, can you split the patch in comment 9 (moving to nsRefPtr) into a new
> bug?  I like reducing the amount of unsafe code.

I am not sure we want/need it.  The code as is is actually OK.