Bug 1394176 - Use default values for scheduler prefs if the parent process didn't send any. r=froydnj draft
authorAndrew McCreight <continuation@gmail.com>
Tue, 29 Aug 2017 10:30:03 -0700
changeset 655335 73afa3eb5accf90f7aabadee9fc6d52f2d5a15af
parent 654398 3529b653ede26f990eb7320649015294ad0f8e76
child 728799 49d86cc5fb5f7bd2ed76007589847a28dcf256df
push id76837
push userbmo:continuation@gmail.com
push dateTue, 29 Aug 2017 21:33:45 +0000
reviewersfroydnj
bugs1394176
milestone57.0a1
Bug 1394176 - Use default values for scheduler prefs if the parent process didn't send any. r=froydnj In some unknown circumstance, possibly if the parent process has a different version than the child process, the child does not receive scheduler prefs, which makes it read out of an uninitialized local variable. This can probably happen because the scheduler prefs are checked before we do the ContentChild::Init version check. Bill also suggested that the pref env var might be getting truncated. This patch improves on the situation by using null for the prefs array if none was sent, and falling back on the default values, which leave the scheduler disabled. This also checks that the pref string is at least long enough to avoid a buffer overflow. Note that if the end of the string isn't an integer we'll end up with an sPrefThreadCount of zero, which can't be good. MozReview-Commit-ID: ByHLFMEpgyZ
dom/ipc/ContentProcess.cpp
xpcom/threads/Scheduler.cpp
--- a/dom/ipc/ContentProcess.cpp
+++ b/dom/ipc/ContentProcess.cpp
@@ -122,17 +122,17 @@ ContentProcess::Init(int aArgc, char* aA
   bool isForBrowser;
 
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
   // If passed in grab the profile path for sandboxing
   bool foundProfile = false;
   nsCOMPtr<nsIFile> profileDir;
 #endif
 
-  char* schedulerPrefs;
+  char* schedulerPrefs = nullptr;
   InfallibleTArray<PrefSetting> prefsArray;
   for (int idx = aArgc; idx > 0; idx--) {
     if (!aArgv[idx]) {
       continue;
     }
 
     if (!strcmp(aArgv[idx], "-appdir")) {
       MOZ_ASSERT(!foundAppdir);
--- a/xpcom/threads/Scheduler.cpp
+++ b/xpcom/threads/Scheduler.cpp
@@ -202,21 +202,21 @@ private:
   ThreadController mController;
 
   static size_t sNumThreadsRunning;
   static bool sUnlabeledEventRunning;
 
   JSContext* mContexts[CooperativeThreadPool::kMaxThreads];
 };
 
-bool SchedulerImpl::sPrefScheduler;
-bool SchedulerImpl::sPrefChaoticScheduling;
-bool SchedulerImpl::sPrefPreemption;
-bool SchedulerImpl::sPrefUseMultipleQueues;
-size_t SchedulerImpl::sPrefThreadCount;
+bool SchedulerImpl::sPrefScheduler = false;
+bool SchedulerImpl::sPrefChaoticScheduling = false;
+bool SchedulerImpl::sPrefPreemption = false;
+bool SchedulerImpl::sPrefUseMultipleQueues = false;
+size_t SchedulerImpl::sPrefThreadCount = 2;
 
 size_t SchedulerImpl::sNumThreadsRunning;
 bool SchedulerImpl::sUnlabeledEventRunning;
 
 bool
 SchedulerEventQueue::PutEvent(already_AddRefed<nsIRunnable>&& aEvent,
                               EventPriority aPriority)
 {
@@ -756,29 +756,45 @@ Scheduler::Shutdown()
   }
 }
 
 /* static */ nsCString
 Scheduler::GetPrefs()
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   nsPrintfCString result("%d%d%d%d,%d",
-                         Preferences::GetBool("dom.ipc.scheduler", false),
-                         Preferences::GetBool("dom.ipc.scheduler.chaoticScheduling", false),
-                         Preferences::GetBool("dom.ipc.scheduler.preemption", false) ,
-                         Preferences::GetBool("dom.ipc.scheduler.useMultipleQueues", false),
-                         Preferences::GetInt("dom.ipc.scheduler.threadCount", 2));
+                         Preferences::GetBool("dom.ipc.scheduler",
+                                              SchedulerImpl::sPrefScheduler),
+                         Preferences::GetBool("dom.ipc.scheduler.chaoticScheduling",
+                                              SchedulerImpl::sPrefChaoticScheduling),
+                         Preferences::GetBool("dom.ipc.scheduler.preemption",
+                                              SchedulerImpl::sPrefPreemption),
+                         Preferences::GetBool("dom.ipc.scheduler.useMultipleQueues",
+                                              SchedulerImpl::sPrefUseMultipleQueues),
+                         Preferences::GetInt("dom.ipc.scheduler.threadCount",
+                                             SchedulerImpl::sPrefThreadCount));
+
   return result;
 }
 
 /* static */ void
 Scheduler::SetPrefs(const char* aPrefs)
 {
   MOZ_ASSERT(XRE_IsContentProcess());
 
+  // If the prefs weren't sent to this process, use the default values.
+  if (!aPrefs) {
+    return;
+  }
+
+  // If the pref string appears truncated, use the default values.
+  if (strlen(aPrefs) < 6) {
+    return;
+  }
+
   SchedulerImpl::sPrefScheduler = aPrefs[0] == '1';
   SchedulerImpl::sPrefChaoticScheduling = aPrefs[1] == '1';
   SchedulerImpl::sPrefPreemption = aPrefs[2] == '1';
   SchedulerImpl::sPrefUseMultipleQueues = aPrefs[3] == '1';
   MOZ_ASSERT(aPrefs[4] == ',');
   SchedulerImpl::sPrefThreadCount = atoi(aPrefs + 5);
 }