From 512562821fe064fd7c232e35831e829f3b4b7e24 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Thu, 21 Apr 2022 21:33:03 +0300 Subject: [PATCH] Backed out 16 changesets (bug 1752332) for causing unrooted hazard failures. CLOSED TREE Backed out changeset 1e57c99c133b (bug 1752332) Backed out changeset 090719a92e33 (bug 1752332) Backed out changeset c9c556d2f676 (bug 1752332) Backed out changeset 1ca918455158 (bug 1752332) Backed out changeset 1e3858df144d (bug 1752332) Backed out changeset 33fb4d7c0f3c (bug 1752332) Backed out changeset 6320b4b3d12d (bug 1752332) Backed out changeset 322bbf59820a (bug 1752332) Backed out changeset fe8f3e1c43b0 (bug 1752332) Backed out changeset e5d5d24b0f3b (bug 1752332) Backed out changeset f48f4c1b0784 (bug 1752332) Backed out changeset 61b6a151b215 (bug 1752332) Backed out changeset 0e70bf8ca3e4 (bug 1752332) Backed out changeset 2dadbfd0b1d7 (bug 1752332) Backed out changeset ce9e1254e82f (bug 1752332) Backed out changeset 3ce1d0529b34 (bug 1752332) --- dom/ipc/ContentParent.cpp | 70 ++--- dom/ipc/ContentParent.h | 2 + dom/ipc/PrefsTypes.ipdlh | 1 - dom/media/ipc/RDDProcessHost.cpp | 6 +- dom/media/ipc/RDDProcessManager.cpp | 10 +- gfx/ipc/GPUProcessHost.cpp | 6 +- gfx/ipc/GPUProcessManager.cpp | 10 +- gfx/vr/ipc/VRProcessManager.cpp | 10 +- gfx/vr/ipc/VRProcessParent.cpp | 7 +- ipc/glue/ProcessUtils.h | 9 +- ipc/glue/ProcessUtils_common.cpp | 17 +- ipc/glue/UtilityProcessHost.cpp | 6 +- ipc/glue/UtilityProcessManager.cpp | 11 +- ipc/ipdl/test/gtest/IPDLUnitTest.cpp | 6 +- modules/libpref/Preferences.cpp | 287 +++------------------ modules/libpref/Preferences.h | 16 +- modules/libpref/SharedPrefMap.cpp | 16 +- modules/libpref/SharedPrefMap.h | 6 - modules/libpref/StaticPrefsBase.h | 6 - modules/libpref/docs/index.md | 57 ++-- modules/libpref/init/StaticPrefList.yaml | 12 - modules/libpref/init/StaticPrefListBegin.h | 50 ++-- modules/libpref/nsIPrefBranch.idl | 18 -- modules/libpref/test/gtest/Basics.cpp | 17 +- netwerk/base/nsIOService.cpp | 7 +- netwerk/ipc/SocketProcessHost.cpp | 6 +- 26 files changed, 182 insertions(+), 487 deletions(-) diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 3ce950fbc17f..d88123ffd312 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -79,7 +79,6 @@ #include "mozilla/Components.h" #include "mozilla/Sprintf.h" #include "mozilla/StaticPrefs_dom.h" -#include "mozilla/StaticPrefs_fission.h" #include "mozilla/StaticPrefs_media.h" #include "mozilla/StaticPrefs_widget.h" #include "mozilla/StyleSheet.h" @@ -648,23 +647,6 @@ static const char* sObserverTopics[] = { "network:socket-process-crashed", }; -static const char kFissionEnforceBlockList[] = - "fission.enforceBlocklistedPrefsInSubprocesses"; -static const char kFissionOmitBlockListValues[] = - "fission.omitBlocklistedPrefsInSubprocesses"; - -static void OnFissionBlocklistPrefChange(const char* aPref, void* aData) { - if (strcmp(aPref, kFissionEnforceBlockList) == 0) { - sCrashOnBlocklistedPref = - StaticPrefs::fission_enforceBlocklistedPrefsInSubprocesses(); - } else if (strcmp(aPref, kFissionOmitBlockListValues) == 0) { - sOmitBlocklistedPrefValues = - StaticPrefs::fission_omitBlocklistedPrefsInSubprocesses(); - } else { - MOZ_CRASH("Unknown pref passed to callback"); - } -} - // PreallocateProcess is called by the PreallocatedProcessManager. // ContentParent then takes this process back within GetNewOrUsedBrowserProcess. /*static*/ already_AddRefed @@ -694,11 +676,6 @@ void ContentParent::StartUp() { BackgroundChild::Startup(); ClientManager::Startup(); - Preferences::RegisterCallbackAndCall(&OnFissionBlocklistPrefChange, - kFissionEnforceBlockList); - Preferences::RegisterCallbackAndCall(&OnFissionBlocklistPrefChange, - kFissionOmitBlockListValues); - #if defined(XP_LINUX) && defined(MOZ_SANDBOX) sSandboxBrokerPolicyFactory = MakeUnique(); #endif @@ -2540,9 +2517,9 @@ bool ContentParent::BeginSubprocessLaunch(ProcessPriority aPriority) { // Instantiate the pref serializer. It will be cleaned up in // `LaunchSubprocessReject`/`LaunchSubprocessResolve`. - mPrefSerializer = MakeUnique(); - if (!mPrefSerializer->SerializeToSharedMemory(GeckoProcessType_Content, - GetRemoteType())) { + mPrefSerializer = MakeUnique( + ShouldSyncPreference); + if (!mPrefSerializer->SerializeToSharedMemory()) { NS_WARNING("SharedPreferenceSerializer::SerializeToSharedMemory failed"); MarkAsDead(); return false; @@ -3668,11 +3645,13 @@ ContentParent::Observe(nsISupports* aSubject, const char* aTopic, // We know prefs are ASCII here. NS_LossyConvertUTF16toASCII strData(aData); - Pref pref(strData, /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); + // A pref changed. If it is useful to do so, inform child processes. + if (!ShouldSyncPreference(strData.Data())) { + return NS_OK; + } - Preferences::GetPreference(&pref, GeckoProcessType_Content, - GetRemoteType()); + Pref pref(strData, /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); if (IsInitialized()) { MOZ_ASSERT(mQueuedPrefs.IsEmpty()); if (!SendPreferenceUpdate(pref)) { @@ -3819,6 +3798,37 @@ ContentParent::Observe(nsISupports* aSubject, const char* aTopic, return NS_OK; } +/* static */ +bool ContentParent::ShouldSyncPreference(const char* aPref) { +#define PARENT_ONLY_PREF_LIST_ENTRY(s) \ + { s, (sizeof(s) / sizeof(char)) - 1 } + struct ParentOnlyPrefListEntry { + const char* mPrefBranch; + size_t mLen; + }; + // These prefs are not useful in child processes. + static const ParentOnlyPrefListEntry sParentOnlyPrefBranchList[] = { + PARENT_ONLY_PREF_LIST_ENTRY("app.update.lastUpdateTime."), + PARENT_ONLY_PREF_LIST_ENTRY("datareporting.policy."), + PARENT_ONLY_PREF_LIST_ENTRY("browser.safebrowsing.provider."), + PARENT_ONLY_PREF_LIST_ENTRY("browser.shell."), + PARENT_ONLY_PREF_LIST_ENTRY("browser.slowStartup."), + PARENT_ONLY_PREF_LIST_ENTRY("browser.startup."), + PARENT_ONLY_PREF_LIST_ENTRY("extensions.getAddons.cache."), + PARENT_ONLY_PREF_LIST_ENTRY("media.gmp-manager."), + PARENT_ONLY_PREF_LIST_ENTRY("media.gmp-gmpopenh264."), + PARENT_ONLY_PREF_LIST_ENTRY("privacy.sanitize."), + }; +#undef PARENT_ONLY_PREF_LIST_ENTRY + + for (const auto& entry : sParentOnlyPrefBranchList) { + if (strncmp(entry.mPrefBranch, aPref, entry.mLen) == 0) { + return false; + } + } + return true; +} + void ContentParent::UpdateNetworkLinkType() { nsresult rv; nsCOMPtr nls = diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index a63419ed37de..b6276ef19df0 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -1460,6 +1460,8 @@ class ContentParent final void UpdateNetworkLinkType(); + static bool ShouldSyncPreference(const char* aPref); + already_AddRefed InitJSActor(JS::HandleObject aMaybeActor, const nsACString& aName, ErrorResult& aRv) override; diff --git a/dom/ipc/PrefsTypes.ipdlh b/dom/ipc/PrefsTypes.ipdlh index 403c7aaf4296..762351a42921 100644 --- a/dom/ipc/PrefsTypes.ipdlh +++ b/dom/ipc/PrefsTypes.ipdlh @@ -23,7 +23,6 @@ union PrefValue { struct Pref { nsCString name; bool isLocked; - bool isSanitized; PrefValue? defaultValue; PrefValue? userValue; }; diff --git a/dom/media/ipc/RDDProcessHost.cpp b/dom/media/ipc/RDDProcessHost.cpp index bba6d7a4ca52..c3cff03123bd 100644 --- a/dom/media/ipc/RDDProcessHost.cpp +++ b/dom/media/ipc/RDDProcessHost.cpp @@ -46,9 +46,9 @@ bool RDDProcessHost::Launch(StringVector aExtraOpts) { MOZ_ASSERT(mLaunchPhase == LaunchPhase::Unlaunched); MOZ_ASSERT(!mRDDChild); - mPrefSerializer = MakeUnique(); - if (!mPrefSerializer->SerializeToSharedMemory(GeckoProcessType_RDD, - /* remoteType */ ""_ns)) { + mPrefSerializer = MakeUnique( + dom::ContentParent::ShouldSyncPreference); + if (!mPrefSerializer->SerializeToSharedMemory()) { return false; } mPrefSerializer->AddSharedPrefCmdLineArgs(*this, aExtraOpts); diff --git a/dom/media/ipc/RDDProcessManager.cpp b/dom/media/ipc/RDDProcessManager.cpp index 8a6dff85dc2c..c69d9fa67824 100644 --- a/dom/media/ipc/RDDProcessManager.cpp +++ b/dom/media/ipc/RDDProcessManager.cpp @@ -96,11 +96,13 @@ void RDDProcessManager::OnPreferenceChange(const char16_t* aData) { // We know prefs are ASCII here. NS_LossyConvertUTF16toASCII strData(aData); - mozilla::dom::Pref pref(strData, /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); + // A pref changed. If it is useful to do so, inform child processes. + if (!dom::ContentParent::ShouldSyncPreference(strData.Data())) { + return; + } - Preferences::GetPreference(&pref, GeckoProcessType_RDD, - /* remoteType */ ""_ns); + mozilla::dom::Pref pref(strData, /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); if (!!mRDDChild) { MOZ_ASSERT(mQueuedPrefs.IsEmpty()); mRDDChild->SendPreferenceUpdate(pref); diff --git a/gfx/ipc/GPUProcessHost.cpp b/gfx/ipc/GPUProcessHost.cpp index 7205bc3e05bf..69e79c8b98ce 100644 --- a/gfx/ipc/GPUProcessHost.cpp +++ b/gfx/ipc/GPUProcessHost.cpp @@ -42,9 +42,9 @@ bool GPUProcessHost::Launch(StringVector aExtraOpts) { MOZ_ASSERT(!mGPUChild); MOZ_ASSERT(!gfxPlatform::IsHeadless()); - mPrefSerializer = MakeUnique(); - if (!mPrefSerializer->SerializeToSharedMemory(GeckoProcessType_GPU, - /* remoteType */ ""_ns)) { + mPrefSerializer = MakeUnique( + dom::ContentParent::ShouldSyncPreference); + if (!mPrefSerializer->SerializeToSharedMemory()) { return false; } mPrefSerializer->AddSharedPrefCmdLineArgs(*this, aExtraOpts); diff --git a/gfx/ipc/GPUProcessManager.cpp b/gfx/ipc/GPUProcessManager.cpp index 2d9c4798e794..ac6c627a585c 100644 --- a/gfx/ipc/GPUProcessManager.cpp +++ b/gfx/ipc/GPUProcessManager.cpp @@ -163,11 +163,13 @@ void GPUProcessManager::OnPreferenceChange(const char16_t* aData) { // We know prefs are ASCII here. NS_LossyConvertUTF16toASCII strData(aData); - mozilla::dom::Pref pref(strData, /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); + // A pref changed. If it is useful to do so, inform child processes. + if (!dom::ContentParent::ShouldSyncPreference(strData.Data())) { + return; + } - Preferences::GetPreference(&pref, GeckoProcessType_GPU, - /* remoteType */ ""_ns); + mozilla::dom::Pref pref(strData, /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); if (!!mGPUChild) { MOZ_ASSERT(mQueuedPrefs.IsEmpty()); mGPUChild->SendPreferenceUpdate(pref); diff --git a/gfx/vr/ipc/VRProcessManager.cpp b/gfx/vr/ipc/VRProcessManager.cpp index f3bc13bac03b..01f56b3648a3 100644 --- a/gfx/vr/ipc/VRProcessManager.cpp +++ b/gfx/vr/ipc/VRProcessManager.cpp @@ -216,11 +216,13 @@ void VRProcessManager::OnPreferenceChange(const char16_t* aData) { // We know prefs are ASCII here. NS_LossyConvertUTF16toASCII strData(aData); - mozilla::dom::Pref pref(strData, /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); + // A pref changed. If it is useful to do so, inform child processes. + if (!dom::ContentParent::ShouldSyncPreference(strData.Data())) { + return; + } - Preferences::GetPreference(&pref, GeckoProcessType_VR, - /* remoteType */ ""_ns); + mozilla::dom::Pref pref(strData, /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); if (!!mVRChild) { MOZ_ASSERT(mQueuedPrefs.IsEmpty()); mVRChild->SendPreferenceUpdate(pref); diff --git a/gfx/vr/ipc/VRProcessParent.cpp b/gfx/vr/ipc/VRProcessParent.cpp index 2546c0d31171..37187bb57693 100644 --- a/gfx/vr/ipc/VRProcessParent.cpp +++ b/gfx/vr/ipc/VRProcessParent.cpp @@ -16,7 +16,6 @@ #include "mozilla/ipc/ProcessUtils.h" #include "mozilla/ipc/ProtocolTypes.h" #include "mozilla/ipc/ProtocolUtils.h" // for IToplevelProtocol -#include "mozilla/Preferences.h" #include "mozilla/StaticPrefs_dom.h" #include "mozilla/TimeStamp.h" // for TimeStamp #include "mozilla/Unused.h" @@ -55,9 +54,9 @@ bool VRProcessParent::Launch() { std::vector extraArgs; ProcessChild::AddPlatformBuildID(extraArgs); - mPrefSerializer = MakeUnique(); - if (!mPrefSerializer->SerializeToSharedMemory(GeckoProcessType_VR, - /* remoteType */ ""_ns)) { + mPrefSerializer = MakeUnique( + dom::ContentParent::ShouldSyncPreference); + if (!mPrefSerializer->SerializeToSharedMemory()) { return false; } mPrefSerializer->AddSharedPrefCmdLineArgs(*this, extraArgs); diff --git a/ipc/glue/ProcessUtils.h b/ipc/glue/ProcessUtils.h index b0f146ef6d4a..023750847f82 100644 --- a/ipc/glue/ProcessUtils.h +++ b/ipc/glue/ProcessUtils.h @@ -13,8 +13,6 @@ #include "mozilla/ipc/FileDescriptor.h" #include "base/shared_memory.h" #include "mozilla/Maybe.h" -#include "mozilla/Preferences.h" -#include "nsXULAppAPI.h" namespace mozilla { namespace ipc { @@ -27,12 +25,12 @@ void SetThisProcessName(const char* aName); class SharedPreferenceSerializer final { public: - explicit SharedPreferenceSerializer(); + explicit SharedPreferenceSerializer( + std::function&& aShouldSerializeFn); SharedPreferenceSerializer(SharedPreferenceSerializer&& aOther); ~SharedPreferenceSerializer(); - bool SerializeToSharedMemory(const GeckoProcessType aDestinationProcessType, - const nsACString& aDestinationRemoteType); + bool SerializeToSharedMemory(); size_t GetPrefMapSize() const { return mPrefMapSize; } size_t GetPrefsLength() const { return mPrefsLength; } @@ -50,6 +48,7 @@ class SharedPreferenceSerializer final { size_t mPrefsLength; UniqueFileHandle mPrefMapHandle; UniqueFileHandle mPrefsHandle; + std::function mShouldSerializeFn; }; class SharedPreferenceDeserializer final { diff --git a/ipc/glue/ProcessUtils_common.cpp b/ipc/glue/ProcessUtils_common.cpp index 69eb4bdf287f..5dbe13cc7b2f 100644 --- a/ipc/glue/ProcessUtils_common.cpp +++ b/ipc/glue/ProcessUtils_common.cpp @@ -8,7 +8,6 @@ #include "mozilla/Preferences.h" #include "mozilla/GeckoArgs.h" -#include "mozilla/dom/RemoteType.h" #include "mozilla/ipc/GeckoChildProcessHost.h" #include "mozilla/UniquePtrExtensions.h" #include "nsPrintfCString.h" @@ -18,8 +17,9 @@ namespace mozilla { namespace ipc { -SharedPreferenceSerializer::SharedPreferenceSerializer() - : mPrefMapSize(0), mPrefsLength(0) { +SharedPreferenceSerializer::SharedPreferenceSerializer( + std::function&& aShouldSerializeFn) + : mPrefMapSize(0), mPrefsLength(0), mShouldSerializeFn(aShouldSerializeFn) { MOZ_COUNT_CTOR(SharedPreferenceSerializer); } @@ -36,20 +36,13 @@ SharedPreferenceSerializer::SharedPreferenceSerializer( MOZ_COUNT_CTOR(SharedPreferenceSerializer); } -bool SharedPreferenceSerializer::SerializeToSharedMemory( - const GeckoProcessType aDestinationProcessType, - const nsACString& aDestinationRemoteType) { +bool SharedPreferenceSerializer::SerializeToSharedMemory() { mPrefMapHandle = Preferences::EnsureSnapshot(&mPrefMapSize).TakePlatformHandle(); - bool destIsWebContent = - aDestinationProcessType == GeckoProcessType_Content && - (StringBeginsWith(aDestinationRemoteType, WEB_REMOTE_TYPE) || - StringBeginsWith(aDestinationRemoteType, PREALLOC_REMOTE_TYPE)); - // Serialize the early prefs. nsAutoCStringN<1024> prefs; - Preferences::SerializePreferences(prefs, destIsWebContent); + Preferences::SerializePreferences(prefs, mShouldSerializeFn); mPrefsLength = prefs.Length(); base::SharedMemory shm; diff --git a/ipc/glue/UtilityProcessHost.cpp b/ipc/glue/UtilityProcessHost.cpp index 2f95d4eb56a0..45e7e59f02d6 100644 --- a/ipc/glue/UtilityProcessHost.cpp +++ b/ipc/glue/UtilityProcessHost.cpp @@ -60,9 +60,9 @@ bool UtilityProcessHost::Launch(StringVector aExtraOpts) { MOZ_ASSERT(mLaunchPhase == LaunchPhase::Unlaunched); MOZ_ASSERT(!mUtilityProcessParent); - mPrefSerializer = MakeUnique(); - if (!mPrefSerializer->SerializeToSharedMemory(GeckoProcessType_Utility, - /* remoteType */ ""_ns)) { + mPrefSerializer = MakeUnique( + dom::ContentParent::ShouldSyncPreference); + if (!mPrefSerializer->SerializeToSharedMemory()) { return false; } mPrefSerializer->AddSharedPrefCmdLineArgs(*this, aExtraOpts); diff --git a/ipc/glue/UtilityProcessManager.cpp b/ipc/glue/UtilityProcessManager.cpp index 040702b97aa6..c52c6e6328bc 100644 --- a/ipc/glue/UtilityProcessManager.cpp +++ b/ipc/glue/UtilityProcessManager.cpp @@ -94,10 +94,13 @@ void UtilityProcessManager::OnPreferenceChange(const char16_t* aData) { // We know prefs are ASCII here. NS_LossyConvertUTF16toASCII strData(aData); - mozilla::dom::Pref pref(strData, /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); - Preferences::GetPreference(&pref, GeckoProcessType_Utility, - /* remoteType */ ""_ns); + // A pref changed. If it is useful to do so, inform child processes. + if (!dom::ContentParent::ShouldSyncPreference(strData.Data())) { + return; + } + + mozilla::dom::Pref pref(strData, /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); for (auto& p : mProcesses) { if (!p) { diff --git a/ipc/ipdl/test/gtest/IPDLUnitTest.cpp b/ipc/ipdl/test/gtest/IPDLUnitTest.cpp index 9e586d3305f2..4eab8031c3f2 100644 --- a/ipc/ipdl/test/gtest/IPDLUnitTest.cpp +++ b/ipc/ipdl/test/gtest/IPDLUnitTest.cpp @@ -49,9 +49,9 @@ already_AddRefed IPDLUnitTestParent::CreateCrossProcess() { std::vector extraArgs; - auto prefSerializer = MakeUnique(); - if (!prefSerializer->SerializeToSharedMemory(GeckoProcessType_IPDLUnitTest, - /* remoteType */ ""_ns)) { + auto prefSerializer = MakeUnique( + mozilla::dom::ContentParent::ShouldSyncPreference); + if (!prefSerializer->SerializeToSharedMemory()) { ADD_FAILURE() << "SharedPreferenceSerializer::SerializeToSharedMemory failed"; return nullptr; diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 70cb47bde91d..1c1a8648f0c7 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -21,7 +21,6 @@ #include "mozilla/Attributes.h" #include "mozilla/Components.h" #include "mozilla/dom/PContent.h" -#include "mozilla/dom/RemoteType.h" #include "mozilla/HashFunctions.h" #include "mozilla/HashTable.h" #include "mozilla/Logging.h" @@ -428,28 +427,17 @@ struct PrefsSizes { static StaticRefPtr gSharedMap; -// Arena for Pref names. -// Never access sPrefNameArena directly, always use PrefNameArena() -// because it must only be accessed on the Main Thread -typedef ArenaAllocator<4096, 1> NameArena; -static NameArena* sPrefNameArena; - -static inline NameArena& PrefNameArena() { +// Arena for Pref names. Inside a function so we can assert it's only accessed +// on the main thread. +static inline ArenaAllocator<4096, 1>& PrefNameArena() { MOZ_ASSERT(NS_IsMainThread()); - if (!sPrefNameArena) { - sPrefNameArena = new NameArena(); - } - return *sPrefNameArena; + static ArenaAllocator<4096, 1> sPrefNameArena; + return sPrefNameArena; } class PrefWrapper; -// Two forward declarations for immediately below -class Pref; -static bool ShouldSanitizePreference(const Pref* const aPref, - bool aIsWebContentProcess = true); - class Pref { public: explicit Pref(const nsACString& aName) @@ -457,7 +445,6 @@ class Pref { mType(static_cast(PrefType::None)), mIsSticky(false), mIsLocked(false), - mIsSanitized(false), mHasDefaultValue(false), mHasUserValue(false), mIsSkippedByIteration(false), @@ -495,18 +482,14 @@ class Pref { bool IsSticky() const { return mIsSticky; } - bool IsSanitized() const { return mIsSanitized; } - bool HasDefaultValue() const { return mHasDefaultValue; } bool HasUserValue() const { return mHasUserValue; } template void AddToMap(SharedPrefMapBuilder& aMap) { - // Sanitized preferences should never be added to the shared pref map - MOZ_ASSERT(!ShouldSanitizePreference(this, true)); aMap.Add(NameString(), {HasDefaultValue(), HasUserValue(), IsSticky(), IsLocked(), - /* isSanitized */ false, IsSkippedByIteration()}, + IsSkippedByIteration()}, HasDefaultValue() ? mDefaultValue.Get() : T(), HasUserValue() ? mUserValue.Get() : T()); } @@ -530,13 +513,6 @@ class Pref { MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() : HasUserValue()); - if (!XRE_IsParentProcess() && sCrashOnBlocklistedPref && - ShouldSanitizePreference(this, XRE_IsContentProcess())) { - MOZ_CRASH_UNSAFE_PRINTF( - "Should not access the preference '%s' in the Content Processes", - Name()); - } - return aKind == PrefValueKind::Default ? mDefaultValue.mBoolVal : mUserValue.mBoolVal; } @@ -546,13 +522,6 @@ class Pref { MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() : HasUserValue()); - if (!XRE_IsParentProcess() && sCrashOnBlocklistedPref && - ShouldSanitizePreference(this, XRE_IsContentProcess())) { - MOZ_CRASH_UNSAFE_PRINTF( - "Should not access the preference '%s' in the Content Processes", - Name()); - } - return aKind == PrefValueKind::Default ? mDefaultValue.mIntVal : mUserValue.mIntVal; } @@ -563,13 +532,6 @@ class Pref { MOZ_ASSERT(aKind == PrefValueKind::Default ? HasDefaultValue() : HasUserValue()); - if (!XRE_IsParentProcess() && sCrashOnBlocklistedPref && - ShouldSanitizePreference(this, XRE_IsContentProcess())) { - MOZ_CRASH_UNSAFE_PRINTF( - "Should not access the preference '%s' in the Content Processes", - Name()); - } - return aKind == PrefValueKind::Default ? mDefaultValue.mStringVal : mUserValue.mStringVal; } @@ -579,16 +541,13 @@ class Pref { return nsDependentCString(GetBareStringValue(aKind)); } - void ToDomPref(dom::Pref* aDomPref, bool aIsDestinationContentProcess) { + void ToDomPref(dom::Pref* aDomPref) { MOZ_ASSERT(XRE_IsParentProcess()); aDomPref->name() = mName; aDomPref->isLocked() = mIsLocked; - aDomPref->isSanitized() = - ShouldSanitizePreference(this, aIsDestinationContentProcess); - if (mHasDefaultValue) { aDomPref->defaultValue() = Some(dom::PrefValue()); mDefaultValue.ToDomPrefValue(Type(), &aDomPref->defaultValue().ref()); @@ -596,8 +555,7 @@ class Pref { aDomPref->defaultValue() = Nothing(); } - if (mHasUserValue && - !(aDomPref->isSanitized() && sOmitBlocklistedPrefValues)) { + if (mHasUserValue) { aDomPref->userValue() = Some(dom::PrefValue()); mUserValue.ToDomPrefValue(Type(), &aDomPref->userValue().ref()); } else { @@ -606,7 +564,6 @@ class Pref { MOZ_ASSERT(aDomPref->defaultValue().isNothing() || aDomPref->userValue().isNothing() || - (mIsSanitized && sOmitBlocklistedPrefValues) || (aDomPref->defaultValue().ref().type() == aDomPref->userValue().ref().type())); } @@ -616,7 +573,6 @@ class Pref { MOZ_ASSERT(mName == aDomPref.name()); mIsLocked = aDomPref.isLocked(); - mIsSanitized = aDomPref.isSanitized(); const Maybe& defaultValue = aDomPref.defaultValue(); bool defaultValueChanged = false; @@ -762,11 +718,9 @@ class Pref { // // The grammar for the serialized prefs has the following form. // - // = ':' ':' ? ':' - // ? '\n' + // = ':' ':' ? ':' ? '\n' // = 'B' | 'I' | 'S' // = 'L' | '-' - // = 'S' | '-' // = // = | | // = 'T' | 'F' @@ -793,26 +747,21 @@ class Pref { // print it and inspect it easily in a debugger. // // Examples of unlocked boolean prefs: - // - "B--:8/my.bool1:F:T\n" - // - "B--:8/my.bool2:F:\n" - // - "B--:8/my.bool3::T\n" - // - // Examples of sanitized, unlocked boolean prefs: - // - "B-S:8/my.bool1:F:T\n" - // - "B-S:8/my.bool2:F:\n" - // - "B-S:8/my.bool3::T\n" + // - "B-:8/my.bool1:F:T\n" + // - "B-:8/my.bool2:F:\n" + // - "B-:8/my.bool3::T\n" // // Examples of locked integer prefs: - // - "IL-:7/my.int1:0:1\n" - // - "IL-:7/my.int2:123:\n" - // - "IL-:7/my.int3::-99\n" + // - "IL:7/my.int1:0:1\n" + // - "IL:7/my.int2:123:\n" + // - "IL:7/my.int3::-99\n" // // Examples of unlocked string prefs: - // - "S--:10/my.string1:3/abc:4/wxyz\n" - // - "S--:10/my.string2:5/1.234:\n" - // - "S--:10/my.string3::7/string!\n" + // - "S-:10/my.string1:3/abc:4/wxyz\n" + // - "S-:10/my.string2:5/1.234:\n" + // - "S-:10/my.string3::7/string!\n" - void SerializeAndAppend(nsCString& aStr, bool aSanitizeUserValue) { + void SerializeAndAppend(nsCString& aStr) { switch (Type()) { case PrefType::Bool: aStr.Append('B'); @@ -833,7 +782,6 @@ class Pref { } aStr.Append(mIsLocked ? 'L' : '-'); - aStr.Append(aSanitizeUserValue ? 'S' : '-'); aStr.Append(':'); SerializeAndAppendString(mName, aStr); @@ -844,7 +792,7 @@ class Pref { } aStr.Append(':'); - if (mHasUserValue && !(aSanitizeUserValue && sOmitBlocklistedPrefValues)) { + if (mHasUserValue) { mUserValue.SerializeAndAppend(Type(), aStr); } aStr.Append('\n'); @@ -879,18 +827,6 @@ class Pref { } p++; // move past the isLocked char - // Sanitize? - bool isSanitized; - if (*p == 'S') { - isSanitized = true; - } else if (*p == '-') { - isSanitized = false; - } else { - NS_ERROR("bad pref sanitized status"); - isSanitized = false; - } - p++; // move past the isSanitized char - MOZ_ASSERT(*p == ':'); p++; // move past the ':' @@ -919,8 +855,7 @@ class Pref { MOZ_ASSERT(*p == '\n'); p++; // move past the '\n' following the user value - *aDomPref = dom::Pref(name, isLocked, isSanitized, maybeDefaultValue, - maybeUserValue); + *aDomPref = dom::Pref(name, isLocked, maybeDefaultValue, maybeUserValue); return p; } @@ -938,17 +873,12 @@ class Pref { } } - void RelocateName(NameArena* aArena) { - mName.Rebind(ArenaStrdup(mName.get(), *aArena), mName.Length()); - } - private: - nsDependentCString mName; // allocated in sPrefNameArena + const nsDependentCString mName; // allocated in sPrefNameArena uint32_t mType : 2; uint32_t mIsSticky : 1; uint32_t mIsLocked : 1; - uint32_t mIsSanitized : 1; uint32_t mHasDefaultValue : 1; uint32_t mHasUserValue : 1; uint32_t mIsSkippedByIteration : 1; @@ -1000,7 +930,6 @@ class MOZ_STACK_CLASS PrefWrapper : public PrefWrapperBase { } FORWARD(bool, IsLocked) - FORWARD(bool, IsSanitized) FORWARD(bool, IsSticky) FORWARD(bool, HasDefaultValue) FORWARD(bool, HasUserValue) @@ -1155,7 +1084,6 @@ void Pref::FromWrapper(PrefWrapper& aWrapper) { mType = uint32_t(pref.Type()); mIsLocked = pref.IsLocked(); - mIsSanitized = pref.IsSanitized(); mIsSticky = pref.IsSticky(); mHasDefaultValue = pref.HasDefaultValue(); @@ -2593,16 +2521,6 @@ nsPrefBranch::PrefIsLocked(const char* aPrefName, bool* aRetVal) { return NS_OK; } -NS_IMETHODIMP -nsPrefBranch::PrefIsSanitized(const char* aPrefName, bool* aRetVal) { - NS_ENSURE_ARG_POINTER(aRetVal); - NS_ENSURE_ARG(aPrefName); - - const PrefName& pref = GetPrefName(aPrefName); - *aRetVal = Preferences::IsSanitized(pref.get()); - return NS_OK; -} - NS_IMETHODIMP nsPrefBranch::UnlockPref(const char* aPrefName) { NS_ENSURE_ARG(aPrefName); @@ -3628,18 +3546,18 @@ NS_IMPL_ISUPPORTS(Preferences, nsIPrefService, nsIObserver, nsIPrefBranch, nsISupportsWeakReference) /* static */ -void Preferences::SerializePreferences(nsCString& aStr, - bool aIsDestinationWebContentProcess) { +void Preferences::SerializePreferences( + nsCString& aStr, + const std::function& aShouldSerializeFn) { MOZ_RELEASE_ASSERT(InitStaticMembers()); aStr.Truncate(); for (auto iter = HashTable()->iter(); !iter.done(); iter.next()) { Pref* pref = iter.get().get(); - if (!pref->IsTypeNone() && pref->HasAdvisablySizedValues()) { - pref->SerializeAndAppend( - aStr, - ShouldSanitizePreference(pref, aIsDestinationWebContentProcess)); + if (!pref->IsTypeNone() && pref->HasAdvisablySizedValues() && + aShouldSerializeFn(pref->Name())) { + pref->SerializeAndAppend(aStr); } } @@ -3684,21 +3602,12 @@ static void RegisterOncePrefs(SharedPrefMapBuilder& aBuilder); /* static */ FileDescriptor Preferences::EnsureSnapshot(size_t* aSize) { MOZ_ASSERT(XRE_IsParentProcess()); - MOZ_ASSERT(NS_IsMainThread()); if (!gSharedMap) { SharedPrefMapBuilder builder; - nsTArray toRepopulate; - NameArena* newPrefNameArena = new NameArena(); - for (auto iter = HashTable()->modIter(); !iter.done(); iter.next()) { - if (!ShouldSanitizePreference(iter.get().get(), true)) { - iter.get()->AddToMap(builder); - } else { - Pref* pref = iter.getMutable().release(); - pref->RelocateName(newPrefNameArena); - toRepopulate.AppendElement(pref); - } + for (auto iter = HashTable()->iter(); !iter.done(); iter.next()) { + iter.get()->AddToMap(builder); } // Store the current value of `once`-mirrored prefs. After this point they @@ -3719,16 +3628,8 @@ FileDescriptor Preferences::EnsureSnapshot(size_t* aSize) { HashTable()->clearAndCompact(); Unused << HashTable()->reserve(kHashTableInitialLengthContent); - delete sPrefNameArena; - sPrefNameArena = newPrefNameArena; + PrefNameArena().Clear(); gCallbackPref = nullptr; - - for (uint32_t i = 0; i < toRepopulate.Length(); i++) { - auto pref = toRepopulate[i]; - auto p = HashTable()->lookupForAdd(pref->Name()); - MOZ_ASSERT(!p.found()); - Unused << HashTable()->add(p, pref); - } } *aSize = gSharedMap->MapSize(); @@ -3973,18 +3874,12 @@ void Preferences::SetPreference(const dom::Pref& aDomPref) { } /* static */ -void Preferences::GetPreference(dom::Pref* aDomPref, - const GeckoProcessType aDestinationProcessType, - const nsACString& aDestinationRemoteType) { +void Preferences::GetPreference(dom::Pref* aDomPref) { MOZ_ASSERT(XRE_IsParentProcess()); - bool destIsWebContent = - aDestinationProcessType == GeckoProcessType_Content && - (StringBeginsWith(aDestinationRemoteType, WEB_REMOTE_TYPE) || - StringBeginsWith(aDestinationRemoteType, PREALLOC_REMOTE_TYPE)); Pref* pref = pref_HashTableLookup(aDomPref->name().get()); if (pref && pref->HasAdvisablySizedValues()) { - pref->ToDomPref(aDomPref, destIsWebContent); + pref->ToDomPref(aDomPref); } } @@ -5080,14 +4975,6 @@ bool Preferences::IsLocked(const char* aPrefName) { return pref.isSome() && pref->IsLocked(); } -/* static */ -bool Preferences::IsSanitized(const char* aPrefName) { - NS_ENSURE_TRUE(InitStaticMembers(), false); - - Maybe pref = pref_Lookup(aPrefName); - return pref.isSome() && pref->IsSanitized(); -} - /* static */ nsresult Preferences::ClearUser(const char* aPrefName) { ENSURE_PARENT_PROCESS("ClearUser", aPrefName); @@ -5741,114 +5628,6 @@ void UnloadPrefsModule() { Preferences::Shutdown(); } } // namespace mozilla -// Preference Sanitization Related Code --------------------------------------- - -#define PREF_LIST_ENTRY(s) \ - { s, (sizeof(s) / sizeof(char)) - 1 } - struct PrefListEntry { - const char* mPrefBranch; - size_t mLen; - }; - - // These prefs are not useful in child processes. - static const PrefListEntry sParentOnlyPrefBranchList[] = { - PREF_LIST_ENTRY("app.update.lastUpdateTime."), - PREF_LIST_ENTRY("datareporting.policy."), - // PREF_LIST_ENTRY("browser.safebrowsing.provider."), - // PREF_LIST_ENTRY("browser.shell."), - // PREF_LIST_ENTRY("browser.slowStartup."), - // PREF_LIST_ENTRY("browser.startup."), - // PREF_LIST_ENTRY("extensions.getAddons.cache."), - // PREF_LIST_ENTRY("media.gmp-manager."), - // PREF_LIST_ENTRY("media.gmp-gmpopenh264."), - // PREF_LIST_ENTRY("privacy.sanitize."), - }; - - static const PrefListEntry sDynamicPrefOverrideList[]{ - PREF_LIST_ENTRY("print.printer_")}; - -#undef PREF_LIST_ENTRY - -// Forward Declaration - it's not defined in the .h, because we don't need to; -// it's only used here. -template -static bool ShouldSanitizePreference_Impl(const T& aPref, - bool aIsDestWebContentProcess); - -static bool ShouldSanitizePreference(const Pref* const aPref, - bool aIsDestWebContentProcess) { - return ShouldSanitizePreference_Impl(*aPref, aIsDestWebContentProcess); -} - -static bool ShouldSanitizePreference(const PrefWrapper& aPref, - bool aIsDestWebContentProcess) { - return ShouldSanitizePreference_Impl(aPref, aIsDestWebContentProcess); -} - -template -static bool ShouldSanitizePreference_Impl(const T& aPref, - bool aIsDestWebContentProcess) { - // In the parent process, we use a heuristic to decide if a pref - // value should be sanitized before sending to subprocesses. - if (XRE_IsParentProcess()) { - const char* prefName = aPref.Name(); - - // First check against the denylist, the denylist is used for - // all subprocesses to reduce IPC traffic. - for (const auto& entry : sParentOnlyPrefBranchList) { - if (strncmp(entry.mPrefBranch, prefName, entry.mLen) == 0) { - return true; - } - } - - if (!aIsDestWebContentProcess) { - return false; - } - - // If it's a Web Content Process, also check if it's a dynamically - // named string preference - if (aPref.Type() == PrefType::String && !aPref.HasDefaultValue()) { - for (const auto& entry : sDynamicPrefOverrideList) { - if (strncmp(entry.mPrefBranch, prefName, entry.mLen) == 0) { - return false; - } - } - return true; - } - - return false; - } - - // In subprocesses we only check the sanitized bit - return aPref.IsSanitized(); -} - -namespace mozilla { - -// Of these four ShouldSanitizePreference* fuctions, this is the -// only one exposed outside of Preferences.cpp, because this is the -// only one ever called from outside this file. -bool ShouldSanitizePreference(const char* aPrefName, - bool aIsDestWebContentProcess) { - if (!aIsDestWebContentProcess) { - return false; - } - - if (Maybe pref = pref_Lookup(aPrefName)) { - if (pref.isNothing()) { - return true; - } - return ShouldSanitizePreference(pref.value(), aIsDestWebContentProcess); - } - - return true; -} - -Atomic sOmitBlocklistedPrefValues(false); -Atomic sCrashOnBlocklistedPref(false); - -} // namespace mozilla - // This file contains the C wrappers for the C++ static pref getters, as used // by Rust code. #include "init/StaticPrefsCGetters.cpp" diff --git a/modules/libpref/Preferences.h b/modules/libpref/Preferences.h index 91050f5934e2..682a465a1ee9 100644 --- a/modules/libpref/Preferences.h +++ b/modules/libpref/Preferences.h @@ -23,7 +23,6 @@ #include "nsString.h" #include "nsTArray.h" #include "nsWeakReference.h" -#include "nsXULAppAPI.h" #include #include @@ -217,7 +216,6 @@ class Preferences final : public nsIPrefService, static nsresult Lock(const char* aPrefName); static nsresult Unlock(const char* aPrefName); static bool IsLocked(const char* aPrefName); - static bool IsSanitized(const char* aPrefName); // Clears user set pref. Fails if run outside the parent process. static nsresult ClearUser(const char* aPrefName); @@ -400,8 +398,9 @@ class Preferences final : public nsIPrefService, // When a content process is created these methods are used to pass changed // prefs in bulk from the parent process, via shared memory. - static void SerializePreferences(nsCString& aStr, - bool aIsDestinationWebContentProcess); + static void SerializePreferences( + nsCString& aStr, + const std::function& aShouldSerializeFn); static void DeserializePreferences(char* aStr, size_t aPrefsLen); static mozilla::ipc::FileDescriptor EnsureSnapshot(size_t* aSize); @@ -409,9 +408,7 @@ class Preferences final : public nsIPrefService, // When a single pref is changed in the parent process, these methods are // used to pass the update to content processes. - static void GetPreference(dom::Pref* aPref, - const GeckoProcessType aDestinationProcessType, - const nsACString& aDestinationRemoteType); + static void GetPreference(dom::Pref* aPref); static void SetPreference(const dom::Pref& aPref); #ifdef DEBUG @@ -534,11 +531,6 @@ class Preferences final : public nsIPrefService, static bool InitStaticMembers(); }; -extern Atomic sOmitBlocklistedPrefValues; -extern Atomic sCrashOnBlocklistedPref; - -bool ShouldSanitizePreference(const char* aPref, bool aIsDestWebContentProcess); - } // namespace mozilla #endif // mozilla_Preferences_h diff --git a/modules/libpref/SharedPrefMap.cpp b/modules/libpref/SharedPrefMap.cpp index 81f8d9248537..a1f1774ab564 100644 --- a/modules/libpref/SharedPrefMap.cpp +++ b/modules/libpref/SharedPrefMap.cpp @@ -81,7 +81,6 @@ void SharedPrefMapBuilder::Add(const nsCString& aKey, const Flags& aFlags, aFlags.mHasUserValue, aFlags.mIsSticky, aFlags.mIsLocked, - aFlags.mIsSanitized, aFlags.mIsSkippedByIteration, }); } @@ -104,7 +103,6 @@ void SharedPrefMapBuilder::Add(const nsCString& aKey, const Flags& aFlags, aFlags.mHasUserValue, aFlags.mIsSticky, aFlags.mIsLocked, - aFlags.mIsSanitized, aFlags.mIsSkippedByIteration, }); } @@ -130,7 +128,6 @@ void SharedPrefMapBuilder::Add(const nsCString& aKey, const Flags& aFlags, aFlags.mHasUserValue, aFlags.mIsSticky, aFlags.mIsLocked, - aFlags.mIsSanitized, aFlags.mIsSkippedByIteration, }); } @@ -193,15 +190,10 @@ Result SharedPrefMapBuilder::Finalize(loader::AutoMemMap& aMap) { auto* entryPtr = reinterpret_cast(&headerPtr[1]); for (auto* entry : entries) { *entryPtr = { - entry->mKey, - GetValue(*entry), - entry->mType, - entry->mHasDefaultValue, - entry->mHasUserValue, - entry->mIsSticky, - entry->mIsLocked, - entry->mIsSanitized, - entry->mIsSkippedByIteration, + entry->mKey, GetValue(*entry), + entry->mType, entry->mHasDefaultValue, + entry->mHasUserValue, entry->mIsSticky, + entry->mIsLocked, entry->mIsSkippedByIteration, }; entryPtr++; } diff --git a/modules/libpref/SharedPrefMap.h b/modules/libpref/SharedPrefMap.h index 7c949e137d80..2ae92ec04d76 100644 --- a/modules/libpref/SharedPrefMap.h +++ b/modules/libpref/SharedPrefMap.h @@ -307,9 +307,6 @@ class SharedPrefMap { uint8_t mIsSticky : 1; // True if the preference is locked, as defined by the preference service. uint8_t mIsLocked : 1; - // True if the preference is sanitized, as defined by the preference - // service. - uint8_t mIsSanitized : 1; // True if the preference should be skipped while iterating over the // SharedPrefMap. This is used to internally store Once StaticPrefs. // This property is not visible to users the way sticky and locked are. @@ -342,7 +339,6 @@ class SharedPrefMap { bool HasDefaultValue() const { return mEntry->mHasDefaultValue; } bool HasUserValue() const { return mEntry->mHasUserValue; } bool IsLocked() const { return mEntry->mIsLocked; } - bool IsSanitized() const { return mEntry->mIsSanitized; } bool IsSticky() const { return mEntry->mIsSticky; } bool IsSkippedByIteration() const { return mEntry->mIsSkippedByIteration; } @@ -573,7 +569,6 @@ class MOZ_RAII SharedPrefMapBuilder { uint8_t mHasUserValue : 1; uint8_t mIsSticky : 1; uint8_t mIsLocked : 1; - uint8_t mIsSanitized : 1; uint8_t mIsSkippedByIteration : 1; }; @@ -812,7 +807,6 @@ class MOZ_RAII SharedPrefMapBuilder { uint8_t mHasUserValue : 1; uint8_t mIsSticky : 1; uint8_t mIsLocked : 1; - uint8_t mIsSanitized : 1; uint8_t mIsSkippedByIteration : 1; }; diff --git a/modules/libpref/StaticPrefsBase.h b/modules/libpref/StaticPrefsBase.h index f1cc5a5e4f34..23df6d918b52 100644 --- a/modules/libpref/StaticPrefsBase.h +++ b/modules/libpref/StaticPrefsBase.h @@ -19,12 +19,6 @@ class SharedPrefMapBuilder; typedef const char* String; -template -struct IsString : std::false_type {}; - -template <> -struct IsString : std::true_type {}; - typedef Atomic RelaxedAtomicBool; typedef Atomic ReleaseAcquireAtomicBool; typedef Atomic SequentiallyConsistentAtomicBool; diff --git a/modules/libpref/docs/index.md b/modules/libpref/docs/index.md index 40c8977015bb..d0e16fec0c25 100644 --- a/modules/libpref/docs/index.md +++ b/modules/libpref/docs/index.md @@ -159,23 +159,6 @@ variable if the pref is deleted? Currently there is a missing the deletion. The cleanest solution is probably to disallow static prefs from being deleted. -### Sanitized Prefs -We restrict certain prefs from entering certain subprocesses. In subprocesses, -a preference may be marked as 'Sanitized' to indicate that it may or may not -have a user value, but that value is not present in this process. In the -parent process no pref is marked as Sanitized. - -Pref Sanitization is overloaded for two purposes: - 1. To protect private user data that may be stored in preferences from a - Spectre adversary. - 2. To reduce IPC use for commonly modified preferences. - -A pref is sanitized from entering the web content process if it matches a -denylist _or_ it is a dynamically-named string preference. - -A pref is sanitized from entering other subprocesses if it matches the -denylist. - ### Loading and Saving Default pref values are initialized from various pref data files. Notable ones include: @@ -231,10 +214,10 @@ activity. ### about:support about:support contains an "Important Modified Preferences" table. It contains all prefs that (a) have had their value changed from the default, and (b) whose -prefix match a allowlist in `Troubleshoot.jsm`. The allowlist matching is to +prefix match a whitelist in `Troubleshoot.jsm`. The whitelist matching is to avoid exposing pref values that might be privacy-sensitive. -**Problem:** The allowlist of prefixes is specified separately from the prefs +**Problem:** The whitelist of prefixes is specified separately from the prefs themselves. Having an attribute on a pref definition would be better. ### Sync @@ -366,22 +349,18 @@ process. They exist so that all child processes can be given the same `once` values as the parent process. ### Child process startup (parent side) -When the first child process is created, the parent process serializes most of -its hash table into a shared, immutable snapshot. This snapshot is stored in a -shared memory region managed by a `SharedPrefMap` instance. - -Sanitized preferences (matching _either_ the denylist of the dynamically named -heuristic) are not included in the shared memory region. After building the -shared memory region, the parent process clears the hash table and then -re-enters sanitized prefs into it. Besides the sanitized prefs, the hash table -is subsequently used only to store changed pref values. +When the first child process is created, the parent process serializes its hash +table into a shared, immutable snapshot. This snapshot is stored in a shared +memory region managed by a `SharedPrefMap` instance. The parent process then +clears the hash table. The hash table is subsequently used only to store +changed pref values. When any child process is created, the parent process serializes all pref values present in the hash table (i.e. those that have changed since the -snapshot was made) _except sanitized prefs__ and stores them in a second, -short-lived shared memory region. This represents the set of changes the child -process needs to apply on top of the snapshot, and allows it to build a hash -table which should exactly match the parent's, modulo the sanitized prefs. +snapshot was made) and stores them in a second, short-lived shared memory +region. This represents the set of changes the child process needs to apply on +top of the snapshot, and allows it to build a hash table which should exactly +match the parent's. The parent process passes two file descriptors to the child process, one for each region of memory. The snapshot is the same for all child processes. @@ -442,13 +421,15 @@ want to waste memory creating an unnecessary hash table entry. Content processes must be told about any visible pref value changes. (A change to a default value that is hidden by a user value is unimportant.) When this -happens, `ContentParent` detects the change (via an observer). Sanitized prefs -do not produce an update; and for string prefs it also checks the value(s) -don't exceed 4 KiB. If the checks pass, it sends an IPC message -(`PreferenceUpdate`) to the child process, and the child process updates the -pref (default and user value) accordingly. +happens, `ContentParent` detects the change (via an observer). It checks the +pref name against a small blacklist of prefixes that child processes should not +care about (this is an optimization to reduce IPC rather than a +capabilities/security consideration), and for string prefs it also checks the +value(s) don't exceed 4 KiB. If the checks pass, it sends an IPC message +(`PreferenceUpdate`) to the child process, and the child process updates +the pref (default and user value) accordingly. -**Problem:** The denylist of prefixes is specified separately from the prefs +**Problem:** The blacklist of prefixes is specified separately from the prefs themselves. Having an attribute on a pref definition would be better. **Problem:** The 4 KiB limit can lead to inconsistencies between the parent diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 51d42989d0f7..8cea64214864 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -4697,18 +4697,6 @@ value: @IS_ANDROID@ mirror: always -# If true, do not send blocklisted preference values to the subprocess -- name: fission.omitBlocklistedPrefsInSubprocesses - type: RelaxedAtomicBool - value: false - mirror: always - -# If true, crash when a blocklisted preference is accessed in a subprocess -- name: fission.enforceBlocklistedPrefsInSubprocesses - type: RelaxedAtomicBool - value: false - mirror: always - #--------------------------------------------------------------------------- # Prefs starting with "font." #--------------------------------------------------------------------------- diff --git a/modules/libpref/init/StaticPrefListBegin.h b/modules/libpref/init/StaticPrefListBegin.h index 8af91795549b..71b2cb505d39 100644 --- a/modules/libpref/init/StaticPrefListBegin.h +++ b/modules/libpref/init/StaticPrefListBegin.h @@ -9,9 +9,7 @@ // namespace. #include "StaticPrefsBase.h" -#include "Preferences.h" #include "MainThreadUtils.h" // for NS_IsMainThread() -#include "nsXULAppAPI.h" // for XRE_IsContentProcess() namespace mozilla { namespace StaticPrefs { @@ -19,35 +17,23 @@ namespace StaticPrefs { // For mirrored prefs we generate an extern variable declaration and three // getter declarations/definitions. #define NEVER_PREF(name, cpp_type, default_value) -#define ALWAYS_PREF(name, base_id, full_id, cpp_type, default_value) \ - extern cpp_type sMirror_##full_id; \ - inline StripAtomic full_id() { \ - if (!XRE_IsParentProcess() && IsString::value && \ - sCrashOnBlocklistedPref) { \ - MOZ_DIAGNOSTIC_ASSERT( \ - !ShouldSanitizePreference(name, XRE_IsContentProcess()), \ - "Should not access the preference '" name "' in Content Processes"); \ - } \ - MOZ_DIAGNOSTIC_ASSERT(IsAtomic::value || NS_IsMainThread(), \ - "Non-atomic static pref '" name \ - "' being accessed on background thread by getter"); \ - return sMirror_##full_id; \ - } \ - inline const char* GetPrefName_##base_id() { return name; } \ - inline StripAtomic GetPrefDefault_##base_id() { \ - return default_value; \ +#define ALWAYS_PREF(name, base_id, full_id, cpp_type, default_value) \ + extern cpp_type sMirror_##full_id; \ + inline StripAtomic full_id() { \ + MOZ_DIAGNOSTIC_ASSERT(IsAtomic::value || NS_IsMainThread(), \ + "Non-atomic static pref '" name \ + "' being accessed on background thread by getter"); \ + return sMirror_##full_id; \ + } \ + inline const char* GetPrefName_##base_id() { return name; } \ + inline StripAtomic GetPrefDefault_##base_id() { \ + return default_value; \ } -#define ONCE_PREF(name, base_id, full_id, cpp_type, default_value) \ - extern cpp_type sMirror_##full_id; \ - inline cpp_type full_id() { \ - MaybeInitOncePrefs(); \ - if (!XRE_IsParentProcess() && IsString::value && \ - sCrashOnBlocklistedPref) { \ - MOZ_DIAGNOSTIC_ASSERT( \ - !ShouldSanitizePreference(name, XRE_IsContentProcess()), \ - "Should not access the preference '" name "' in Content Processes"); \ - } \ - return sMirror_##full_id; \ - } \ - inline const char* GetPrefName_##base_id() { return name; } \ +#define ONCE_PREF(name, base_id, full_id, cpp_type, default_value) \ + extern cpp_type sMirror_##full_id; \ + inline cpp_type full_id() { \ + MaybeInitOncePrefs(); \ + return sMirror_##full_id; \ + } \ + inline const char* GetPrefName_##base_id() { return name; } \ inline cpp_type GetPrefDefault_##base_id() { return default_value; } diff --git a/modules/libpref/nsIPrefBranch.idl b/modules/libpref/nsIPrefBranch.idl index 8e45d5d3122f..1a567ba3b26f 100644 --- a/modules/libpref/nsIPrefBranch.idl +++ b/modules/libpref/nsIPrefBranch.idl @@ -315,24 +315,6 @@ interface nsIPrefBranch : nsISupports */ boolean prefIsLocked(in string aPrefName); - /** - * Called to check if a specific preference has been sanitized. If a - * preference is sanitized, any user value the preference may have will not - * be present in a sub-process. A preference is never sanitized in the - * parent process; it is only marked as sanitized when it is converted - * to a dom::Pref for serialization to a child process. - * - * @param aPrefName The preference to be tested. - * - * @note - * This method can be called on either a default or user branch but it - * makes no difference. - * - * @return boolean true The preference is sanitized. - * false The preference is not sanitized. - */ - boolean prefIsSanitized(in string aPrefName); - /** * Called to unlock a specific preference. Unlocking a previously locked * preference allows the preference service to once again return the user set diff --git a/modules/libpref/test/gtest/Basics.cpp b/modules/libpref/test/gtest/Basics.cpp index 6e4bac67fbe5..8f6e0bef1fc3 100644 --- a/modules/libpref/test/gtest/Basics.cpp +++ b/modules/libpref/test/gtest/Basics.cpp @@ -43,13 +43,12 @@ TEST(PrefsBasics, Serialize) true); nsCString str; - Preferences::SerializePreferences(str, true); - fprintf(stderr, "%s\n", str.Data()); - // Assert that some prefs were not sanitized - ASSERT_NE(nullptr, strstr(str.Data(), "B--:")); - ASSERT_NE(nullptr, strstr(str.Data(), "I--:")); - ASSERT_NE(nullptr, strstr(str.Data(), "S--:")); - // Assert that something was sanitized - ASSERT_NE(nullptr, - strstr(str.Data(), "I-S:56/datareporting.policy.dataSubmissionPolicyAcceptedVersion")); + Preferences::SerializePreferences( + str, [](const char* aPref) -> bool { return false; }); + ASSERT_STREQ(str.Data(), ""); + + Preferences::SerializePreferences(str, [](const char* aPref) -> bool { + return strncmp(aPref, "foo.bool", 8) == 0; + }); + ASSERT_STREQ(str.Data(), "B-:8/foo.bool:T:F\n"); } diff --git a/netwerk/base/nsIOService.cpp b/netwerk/base/nsIOService.cpp index 035e3264c4f0..60b01e4eb55a 100644 --- a/netwerk/base/nsIOService.cpp +++ b/netwerk/base/nsIOService.cpp @@ -646,11 +646,8 @@ void nsIOService::NotifySocketProcessPrefsChanged(const char* aName) { return; } - dom::Pref pref(nsCString(aName), /* isLocked */ false, - /* isSanitized */ false, Nothing(), Nothing()); - - Preferences::GetPreference(&pref, GeckoProcessType_Socket, - /* remoteType */ ""_ns); + dom::Pref pref(nsCString(aName), /* isLocked */ false, Nothing(), Nothing()); + Preferences::GetPreference(&pref); auto sendPrefUpdate = [pref]() { Unused << gIOService->mSocketProcess->GetActor()->SendPreferenceUpdate( pref); diff --git a/netwerk/ipc/SocketProcessHost.cpp b/netwerk/ipc/SocketProcessHost.cpp index 2677cd510353..b9e3ca93b512 100644 --- a/netwerk/ipc/SocketProcessHost.cpp +++ b/netwerk/ipc/SocketProcessHost.cpp @@ -68,9 +68,9 @@ bool SocketProcessHost::Launch() { std::vector extraArgs; ProcessChild::AddPlatformBuildID(extraArgs); - SharedPreferenceSerializer prefSerializer; - if (!prefSerializer.SerializeToSharedMemory(GeckoProcessType_VR, - /* remoteType */ ""_ns)) { + SharedPreferenceSerializer prefSerializer( + mozilla::dom::ContentParent::ShouldSyncPreference); + if (!prefSerializer.SerializeToSharedMemory()) { return false; } prefSerializer.AddSharedPrefCmdLineArgs(*this, extraArgs);