[e10s] Forbid CPOW usage if add-on declares it is multiprocessCompatible

RESOLVED FIXED in Firefox 50

Status

()

Core
DOM: Content Processes
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

({addon-compat})

Trunk
mozilla50
addon-compat
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox50 fixed)

Details

Attachments

(1 attachment)

The idea is to disable CPOWs in compartments associated with add-ons that are multiprocessCompatible. That will guarantee that these add-ons aren't getting CPOWs through some accidental route (i.e., something other than the shims).
Assignee: nobody → wmccloskey
Summary: [e10s] Forbid CPOW usage is add-on declares it is multiprocessCompatible → [e10s] Forbid CPOW usage if add-on declares it is multiprocessCompatible
Created attachment 8764442 [details] [diff] [review]
patch

We're getting back some data showing that add-ons are setting multiprocessCompatible but still using CPOWs. This patch will prevent that.

It's enabled in two ways:
1. Unless we set dom.ipc.cpows.forbid-cpows-in-compat-addons, it does nothing.
2. If that pref is set, then we check if the add-on ID is listed in dom.ipc.cpows.allow-cpows-in-compat-addons. If it is, then the patch does nothing.

Otherwise we block the CPOW. The patch also will collect telemetry about which add-ons marked as being compatible use CPOWs. We can use this to generate the initial whitelist and then contact those add-on authors.
Attachment #8764442 - Flags: review?(mrbkap)
Comment on attachment 8764442 [details] [diff] [review]
patch

Review of attachment 8764442 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/ipc/JavaScriptParent.cpp
@@ +123,4 @@
>                  Telemetry::Accumulate(Telemetry::BROWSER_SHIM_USAGE_BLOCKED, 1);
>                  JS_ReportError(cx, "unsafe CPOW usage forbidden");
>                  return false;
> +            } else if (addonId) {

Nit (here and below): We at least used to avoid else-after-return. I think in this case, it makes sense.

::: js/xpconnect/src/XPCComponents.cpp
@@ +3366,5 @@
> +nsXPCComponents_Utils::AllowCPOWsInAddon(const nsACString& addonIdStr,
> +                                         bool allow,
> +                                         JSContext* cx)
> +{
> +    printf("!allow in %s\n", nsCString(addonIdStr).get());

This should go away.
Attachment #8764442 - Flags: review?(mrbkap) → review+
Keywords: addon-compat

Comment 3

2 years ago
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14928a6b38f3
Forbid CPOW usage if add-on declares it is multiprocessCompatible (r=mrbkap)
e10s debug devtools (which means Mac and Win7, those being the only place they run), leaked a StringBuffer, https://treeherder.mozilla.org/logviewer.html#?job_id=31029495&repo=mozilla-inbound

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/0a03bb6af6043ce323142d24fb43c2d63deefbcb

Comment 5

2 years ago
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/960aa6832bf7
Forbid CPOW usage if add-on declares it is multiprocessCompatible (r=mrbkap)

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/960aa6832bf7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.