win: crashpad_handler should create its own pipe name in ephemeral mode

client/crashpad_client_win.cc

Index: client/crashpad_client_win.cc
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
index 3f23dde16e0ec74aac2de236f03f58d3e59889d2..a57a33f20595c10188098dc7e48cf7325cdb25ce 100644
--- a/client/crashpad_client_win.cc
+++ b/client/crashpad_client_win.cc
@@ -19,7 +19,7 @@
 
 #include "base/atomicops.h"
 #include "base/logging.h"
-#include "base/rand_util.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -129,12 +129,17 @@ bool CrashpadClient::StartHandler(
     const std::vector<std::string>& arguments) {
   DCHECK(ipc_pipe_.empty());
 
-  std::string ipc_pipe =
-      base::StringPrintf("\\\\.\\pipe\\crashpad_%d_", GetCurrentProcessId());
-  for (int index = 0; index < 16; ++index) {
-    ipc_pipe.append(1, static_cast<char>(base::RandInt('A', 'Z')));
+  HANDLE pipe_read;
+  HANDLE pipe_write;
+  SECURITY_ATTRIBUTES security_attributes = {};
+  security_attributes.nLength = sizeof(security_attributes);
+  security_attributes.bInheritHandle = TRUE;
+  if (!CreatePipe(&pipe_read, &pipe_write, &security_attributes, 0)) {
+    PLOG(ERROR) << "CreatePipe";
+    return false;
   }
-  ipc_pipe_ = base::UTF8ToUTF16(ipc_pipe);
+  ScopedFileHandle pipe_read_owner(pipe_read);

[scottmg, 2015/11/03 20:52:07]

SetHandleInformation(pipe_read, HANDLE_FLAG_INHERIT, 0) so the read side isn't
inherited by the handler.

[Mark Mentovai, 2015/11/03 23:58:48]

scottmg wrote:
Done.

+  ScopedFileHandle pipe_write_owner(pipe_write);
 
   std::wstring command_line;
   AppendCommandLineArgument(handler.value(), &command_line);
@@ -157,8 +162,16 @@ bool CrashpadClient::StartHandler(
                              base::UTF8ToUTF16(kv.first + '=' + kv.second)),
         &command_line);
   }
-  AppendCommandLineArgument(FormatArgumentString("pipe-name", ipc_pipe_),
-                            &command_line);
+
+  // According to
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203, HANDLEs
+  // are always 32 bits.
+  unsigned int pipe_write_uint =
+      base::checked_cast<unsigned int>(reinterpret_cast<uintptr_t>(pipe_write));
+  AppendCommandLineArgument(
+      base::UTF8ToUTF16(base::StringPrintf("--handshake-handle=0x%x",
+                                           pipe_write_uint)),
+      &command_line);
 
   STARTUPINFO startup_info = {};
   startup_info.cb = sizeof(startup_info);
@@ -188,6 +201,20 @@ bool CrashpadClient::StartHandler(
   rv = CloseHandle(process_info.hProcess);
   PLOG_IF(WARNING, !rv) << "CloseHandle process";
 
+  pipe_write_owner.reset();
+
+  uint32_t ipc_pipe_length;
+  if (!LoggingReadFile(pipe_read, &ipc_pipe_length, sizeof(ipc_pipe_length))) {
+    return false;
+  }
+
+  ipc_pipe_.resize(ipc_pipe_length);
+  if (ipc_pipe_length &&
+      !LoggingReadFile(
+          pipe_read, &ipc_pipe_[0], ipc_pipe_length * sizeof(ipc_pipe_[0]))) {

[scottmg, 2015/11/03 20:52:07]

I'm a little worried about the implicitly-wide char length here, but I guess it
will be obvious enough if something is mismatched, so not really a problem.

[Mark Mentovai, 2015/11/03 23:58:48]

scottmg wrote:
It seemed better than converting to and from UTF-8, since it seems that these
really are path strings and we do want UTF-16 on each side.

+    return false;
+  }
+
   return true;
 }