crash in nsPluginHost::FindPlugins

VERIFIED FIXED in Firefox 20

Status

()

defect
P2
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: scoobidiver, Assigned: gfritzsche)

Tracking

({crash})

17 Branch
mozilla21
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox20 verified)

Details

(crash signature)

Attachments

(2 attachments, 3 obsolete attachments)

It's #46 top browser crasher in 17.0b6.

Stack traces are various:
Frame 	Module 	Signature 	Source
0 	mozalloc.dll 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:23
1 	mozalloc.dll 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:27
2 	mozalloc.dll 	moz_xmalloc 	memory/mozalloc/mozalloc.cpp:59
3 	xul.dll 	nsPluginHost::FindPlugins 	dom/plugins/base/nsPluginHost.cpp:2378
4 	xul.dll 	nsPluginHost::LoadPlugins 	dom/plugins/base/nsPluginHost.cpp:2344
5 	xul.dll 	nsPluginHost::GetPluginCount 	dom/plugins/base/nsPluginHost.cpp:1497
6 	xul.dll 	nsPluginArray::GetLength 	dom/base/nsPluginArray.cpp:61
7 	xul.dll 	nsPluginArray::GetPlugins 	dom/base/nsPluginArray.cpp:221
8 	xul.dll 	nsPluginArray::GetNamedItem 	dom/base/nsPluginArray.cpp:115
9 	xul.dll 	nsPluginArraySH::GetNamedItem 	dom/base/nsDOMClassInfo.cpp:10046
...

Frame 	Module 	Signature 	Source
0 	mozalloc.dll 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:79
1 	mozalloc.dll 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:60
2 	mozalloc.dll 	moz_xmalloc 	
3 	xul.dll 	nsPluginHost::FindPlugins 	dom/plugins/base/nsPluginHost.cpp:2341
4 	xul.dll 	nsPluginHost::LoadPlugins 	dom/plugins/base/nsPluginHost.cpp:2307
5 	xul.dll 	nsPluginHost::GetPluginCount 	dom/plugins/base/nsPluginHost.cpp:1537
6 	xul.dll 	nsPluginArray::GetLength 	dom/base/nsPluginArray.cpp:93
7 	xul.dll 	nsMimeTypeArray::GetMimeTypes 	dom/base/nsMimeTypeArray.cpp:245
8 	xul.dll 	mozilla::dom::Navigator::RefreshMIMEArray 	dom/base/Navigator.cpp:560
9 	xul.dll 	mozilla::dom::Navigator::JavaEnabled 	dom/base/Navigator.cpp:523
10 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
...

Frame 	Module 	Signature 	Source
0 	mozalloc.dll 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:23
1 	mozalloc.dll 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:27
2 	mozalloc.dll 	moz_xmalloc 	memory/mozalloc/mozalloc.cpp:59
3 	xul.dll 	nsPluginHost::FindPlugins 	dom/plugins/base/nsPluginHost.cpp:2378
4 	xul.dll 	nsPluginHost::LoadPlugins 	dom/plugins/base/nsPluginHost.cpp:2344
5 	xul.dll 	nsPluginHost::FindPluginEnabledForExtension 	dom/plugins/base/nsPluginHost.cpp:1621
6 	xul.dll 	nsPluginHost::IsPluginEnabledForExtension 	dom/plugins/base/nsPluginHost.cpp:1369
7 	xul.dll 	nsExternalHelperAppService::GetTypeFromExtension 	uriloader/exthandler/nsExternalHelperAppService.cpp:2594
8 	xul.dll 	nsExternalHelperAppService::GetTypeFromFile 	uriloader/exthandler/nsExternalHelperAppService.cpp:2718
9 	xul.dll 	nsFileChannel::MakeFileInputStream 	netwerk/protocol/file/nsFileChannel.cpp:298
10 	xul.dll 	nsFileChannel::OpenContentStream 	netwerk/protocol/file/nsFileChannel.cpp:367
...

Frame 	Module 	Signature 	Source
0 	mozalloc.dll 	mozalloc_abort 	memory/mozalloc/mozalloc_abort.cpp:23
1 	mozalloc.dll 	mozalloc_handle_oom 	memory/mozalloc/mozalloc_oom.cpp:27
2 	mozalloc.dll 	moz_xmalloc 	memory/mozalloc/mozalloc.cpp:59
3 	xul.dll 	nsPluginHost::FindPlugins 	dom/plugins/base/nsPluginHost.cpp:2378
4 	xul.dll 	nsPluginHost::LoadPlugins 	dom/plugins/base/nsPluginHost.cpp:2344
5 	xul.dll 	nsPluginHost::GetPluginTags 	dom/plugins/base/nsPluginHost.cpp:1534
6 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
7 	xul.dll 	XPCWrappedNative::CallMethod 	js/xpconnect/src/XPCWrappedNative.cpp:2406
8 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1478
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+mozalloc_handle_oom%28unsigned+int%29+|+moz_xmalloc+|+nsPluginHost%3A%3AFindPlugins%28bool%2C+bool*%29
https://crash-stats.mozilla.com/report/index/a69c84d6-f115-42e6-9d68-7e4612121119
OOMAllocationSize	4294967295

== 0xFFFFFFFF underflow.

This is similar or identical to bug 786233.
Workaround: 

Delete/rename the file %APPDATA%\Mozilla\Firefox\Profiles\%PID%.default\pluginreg.dat
Thanks Cristian, i can reproduce the crash with that. 

We try allocate an array of size |0x7fffffff * 3| here:
http://hg.mozilla.org/mozilla-central/annotate/dd277d439d31/dom/plugins/base/nsPluginHost.cpp#l2808
... from |atoi("1347734823984|0|5")| here:
http://hg.mozilla.org/mozilla-central/annotate/dd277d439d31/dom/plugins/base/nsPluginHost.cpp#l2800

We could do a sanity bounding here for safety, but the interesting question is how those corrupt values for the mimetype count got in there.
Reading through |nsPluginHost::WritePluginInfo()|, this looks definitely broken. For the npgtpo3dautoplugin the whole block with description, name and mimetype-count is missing that should have been written here:
http://hg.mozilla.org/mozilla-central/annotate/dd277d439d31/dom/plugins/base/nsPluginHost.cpp#l2538
Maybe there was a crash. Maybe the computer shut off. We should definitely be sanity-checking the numbers on input regardless of whether there's a bug we can fix at output time. It's a cache, so we should be safe to just throw the whole thing away if we detect anything awry.
Priority: -- → P2
Straight-forward sanity check on the parsed mimetype count.

Do we have any test-environment that could cover this?
Attachment #693899 - Flags: review?(benjamin)
Good question. You might be able to do this in an xpcshell test by writing the "bad" pluginreg.dat before you "start" a profile.
Assignee: nobody → georg.fritzsche
Comment on attachment 693899 [details] [diff] [review]
Sanity check for parsed mimetype count

Ah, thanks - turns out we already do write some plugin registries in dom/plugins/test/unit.

Holding off the review request until i added the test.
Attachment #693899 - Flags: review?(benjamin)
Added test.
Try run: https://tbpl.mozilla.org/?tree=Try&rev=d0c94879d370

control try run to see that this triggers properly (only the xpcshell test):
https://tbpl.mozilla.org/?tree=Try&rev=decae962dd56
Attachment #693899 - Attachment is obsolete: true
Attachment #694840 - Flags: review?(benjamin)
Fix Linux build bustage. Also updated to tip and killed a redundant JS function.
https://tbpl.mozilla.org/?tree=Try&rev=763f9efc76cd
Attachment #694840 - Attachment is obsolete: true
Attachment #694840 - Flags: review?(benjamin)
The Windows debug builds for the control try run are timing out. 
Looks like this is the runtime throwing up an error message box (for the invalid allocation), which is pretty unhelpful in automated runs.
Windows debug builds hang with the call stack below for the unit test, never even showing a message box. This is apparently specific to xpcshell-tests, as the MsgBox shows up for the test-registry with Fx. 
Benjamin or Jim, do you see some easy solution for this? 

user32.dll!_NtUserWaitMessage@0()  + 0x15 bytes	
user32.dll!_NtUserWaitMessage@0()  + 0x15 bytes	
xul.dll!nsAppShell::ProcessNextNativeEvent(bool mayWait)  Line 335	C++
xul.dll!nsBaseAppShell::DoProcessNextNativeEvent(bool mayWait, unsigned int recursionDepth)  Line 139 + 0x12 bytes	C++
xul.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr, bool mayWait, unsigned int recursionDepth)  Line 298 + 0x14 bytes	C++
xul.dll!nsThread::ProcessNextEvent(bool mayWait, bool * result)  Line 603	C++
xul.dll!NS_ProcessNextEvent_P(nsIThread * thread, bool mayWait)  Line 237 + 0x17 bytes	C++
xul.dll!nsThread::Shutdown()  Line 474 + 0xb bytes	C++
xul.dll!mozilla::crashreporter::LSPAnnotationGatherer::Annotate()  Line 47	C++
xul.dll!nsRunnableMethodImpl<void (__thiscall mozilla::crashreporter::LSPAnnotationGatherer::*)(void),1>::Run()  Line 368	C++
xul.dll!nsThread::ProcessNextEvent(bool mayWait, bool * result)  Line 627 + 0x19 bytes	C++
xul.dll!NS_ProcessPendingEvents_P(nsIThread * thread, unsigned int timeout)  Line 187 + 0x14 bytes	C++
xul.dll!nsBaseAppShell::NativeEventCallback()  Line 98	C++
xul.dll!nsAppShell::EventWindowProc(HWND__ * hwnd, unsigned int uMsg, unsigned int wParam, long lParam)  Line 94	C++
user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
user32.dll!_DispatchMessageWorker@8()  + 0xed bytes	
user32.dll!_DispatchMessageW@4()  + 0xf bytes	
user32.dll!_DialogBox2@16()  + 0x1c0 bytes	
user32.dll!_InternalDialogBox@24()  + 0xc9 bytes	
user32.dll!_SoftModalMessageBox@4()  + 0x757 bytes	
user32.dll!_MessageBoxWorker@4()  + 0x269 bytes	
user32.dll!_MessageBoxTimeoutW@24()  + 0x52 bytes	
user32.dll!_MessageBoxTimeoutA@24()  + 0x76 bytes	
user32.dll!_MessageBoxExA@20()  + 0x1b bytes	
user32.dll!_MessageBoxA@16()  + 0x18 bytes	
msvcr100d.dll!___crtMessageBoxA()  + 0x20c bytes	
msvcr100d.dll!___crtMessageWindowA()  + 0x3b7 bytes	
msvcr100d.dll!__VCrtDbgReportA()  + 0x7d8 bytes	
msvcr100d.dll!__CrtDbgReportV()  + 0x22 bytes	
msvcr100d.dll!__CrtDbgReport()  + 0x2b bytes	
msvcr100d.dll!__nh_malloc_dbg()  + 0x265 bytes	
msvcr100d.dll!__nh_malloc_dbg()  + 0x7f bytes	
msvcr100d.dll!__nh_malloc_dbg()  + 0x2c bytes	
msvcr100d.dll!_malloc()  + 0x1b bytes	
mozalloc.dll!moz_xmalloc(unsigned int size)  Line 54 + 0xa bytes	C++
xul.dll!operator new[](unsigned int size)  Line 212 + 0xa bytes	C++
xul.dll!nsPluginHost::ReadPluginInfo()  Line 2821 + 0x1f bytes	C++
[...]
FYI bug 587982 got rid of these dialogs in the browser. We probably just need to use that same code in xpcshell. (The hang in the nested event loop is just a horrible side effect of that.)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #14)
> FYI bug 587982 got rid of these dialogs in the browser. We probably just
> need to use that same code in xpcshell. (The hang in the nested event loop
> is just a horrible side effect of that.)

Ah, thanks Ted, will check that out.
Doing a straight-forward adoption i get a hang which is apparently waiting on the CrashReporter thread, which hangs in allocation related code (hanging on heap unlock in the CRT), stack below.

We receive allocation failure crash reports for Fx, so this must be another setup issue? Ted, any ideas?

ntdll.dll!_NtWaitForSingleObject@12()  + 0x15 bytes	
ntdll.dll!_NtWaitForSingleObject@12()  + 0x15 bytes	
browsercomps.dll!_heap_alloc_dbg_impl(unsigned int nSize, int nBlockUse, const char * szFileName, int nLine, int * errno_tmp)  Line 507 + 0x7 bytes	C++
ntdll.dll!_RtlInitUnicodeStringEx@8() 	
msvcr100d.dll!__lock()  + 0x3d bytes	
msvcr100d.dll!__free_dbg()  + 0x39 bytes	
msvcr100d.dll!_free()  + 0x10 bytes	
mozalloc.dll!moz_free(void * ptr)  Line 48 + 0xa bytes	C++
xul.dll!operator delete[](void * ptr)  Line 236 + 0xa bytes	C++
xul.dll!nsAutoArrayPtr<wchar_t>::~nsAutoArrayPtr<wchar_t>()  Line 475 + 0x11 bytes	C++
xul.dll!`anonymous namespace'::patched_LdrLoadDll(wchar_t * filePath, unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle)  Line 453 + 0x27 bytes	C++
KernelBase.dll!74d92c95() 	
[Frames below may be incorrect and/or missing, no symbols loaded for KernelBase.dll]	
Nvd3d9wrap.dll!74642654() 	
dbghelp.dll!Win32LiveSystemProvider::Initialize()  + 0x10 bytes	
dbghelp.dll!NtWin32LiveSystemProvider::Initialize()  + 0xa bytes	
dbghelp.dll!_MiniDumpCreateLiveSystemProvider@8()  + 0xb3 bytes	
dbghelp.dll!_MiniDumpWriteDump@28()  + 0x38 bytes	
xul.dll!google_breakpad::ExceptionHandler::WriteMinidumpWithExceptionForProcess(unsigned long requesting_thread_id, _EXCEPTION_POINTERS * exinfo, MDRawAssertionInfo * assertion, void * process, bool write_requester_stream)  Line 990 + 0x43 bytes	C++
xul.dll!google_breakpad::ExceptionHandler::WriteMinidumpWithException(unsigned long requesting_thread_id, _EXCEPTION_POINTERS * exinfo, MDRawAssertionInfo * assertion)  Line 819 + 0x1d bytes	C++
xul.dll!google_breakpad::ExceptionHandler::ExceptionHandlerThreadMain(void * lpParameter)  Line 374 + 0x26 bytes	C++
Flags: needinfo?(ted)
Comment on attachment 694879 [details] [diff] [review]
Sanity check for parsed mimetype count, v4

It looks like adressing the timeouts might take a little bit longer, but given that it's not an issue with this patch applied i'd like to get this landed.
I'd address the xpcshell timeouts in a separate bug as they are independent of the issue here anyway.
Attachment #694879 - Flags: review?(benjamin)
Comment on attachment 694879 [details] [diff] [review]
Sanity check for parsed mimetype count, v4

I don't think 'return rv' is correct here, because rv will probably be a success code. You really want to return a failure code here, I think, so think should be return NS_ERROR_FAILURE;

r=me with that change
Attachment #694879 - Flags: review?(benjamin) → review+
Fixed the return statement, thanks for catching that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef505122f02a
Attachment #694879 - Attachment is obsolete: true
Flags: needinfo?(ted)
https://hg.mozilla.org/mozilla-central/rev/ef505122f02a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 703983 [details] [diff] [review]
Sanity check for parsed mimetype count, v5

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Start-up crashes with certain broken plugin registries.
Testing completed (on m-c, etc.): Fine on try, landed on m-c.
Risk to taking this patch (and alternatives if risky): Low-risk, it just guards against over-allocating.
String or UUID changes made by this patch: None.
Attachment #703983 - Flags: approval-mozilla-beta?
Attachment #703983 - Flags: approval-mozilla-aurora?
Comment on attachment 703983 [details] [diff] [review]
Sanity check for parsed mimetype count, v5

Not a tracked top crasher, so let's take this plugin code change on Aurora 20 and let it ride the trains.
Attachment #703983 - Flags: approval-mozilla-beta?
Attachment #703983 - Flags: approval-mozilla-beta-
Attachment #703983 - Flags: approval-mozilla-aurora?
Attachment #703983 - Flags: approval-mozilla-aurora+
I stumbled over a slightly different crash signature:
mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | nsPluginHost::ReadPluginInfo()
https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char+const*+const%29+|+mozalloc_handle_oom%28unsigned+int%29+|+moz_xmalloc+|+nsPluginHost%3A%3AReadPluginInfo%28%29

This bug doesn't show up there - would i have to edit the crash signature field on this bug to get it to?
(In reply to Georg Fritzsche [:gfritzsche] from comment #23) 
> would i have to edit the crash signature field on this bug to get it to?
Done.
Crash Signature: [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | nsPluginHost::FindPlugins(bool, bool*)] → [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | nsPluginHost::FindPlugins(bool, bool*)] [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | nsPluginHost::ReadPluginInfo() ]
There are no crashes in crash-stats for FF 20.b1, Aurora 21 or Nightly 22 for the signatures mentioned in this Bug, so I guess Georg fix is verified. Scoobi can you please confirm this?
There are a few crashes in 19.0 and below but not crashes in 20.0 and above.
Status: RESOLVED → VERIFIED
(In reply to Scoobidiver from comment #27)
> There are a few crashes in 19.0 and below but not crashes in 20.0 and above.

Thanks for confirm!
You need to log in before you can comment on or make changes to this bug.