775206 - Random IndexedDB crashes with Trial Tool

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

If I open up the "Trial Tool" at http://nparashuram.com/trialtool/index.html#example=/IndexedDB/trialtool/moz_indexedDB.html&selected=#db&

and then do a bunch of running of various testcases in somewhat random orders (I only used prereqs, open, open+1, delete, close, create obj store, delete obj store, list obj store -- didn't touch anything below that), I can often get a crash when I reload the page.  I've reproduced it twice with the following two different crash reports

https://crash-stats.mozilla.com/report/index/bp-f489fa2b-96a0-4031-b91a-5966a2120718
https://crash-stats.mozilla.com/report/index/bp-4439b6f2-778c-4153-80af-324ee2120718

Had a harder time doing it a third time. 2012-07-18 m-c nightly.

Comment 1

[Daniel Veditz [:dveditz]]

marking sec-critical based on bp-f489fa2b-96a0-4031-b91a-5966a2120718 -- but it would really help to have a reproducible case.

Comment 2

[Andrew McCreight [:mccr8]]

Kyle, do you have any ideas about this?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I can look at the code and think really hard, but a testcase would be much easier.

Comment 4

[Daniel Veditz [:dveditz]]

I was able to get a debug version to crash -- took a while, not sure exactly how much of what I was doing was relevant to the testcase. Before the crash I saw the following assertions

###!!! ASSERTION: Release()ing something that hasn't been Acquire()ed: 'chainFront && CallStack::kNone != mDDEntry->mAcquisitionContext', file /Users/daniel/dev/ffcentral/mozilla/obj-dbg/xpcom/build/BlockingResourceBase.cpp, line 160
WARNING: Resource acquired at calling context
: file /Users/daniel/dev/ffcentral/mozilla/obj-dbg/xpcom/build/BlockingResourceBase.cpp, line 167
  [stack trace unavailable]
WARNING: 
is being released in non-LIFO order; why?: file /Users/daniel/dev/ffcentral/mozilla/obj-dbg/xpcom/build/BlockingResourceBase.cpp, line 169

then it crashed with

0   libSystem.B.dylib             	0xffff12c1 __longcopy + 193 (cpu_capabilities.h:249)
1   libSystem.B.dylib             	0xffff086e __memcpy + 206 (cpu_capabilities.h:246)
2   XUL                           	0x06e7eb74 nsTArray_base<nsTArrayDefaultAllocator>::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int, unsigned long) + 204 (nsTArray-inl.h:245)
3   XUL                           	0x07365232 nsTArray<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, nsTArrayDefaultAllocator>::RemoveElementsAt(unsigned int, unsigned int) + 232 (nsTArray.h:932)
4   XUL                           	0x07365284 nsTArray<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, nsTArrayDefaultAllocator>::RemoveElementAt(unsigned int) + 32 (nsTArray.h:937)
5   XUL                           	0x073652d1 bool nsTArray<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, nsTArrayDefaultAllocator>::RemoveElement<mozilla::dom::indexedDB::IDBDatabase*, nsDefaultComparator<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, mozilla::dom::indexedDB::IDBDatabase*> >(mozilla::dom::indexedDB::IDBDatabase* const&, nsDefaultComparator<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, mozilla::dom::indexedDB::IDBDatabase*> const&) + 75 (nsTArray.h:957)
6   XUL                           	0x073652fd bool nsTArray<nsRefPtr<mozilla::dom::indexedDB::IDBDatabase>, nsTArrayDefaultAllocator>::RemoveElement<mozilla::dom::indexedDB::IDBDatabase*>(mozilla::dom::indexedDB::IDBDatabase* const&) + 31 (nsTArray.h:964)
7   XUL                           	0x0735ef28 mozilla::dom::indexedDB::IndexedDatabaseManager::OnDatabaseClosed(mozilla::dom::indexedDB::IDBDatabase*) + 210 (IndexedDatabaseManager.cpp:687)
8   XUL                           	0x0731b103 mozilla::dom::indexedDB::IDBDatabase::CloseInternal(bool) + 257 (IDBDatabase.cpp:294)
9   XUL                           	0x0731b1d4 mozilla::dom::indexedDB::IDBDatabase::Close() + 94 (IDBDatabase.cpp:755)
10  XUL                           	0x07359301 mozilla::dom::indexedDB::IndexedDatabaseManager::AbortCloseDatabasesForWindow(nsPIDOMWindow*) + 283 (IndexedDatabaseManager.cpp:632)
11  XUL                           	0x071baa2e nsGlobalWindow::FreeInnerObjects() + 198 (nsGlobalWindow.cpp:1105)
12  XUL                           	0x071baf13 nsGlobalWindow::DetachFromDocShell() + 359 (nsGlobalWindow.cpp:2299)
13  XUL                           	0x07909f03 nsDocShell::Destroy() + 1053 (nsDocShell.cpp:4745)
14  XUL                           	0x06d7f3ae nsFrameLoader::Finalize() + 88 (nsFrameLoader.cpp:546)
15  XUL                           	0x06d4ce8b nsDocument::MaybeInitializeFinalizeFrameLoaders() + 611 (nsDocument.cpp:5423)
16  XUL                           	0x06d6b3dd nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() + 101 (nsThreadUtils.h:350)
17  XUL                           	0x06d013f8 nsContentUtils::AddScriptRunner(nsIRunnable*) + 120 (nsContentUtils.cpp:4911)
18  XUL                           	0x06d4519a nsDocument::FinalizeFrameLoader(nsFrameLoader*) + 296 (nsDocument.cpp:5382)
19  XUL                           	0x06d8416c nsFrameLoader::Destroy() + 966 (nsFrameLoader.cpp:1328)
20  XUL                           	0x06f675e9 nsGenericHTMLFrameElement::DestroyContent() + 57 (nsGenericHTMLFrameElement.cpp:224)
21  XUL                           	0x06d98731 nsGenericElement::DestroyContent() + 167 (nsGenericElement.cpp:2735)
22  XUL                           	0x06d98731 nsGenericElement::DestroyContent() + 167 (nsGenericElement.cpp:2735)
23  XUL                           	0x06d98731 nsGenericElement::DestroyContent() + 167 (nsGenericElement.cpp:2735)
24  XUL                           	0x06d98731 nsGenericElement::DestroyContent() + 167 (nsGenericElement.cpp:2735)
25  XUL                           	0x06d98731 nsGenericElement::DestroyContent() + 167 (nsGenericElement.cpp:2735)
26  XUL                           	0x06d495e1 nsDocument::Destroy() + 155 (nsDocument.cpp:6963)
27  XUL                           	0x069bbe6c DocumentViewerImpl::Destroy() + 1362 (nsDocumentViewer.cpp:1598)
28  XUL                           	0x069be57b DocumentViewerImpl::Show() + 207 (nsDocumentViewer.cpp:1915)
29  XUL                           	0x069e5f47 nsPresContext::EnsureVisible() + 311 (nsPresContext.cpp:1822)
30  XUL                           	0x07b67250 nsPluginInstanceOwner::Init(nsIContent*) + 176 (nsPluginInstanceOwner.cpp:3209)
31  XUL                           	0x07b532a7 nsPluginHost::InstantiateEmbeddedPluginInstance(char const*, nsIURI*, nsObjectLoadingContent*, nsPluginInstanceOwner**) + 343 (nsPluginHost.cpp:931)
32  XUL                           	0x06dcda16 nsObjectLoadingContent::InstantiatePluginInstance(char const*, nsIURI*) + 974 (nsObjectLoadingContent.cpp:691)
33  XUL                           	0x06dcdd3f nsObjectLoadingContent::SyncStartPluginInstance() + 225 (nsObjectLoadingContent.cpp:2036)
34  XUL                           	0x06dc6af2 nsAsyncInstantiateEvent::Run() + 70 (nsObjectLoadingContent.cpp:123)
35  XUL                           	0x080ea129 nsThread::ProcessNextEvent(bool, bool*) + 1265 (nsThread.cpp:624)
36  XUL                           	0x08077d2c NS_ProcessPendingEvents_P(nsIThread*, unsigned int) + 146
37  XUL                           	0x07cf3237 nsBaseAppShell::NativeEventCallback() + 169 (nsBaseAppShell.cpp:98)
38  XUL                           	0x07c94601 nsAppShell::ProcessGeckoEvents(void*) + 545 (nsAppShell.mm:403)

Maybe things would be clearer if we could reproduce this in an ASan build.

Comment 5

[Christian Holler (:decoder)]

I tried the 2012-07-20 m-c nightly ASan builds (both debug and opt) because they don't go further back in time right now. I tried to run various tests as described and also reloaded the site a few times, but I was not able to reproduce the issue (x86-64 Linux).

Comment 6

[Christian Holler (:decoder)]

I quickly implemented an indexedDB fuzzer that is capable of performing the actions in comment 0. I don't have a testcase yet but I hit this ASan trace pretty quickly:

==15301== ERROR: AddressSanitizer heap-use-after-free on address 0x7f2275e58f38 at pc 0x7f22d57131df bp 0x7fff7d522870 sp 0x7fff7d522868
READ of size 8 at 0x7f2275e58f38 thread T0
    #0 0x7f22d57131df in mozilla::dom::indexedDB::IDBDatabase::CloseInternal(bool) /builds/slave/try-lnx64/build/dom/indexedDB/IDBDatabase.cpp:294
    #1 0x7f22d5717f58 in mozilla::dom::indexedDB::IDBDatabase::Close() /builds/slave/try-lnx64/build/dom/indexedDB/IDBDatabase.cpp:753
    #2 0x7f22d545bfa4 in nsGlobalWindow::FirstTimeout() /builds/slave/try-lnx64/build/dom/base/nsGlobalWindow.h:839
    #3 0x7f22d5468718 in nsGlobalWindow::DetachFromDocShell() /builds/slave/try-lnx64/build/dom/base/nsGlobalWindow.cpp:2305
    #4 0x7f22d63448b3 in nsCOMPtr<nsIScriptGlobalObject>::operator=(nsIScriptGlobalObject*) /builds/slave/try-lnx64/build/../../dist/include/nsCOMPtr.h:622
    #5 0x7f22d6344cfd in non-virtual thunk to nsDocShell::Destroy() /builds/slave/try-lnx64/build/build/unix/stdc++compat/stdc++compat.cpp:0
    #6 0x7f22d4ce3c9e in nsDocument::MaybeInitializeFinalizeFrameLoaders() /builds/slave/try-lnx64/build/content/base/src/nsDocument.cpp:5437
    #7 0x7f22d4ce353f in nsDocument::EndUpdate(unsigned int) /builds/slave/try-lnx64/build/content/base/src/nsDocument.cpp:3978
    #8 0x7f22d53c8be9 in nsXULDocument::EndUpdate(unsigned int) /builds/slave/try-lnx64/build/content/xul/document/src/nsXULDocument.cpp:3304
0x7f2275e58f38 is located 184 bytes inside of 208-byte region [0x7f2275e58e80,0x7f2275e58f50)
freed by thread T0 here:
    #0 0x4248b2 in free ??:0
    #1 0x7f22d50349e9 in nsDOMEventTargetHelper::Release() /builds/slave/try-lnx64/build/content/events/src/nsDOMEventTargetHelper.cpp:76
previously allocated by thread T0 here:
    #0 0x424972 in __interceptor_malloc ??:0
    #1 0x7f22d9ea11a9 in moz_xmalloc /builds/slave/try-lnx64/build/memory/mozalloc/mozalloc.cpp:54
==15301== ABORTING


Not sure if this is what we're looking for, but it might be a good start.

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Christian Holler (:decoder) from comment #6)
> I quickly implemented an indexedDB fuzzer that is capable of performing the
> actions in comment 0. I don't have a testcase yet but I hit this ASan trace
> pretty quickly:

Awesome, thanks!

Comment 8

[Christian Holler (:decoder)]

I'm also seeing this assertion in debug builds, not sure if related:

###!!! ASSERTION: This shouldn't be failing at this point!: 'NS_SUCCEEDED(rv)', file /builds/slave/try-lnx64-dbg/build/dom/indexedDB/OpenDatabaseHelper.cpp, line 2201

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

Kyle can you comment on comment 8? (If you unassign this bug please reassign to a best guess alternative)

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't have any ideas about what would cause the assertion in comment 8 to fire.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Christian provided me with a fuzzer that reproduces the crash and I caught it in a VM.  The stacks match those from comments 4 and 6.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The problem is that we're removing a database from an array.  Doing so causes us to destroy the database (because the array has the last reference to it) which reenters the code that causes us to remove the database from an array.  Since we're in the middle of removing something from the array, unwinding the stack leaves the nsTArray in an incoherent state and causes memory corruption which later brings down the browser.

I think this goes back all the way to the IndexedDB synchronization rewrite in the Firefox 6/7/8 timeframe.

I'll write a patch tomorrow.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Pretty simple fix.

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Unfortunately I can't really write a deterministic test for this, because in order for this condition to trigger the synchronization queue has to hold the last reference to the database.

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b656b2c1037

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 650553 [details] [diff] [review]
Patch

I think we should take this everywhere we're willing to take patches.  This would probably be a bit tricky to exploit, and has been around for a long time (since 7 or so), but does lead to memory corruption.

Risk is minimal, no string/uuid changes/etc.

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6b656b2c1037

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/99b89297c6c8
https://hg.mozilla.org/releases/mozilla-beta/rev/3b33a689e63d
https://hg.mozilla.org/releases/mozilla-esr10/rev/3faafa7a0daa

Comment 19

[u279076]

Given this is in-testsuite-, does it need a manual test case?

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't know of any way to test this deterministically.