Closed Bug 958780 Opened 6 years ago Closed 6 years ago

B2G Crash during FTU

Categories

(Core :: DOM: Core & HTML, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)
blocking-b2g 1.3+
Tracking Status
firefox27 + fixed
firefox28 --- fixed
firefox29 --- fixed
firefox-esr24 27+ fixed
b2g18 --- fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: gwagner, Assigned: gwagner)

References

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [systemsfe][needed 964462 to be truly fixed])

Attachments

(1 file, 1 obsolete file)

On current trunk with nexus 4 during FTU when opening mozilla privacy page.

Program received signal SIGSEGV, Segmentation fault.
0xb4cdec1a in nsCOMPtr<nsIFile>::~nsCOMPtr (this=0xafdc876c, __in_chrg=<optimized out>) at ../../../xpcom/glue/nsCOMPtr.h:517
517	            NSCAP_RELEASE(this, mRawPtr);
(gdb) bt
#0  0xb4cdec1a in nsCOMPtr<nsIFile>::~nsCOMPtr (this=0xafdc876c, __in_chrg=<optimized out>) at ../../../xpcom/glue/nsCOMPtr.h:517
#1  0xb5579000 in nsDOMOfflineResourceList::~nsDOMOfflineResourceList (this=0xafdc8710, __in_chrg=<optimized out>)
    at ../../../../dom/src/offline/nsDOMOfflineResourceList.cpp:96
#2  0xb5579064 in nsDOMOfflineResourceList::~nsDOMOfflineResourceList (this=0xafdc8710, __in_chrg=<optimized out>)
    at ../../../../dom/src/offline/nsDOMOfflineResourceList.cpp:96
#3  0xb5410ce6 in nsTransactionManager::DeleteCycleCollectable (this=<optimized out>) at ../../../../editor/txmgr/src/nsTransactionManager.cpp:58
#4  0xb51a41c6 in nsDOMEventTargetHelper::cycleCollection::DeleteCycleCollectable (this=<optimized out>, p=0xafdc8710)
    at ../../../content/events/src/nsDOMEventTargetHelper.h:58
#5  0xb4cf7e88 in SnowWhiteKiller::~SnowWhiteKiller (this=0xbe938be8, __in_chrg=<optimized out>) at ../../../xpcom/base/nsCycleCollector.cpp:2186
#6  0xb4cf7efc in nsCycleCollector::FreeSnowWhite (this=0xb3e76000, aUntilNoSWInPurpleBuffer=<optimized out>) at ../../../xpcom/base/nsCycleCollector.cpp:2297
#7  0xb4cf80aa in nsCycleCollector::BeginCollection (this=0xb3e76000, aCCType=SliceCC, aManualListener=0x0) at ../../../xpcom/base/nsCycleCollector.cpp:3144
#8  0xb4cf8262 in Collect (aManualListener=0x0, aBudget=..., aCCType=SliceCC, this=0xb3e76000) at ../../../xpcom/base/nsCycleCollector.cpp:3009
#9  nsCycleCollector::Collect (this=0xb3e76000, aCCType=SliceCC, aBudget=..., aManualListener=0x0) at ../../../xpcom/base/nsCycleCollector.cpp:2983
#10 0xb4cf83b0 in nsCycleCollector_collectSlice (aSliceTime=<optimized out>) at ../../../xpcom/base/nsCycleCollector.cpp:3566
#11 0xb54d5afa in RunCycleCollectorSlice (aSliceTime=-1) at ../../../dom/base/nsJSEnvironment.cpp:2148
#12 nsJSContext::RunCycleCollectorSlice (aSliceTime=<optimized out>) at ../../../dom/base/nsJSEnvironment.cpp:2137
#13 0xb54d620c in CCTimerFired (aTimer=<optimized out>, aClosure=<optimized out>) at ../../../dom/base/nsJSEnvironment.cpp:2446
#14 CCTimerFired (aTimer=<optimized out>, aClosure=<optimized out>) at ../../../dom/base/nsJSEnvironment.cpp:2400
#15 0xb4d2d14e in nsTimerImpl::Fire (this=0xaff6c470) at ../../../xpcom/threads/nsTimerImpl.cpp:551
#16 0xb4d2d2d8 in nsTimerEvent::Run (this=0xb1a2ddd0) at ../../../xpcom/threads/nsTimerImpl.cpp:635
#17 0xb4d2a6da in ProcessNextEvent (result=0xbe938da7, mayWait=<optimized out>, this=0xb3e2f600) at ../../../xpcom/threads/nsThread.cpp:637
#18 nsThread::ProcessNextEvent (this=0xb3e2f600, mayWait=<optimized out>, result=0xbe938da7) at ../../../xpcom/threads/nsThread.cpp:568
#19 0xb4ce61bc in NS_ProcessNextEvent (thread=0xb3e2f600, mayWait=<optimized out>) at ../../../xpcom/glue/nsThreadUtils.cpp:263
#20 0xb4eb74e2 in mozilla::ipc::MessagePump::Run (this=0xb3e01b80, aDelegate=0xbe938f00) at ../../../ipc/glue/MessagePump.cpp:134
#21 0xb4ea970a in MessageLoop::RunInternal (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:226
#22 0xb4ea9722 in RunHandler (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:219
#23 MessageLoop::Run (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:193
#24 0xb5406ffe in nsBaseAppShell::Run (this=0xb313b640) at ../../../widget/xpwidgets/nsBaseAppShell.cpp:161
#25 0xb5c75c0e in XRE_RunAppShell () at ../../../toolkit/xre/nsEmbedFunctions.cpp:679
#26 0xb4eb759a in mozilla::ipc::MessagePumpForChildProcess::Run (this=0xb3e01b80, aDelegate=0xbe938f00) at ../../../ipc/glue/MessagePump.cpp:251
#27 0xb4ea970a in MessageLoop::RunInternal (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:226
#28 0xb4ea9722 in RunHandler (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:219
#29 MessageLoop::Run (this=0xbe938f00) at ../../../ipc/chromium/src/base/message_loop.cc:193
#30 0xb5c75af2 in XRE_InitChildProcess (aArgc=5, aArgv=<optimized out>, aProcess=<optimized out>) at ../../../toolkit/xre/nsEmbedFunctions.cpp:516
#31 0x0000876e in main (argc=6, argv=0xbe9399f4) at ../../../ipc/app/MozillaRuntimeMain.cpp:137
(gdb) up
#1  0xb5579000 in nsDOMOfflineResourceList::~nsDOMOfflineResourceList (this=0xafdc8710, __in_chrg=<optimized out>)
    at ../../../../dom/src/offline/nsDOMOfflineResourceList.cpp:96
96	}
(gdb) p this
$1 = (nsDOMOfflineResourceList * const) 0xafdc8710
(gdb) p *this
$2 = {<nsDOMEventTargetHelper> = {<mozilla::dom::EventTarget> = {<nsIDOMEventTarget> = {<nsISupports> = {
          _vptr.nsISupports = 0xb6c0b528}, <No data fields>}, <nsWrapperCache> = {_vptr.nsWrapperCache = 0xb6c0b624, 
        mWrapper = {<js::HeapBase<JSObject*>> = {<No data fields>}, ptr = 0x0}, mFlags = 2}, <No data fields>}, mRefCnt = {mRefCntAndFlags = 5}, 
    _mOwningThread = {mThread = 0xb3e2e080}, static _cycleCollectorGlobal = {
    <nsXPCOMCycleCollectionParticipant> = {<nsScriptObjectTracer> = {<nsCycleCollectionParticipant> = {_vptr.nsCycleCollectionParticipant = 0xb6c26b70, 
            mMightSkip = true}, <No data fields>}, <No data fields>}, <No data fields>}, mListenerManager = {mRawPtr = 0x0}, mParentObject = 0x0, 
    mOwnerWindow = 0x0, mHasOrHasHadOwnerWindow = true}, <nsIDOMOfflineResourceList> = {<nsISupports> = {
      _vptr.nsISupports = 0xb6c0b630}, <No data fields>}, <nsIObserver> = {<nsISupports> = {
      _vptr.nsISupports = 0xb6c0b6a8}, <No data fields>}, <nsIOfflineCacheUpdateObserver> = {<nsISupports> = {
      _vptr.nsISupports = 0xb6c0b6c0}, <No data fields>}, <nsSupportsWeakReference> = {<nsISupportsWeakReference> = {<nsISupports> = {
        _vptr.nsISupports = 0xb6c0b6dc}, <No data fields>}, mProxy = 0xaf0dc970}, 
  static _cycleCollectorGlobal = {<nsDOMEventTargetHelper::cycleCollection> = {<nsXPCOMCycleCollectionParticipant> = {<nsScriptObjectTracer> = {<nsCycleCollectionParticipant> = {_vptr.nsCycleCollectionParticipant = 0xb6c0b6f8, 
            mMightSkip = true}, <No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, mInitialized = true, mManifestURI = {mRawPtr = 
    0xafdd0140}, mManifestSpec = {<nsACString_internal> = {mData = 0xaff6c6f8 "https://marketplace.firefox.com/manifest.appcache?repo=fireplace", 
      mLength = 64, mFlags = 5}, <No data fields>}, mDocumentURI = {mRawPtr = 0xb31d9a00}, mApplicationCacheService = {mRawPtr = 0x0}, 
  mAvailableApplicationCache = {mRawPtr = 0x0}, mCacheUpdate = {mRawPtr = 0xafdc85c0}, mExposeCacheUpdateStatus = true, mStatus = 1, mCachedKeys = 0x0, 
  mCachedKeysCount = 0, mPendingEvents = {<nsCOMArray_base> = {
      mArray = {<nsTArray_Impl<nsISupports*, nsTArrayInfallibleAllocator>> = {<nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>> = {
            mHdr = 0xb6d6a6b8}, <nsTArray_TypedBase<nsISupports*, nsTArray_Impl<nsISupports*, nsTArrayInfallibleAllocator> >> = {<nsTArray_SafeElementAtHelper<nsISupports*, nsTArray_Impl<nsISupports*, nsTArrayInfallibleAllocator> >> = {<No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}}, <No data fields>}}
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
I tried to reproduce bug 956325. Maybe they are related.
Blocks: 956325
Attached patch 958780.diff (obsolete) — Splinter Review
Comment on attachment 8359350 [details] [diff] [review]
958780.diff

Smaug, could this cause crashes? Or would you expect it to cause leaks.
Attachment #8359350 - Flags: review?(bugs)
And please assume that we cast in the Unlink implementation like we should.
Attached patch 958780.diffSplinter Review
mrbkaps patch.
Attachment #8359350 - Attachment is obsolete: true
Attachment #8359350 - Flags: review?(bugs)
Assignee: nobody → mrbkap
blocking-b2g: --- → 1.3?
Comment on attachment 8359355 [details] [diff] [review]
958780.diff

Don't see how this would fix the bug. That Disconnect doesn't do anything too interesting, or am I missing something?
Attachment #8359355 - Flags: review?(bugs)
Comment on attachment 8359355 [details] [diff] [review]
958780.diff

But this is good anyway.
Attachment #8359355 - Flags: review+
Whiteboard: checkin-needed
Whiteboard: checkin-needed → checkin-needed, [systemsfe]
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
https://hg.mozilla.org/integration/b2g-inbound/rev/747121b2bb50
Whiteboard: checkin-needed, [systemsfe] → [systemsfe]
fixed on central https://hg.mozilla.org/mozilla-central/rev/747121b2bb50
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks an existing 1.3+ bug.
blocking-b2g: 1.3? → 1.3+
https://hg.mozilla.org/releases/mozilla-aurora/rev/fce670bb28db

Does this affect v1.2?
Assignee: mrbkap → anygregor
status-b2g-v1.2: --- → ?
Flags: needinfo?(anygregor)
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #11)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/fce670bb28db
> 
> Does this affect v1.2?

Yes. Lets uplift.
Flags: needinfo?(anygregor)
The initial crash looks a little weird, but from the gdb output, it doesn't look like the object is dead, so it is probably okay to just treat this as a DOS kind of crash.
For what it's worth, the object was definitely dead (its memory was all 0x5a'd).
Well, in that case this should get backported everywhere. ;)  Fortunately the patch is small.
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/55c994a4f146
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e7a8e7b9b7d2

Given that this is sec-critical (and it isn't clear that this is B2G-only), do we want this on beta/esr24 as well? If so, please nominate the patch accordingly (or set their status flags to wontfix) :)
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #17)
> https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/e7a8e7b9b7d2

So it's fixed on all b2g branches right? What else do we have to figure out here?
Flags: needinfo?(anygregor) → needinfo?(ryanvm)
(In reply to Gregor Wagner [:gwagner] from comment #18)
> So it's fixed on all b2g branches right? What else do we have to figure out
> here?

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #16)
> Given that this is sec-critical (and it isn't clear that this is B2G-only),
> do we want this on beta/esr24 as well? If so, please nominate the patch
> accordingly (or set their status flags to wontfix) :)
Flags: needinfo?(ryanvm) → needinfo?(anygregor)
Attachment #8359355 - Flags: approval-mozilla-esr24?
Flags: needinfo?(anygregor)
Comment on attachment 8359355 [details] [diff] [review]
958780.diff

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: accessing dead object 
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none
Attachment #8359355 - Flags: approval-mozilla-beta?
Attachment #8359355 - Flags: approval-mozilla-esr24?
Attachment #8359355 - Flags: approval-mozilla-esr24+
Attachment #8359355 - Flags: approval-mozilla-beta?
Attachment #8359355 - Flags: approval-mozilla-beta+
Whiteboard: [systemsfe] → [systemsfe][adv-main27+][adv-esr24.3+]
Depends on: 964462
Anyone have steps to reproduce for desktop Firefox? We'd like to verify the fix there, if possible. 

If not, let us know.
(In reply to Matt Wobensmith from comment #22)
> Anyone have steps to reproduce for desktop Firefox? We'd like to verify the
> fix there, if possible. 

Please see bug 964462. As Olli mentioned in comment 6, this patch probably didn't fix this bug.
(In reply to Blake Kaplan (:mrbkap) from comment #23)
 
> Please see bug 964462. As Olli mentioned in comment 6, this patch probably
> didn't fix this bug.

Then why is it resolved fixed everywhere and we went ahead and checked the "fix" in? Comment 15 implies it did fix it.
(In reply to Al Billings [:abillings] from comment #24)
> Then why is it resolved fixed everywhere and we went ahead and checked the
> "fix" in? Comment 15 implies it did fix it.

We thought it was fixed (the fix here went in at the same time as some other stuff and we could no longer reproduce it until Gregor found better STR). We're still debugging.

At the same time, the patch that went in here is obviously correct and low-to-no-risk, so no damage really...
Well, it is included in an advisory as a fixed security issue...
To put everyone on the same page:

If the bug is not fixed, it shouldn't be marked resolved, and branches should not be marked fixed either. If anyone has any concern that issue(s) still linger(s), reopen this. Or, if this is a dupe of the related bug 964462, mark it as such. 

Too much confusion here, and we're running out of time for this release.
If this isn't fixed, I don't want to publicly discuss it in an advisory. Please advise, Blake or whomever.
I suppose what should have happened is that when Gregor reproduced bug 964462, we should have reopened this bug or something like that (except that bugzilla stinks at tracking multiple patches). Can we mark this bug as somehow depending on bug 964462 or at least to not release the advisory for this bug until that bug is fixed?
Whiteboard: [systemsfe][adv-main27+][adv-esr24.3+] → [systemsfe][needed 964462 to be truly fixed]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Matt Wobensmith from comment #27)
> To put everyone on the same page:
> 
> If the bug is not fixed, it shouldn't be marked resolved, and branches
> should not be marked fixed either. If anyone has any concern that issue(s)
> still linger(s), reopen this. Or, if this is a dupe of the related bug
> 964462, mark it as such. 
> 
> Too much confusion here, and we're running out of time for this release.

Actually that's not right. The resolution path in Bugzilla follows a landing process, not a user behavioral process for tracking resolution. This will screw up tree management right now if we reopen this bug, as the landing has already occurred. What we usually do here instead is file followups for tracking what needs to be resolved here. bug 964462 has been opened as a followup to this issue, so we've got a bug already open tracking the problem here as a dependency.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Group: core-security
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.