Bug 990230 - Fix heap use-after-free in memory reporter. r=n.nethercote

xpcom/base/nsDumpUtils.cpp

@@ -11,7 +11,7 @@
 #include <errno.h>
 #include "mozilla/Services.h"
 #include "nsIObserverService.h"
- #include "mozilla/ClearOnShutdown.h"
+#include "mozilla/ClearOnShutdown.h"
 
 #if defined(XP_LINUX) || defined(__FreeBSD__) // {
 #include "mozilla/Preferences.h"
@@ -20,6 +20,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+using namespace mozilla;
+
 /*
  * The following code supports triggering a registered callback upon
  * receiving a specific signal.
@@ -118,25 +120,26 @@ SignalPipeWatcher::GetSingleton()
   return sSingleton;
 }
 
-/* static */ void
+void
-SignalPipeWatcher::RegisterCallback(const uint8_t aSignal,
+SignalPipeWatcher::RegisterCallback(uint8_t aSignal,
                                     PipeCallback aCallback)
 {
-  for (SignalInfoArray::index_type i = 0; 
+  MutexAutoLock lock(mSignalInfoLock);
-       i < SignalPipeWatcher::mSignalInfo.Length(); i++)
+
+  for (SignalInfoArray::index_type i = 0; i < mSignalInfo.Length(); i++)
   {
-    if (SignalPipeWatcher::mSignalInfo[i].mSignal == aSignal) {
+    if (mSignalInfo[i].mSignal == aSignal) {
       LOG("Register Signal(%d) callback failed! (DUPLICATE)", aSignal);
       return;
     }
   }
-  SignalInfo aSignalInfo = { aSignal, aCallback };
+  SignalInfo signalInfo = { aSignal, aCallback };
-  SignalPipeWatcher::mSignalInfo.AppendElement(aSignalInfo);
+  mSignalInfo.AppendElement(signalInfo);
-  SignalPipeWatcher::RegisterSignalHandler(aSignalInfo.mSignal);
+  RegisterSignalHandler(signalInfo.mSignal);
 }
 
-/* static */ void
+void
-SignalPipeWatcher::RegisterSignalHandler(const uint8_t aSignal)
+SignalPipeWatcher::RegisterSignalHandler(uint8_t aSignal)
 {
   struct sigaction action;
   memset(&action, 0, sizeof(action));
@@ -148,10 +151,11 @@ SignalPipeWatcher::RegisterSignalHandler(const uint8_t aSignal)
       LOG("SignalPipeWatcher failed to register sig %d.", aSignal);
     }
   } else {
-    for (SignalInfoArray::index_type i = 0; i < SignalPipeWatcher::mSignalInfo.Length(); i++) {
+    MutexAutoLock lock(mSignalInfoLock);
-      if (sigaction(SignalPipeWatcher::mSignalInfo[i].mSignal, &action, nullptr)) {
+    for (SignalInfoArray::index_type i = 0; i < mSignalInfo.Length(); i++) {
+      if (sigaction(mSignalInfo[i].mSignal, &action, nullptr)) {
         LOG("SignalPipeWatcher failed to register signal(%d) "
-            "dump signal handler.",SignalPipeWatcher::mSignalInfo[i].mSignal);
+            "dump signal handler.", mSignalInfo[i].mSignal);
       }
     }
   }
@@ -159,8 +163,9 @@ SignalPipeWatcher::RegisterSignalHandler(const uint8_t aSignal)
 
 SignalPipeWatcher::~SignalPipeWatcher()
 {
-  if (sDumpPipeWriteFd != -1)
+  if (sDumpPipeWriteFd != -1) {
-    SignalPipeWatcher::StopWatching();
+    StopWatching();
+  }
 }
 
 int SignalPipeWatcher::OpenFd()
@@ -182,7 +187,7 @@ int SignalPipeWatcher::OpenFd()
   int readFd = pipeFds[0];
   sDumpPipeWriteFd = pipeFds[1];
 
-  SignalPipeWatcher::RegisterSignalHandler();
+  RegisterSignalHandler();
   return readFd;
 }
 
@@ -215,12 +220,15 @@ void SignalPipeWatcher::OnFileCanReadWithoutBlocking(int aFd)
     return;
   }
 
-  for (SignalInfoArray::index_type i = 0; i < SignalPipeWatcher::mSignalInfo.Length(); i++) {
+  {
-    if(signum == SignalPipeWatcher::mSignalInfo[i].mSignal) {
+    MutexAutoLock lock(mSignalInfoLock);
-      SignalPipeWatcher::mSignalInfo[i].mCallback(signum);
+    for (SignalInfoArray::index_type i = 0; i < mSignalInfo.Length(); i++) {
+      if(signum == mSignalInfo[i].mSignal) {
+        mSignalInfo[i].mCallback(signum);
         return;
       }
     }
+  }
   LOG("SignalPipeWatcher got unexpected signum.");
 }
 
@@ -257,25 +265,26 @@ FifoWatcher::MaybeCreate()
   }
 
   // The FifoWatcher is held alive by the observer service.
-  if (!FifoWatcher::sSingleton) {
+  if (!sSingleton) {
-    FifoWatcher::GetSingleton();
+    GetSingleton();
   }
   return true;
 }
 
 void
-FifoWatcher::RegisterCallback(const nsCString& aCommand,FifoCallback aCallback)
+FifoWatcher::RegisterCallback(const nsCString& aCommand, FifoCallback aCallback)
 {
-  for (FifoInfoArray::index_type i = 0;
+  MutexAutoLock lock(mFifoInfoLock);
-       i < FifoWatcher::mFifoInfo.Length(); i++)
+
+  for (FifoInfoArray::index_type i = 0; i < mFifoInfo.Length(); i++)
   {
-    if (FifoWatcher::mFifoInfo[i].mCommand.Equals(aCommand)) {
+    if (mFifoInfo[i].mCommand.Equals(aCommand)) {
       LOG("Register command(%s) callback failed! (DUPLICATE)", aCommand.get());
       return;
     }
   }
   FifoInfo aFifoInfo = { aCommand, aCallback };
-  FifoWatcher::mFifoInfo.AppendElement(aFifoInfo);
+  mFifoInfo.AppendElement(aFifoInfo);
 }
 
 FifoWatcher::~FifoWatcher()
@@ -392,13 +401,17 @@ void FifoWatcher::OnFileCanReadWithoutBlocking(int aFd)
   // it'll actually write "foo\n" to the fifo.
   inputStr.Trim("\b\t\r\n");
 
-  for (FifoInfoArray::index_type i = 0; i < FifoWatcher::mFifoInfo.Length(); i++) {
+  {
-    const nsCString commandStr = FifoWatcher::mFifoInfo[i].mCommand;
+    MutexAutoLock lock(mFifoInfoLock);
+
+    for (FifoInfoArray::index_type i = 0; i < mFifoInfo.Length(); i++) {
+      const nsCString commandStr = mFifoInfo[i].mCommand;
       if(inputStr == commandStr.get()) {
-      FifoWatcher::mFifoInfo[i].mCallback(inputStr);
+        mFifoInfo[i].mCallback(inputStr);
         return;
       }
     }
+  }
   LOG("Got unexpected value from fifo; ignoring it.");
 }
 

xpcom/base/nsDumpUtils.h

@@ -11,6 +11,7 @@
 #include "base/message_loop.h"
 #include "nsXULAppAPI.h"
 #include "nsThreadUtils.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/StaticPtr.h"
 #include "nsTArray.h"
 
@@ -126,8 +127,10 @@ private:
 
   FifoWatcher(nsCString aPath)
     : mDirPath(aPath)
+    , mFifoInfoLock("FifoWatcher.mFifoInfoLock")
   {}
 
+  mozilla::Mutex mFifoInfoLock; // protects mFifoInfo
   FifoInfoArray mFifoInfo;
 };
 
@@ -143,9 +146,9 @@ class SignalPipeWatcher : public FdWatcher
 public:
   static SignalPipeWatcher* GetSingleton();
 
-  void RegisterCallback(const uint8_t aSignal, PipeCallback aCallback);
+  void RegisterCallback(uint8_t aSignal, PipeCallback aCallback);
 
-  void RegisterSignalHandler(const uint8_t aSignal = 0);
+  void RegisterSignalHandler(uint8_t aSignal = 0);
 
   virtual ~SignalPipeWatcher();
 
@@ -159,10 +162,12 @@ private:
   static StaticRefPtr<SignalPipeWatcher> sSingleton;
 
   SignalPipeWatcher()
+    : mSignalInfoLock("SignalPipeWatcher.mSignalInfoLock")
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
+  mozilla::Mutex mSignalInfoLock; // protects mSignalInfo
   SignalInfoArray mSignalInfo;
 };