Bug 680279 - [Patch included] nspluginwrapper has a race condition on NPP_Destroy and may crash Flash
[Patch included] nspluginwrapper has a race condition on NPP_Destroy and may ...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: nspluginwrapper (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Peter Hatina
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-24 17:00 EST by David Benjamin
Modified: 2016-05-31 21:30 EDT (History)
3 users (show)

See Also:
Fixed In Version: nspluginwrapper-1.3.0-18.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-03-26 01:08:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Delay calls to NPP_Destroy when the plugin instance is on the stack. (5.78 KB, patch)
2011-03-08 06:01 EST, Peter Hatina
no flags Details | Diff
Make delayed_calls_process re-entrant (1.57 KB, patch)
2011-03-08 06:02 EST, Peter Hatina
no flags Details | Diff
Delay calls to NPP_Destroy when the plugin instance is on the stack (1.80 KB, patch)
2011-03-08 06:04 EST, Peter Hatina
no flags Details | Diff
Delay calls to NPP_Destroy when the plugin instance is on the stack. (1.43 KB, patch)
2011-03-08 06:05 EST, Peter Hatina
no flags Details | Diff

  None (edit)
Description David Benjamin 2011-02-24 17:00:08 EST
This is the same report as https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/724587 on Ubuntu;  I'm filing here too because, as far as I can tell, nspluginwrapper no longer has an upstream.
---

nspluginwrapper has a race condition during NPP_Destroy (called when a tab is closed) that can crash the plugin. The race happens particularly often when another tab has a video playing; I suspect this is because it causes enough traffic over the IPC to delay the processes and trigger the race.

If NPP_Destroy is called by the wrapper process at the same time the viewer (plugin) process makes some call, then, from the plugin's perspective, its call to NPN_InvalidateRect resulted in the plugin instance being destroyed from under its feet. This is, of course, nonsense, so Flash shortly crashes to let us know how silly we are being. :-)

I've written patches for this issue here
https://github.com/davidben/nspluginwrapper/commits/master

Only the second of the two patches is strictly relevant. (The other is a separate race I came across in a previous iteration of this patch.) It detects when NPP_Destroy is being called at an unsafe point and delays it to another message loop iteration. With the caveat that requests can't be reordered. So, when it must, the patch lies to the wrapper about NPP_Destroy's return values. Any delayed NPSavedData gets discarded. That said, I've never seen Flash use this feature, and the docs do allow the browser to discard them arbitrarily.

The relevant bug in Chromium is here:
http://code.google.com/p/chromium/issues/detail?id=53940
Comment 1 Peter Hatina 2011-03-08 06:01:32 EST
Created attachment 482872 [details]
Delay calls to NPP_Destroy when the plugin instance is on the stack.

Delay calls to NPP_Destroy when the plugin instance is on the stack

Otherwise, from the plugin's perspective, NPN_InvalidateRect results in
the plugin instance exploding. One can hardly blame Adobe that Flash
crashes in this situation.

Unfortunately, because we attempt to speak synchronous NPAPI on both
ends and there's the additional SYNC mechanism forcing a particular
order, we cannot reorder the calls. As a result, when NPP_Destroy must
be delayed, we lie to the browser and actually destroy the plugin later.
This means, however, that any NPSavedData provided by the plugin is
ignored. In this case, a warning is emitted.

We do a slightly more conservative check and delay whenever we have any
invoke on the call stack, be it this instance or any other. This is
better than checking the refcount because any NPObjectInfo will hold a
reference.
Comment 2 Peter Hatina 2011-03-08 06:02:56 EST
Created attachment 482873 [details]
Make delayed_calls_process re-entrant

And I also attach all the other relevant patches.
Comment 3 Peter Hatina 2011-03-08 06:04:03 EST
Created attachment 482874 [details]
Delay calls to NPP_Destroy when the plugin instance is on the stack
Comment 4 Peter Hatina 2011-03-08 06:05:09 EST
Created attachment 482875 [details]
Delay calls to NPP_Destroy when the plugin instance is on the stack.
Comment 5 Fedora Update System 2011-03-09 05:17:46 EST
nspluginwrapper-1.3.0-18.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/nspluginwrapper-1.3.0-18.fc15
Comment 6 Fedora Update System 2011-03-26 01:08:37 EDT
nspluginwrapper-1.3.0-18.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.