Bug 1395952: Enhance telemetry for failed launch of Windows sandboxed…

… process by process type/error code key. r=jimm, data-r=rweiss Only one telemetry accumlation will occur for each key per session.
mozilla-spidermonkey · Sep 12, 2017 · 67b3024 · 67b3024
1 parent bd7cddf
commit 67b3024
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 4 deletions.
diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp
@@ -1119,6 +1119,7 @@ GeckoChildProcessHost::PerformAsyncLaunchInternal(std::vector<std::string>& aExt
   if (shouldSandboxCurrentProcess) {
     if (mSandboxBroker.LaunchApp(cmdLine.program().c_str(),
                                  cmdLine.command_line_string().c_str(),
+                                 mProcessType,
                                  mEnableSandboxLogging,
                                  &process)) {
       EnvironmentLog("MOZ_PROCESS_LOG").print(

diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -23,6 +23,7 @@
 #include "nsIProperties.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
+#include "nsTHashtable.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/security_level.h"
 #include "WinUtils.h"
@@ -50,6 +51,9 @@ static LazyLogModule sSandboxBrokerLog("SandboxBroker");
 #define LOG_E(...) MOZ_LOG(sSandboxBrokerLog, LogLevel::Error, (__VA_ARGS__))
 #define LOG_W(...) MOZ_LOG(sSandboxBrokerLog, LogLevel::Warning, (__VA_ARGS__))
 
+// Used to store whether we have accumulated an error combination for this session.
+static UniquePtr<nsTHashtable<nsCStringHashKey>> sLaunchErrors;
+
 /* static */
 void
 SandboxBroker::Initialize(sandbox::BrokerServices* aBrokerServices)
@@ -135,6 +139,7 @@ SandboxBroker::SandboxBroker()
 bool
 SandboxBroker::LaunchApp(const wchar_t *aPath,
                          const wchar_t *aArguments,
+                         GeckoProcessType aProcessType,
                          const bool aEnableLogging,
                          void **aProcessHandle)
 {
@@ -206,9 +211,25 @@ SandboxBroker::LaunchApp(const wchar_t *aPath,
   result = sBrokerService->SpawnTarget(aPath, aArguments, mPolicy,
                                        &last_warning, &last_error, &targetInfo);
   if (sandbox::SBOX_ALL_OK != result) {
-    Telemetry::Accumulate(Telemetry::SANDBOX_FAILED_LAUNCH, result);
+    nsAutoCString key;
+    key.AppendASCII(XRE_ChildProcessTypeToString(aProcessType));
+    key.AppendLiteral("/0x");
+    key.AppendInt(static_cast<uint32_t>(last_error), 16);
+
+    if (!sLaunchErrors) {
+      sLaunchErrors = MakeUnique<nsTHashtable<nsCStringHashKey>>();
+      ClearOnShutdown(&sLaunchErrors);
+    }
+
+    // Only accumulate for each combination once per session.
+    if (!sLaunchErrors->Contains(key)) {
+      Telemetry::Accumulate(Telemetry::SANDBOX_FAILED_LAUNCH_KEYED, key, result);
+      sLaunchErrors->PutEntry(key);
+    }
+
     LOG_E("Failed (ResultCode %d) to SpawnTarget with last_error=%d, last_warning=%d",
           result, last_error, last_warning);
+
     return false;
   } else if (sandbox::SBOX_ALL_OK != last_warning) {
     // If there was a warning (but the result was still ok), log it and proceed.

diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -11,6 +11,7 @@
 #include <windows.h>
 
 #include "base/child_privileges.h"
+#include "nsXULAppAPI.h"
 
 namespace sandbox {
   class BrokerServices;
@@ -34,6 +35,7 @@ class SandboxBroker
 
   bool LaunchApp(const wchar_t *aPath,
                  const wchar_t *aArguments,
+                 GeckoProcessType aProcessType,
                  const bool aEnableLogging,
                  void **aProcessHandle);
   virtual ~SandboxBroker();

diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json
@@ -12590,15 +12590,16 @@
     "cpp_guard": "XP_LINUX",
     "description": "System calls blocked by a seccomp-bpf sandbox policy; limited to syscalls where we would crash on Nightly.  The key is generally the architecture and syscall ID but in some cases we include non-personally-identifying information from the syscall arguments; see the function SubmitToTelemetry in security/sandbox/linux/reporter/SandboxReporter.cpp for details."
   },
-  "SANDBOX_FAILED_LAUNCH": {
+  "SANDBOX_FAILED_LAUNCH_KEYED": {
     "record_in_processes": ["main"],
     "alert_emails": ["bowen@mozilla.com"],
-    "expires_in_version": "60",
+    "expires_in_version": "never",
     "kind": "enumerated",
+    "keyed": true,
     "n_values": 50,
     "bug_numbers": [1368600],
     "cpp_guard": "XP_WIN",
-    "description": "Error code when a Windows sandboxed process fails to launch. See https://dxr.mozilla.org/mozilla-central/search?q=ResultCode++path%3Asandbox_types.h&redirect=true for definitions of the error codes."
+    "description": "Error code when a Windows sandboxed process fails to launch, keyed by process type and Windows error code. See https://dxr.mozilla.org/mozilla-central/search?q=ResultCode++path%3Asandbox_types.h&redirect=true for definitions of the error codes."
   },
   "SYNC_WORKER_OPERATION": {
     "record_in_processes": ["main", "content"],