645289 - Crash [@ nsNPAPIPluginInstance::GetImage ] mainly with Webex

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Scoobidiver (away)]

It is a new crash signature that first appeared in 4.0b12.
It is #7 top crasher on Mac OS X in 4.0.

Most comments talk about Webex.

Signature	nsNPAPIPluginInstance::GetImage
UUID	cad2d71a-1982-494b-b7a6-5d4f22110324
Time 	2011-03-24 21:48:04.45510
Uptime	91254
Last Crash	9751821 seconds (more than 3 months) before submission
Install Age	450096 seconds (5.2 days) since version was first installed.
Product	Firefox
Version	4.0
Build ID	20110318052756
Branch	2.0
OS	Mac OS X
OS Version	10.5.8 9L31a
CPU	x86
CPU Info	family 6 model 23 stepping 6
Crash Reason	EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash Address	0x6800003d
User Comments	www.xbox.com
App Notes 	Renderers: 0x22600,0x20400

Frame 	Module 	Signature [Expand] 	Source
0 	XUL 	nsNPAPIPluginInstance::GetImage 	modules/plugin/base/src/nsNPAPIPluginInstance.cpp:882
1 	XUL 	nsPluginInstanceOwner::SetCurrentImage 	layout/generic/nsObjectFrame.cpp:2000
2 	XUL 	nsPluginInstanceOwner::InvalidateRect 	layout/generic/nsObjectFrame.cpp:3499
3 	XUL 	nsNPAPIPluginInstance::InvalidateRect 	modules/plugin/base/src/nsNPAPIPluginInstance.cpp:1215
4 	XUL 	mozilla::plugins::parent::_invalidaterect 	modules/plugin/base/src/nsNPAPIPlugin.cpp:1268
5 	agcore 	agcore@0x898e1b 	
6 	agcore 	agcore@0x6520d8 	
7 	agcore 	agcore@0x6632eb 	
8 	agcore 	agcore@0x663f56 	
9 	agcore 	agcore@0x664148 	
10 	CoreFoundation 	__CFRunLoopDoObservers 	
11 	CoreFoundation 	CFRunLoopRunSpecific 	
12 	CoreFoundation 	CFRunLoopRunInMode 	
13 	Foundation 	-[NSRunLoop runMode:beforeDate:] 	
14 	XUL 	nsAppShell::ProcessNextNativeEvent 	widget/src/cocoa/nsAppShell.mm:664
15 	XUL 	nsBaseAppShell::OnProcessNextEvent 	widget/src/xpwidgets/nsBaseAppShell.cpp:173
16 	XUL 	nsAppShell::OnProcessNextEvent 	widget/src/cocoa/nsAppShell.mm:833
17 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:597
18 	XUL 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:200
19 	XUL 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:132
20 	XUL 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:399
21 	CoreFoundation 	CFRunLoopRunSpecific 	
22 	CoreFoundation 	CFRunLoopRunInMode 	
23 	HIToolbox 	RunCurrentEventLoopInMode 	
24 	HIToolbox 	ReceiveNextEventCommon 	
25 	HIToolbox 	BlockUntilNextEventMatchingListInMode 	
26 	AppKit 	_DPSNextEvent 	
27 	AppKit 	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 	
28 	AppKit 	-[NSApplication run] 	
29 	XUL 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:746
30 	XUL 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:220
31 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3786
32 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
33 	firefox-bin 	firefox-bin@0xa05 	
34 		@0x1 	

More reports at:
https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=nsNPAPIPluginInstance%3A%3AGetImage

Comment 1

[Josh Aas]

I've been debugging bug 633452, I'm pretty sure this is the same crash. The stack is different every once in a while but the underlying problem is the same. I can reproduce bug 633452 reliably and every once in a while I get this stack.

Comment 2

[Josh Aas]

Attachment: fix v1.0

The problem here is that we miscalculate whether or not a plugin has running instances. "nsPluginHost::IsRunningPlugin" is only asking the first instance for any given MIME type if it is running. In the case where the first instance isn't running but a second instance is, "IsRunningPlugin" will incorrectly claim that there is no running instance. That can lead to us deleting the nsNPAPIPlugin object from under a running instance, bad times.

This crash shows up the second time you try to load a WebEx meeting because it's the second time we try to load an instance for the MIME type "x-java-vm". The first instance is no longer running but the second one is. Then the page refreshes the plugin list to look for a newly installed WebEx plugin (which would have been installed by the Java instance), and refreshing the plugin list attempts to delete nsNPAPIPlugin objects that don't have running instances. The nsNPAPIPlugin object gets incorrectly deleted from under the running Java instance and we crash.

I'm putting this patch on this bug because I saw this stack most often while debugging, however I'm pretty sure this will also fix bug 633452. Same underlying problem.

Comment 3

[Josh Aas]

Marking this as security sensitive in case this is exploitable. Seems like that might be possible since content can trigger access to a deleted nsNPAPIPlugin object. We can open this up again if others think this isn't an issue but I want to play it safe.

Comment 4

[Josh Aas]

To be a little more clear about the way in which the object gets incorrectly deleted... "nsPluginHost::ReloadPlugins" calls "IsRunningPlugin" to determine whether it can call "TryUnloadPlugin" on the plugin's nsPluginTag object. "nsPluginTag::TryUnloadPlugin" will null out its entry point, an nsNPAPIPlugin object, which releases it and often destroys it. This crash happens when "IsRunningPlugin" incorrectly claims there is no running instance for the plugin and "TryUnloadPlugin" ends up destroying the nsNPAPIPlugin object out from underneath a running instance. At some later point the running instance tries to call a method on the deleted object, in this case during an invalidation.

Comment 5

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/1b56b31b790f

Comment 6

[Josh Aas]

We should take this for Firefox 4. I've been working on a minimal test case but so far I haven't been able to figure out exactly what the web page is doing to get the first instance that is no longer running to remain in the plugin instances array.

Comment 7

[Josh Aas]

I looked at the 1.9.2 code and while the bug that caused this problem exists there (miscalculating the number of live plugin instances for a plugin), the pointer to the deleted object that is causing the specific dangerous crash here does not exist. At least not in the same place. The live instance calculation bug could cause all sorts of hard-to-guess trouble as a result of premature deletion (among other things), I think we should fix it on 1.9.2.

Comment 8

[Josh Aas]

Attachment: fix v1.0 for 1.9.2

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 522606 [details] [diff] [review]
fix v1.0

Approved for mozilla2.0 repository, a=dveditz for release-drivers

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 522823 [details] [diff] [review]
fix v1.0 for 1.9.2

Approved for 1.9.2.17, a=dveditz for release-drivers

Please land ASAP so we get this into 3.6.17 which will release around the same time as 4.0.1

Comment 11

[Josh Aas]

pushed to mozilla-1.9.2

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/57f12d60226f

Comment 12

[Josh Aas]

pushed to mozilla-2.0

http://hg.mozilla.org/releases/mozilla-2.0/rev/62e68e402bfd