1322024 - Remove com.apple.windowserver.active from the content process Mac sandbox

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Haik Aftandilian [:haik]]

Removing access to com.apple.windowserver.active exposes performance regressions on Talos tests "damp opt e10s" (~10%) and "tps opt e10s" (~20%). This bug is to cover investigating why this is happening and then remove windowserver.active without performance issues. Allowing access to windowserver.active might allow a compromised content process to draw to the screen and it's one more API we'd rather not expose to content if we don't have to.

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

The set of regressions here sound like they're on the same benchmarks as were improved by disabling logging (bug 1383841). We should trigger a talos run and see if removing windserver.active still has any performance impact.

Comment 2

[Haik Aftandilian [:haik]]

I'll re-do the talos tests. Gabor also mentioned that those regressing tests have been changed since this was last run. I pushed a try run on the 24th and hit a handful of test failures which I didn't get to look into yet. They look mostly unrelated, but I'll retry and see where things are today.

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=6917c33c25b23218fb4efe71a690917b2df4a131

Comment 3

[Haik Aftandilian [:haik]]

Talos try:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=04d7f75bcc600dc113ecf4e10ea1678f4bee134d

Comparison link:

  https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=04d7f75bcc600dc113ecf4e10ea1678f4bee134d

Comment 4

[Haik Aftandilian [:haik]]

(In reply to Haik Aftandilian [:haik] from comment #3)
> https://treeherder.mozilla.org/perf.html#/
> comparechooser?newProject=try&newRevision=04d7f75bcc600dc113ecf4e10ea1678f4be
> e134d

The largest delta is "damp opt e10s" being 10.47% worse, although it isn't included when selecting "Show only important changes
Non-trivial changes (2%+)". No other results look significant. I will run this by someone with more experience interpreting talos results.

Comment 5

[Haik Aftandilian [:haik]]

(In reply to Haik Aftandilian [:haik] from comment #4)
> (In reply to Haik Aftandilian [:haik] from comment #3)
> > https://treeherder.mozilla.org/perf.html#/
> > comparechooser?newProject=try&newRevision=04d7f75bcc600dc113ecf4e10ea1678f4be
> > e134d
> 
> The largest delta is "damp opt e10s" being 10.47% worse, although it isn't
> included when selecting "Show only important changes
> Non-trivial changes (2%+)". No other results look significant. I will run
> this by someone with more experience interpreting talos results.

After adding more repeats to the try run, none of the perf differences are significant.

Local tests and try results haven't turned up any issues with removing windowserver.active so I'm moving forward with this fix.

Comment 6

[Haik Aftandilian [:haik]]

Attachment: Bug 1322024 - [Mac] Remove com.apple.windowserver.active access from the content sandbox.

Review commit: https://reviewboard.mozilla.org/r/165780/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165780/

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8894626 [details]
Bug 1322024 - [Mac] Remove com.apple.windowserver.active access from the content sandbox.

https://reviewboard.mozilla.org/r/165780/#review170884

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da3110f2da8b
[Mac] Remove com.apple.windowserver.active access from the content sandbox. r=Alex_Gaynor

Comment 9

[Carsten Book [:Tomcat]]

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