Bug 1272707, part 2 - Limit the size of preference values sent to child processes. r=bsmedberg
authorAndrew McCreight <continuation@gmail.com>
Tue, 17 May 2016 07:55:49 -0700
changeset 297672 49b09d2f702f747e1b594649e947653a1c5c8c14
parent 297671 a97d0c36d9934997f227f99ceae6a5ca4a4cad21
child 297673 9dfa2037416f95b7f7d66de9cc964e53f5f2901d
push id76856
push useramccreight@mozilla.com
push dateTue, 17 May 2016 14:56:05 +0000
treeherdermozilla-inbound@9dfa2037416f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs1272707
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1272707, part 2 - Limit the size of preference values sent to child processes. r=bsmedberg Don't send any preferences that have a string value that is longer than MAX_ADVISABLE_PREF_LENGTH. This is intended to mitigate OOM issues, as I've seen a parent process crash trying to create a 100mb message to send to the child. Such users likely cannot use e10s at all. This has a test for all combinations of setting the default and user values of a preference to large or small string values, or not setting them at all. I manually verified that filtering out preferences reduces the size of the IPC::Message that is sent to the child by printing out the size of the reply message in PContentParent::OnMessageReceived().
modules/libpref/Preferences.cpp
modules/libpref/nsPrefBranch.cpp
modules/libpref/prefapi.cpp
modules/libpref/prefapi_private_data.h
modules/libpref/test/unit_ipc/test_large_pref.js
modules/libpref/test/unit_ipc/xpcshell.ini
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -723,25 +723,32 @@ Preferences::SetPreference(const PrefSet
 
 void
 Preferences::GetPreference(PrefSetting* aPref)
 {
   PrefHashEntry *entry = pref_HashTableLookup(aPref->name().get());
   if (!entry)
     return;
 
-  pref_GetPrefFromEntry(entry, aPref);
+  if (pref_EntryHasAdvisablySizedValues(entry)) {
+    pref_GetPrefFromEntry(entry, aPref);
+  }
 }
 
 void
 Preferences::GetPreferences(InfallibleTArray<PrefSetting>* aPrefs)
 {
   aPrefs->SetCapacity(gHashTable->Capacity());
   for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
     auto entry = static_cast<PrefHashEntry*>(iter.Get());
+
+    if (!pref_EntryHasAdvisablySizedValues(entry)) {
+      continue;
+    }
+
     dom::PrefSetting *pref = aPrefs->AppendElement();
     pref_GetPrefFromEntry(entry, pref);
   }
 }
 
 NS_IMETHODIMP
 Preferences::GetBranch(const char *aPrefRoot, nsIPrefBranch **_retval)
 {
--- a/modules/libpref/nsPrefBranch.cpp
+++ b/modules/libpref/nsPrefBranch.cpp
@@ -375,17 +375,20 @@ nsresult nsPrefBranch::CheckSanityOfStri
   if (aLength <= MAX_ADVISABLE_PREF_LENGTH) {
     return NS_OK;
   }
   nsresult rv;
   nsCOMPtr<nsIConsoleService> console = do_GetService("@mozilla.org/consoleservice;1", &rv);
   if (NS_FAILED(rv)) {
     return rv;
   }
-  nsAutoCString message(nsPrintfCString("Warning: attempting to write %d bytes to preference %s. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file.",
+  nsAutoCString message(nsPrintfCString("Warning: attempting to write %d bytes to preference %s. This is bad "
+                                        "for general performance and memory usage. Such an amount of data "
+                                        "should rather be written to an external file. This preference will "
+                                        "not be sent to any content processes.",
                                         aLength,
                                         getPrefName(aPrefName)));
   rv = console->LogStringMessage(NS_ConvertUTF8toUTF16(message).get());
   if (NS_FAILED(rv)) {
     return rv;
   }
   return NS_OK;
 }
--- a/modules/libpref/prefapi.cpp
+++ b/modules/libpref/prefapi.cpp
@@ -361,16 +361,41 @@ pref_savePrefs(PLDHashTable* aTable)
                                        NS_LITERAL_CSTRING("\", ") +
                                        prefValue +
                                        NS_LITERAL_CSTRING(");"));
     }
 
     return savedPrefs;
 }
 
+bool
+pref_EntryHasAdvisablySizedValues(PrefHashEntry* aHashEntry)
+{
+    if (aHashEntry->prefFlags.GetPrefType() != PrefType::String) {
+        return true;
+    }
+
+    char* stringVal;
+    if (aHashEntry->prefFlags.HasDefault()) {
+        stringVal = aHashEntry->defaultPref.stringVal;
+        if (strlen(stringVal) > MAX_ADVISABLE_PREF_LENGTH) {
+            return false;
+        }
+    }
+
+    if (aHashEntry->prefFlags.HasUserValue()) {
+        stringVal = aHashEntry->userPref.stringVal;
+        if (strlen(stringVal) > MAX_ADVISABLE_PREF_LENGTH) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static void
 GetPrefValueFromEntry(PrefHashEntry *aHashEntry, dom::PrefSetting* aPref,
                       WhichValue aWhich)
 {
     PrefValue* value;
     dom::PrefValue* settingValue;
     if (aWhich == USER_VALUE) {
         value = &aHashEntry->userPref;
--- a/modules/libpref/prefapi_private_data.h
+++ b/modules/libpref/prefapi_private_data.h
@@ -24,15 +24,18 @@ mozilla::UniquePtr<char*[]>
 pref_savePrefs(PLDHashTable* aTable);
 
 nsresult
 pref_SetPref(const mozilla::dom::PrefSetting& aPref);
 
 int pref_CompareStrings(const void *v1, const void *v2, void* unused);
 PrefHashEntry* pref_HashTableLookup(const char *key);
 
+bool
+pref_EntryHasAdvisablySizedValues(PrefHashEntry* aHashEntry);
+
 void pref_GetPrefFromEntry(PrefHashEntry *aHashEntry,
                            mozilla::dom::PrefSetting* aPref);
 
 size_t
 pref_SizeOfPrivateData(mozilla::MallocSizeOf aMallocSizeOf);
 
 #endif
new file mode 100644
--- /dev/null
+++ b/modules/libpref/test/unit_ipc/test_large_pref.js
@@ -0,0 +1,98 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Large preferences should not be set in the child process.
+// Non-string preferences are not tested here, because their behavior
+// should not be affected by this filtering.
+
+var Ci = Components.interfaces;
+var Cc = Components.classes;
+
+function isParentProcess() {
+    let appInfo = Cc["@mozilla.org/xre/app-info;1"];
+    return (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType == Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT);
+}
+
+function makeBuffer(length) {
+    let string = "x";
+    while (string.length < length) {
+      string = string + string;
+    }
+    if (string.length > length) {
+      string = string.substring(length - string.length);
+    }
+    return string;
+}
+
+// from prefapi.h
+const MAX_ADVISABLE_PREF_LENGTH = 4 * 1024;
+
+const largeString = makeBuffer(MAX_ADVISABLE_PREF_LENGTH + 1);
+const smallString = makeBuffer(4);
+
+const testValues = [
+  {name: "None", value: undefined},
+  {name: "Small", value: smallString},
+  {name: "Large", value: largeString},
+];
+
+function prefName(def, user) {
+  return "Test.IPC.default" + def.name + "User" + user.name;
+}
+
+function expectedPrefValue(def, user) {
+  if (user.value) {
+    return user.value;
+  }
+  return def.value;
+}
+
+function run_test() {
+  let pb = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+  let ps = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefService);
+  let defaultBranch = ps.getDefaultBranch("");
+
+  let isParent = isParentProcess();
+  if (isParent) {
+    // Set all combinations of none, small and large, for default and user prefs.
+    for (let def of testValues) {
+      for (let user of testValues) {
+        let currPref = prefName(def, user);
+        if (def.value) {
+          defaultBranch.setCharPref(currPref, def.value);
+        }
+        if (user.value) {
+          pb.setCharPref(currPref, user.value);
+        }
+      }
+    }
+
+    run_test_in_child("test_large_pref.js");
+  }
+
+  // Check that each preference is set or not set, as appropriate.
+  for (let def of testValues) {
+    for (let user of testValues) {
+      if (!def.value && !user.value) {
+        continue;
+      }
+      let pref_name = prefName(def, user);
+      if (isParent || (def.name != "Large" && user.name != "Large")) {
+        do_check_eq(pb.getCharPref(pref_name), expectedPrefValue(def, user));
+      } else {
+        // This is the child, and either the default or user value is
+        // large, so the preference should not be set.
+        let prefExists;
+        try {
+          pb.getCharPref(pref_name);
+          prefExists = true;
+        } catch(e) {
+          prefExists = false;
+        }
+        ok(!prefExists,
+           "Pref " + pref_name + " should not be set in the child");
+      }
+    }
+  }
+}
--- a/modules/libpref/test/unit_ipc/xpcshell.ini
+++ b/modules/libpref/test/unit_ipc/xpcshell.ini
@@ -1,10 +1,11 @@
 [DEFAULT]
 head = 
 tail = 
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 
 [test_existing_prefs.js]
 [test_initial_prefs.js]
+[test_large_pref.js]
 [test_observed_prefs.js]
 [test_update_prefs.js]
 [test_user_default_prefs.js]