1966716 - Use USER_LIMITED for the Windows GPU process sandbox on Nightly

Status: RESOLVED FIXED

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

Description

[Bob Owen (:bobowen)]

Landing USER_LIMITED for the Windows GPU process sandbox on Nightly only, to see if we still have the user reported issues we had previously.

Comment 1

[Bob Owen (:bobowen)]

Attachment: Bug 1966716: Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman!

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d504e5a2f9a
Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman

Comment 3

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/3d504e5a2f9a

Comment 4

[Mayank Bansal]

Lead to huge regressions on many Talos startup-y tests only on Windows hw-wr. Sw-wr is unaffected. So the sandbox is delaying/hampering stuff at startup.

122% on ts_paint

90% on startup_about_home_paint

104% on sessionrestore hw-wr but sw-wr is unaffected

Comment 5

[:Gijs (he/him)]

(In reply to Mayank Bansal from comment #4)
Mayank: I really appreciate that you're helping us track performance issues. However, please stop CC'ing me on everything you find; I'm not on the performance team and I don't know about everything in Firefox (case in point: I know approximately nothing about our graphics code and sandboxing, i.e. this bug). Usually if you're asking questions or think action needs to be taken, pinging the assignee of the bug or triage owner (if there isn't an assignee) would be more appropriate.

Lead to huge regressions on many Talos startup-y tests only on Windows hw-wr. Sw-wr is unaffected. So the sandbox is delaying/hampering stuff at startup.

Is there a separate perf alert bug on file? That is the typical way to deal with this, right? If not, let's get one on file and needinfo the assignee of the regressing bug (Bob) there. Even better if you have profiles of a before/after build to show what is causing the slowdown.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3a91d82cca8
Revert "Bug 1966716: Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman" for causing sandboxing crashes related to font loading (bug 1967071)

Comment 7

[Sandor Molnar[:smolnar]]

Backed out for causing sandboxing crashes related to font loading

Backout link: https://hg.mozilla.org/integration/autoland/rev/f3a91d82cca84a81609f5c5e9bf827198b8c9ebc

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/f927eb67efc3
Revert "Bug 1966716: Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman" for causing sandboxing crashes related to font loading (bug 1967071)

Comment 9

[Mayank Bansal]

Gijs: I had cc'd you based on the test owner mentioned here for startup_about_home_paint

Comment 10

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/f3a91d82cca8

Comment 11

[Acasandrei Beatrice (needinfo me)]

(In reply to Sandor Molnar[:smolnar] from comment #7)

Backed out for causing sandboxing crashes related to font loading

Backout link: https://hg.mozilla.org/integration/autoland/rev/f3a91d82cca84a81609f5c5e9bf827198b8c9ebc

Perfherder has detected a talos performance change from push f3a91d82cca84a81609f5c5e9bf827198b8c9ebc.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
56%	ts_paint	windows11-64-24h2-shippable	e10s fission stylo webrender	725.50 -> 320.92
53%	sessionrestore_no_auto_restore	windows11-64-24h2-shippable	e10s fission stylo webrender	762.50 -> 358.08
52%	sessionrestore	windows11-64-24h2-shippable	e10s fission stylo webrender	740.33 -> 354.67
47%	startup_about_home_paint_cached	windows11-64-24h2-shippable	e10s fission stylo webrender	869.25 -> 458.83
47%	startup_about_home_paint	windows11-64-24h2-shippable	e10s fission stylo webrender	870.67 -> 463.17
46%	startup_about_home_paint_realworld_webextensions	windows11-64-24h2-shippable	e10s fission stylo webrender	891.00 -> 483.67

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 45210

The following documentation link provides more information about this command.

Comment 12

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=13ac4b16953183afa2960d38818e707360cc4d53

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1ddea5111f80
https://hg.mozilla.org/integration/autoland/rev/e01369503dde
Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman

Comment 14

[Norisz Fay [:noriszfay]]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0739bb7ed078
https://hg.mozilla.org/integration/autoland/rev/584aadc43139
Revert "Bug 1966716: Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman"  as requested

Comment 16

[pstanciu]

Backed out as requested

• https://hg.mozilla.org/integration/autoland/rev/584aadc4313965c02a9d0a6b637714d3f812f4b2

Comment 17

[Norisz Fay [:noriszfay]]

Backout merged to central: https://hg-edge.mozilla.org/mozilla-central/rev/584aadc43139

Comment 18

[Bob Owen (:bobowen)]

It looks like this is down to people having user only fonts in places other than the standard directory.
This could be manually done or more likely to be third party font managers.
This would not normally cause issues as the content process can't access these either (issues other than not being able to use those fonts).

However, dropdown/combo boxes still have their layout done in the parent, so it can access these fonts.
So, if they are used the GPU process can't find them causing either garbled fonts or crashes.
Even if it is possible moving the layout back to the content process, doesn't seem to be trivial.

The simplest solution is to add policy rules to allow read access to either the fonts or their dirs.
While not ideal, only the user, SYSTEM and Administrators have write access to the registry key, so to add malicious entry an attacker would already have user or higher access to the system.

Comment 19

[Bob Owen (:bobowen)]

try push with latest fix from bug 1977201:
https://treeherder.mozilla.org/jobs?repo=try&revision=50699f5a6fdfbb0857459681f6ad635a6ffd2548

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b2a64144d136
https://hg.mozilla.org/integration/autoland/rev/82c61d1dcc19
Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8f85432344cf
https://hg.mozilla.org/integration/autoland/rev/97ed289b2fc7
Revert "Bug 1966716: Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman" for causing marionetter failures on test_refresh_firefox.py

Comment 22

[Cosmin Sabou [:CosminS]]

Backed out for causing marionette failures on test_refresh_firefox.py

Push with failures

Failure log

Backout link

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7fbdf5cb7743
https://hg.mozilla.org/integration/autoland/rev/ad84e34ff5f7
Use USER_LIMITED for the Windows GPU process sandbox on Nightly. r=handyman

Comment 24

[Iulian Moraru]

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

Comment 25

[Donal Meehan [:dmeehan]]

:bobowen, anything you want to mention here in a release note? (Process info)
We could include it in the nightly only release notes if you wanted some extra visibility.

Comment 26

[Bob Owen (:bobowen)]

(In reply to Donal Meehan [:dmeehan] from comment #25)

:bobowen, anything you want to mention here in a release note? (Process info)
We could include it in the nightly only release notes if you wanted some extra visibility.

Possibly, but I'll wait a couple of days to make sure it sticks and the existing issues on previous attempts seem to be fixed.
There isn't anything specific people can do testing-wise, apart from look out for GPU process crashes and possibly rendering quirks, so we'll have to explain that, but I guess people using Nightly are more used to looking out for such things.

Comment 27

[Bob Owen (:bobowen)]

I'm going to wait until I'm back to think about this again.