Bug 1394176 - Use default values for scheduler prefs if the parent process didn't send any. r=billm draft
authorAndrew McCreight <continuation@gmail.com>
Tue, 29 Aug 2017 10:30:03 -0700
changeset 655336 bef3172da85252a67fc7fd03fabb9c8edc548918
parent 654398 3529b653ede26f990eb7320649015294ad0f8e76
child 728800 fd062093d523808db28254edafad53313a1b118f
push id76838
push userbmo:continuation@gmail.com
push dateTue, 29 Aug 2017 21:34:12 +0000
reviewersbillm
bugs1394176
milestone57.0a1
Bug 1394176 - Use default values for scheduler prefs if the parent process didn't send any. r=billm 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);
 }