Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r?bsmedberg draft
authorAaron Klotz <aklotz@mozilla.com>
Wed, 16 Mar 2016 12:35:50 -0600
changeset 341729 5aa1e9cb8b9ab976a6af17dee750c8ff59daaf26
parent 339874 f0c0480732d36153e8839c7f17394d45f679f87d
child 516446 9ee6f4f77c2ad6df83e92f164996fe097e48d9ca
push id13272
push useraklotz@mozilla.com
push dateThu, 17 Mar 2016 17:40:34 +0000
reviewersbsmedberg
bugs1256541, 1257098
milestone48.0a1
Bug 1256541: Fix incorrect generation of path for child process .extra files when content sandboxing is disabled; r?bsmedberg Since the minidump path can be overridden programmatically in the chrome process, using that path as the base for .extra files won't work since content is unaware of it. This patch changes everything to use the temp path when MOZ_CONTENT_SANDBOX is not defined or when sandboxing is disabled via pref. It also moves the derivation of the content temp path out of exception context on Windows and Mac, as I found out that those functions touch the heap. I also noticed that xpcshell is not sandbox-aware when utilized as a parent process. I've filed bug 1257098 to take care of that, but this patch includes a hack for the immediate term. MozReview-Commit-ID: 3SIB5Nihqxh
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)
@@ -259,19 +261,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;
 
@@ -1102,21 +1104,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)
@@ -1219,38 +1221,52 @@ 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))
+  MOZ_ASSERT(childProcessTmpDir && !childProcessTmpDir->empty());
+  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);
@@ -2877,38 +2893,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"
@@ -2918,36 +2927,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)
 {
@@ -3019,18 +3012,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;
     }
   }
@@ -3157,23 +3149,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(
@@ -3398,16 +3402,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
@@ -231,16 +231,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;
   }