Scoobidiver (away) Reporter
	Description • 12 years ago

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

	Benjamin Smedberg
	Comment 1 • 12 years ago

https://crash-stats.mozilla.com/report/index/a69c84d6-f115-42e6-9d68-7e4612121119
OOMAllocationSize	4294967295

== 0xFFFFFFFF underflow.

This is similar or identical to bug 786233.

	cristian.cornea
	Comment 2 • 12 years ago

Workaround: 

Delete/rename the file %APPDATA%\Mozilla\Firefox\Profiles\%PID%.default\pluginreg.dat

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 4 • 12 years ago

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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 5 • 12 years ago

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

	Benjamin Smedberg
	Comment 6 • 12 years ago

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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 7 • 12 years ago

Straight-forward sanity check on the parsed mimetype count.

Do we have any test-environment that could cover this?

	Benjamin Smedberg
	Comment 8 • 12 years ago

Good question. You might be able to do this in an xpcshell test by writing the "bad" pluginreg.dat before you "start" a profile.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 9 • 12 years ago

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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 10 • 12 years ago

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

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 11 • 12 years ago

Fix Linux build bustage. Also updated to tip and killed a redundant JS function.
https://tbpl.mozilla.org/?tree=Try&rev=763f9efc76cd

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 12 • 12 years ago

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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 13 • 12 years ago

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++
[...]

	(not currently active) Ted Mielczarek
	Comment 14 • 12 years ago

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.)

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 15 • 12 years ago

(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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 16 • 12 years ago

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++

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 17 • 12 years ago

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.

	Benjamin Smedberg
	Comment 18 • 12 years ago

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

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 19 • 12 years ago

Fixed the return statement, thanks for catching that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef505122f02a

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 20 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/ef505122f02a

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 21 • 12 years ago

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.

	Alex Keybl [:akeybl]
	Comment 22 • 12 years ago

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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 23 • 12 years ago

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?

	Scoobidiver (away) Reporter
	Comment 24 • 12 years ago

(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.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 25 • 12 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c3bea6e07ab

	Mihai Morar, (:MihaiMorar)
	Comment 26 • 12 years ago

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?

	Scoobidiver (away) Reporter
	Comment 27 • 12 years ago

There are a few crashes in 19.0 and below but not crashes in 20.0 and above.

	Mihai Morar, (:MihaiMorar)
	Comment 28 • 12 years ago

(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!