Linux Breakpad: Keep reading from the crash upload progress until a complete crash id has been rece…

components/breakpad/app/breakpad_linux.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // For linux_syscall_support.h. This makes it safe to call embedded system
 // calls when in seccomp mode.
 
 #include "components/breakpad/app/breakpad_linux.h"
 
 #include <fcntl.h>
@@ skipping 1002 matching lines @@
     NULL,
   };
   static const char msg[] = "Cannot upload crash dump: cannot exec "
                             "/usr/bin/wget\n";
 #endif
   execve(args[0], const_cast<char**>(args), environ);
   WriteLog(msg, sizeof(msg) - 1);
   sys__exit(1);
 }
 
+bool IsValidCrashReportId(const char* buf, size_t buflen, size_t expected_len) {
+  if (buflen != expected_len)
+    return false;
+#if defined(OS_CHROMEOS)
+  return strcmp(buf, "_sys_cr_finished") == 0;
+#else
+  for (size_t i = 0; i < buflen; ++i) {
+    char c = buf[i];

[vapier, 2014/04/23 22:03:21]

i think you want:

 for (size_t i = 0; i < buflen; ++i)
   if (!isxdigit(buf[i]))
     return false;
 return true;

is there not a libbase func to do this too ?

[Lei Zhang, 2014/04/24 00:37:25]

We can't call into libc in the middle of a crash. Note the numerous my_foo
functions used in this file.

+    if ((c >= '0' && c <= '9') || (c >= 'a' || c <= 'f'))
+      continue;
+    return false;
+  }
+  return true;
+#endif
+}
+
 #if defined(OS_CHROMEOS)
 const char* GetCrashingProcessName(const BreakpadInfo& info,
                                    google_breakpad::PageAllocator* allocator) {
   // Symlink to process binary is at /proc/###/exe.
   char linkpath[kUint64StringSize + sizeof("/proc/") + sizeof("/exe")] =
     "/proc/";
   uint64_t pid_value_len = my_uint64_len(info.pid);
   my_uint64tos(linkpath + sizeof("/proc/") - 1, info.pid, pid_value_len);
   linkpath[sizeof("/proc/") - 1 + pid_value_len] = '\0';
   my_strlcat(linkpath, "/exe", sizeof(linkpath));
@@ skipping 369 matching lines @@
       if (!upload_child) {
         // Upload process.
         IGNORE_RET(sys_close(fds[0]));
         IGNORE_RET(sys_dup2(fds[1], 3));
         ExecUploadProcessOrTerminate(info, temp_file, mime_boundary, exe_buf,
                                      &allocator);
       }
 
       // Helper process.
       if (upload_child > 0) {
+        const size_t kCrashIdLength = 16;
         IGNORE_RET(sys_close(fds[1]));
-        char id_buf[17];  // Crash report IDs are expected to be 16 chars.
-        ssize_t len = -1;
+        char id_buf[kCrashIdLength + 1];
+        size_t bytes_read = 0;
         // Upload should finish in about 10 seconds. Add a few more 500 ms
         // internals to account for process startup time.
         for (size_t wait_count = 0; wait_count < 24; ++wait_count) {
           struct kernel_pollfd poll_fd;
           poll_fd.fd = fds[0];
           poll_fd.events = POLLIN | POLLPRI | POLLERR;
           int ret = sys_poll(&poll_fd, 1, 500);
           if (ret < 0) {
             // Error
             break;
           } else if (ret > 0) {
             // There is data to read.
-            len = HANDLE_EINTR(sys_read(fds[0], id_buf, sizeof(id_buf) - 1));
-            break;
+            ssize_t len = HANDLE_EINTR(
+                sys_read(fds[0],
+                         &id_buf[bytes_read],
+                         sizeof(id_buf) - bytes_read - 1));
+            if (len < 0)
+              break;
+            bytes_read += len;
+            if (bytes_read == kCrashIdLength)
+              break;
           }
-          // ret == 0 -> timed out, continue waiting.
+          // |ret| == 0 -> timed out, continue waiting.
+          // or |bytes_read| < |kCrashIdLength| still, keep reading.
         }
-        if (len > 0) {
+        id_buf[kCrashIdLength] = 0;  // Always NULL terminate the buffer.
+        if (IsValidCrashReportId(id_buf, bytes_read, kCrashIdLength)) {
           // Write crash dump id to stderr.
-          id_buf[len] = 0;
           static const char msg[] = "\nCrash dump id: ";
           WriteLog(msg, sizeof(msg) - 1);
           WriteLog(id_buf, my_strlen(id_buf));
           WriteLog("\n", 1);
 
           // Write crash dump id to crash log as: seconds_since_epoch,crash_id
           struct kernel_timeval tv;
           if (g_crash_log_path && !sys_gettimeofday(&tv, NULL)) {
             uint64_t time = kernel_timeval_to_ms(&tv) / 1000;
             char time_str[kUint64StringSize];
             const unsigned time_len = my_uint64_len(time);
             my_uint64tos(time_str, time, time_len);
 
             int log_fd = sys_open(g_crash_log_path,
                                   O_CREAT | O_WRONLY | O_APPEND,
                                   0600);
             if (log_fd > 0) {
               sys_write(log_fd, time_str, time_len);
               sys_write(log_fd, ",", 1);
               sys_write(log_fd, id_buf, my_strlen(id_buf));
               sys_write(log_fd, "\n", 1);
               IGNORE_RET(sys_close(log_fd));
             }
           }
+        } else {
+          static const char msg[] = "\nFailed to get crash dump id.\n";
+          WriteLog(msg, sizeof(msg) - 1);
         }
+
         if (sys_waitpid(upload_child, NULL, WNOHANG) == 0) {
           // Upload process is still around, kill it.
           sys_kill(upload_child, SIGKILL);
         }
       }
     }
 
     // Helper process.
     IGNORE_RET(sys_unlink(info.filename));
 #if defined(ADDRESS_SANITIZER)
@@ skipping 75 matching lines @@
     }
   }
 }
 #endif  // OS_ANDROID
 
 bool IsCrashReporterEnabled() {
   return g_is_crash_reporter_enabled;
 }
 
 }  // namespace breakpad