Closed Bug 1752332 Opened 7 months ago Closed 4 months ago

Expand the pref-syncing-to-subprocesses blocklist to dynamic preferences

Categories

(Core :: Preferences: Backend, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

Attachments

(20 files, 9 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
No description provided.
Attachment #9261055 - Attachment description: WIP: Bug 1752332: Expand the pref-syncing-to-subprocesses blocklist to dynamic preferences → WIP: Bug 1752332: Do not sync dynamic preferences to subprocesses
Attachment #9261055 - Attachment is obsolete: true

C++ preferences changes are likely going to be the Core::Preferences module, not the Desktop Firefox::Preferences module. Kris Wright could review these patches.

I'm suggesting P1 as this is a blocker for removal spectre mitigations.

Priority: -- → P1
Attachment #9263785 - Attachment description: Bug 1752332: Add in a sanitize property for prefs, and only serialize the user value if we're not sanitized r?jaws → Bug 1752332: Add in a sanitize property for prefs, and only serialize the user value if we're not sanitized r?kriswright
Attachment #9263786 - Attachment description: Bug 1752332: Rename references to parent-only pref structures r?jaws → Bug 1752332: Rename references to parent-only pref structures r?kriswright
Attachment #9263787 - Attachment description: Bug 1752332: Move ShouldSyncPreferences to Preferences module r?jaws → Bug 1752332: Move ShouldSyncPreferences to Preferences module r?kriswright
Attachment #9263788 - Attachment description: Bug 1752332: Rename ShouldSyncPreference to ShouldSanitizePreference r?jaws → Bug 1752332: Rename ShouldSyncPreference to ShouldSanitizePreference r?kriswright
Attachment #9263789 - Attachment description: Bug 1752332: Remove browser.startup from blocklist r?jaws → Bug 1752332: Remove browser.startup from blocklist r?kriswright
Attachment #9263790 - Attachment description: Bug 1752332: Remove the blocklisting check in ::OnPreferenceChange r?jaws → Bug 1752332: Remove the blocklisting check in ::OnPreferenceChange r?kriswright
Attachment #9263791 - Attachment description: Bug 1752332: Improve the blocklisting behavior r?jaws → Bug 1752332: Improve the blocklisting behavior r?kriswright
Attachment #9263792 - Attachment description: Bug 1752332: Crash if a pref is accessed that shouldn't be r?jaws → Bug 1752332: Crash if a pref is accessed that shouldn't be r?kriswright
Attachment #9263793 - Attachment description: Bug 1752332: Add preferences that control whether we send user data and/or crash r?jaws → Bug 1752332: Add preferences that control whether we send user data and/or crash r?kriswright
Attachment #9263794 - Attachment description: Bug 1752332: Do not sanitize anything for non-content processes r?jaws → Bug 1752332: Do not sanitize anything for non-content processes r?kriswright

Also, I think a more precise bugzilla component for changes under modules/libpref/ is Core::Preferences::Backend. (I haven't looked at all of the patches so I don't know if there are any changes here to the frontend part of the preferences code.)

Component: Preferences → Preferences: Backend
Product: Toolkit → Core

All this behavior is off-by-default, so I'm going to try to land it and then let a few people opt-in for testing.

Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a875dc06058
Add in a sanitize property for prefs, and only serialize the user value if we're not sanitized r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/26796b527442
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/be908bb7a985
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/8de253223dbc
Rename ShouldSyncPreference to ShouldSanitizePreference r=necko-reviewers,KrisWright,dragana
https://hg.mozilla.org/integration/autoland/rev/b10197a1ed67
Remove browser.startup from blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/a8832dc94d1b
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/593e5e138927
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/e8b2a80aa796
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/7fb5f0cc44f6
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/5c475692133f
Do not sanitize anything for non-content processes r=KrisWright
Attachment #9263785 - Attachment is obsolete: true
Attachment #9263786 - Attachment is obsolete: true
Attachment #9263787 - Attachment is obsolete: true
Attachment #9263788 - Attachment is obsolete: true
Attachment #9263789 - Attachment is obsolete: true
Attachment #9263790 - Attachment is obsolete: true
Attachment #9263791 - Attachment is obsolete: true
Attachment #9265232 - Attachment is obsolete: true

If anyone ever ends up here in the future doing bug archaeology; https://bugzilla.mozilla.org/show_bug.cgi?id=1751936#c9 and below are relevant to understanding how the above patches came about.

Flags: needinfo?(tom)

Alright; I rebased and fixed a few issues that popped up and got a try run where all the failures seemed to be intermittents...

Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4944ae34f15b
Add a sanitized property to prefs r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/deadcb975866
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/9dfd530f26b8
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/185026a397b0
Tell ShouldSyncPreference if the destination is a web content process r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ce809df435e1
Rename ShouldSyncPreference to ShouldSanitizePreference r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/22c6c04046b1
Correctly populate the sanitized bit for PreferenceUpdate r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/01ca344dedbf
Remove some prefs from the blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/26707a82d896
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/061cdf612d0e
Make SerializePreferences correctly sanitize preferences r=KrisWright,necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/d72dc14eeafd
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ab5c00d2fb02
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/52f6b46250a3
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/258d19fe4e28
Remove the shouldSanitizeFunction member r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/d37dec5c0d9e
Optimize the crash checks in Check*Value functions r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/10ac6886fa5e
Sanitize preferences in the shared memory region r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ec6a5f016318
Update the preferences documentation r=KrisWright

Backed out for causing multiple failures on StaticPrefList_media.h

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))), at /builds/worker/workspace/obj-build/dist/include/mozilla/StaticPrefList_media.h:260
Flags: needinfo?(tom)

Okay; I believe that's fixed...

Flags: needinfo?(tom)
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e40a778cb93f
Add a sanitized property to prefs r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/1b4d316e7f20
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ef50d2618c7f
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/eec507d87b2b
Tell ShouldSyncPreference if the destination is a web content process r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/8566711743a8
Rename ShouldSyncPreference to ShouldSanitizePreference r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/4b04bf33486e
Correctly populate the sanitized bit for PreferenceUpdate r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/a74095dfe3da
Remove some prefs from the blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/21f015b60220
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/2b78f295d903
Make SerializePreferences correctly sanitize preferences r=KrisWright,necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/1bc4f1780f37
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/c184c517de91
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/aeb79413e987
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/5b9bbe252fec
Remove the shouldSanitizeFunction member r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/7d159898e81d
Optimize the crash checks in Check*Value functions r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/8d2932869cbd
Sanitize preferences in the shared memory region r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/2523f5463789
Update the preferences documentation r=KrisWright

Backed out for causing gtest failures.

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | PrefsBasics.Serialize | Expected: (nullptr) != (strstr(str.Data(), "S-S:36/media.gmp-manager.certs.1.commonName")), actual: (nullptr) vs NULL @ /builds/worker/checkouts/gecko/modules/libpref/test/gtest/Basics.cpp:54
Flags: needinfo?(tom)

okay, fixed, try run's green, let's try again.

Flags: needinfo?(tom)
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ce1d0529b34
Add a sanitized property to prefs r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ce9e1254e82f
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/2dadbfd0b1d7
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/0e70bf8ca3e4
Tell ShouldSyncPreference if the destination is a web content process r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/61b6a151b215
Rename ShouldSyncPreference to ShouldSanitizePreference r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/f48f4c1b0784
Correctly populate the sanitized bit for PreferenceUpdate r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/e5d5d24b0f3b
Remove some prefs from the blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/fe8f3e1c43b0
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/322bbf59820a
Make SerializePreferences correctly sanitize preferences r=KrisWright,necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/6320b4b3d12d
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/33fb4d7c0f3c
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/1e3858df144d
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/1ca918455158
Remove the shouldSanitizeFunction member r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/c9c556d2f676
Optimize the crash checks in Check*Value functions r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/090719a92e33
Sanitize preferences in the shared memory region r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/1e57c99c133b
Update the preferences documentation r=KrisWright

Giving another try at landing this

Flags: needinfo?(tom)
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2d5880b528f
Add a sanitized property to prefs r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/b580c2a3bac0
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/a89203990075
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/d2e748ccd01c
Tell ShouldSyncPreference if the destination is a web content process r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/22b910308ecd
Rename ShouldSyncPreference to ShouldSanitizePreference r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/d165042105ea
Correctly populate the sanitized bit for PreferenceUpdate r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ce93b08837d2
Remove some prefs from the blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/3e60b77153a3
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/dd02b8ef0219
Make SerializePreferences correctly sanitize preferences r=KrisWright,necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/f9d6bc72406f
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/e1dcb4c7cb88
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/46ea5b4f9ad3
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/1ff8656b362d
Remove the shouldSanitizeFunction member r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/022a68e8d603
Optimize the crash checks in Check*Value functions r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/69cbec3e9a11
Sanitize preferences in the shared memory region r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/2d794b61fbf7
Update the preferences documentation r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/eaa4213b9e08
Suppress Hazard warnings due to hash table function pointers r=sfink

Once Bug 1766381 lands, I can reland this.

Flags: needinfo?(tom)
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e9f0c59af60a
Add a sanitized property to prefs r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/f920aa7e10ee
Rename references to parent-only pref structures r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/a8c7a3fb8979
Move ShouldSyncPreferences to Preferences module r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/2645ce66e4eb
Tell ShouldSyncPreference if the destination is a web content process r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/bfe9ec21265d
Rename ShouldSyncPreference to ShouldSanitizePreference r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/674d21f5ef9c
Correctly populate the sanitized bit for PreferenceUpdate r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/c4005c136bc8
Remove some prefs from the blocklist r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/baf63e3988b3
Remove the blocklisting check in ::OnPreferenceChange r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/0547ca4f6396
Make SerializePreferences correctly sanitize preferences r=KrisWright,necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/e08344bb3e98
Improve the blocklisting behavior r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/ab581def9c6a
Crash if a pref is accessed that shouldn't be r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/eafd5e87610f
Add preferences that control whether we send user data and/or crash r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/b7d958f0b034
Remove the shouldSanitizeFunction member r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/4610f070c355
Optimize the crash checks in Check*Value functions r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/0c2e8586a0c9
Sanitize preferences in the shared memory region r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/d7155101cf54
Update the preferences documentation r=KrisWright
https://hg.mozilla.org/integration/autoland/rev/6e78cd06efbc
Suppress Hazard warnings due to hash table function pointers r=sfink
Regressions: 1767281
No longer regressions: 1767281
You need to log in before you can comment on or make changes to this bug.