Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r=bsmedberg, a=lizzard
authorAaron Klotz <aklotz@mozilla.com>
Wed, 16 Mar 2016 12:35:50 -0600
changeset 323669 044150f51647a19b5f926f659fff3949436a5d77
parent 323668 b51762dc3de0b6e116b1e02096f691a60a6f5be7
child 323670 a9fd59d20ca17eb27c1096c8cc582463a54d06bd
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg, lizzard
bugs1256541
milestone47.0a2
Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r=bsmedberg, a=lizzard MozReview-Commit-ID: JQgqlntQ6cu
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.h
toolkit/xre/nsEmbedFunctions.cpp
toolkit/xre/nsXREDirProvider.cpp
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -119,16 +119,17 @@ typedef wchar_t XP_CHAR;
 typedef std::wstring xpstring;
 #define XP_TEXT(x) L##x
 #define CONVERT_XP_CHAR_TO_UTF16(x) x
 #define XP_STRLEN(x) wcslen(x)
 #define my_strlen strlen
 #define CRASH_REPORTER_FILENAME "crashreporter.exe"
 #define PATH_SEPARATOR "\\"
 #define XP_PATH_SEPARATOR L"\\"
+#define XP_PATH_SEPARATOR_CHAR L'\\'
 #define XP_PATH_MAX (MAX_PATH + 1)
 // "<reporter path>" "<minidump path>"
 #define CMDLINE_SIZE ((XP_PATH_MAX * 2) + 6)
 #ifdef _USE_32BIT_TIME_T
 #define XP_TTOA(time, buffer, base) ltoa(time, buffer, base)
 #else
 #define XP_TTOA(time, buffer, base) _i64toa(time, buffer, base)
 #endif
@@ -136,16 +137,17 @@ typedef std::wstring xpstring;
 #else
 typedef char XP_CHAR;
 typedef std::string xpstring;
 #define XP_TEXT(x) x
 #define CONVERT_XP_CHAR_TO_UTF16(x) NS_ConvertUTF8toUTF16(x)
 #define CRASH_REPORTER_FILENAME "crashreporter"
 #define PATH_SEPARATOR "/"
 #define XP_PATH_SEPARATOR "/"
+#define XP_PATH_SEPARATOR_CHAR '/'
 #define XP_PATH_MAX PATH_MAX
 #ifdef XP_LINUX
 #define XP_STRLEN(x) my_strlen(x)
 #define XP_TTOA(time, buffer, base) my_inttostring(time, buffer, sizeof(buffer))
 #define XP_STOA(size, buffer, base) my_inttostring(size, buffer, sizeof(buffer))
 #else
 #define XP_STRLEN(x) strlen(x)
 #define XP_TTOA(time, buffer, base) sprintf(buffer, "%ld", time)
@@ -262,19 +264,19 @@ static uint32_t eventloopNestingLevel = 
 
 // Avoid a race during application termination.
 static Mutex* dumpSafetyLock;
 static bool isSafeToDump = false;
 
 // OOP crash reporting
 static CrashGenerationServer* crashServer; // chrome process has this
 
-#if (defined(XP_MACOSX) || defined(XP_WIN)) && defined(MOZ_CONTENT_SANDBOX)
-// This field is only valid in the chrome process, not content.
-static xpstring* contentProcessTmpDir = nullptr;
+#if (defined(XP_MACOSX) || defined(XP_WIN))
+// This field is valid in both chrome and content processes.
+static xpstring* childProcessTmpDir = nullptr;
 #endif
 
 #  if defined(XP_WIN) || defined(XP_MACOSX)
 // If crash reporting is disabled, we hand out this "null" pipe to the
 // child process and don't attempt to connect to a parent server.
 static const char kNullNotifyPipe[] = "-";
 static char* childCrashNotifyPipe;
 
@@ -1111,21 +1113,21 @@ bool MinidumpCallback(
 #endif // XP_MACOSX
 #endif // XP_UNIX
 
   return returnValue;
 }
 
 #if defined(XP_MACOSX) || defined(__ANDROID__)
 static size_t
-EnsureTrailingSlash(char* aBuf, size_t aBufLen)
+EnsureTrailingSlash(XP_CHAR* aBuf, size_t aBufLen)
 {
   size_t len = XP_STRLEN(aBuf);
-  if ((len + 2) < aBufLen && aBuf[len - 1] != '/') {
-    aBuf[len] = '/';
+  if ((len + 2) < aBufLen && aBuf[len - 1] != XP_PATH_SEPARATOR_CHAR) {
+    aBuf[len] = XP_PATH_SEPARATOR_CHAR;
     ++len;
     aBuf[len] = 0;
   }
   return len;
 }
 #endif
 
 #if defined(XP_WIN32)
@@ -1220,38 +1222,51 @@ BuildTempPath(PathStringT& aResult)
 
 static void
 PrepareChildExceptionTimeAnnotations()
 {
   MOZ_ASSERT(!XRE_IsParentProcess());
   static XP_CHAR tempPath[XP_PATH_MAX] = {0};
 
   // Get the temp path
+  int charsAvailable = XP_PATH_MAX;
+  XP_CHAR* p = tempPath;
+#if (defined(XP_MACOSX) || defined(XP_WIN))
+  if (!childProcessTmpDir || childProcessTmpDir->empty()) {
+    return;
+  }
+  p = Concat(p, childProcessTmpDir->c_str(), &charsAvailable);
+  // Ensure that this path ends with a path separator
+  if (p > tempPath && *(p - 1) != XP_PATH_SEPARATOR_CHAR) {
+    p = Concat(p, XP_PATH_SEPARATOR, &charsAvailable);
+  }
+#else
   size_t tempPathLen = BuildTempPath(tempPath);
   if (!tempPathLen) {
     return;
   }
+  p += tempPathLen;
+  charsAvailable -= tempPathLen;
+#endif
 
   // Generate and append the file name
-  int size = XP_PATH_MAX - tempPathLen;
-  XP_CHAR* p = tempPath + tempPathLen;
-  p = Concat(p, childCrashAnnotationBaseName, &size);
+  p = Concat(p, childCrashAnnotationBaseName, &charsAvailable);
   XP_CHAR pidBuffer[32] = XP_TEXT("");
 #if defined(XP_WIN32)
   _ui64tow(GetCurrentProcessId(), pidBuffer, 10);
 #else
   XP_STOA(getpid(), pidBuffer, 10);
 #endif
-  p = Concat(p, pidBuffer, &size);
+  p = Concat(p, pidBuffer, &charsAvailable);
 
   // Now open the file...
   PlatformWriter apiData;
   OpenAPIData(apiData, tempPath);
 
-  // ...and write out any annotations. These should be escaped if necessary
+  // ...and write out any annotations. These must be escaped if necessary
   // (but don't call EscapeAnnotation here, because it touches the heap).
   char oomAllocationSizeBuffer[32] = "";
   if (gOOMAllocationSize) {
     XP_STOA(gOOMAllocationSize, oomAllocationSizeBuffer, 10);
   }
 
   if (oomAllocationSizeBuffer[0]) {
     WriteAnnotation(apiData, "OOMAllocationSize", oomAllocationSizeBuffer);
@@ -2896,38 +2911,31 @@ WriteExtraData(nsIFile* extraFile,
 
 bool
 AppendExtraData(nsIFile* extraFile, const AnnotationTable& data)
 {
   return WriteExtraData(extraFile, data, Blacklist());
 }
 
 static bool
-GetExtraFileForChildPid(nsIFile* aMinidump, uint32_t aPid, nsIFile** aExtraFile)
+GetExtraFileForChildPid(uint32_t aPid, nsIFile** aExtraFile)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
 
   nsCOMPtr<nsIFile> extraFile;
   nsresult rv;
 
 #if defined(XP_WIN) || defined(XP_MACOSX)
-#  if defined(MOZ_CONTENT_SANDBOX)
-  if (!contentProcessTmpDir) {
+  if (!childProcessTmpDir) {
     return false;
   }
-  CreateFileFromPath(*contentProcessTmpDir, getter_AddRefs(extraFile));
+  CreateFileFromPath(*childProcessTmpDir, getter_AddRefs(extraFile));
   if (!extraFile) {
     return false;
   }
-#  else
-  rv = aMinidump->Clone(getter_AddRefs(extraFile));
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-#  endif // defined(MOZ_CONTENT_SANDBOX)
 #elif defined(XP_UNIX)
   rv = NS_NewLocalFile(NS_LITERAL_STRING("/tmp"), false,
                        getter_AddRefs(extraFile));
   if (NS_FAILED(rv)) {
     return false;
   }
 #else
 #error "Implement this for your platform"
@@ -2937,36 +2945,20 @@ GetExtraFileForChildPid(nsIFile* aMinidu
 #if defined(XP_WIN)
   leafName.AppendPrintf("%S%u%S", childCrashAnnotationBaseName, aPid,
                         extraFileExtension);
 #else
   leafName.AppendPrintf("%s%u%s", childCrashAnnotationBaseName, aPid,
                         extraFileExtension);
 #endif
 
-#if defined(XP_WIN) || defined(XP_MACOSX)
-#  if defined(MOZ_CONTENT_SANDBOX)
   rv = extraFile->Append(leafName);
   if (NS_FAILED(rv)) {
     return false;
   }
-#  else
-  rv = extraFile->SetLeafName(leafName);
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-#  endif // defined(MOZ_CONTENT_SANDBOX)
-#elif defined(XP_UNIX)
-  rv = extraFile->Append(leafName);
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-#else
-#error "Implement this for your platform"
-#endif
 
   extraFile.forget(aExtraFile);
   return true;
 }
 
 static bool
 IsDataEscaped(char* aData)
 {
@@ -3038,18 +3030,17 @@ WriteExtraForMinidump(nsIFile* minidump,
                       blacklist,
                       true /*write crash time*/,
                       true /*truncate*/)) {
     return false;
   }
 
   nsCOMPtr<nsIFile> exceptionTimeExtra;
   FILE* fd;
-  if (pid && GetExtraFileForChildPid(minidump, pid,
-                                     getter_AddRefs(exceptionTimeExtra)) &&
+  if (pid && GetExtraFileForChildPid(pid, getter_AddRefs(exceptionTimeExtra)) &&
       NS_SUCCEEDED(exceptionTimeExtra->OpenANSIFileDesc("r", &fd))) {
     AnnotationTable exceptionTimeAnnotations;
     ReadAndValidateExceptionTimeAnnotations(fd, exceptionTimeAnnotations);
     fclose(fd);
     if (!AppendExtraData(extra, exceptionTimeAnnotations)) {
       return false;
     }
   }
@@ -3176,23 +3167,35 @@ OOPInit()
   if (OOPInitialized())
     return;
 
   MOZ_ASSERT(NS_IsMainThread());
 
   MOZ_ASSERT(gExceptionHandler != nullptr,
              "attempt to initialize OOP crash reporter before in-process crashreporter!");
 
-#if (defined(XP_WIN) || defined(XP_MACOSX)) && defined(MOZ_CONTENT_SANDBOX)
+#if (defined(XP_WIN) || defined(XP_MACOSX))
   nsCOMPtr<nsIFile> tmpDir;
-  nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir));
+# if defined(MOZ_CONTENT_SANDBOX)
+  nsresult rv = NS_GetSpecialDirectory(NS_APP_CONTENT_PROCESS_TEMP_DIR,
+                                       getter_AddRefs(tmpDir));
+  if (NS_FAILED(rv) && PR_GetEnv("XPCSHELL_TEST_PROFILE_DIR")) {
+    // Temporary hack for xpcshell, will be fixed in bug 1257098
+    rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir));
+  }
   if (NS_SUCCEEDED(rv)) {
-    contentProcessTmpDir = CreatePathFromFile(tmpDir);
+    childProcessTmpDir = CreatePathFromFile(tmpDir);
   }
-#endif
+# else
+  if (NS_SUCCEEDED(NS_GetSpecialDirectory(NS_OS_TEMP_DIR,
+                                          getter_AddRefs(tmpDir)))) {
+    childProcessTmpDir = CreatePathFromFile(tmpDir);
+  }
+# endif // defined(MOZ_CONTENT_SANDBOX)
+#endif // (defined(XP_WIN) || defined(XP_MACOSX))
 
 #if defined(XP_WIN)
   childCrashNotifyPipe =
     PR_smprintf("\\\\.\\pipe\\gecko-crash-server-pipe.%i",
                 static_cast<int>(::GetCurrentProcessId()));
 
   const std::wstring dumpPath = gExceptionHandler->dump_path();
   crashServer = new CrashGenerationServer(
@@ -3417,16 +3420,31 @@ GetLastRunCrashID(nsAString& id)
   if (!lastRunCrashID) {
     return false;
   }
 
   id = *lastRunCrashID;
   return true;
 }
 
+#if defined(XP_WIN) || defined(XP_MACOSX)
+void
+InitChildProcessTmpDir()
+{
+  MOZ_ASSERT(!XRE_IsParentProcess());
+  // When retrieved by the child process, this will always resolve to the
+  // correct directory regardless of sandbox level.
+  nsCOMPtr<nsIFile> tmpDir;
+  nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpDir));
+  if (NS_SUCCEEDED(rv)) {
+    childProcessTmpDir = CreatePathFromFile(tmpDir);
+  }
+}
+#endif // defined(XP_WIN) || defined(XP_MACOSX)
+
 #if defined(XP_WIN)
 // Child-side API
 bool
 SetRemoteExceptionHandler(const nsACString& crashPipe)
 {
   // crash reporting is disabled
   if (crashPipe.Equals(kNullNotifyPipe))
     return true;
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@@ -232,16 +232,17 @@ public:
 
 // This method implies OOPInit
 void InjectCrashReporterIntoProcess(DWORD processID, InjectorCrashCallback* cb);
 void UnregisterInjectorCallback(DWORD processID);
 #endif
 
 // Child-side API
 bool SetRemoteExceptionHandler(const nsACString& crashPipe);
+void InitChildProcessTmpDir();
 
 #  elif defined(XP_LINUX)
 // Parent-side API for children
 
 // Set the outparams for crash reporter server's fd (|childCrashFd|)
 // and the magic fd number it should be remapped to
 // (|childCrashRemapFd|) before exec() in the child process.
 // |SetRemoteExceptionHandler()| in the child process expects to find
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -607,16 +607,20 @@ XRE_InitChildProcess(int aArgc,
       }
 
       if (!process->Init()) {
         profiler_shutdown();
         NS_LogTerm();
         return NS_ERROR_FAILURE;
       }
 
+#if defined(XP_WIN) || defined(XP_MACOSX)
+      CrashReporter::InitChildProcessTmpDir();
+#endif
+
 #if defined(XP_WIN)
       // Set child processes up such that they will get killed after the
       // chrome process is killed in cases where the user shuts the system
       // down or logs off.
       ::SetProcessShutdownParameters(0x280 - 1, SHUTDOWN_NORETRY);
 #endif
 
 #if defined(MOZ_SANDBOX) && defined(XP_WIN)
--- a/toolkit/xre/nsXREDirProvider.cpp
+++ b/toolkit/xre/nsXREDirProvider.cpp
@@ -37,16 +37,17 @@
 #include "mozilla/Preferences.h"
 #include "mozilla/Telemetry.h"
 
 #include <stdlib.h>
 
 #ifdef XP_WIN
 #include <windows.h>
 #include <shlobj.h>
+#include "mozilla/WindowsVersion.h"
 #endif
 #ifdef XP_MACOSX
 #include "nsILocalFileMac.h"
 // for chflags()
 #include <sys/stat.h>
 #include <unistd.h>
 #endif
 #ifdef XP_UNIX
@@ -638,16 +639,30 @@ GetContentProcessTempBaseDirKey()
 #else
   return NS_OS_TEMP_DIR;
 #endif
 }
 
 nsresult
 nsXREDirProvider::LoadContentProcessTempDir()
 {
+#if defined(XP_WIN)
+  const bool isSandboxDisabled = !mozilla::IsVistaOrLater() ||
+    (Preferences::GetInt("security.sandbox.content.level") < 1);
+#elif defined(XP_MACOSX)
+  const bool isSandboxDisabled =
+    Preferences::GetInt("security.sandbox.content.level") < 1;
+#endif
+
+  if (isSandboxDisabled) {
+    // Just use the normal temp directory if sandboxing is turned off
+    return NS_GetSpecialDirectory(NS_OS_TEMP_DIR,
+                                  getter_AddRefs(mContentTempDir));
+  }
+
   nsCOMPtr<nsIFile> localFile;
 
   nsresult rv = NS_GetSpecialDirectory(GetContentProcessTempBaseDirKey(),
                                        getter_AddRefs(localFile));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }