Not all plugins should be opt out of 32-bit NX.
Created attachment 84619 [details] Patch
Comment on attachment 84619 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84619&action=review > WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp:222 > +bool PluginProcessProxy::pluginNeedsExecutableHeap(const PluginInfoStore::Plugin& pluginInfo) const This doesn't need to be a member function of PluginProcessProxy, it can just be a static free function (and I'd put it above the call site.
Bug title makes no sense. What is NX?
Simon, Take a look at this: <rdar://problem/8105706> and that: http://en.wikipedia.org/wiki/NX_bit
You should spell out "non-executable", and clarify what 32-bit NX is. Does this only apply to 32-bit plug-in, or does it mean something else? And is the plugin opting out, or is the browser forcing this on the plug-in?
Created attachment 84638 [details] Proposed patch.
This patch is confused. The ChangeLog states that Silverlight and QuickTime should opt *in* to a 32-bit NX heap, which means their heap should not be executable. But then it opts them *out* by making pluginNeedsExecutableHeap() claim that those two plugins need an executable heap — i.e. that they cannot run with NX. It also opts all other plugins into running with an NX heap. Assuming you're really trying to say "QuickTime and Silverlight should run with a non-executable heap (opt-in), and other plugins should have an executable one (opt-out)", you need to invert your true/false returns in pluginNeedsExecutableHeap.
I misunderstood this bug, let me clarify the ChangeLog and make changes to the patch. (In reply to comment #7) > This patch is confused. The ChangeLog states that Silverlight and QuickTime should opt *in* to a 32-bit NX heap, which means their heap should not be executable. But then it opts them *out* by making pluginNeedsExecutableHeap() claim that those two plugins need an executable heap — i.e. that they cannot run with NX. It also opts all other plugins into running with an NX heap. > > Assuming you're really trying to say "QuickTime and Silverlight should run with a non-executable heap (opt-in), and other plugins should have an executable one (opt-out)", you need to invert your true/false returns in pluginNeedsExecutableHeap.
Created attachment 84766 [details] Proposed patch.
Looks good to me.
Comment on attachment 84766 [details] Proposed patch. Rejecting attachment 84766 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build-..." exit_code: 2 Last 500 characters of output: ....................................................................................................................................................................................................................... inspector ........... inspector/audits . inspector/audits/audits-panel-functional.html -> failed Exiting early after 1 failures. 12761 tests run. 301.30s total testing time 12760 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 8 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/8082846
Committed r80523: <http://trac.webkit.org/changeset/80523>