linux: verify whether a broker has been successfully forked

Also fix an error in checking that PtraceClient was initialized.

Bug: crashpad:30
Change-Id: I1928340a2a642c2d831f0152bb9faaa12afb07e8
Reviewed-on: https://chromium-review.googlesource.com/978630
Reviewed-by: Mark Mentovai <mark@chromium.org>

handler/linux/crash_report_exception_handler.cc

@@ -63,7 +63,7 @@ bool CrashReportExceptionHandler::HandleExceptionWithBroker(
   Metrics::ExceptionEncountered();
 
   PtraceClient client;
-  if (client.Initialize(broker_sock, client_process_id)) {
+  if (!client.Initialize(broker_sock, client_process_id)) {
     Metrics::ExceptionCaptureResult(
         Metrics::CaptureResult::kBrokeredPtraceFailed);
     return false;

handler/linux/exception_handler_server.cc

@@ -126,8 +126,10 @@ class PtraceStrategyDeciderImpl : public PtraceStrategyDecider {
   Strategy ChooseStrategy(int sock, const ucred& client_credentials) override {
     switch (GetPtraceScope()) {
       case PtraceScope::kClassic:
-        return getuid() == client_credentials.uid ? Strategy::kDirectPtrace
+        if (getuid() == client_credentials.uid) {
-                                                  : Strategy::kForkBroker;
+          return Strategy::kDirectPtrace;
+        }
+        return TryForkingBroker(sock);
 
       case PtraceScope::kRestricted:
         if (!SendMessageToClient(sock,
@@ -143,7 +145,7 @@ class PtraceStrategyDeciderImpl : public PtraceStrategyDecider {
         if (status != 0) {
           errno = status;
           PLOG(ERROR) << "Handler Client SetPtracer";
-          return Strategy::kForkBroker;
+          return TryForkingBroker(sock);
         }
         return Strategy::kDirectPtrace;
 
@@ -163,6 +165,26 @@ class PtraceStrategyDeciderImpl : public PtraceStrategyDecider {
 
     DCHECK(false);
   }
+
+ private:
+  static Strategy TryForkingBroker(int client_sock) {
+    if (!SendMessageToClient(client_sock,
+                             ServerToClientMessage::kTypeForkBroker)) {
+      return Strategy::kError;
+    }
+
+    Errno status;
+    if (!LoggingReadFileExactly(client_sock, &status, sizeof(status))) {
+      return Strategy::kError;
+    }
+
+    if (status != 0) {
+      errno = status;
+      PLOG(ERROR) << "Handler Client ForkBroker";
+      return Strategy::kNoPtrace;
+    }
+    return Strategy::kUseBroker;
+  }
 };
 
 }  // namespace
@@ -427,12 +449,7 @@ bool ExceptionHandlerServer::HandleCrashDumpRequest(
                                  client_info.exception_information_address);
       break;
 
-    case PtraceStrategyDecider::Strategy::kForkBroker:
+    case PtraceStrategyDecider::Strategy::kUseBroker:
-      if (!SendMessageToClient(client_sock,
-                               ServerToClientMessage::kTypeForkBroker)) {
-        return false;
-      }
-
       delegate_->HandleExceptionWithBroker(
           client_process_id,
           client_info.exception_information_address,

handler/linux/exception_handler_server.h

@@ -46,8 +46,8 @@ class PtraceStrategyDecider {
     //! \brief The handler should `ptrace`-attach the client directly.
     kDirectPtrace,
 
-    //! \brief The client should `fork` a PtraceBroker for the handler.
+    //! \brief The client has `fork`ed a PtraceBroker for the handler.
-    kForkBroker,
+    kUseBroker,
   };
 
   //! \brief Chooses an appropriate `ptrace` strategy.

handler/linux/exception_handler_server_test.cc

@@ -289,7 +289,7 @@ TEST_F(ExceptionHandlerServerTest, RequestCrashDumpNoPtrace) {
 }
 
 TEST_F(ExceptionHandlerServerTest, RequestCrashDumpForkBroker) {
-  ExpectCrashDumpUsingStrategy(PtraceStrategyDecider::Strategy::kForkBroker,
+  ExpectCrashDumpUsingStrategy(PtraceStrategyDecider::Strategy::kUseBroker,
                                true);
 }
 

util/linux/exception_handler_client.cc

@@ -112,11 +112,14 @@ int ExceptionHandlerClient::WaitForCrashDumpComplete() {
         Signals::InstallDefaultHandler(SIGCHLD);
 
         pid_t pid = fork();
-        if (pid < 0) {
+        if (pid <= 0) {
-          Errno error = errno;
+          Errno error = pid < 0 ? errno : 0;
           if (!WriteFile(server_sock_, &error, sizeof(error))) {
             return errno;
           }
+        }
+
+        if (pid < 0) {
           continue;
         }