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

client/crashpad_client_win.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #include "client/crashpad_client.h"
 
 #include <string.h>
 #include <windows.h>
 
 #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"
 #include "base/synchronization/lock.h"
 #include "util/file/file_io.h"
 #include "util/win/command_line.h"
 #include "util/win/critical_section_with_debug_info.h"
 #include "util/win/registration_protocol_win.h"
 #include "util/win/scoped_handle.h"
 
@@ skipping 89 matching lines @@
 }
 
 bool CrashpadClient::StartHandler(
     const base::FilePath& handler,
     const base::FilePath& database,
     const std::string& url,
     const std::map<std::string, std::string>& annotations,
     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);
   for (const std::string& argument : arguments) {
     AppendCommandLineArgument(base::UTF8ToUTF16(argument), &command_line);
   }
   if (!database.value().empty()) {
     AppendCommandLineArgument(FormatArgumentString("database",
                                                    database.value()),
                               &command_line);
   }
   if (!url.empty()) {
     AppendCommandLineArgument(FormatArgumentString("url",
                                                    base::UTF8ToUTF16(url)),
                               &command_line);
   }
   for (const auto& kv : annotations) {
     AppendCommandLineArgument(
         FormatArgumentString("annotation",
                              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);
   startup_info.dwFlags = STARTF_USESTDHANDLES;
   startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
   startup_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
   startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE);
   PROCESS_INFORMATION process_info;
   BOOL rv = CreateProcess(handler.value().c_str(),
                           &command_line[0],
                           nullptr,
                           nullptr,
                           true,
                           0,
                           nullptr,
                           nullptr,
                           &startup_info,
                           &process_info);
   if (!rv) {
     PLOG(ERROR) << "CreateProcess";
     return false;
   }
 
   rv = CloseHandle(process_info.hThread);
   PLOG_IF(WARNING, !rv) << "CloseHandle thread";
 
   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;
 }
 
 bool CrashpadClient::SetHandlerIPCPipe(const std::wstring& ipc_pipe) {
   DCHECK(ipc_pipe_.empty());
   DCHECK(!ipc_pipe.empty());
 
   ipc_pipe_ = ipc_pipe;
 
   return true;
@@ skipping 102 matching lines @@
       reinterpret_cast<crashpad::WinVMAddress>(&exception_pointers);
 
   bool set_event_result = !!SetEvent(g_signal_non_crash_dump);
   PLOG_IF(ERROR, !set_event_result) << "SetEvent";
 
   DWORD wfso_result = WaitForSingleObject(g_non_crash_dump_done, INFINITE);
   PLOG_IF(ERROR, wfso_result != WAIT_OBJECT_0) << "WaitForSingleObject";
 }
 
 }  // namespace crashpad