Add an observer interface to CrashHandlerHost

This interface allows interested classes, including
AwBrowserTerminator, ChromeStabilityMetricsProvider,
OutOfMemoryReporter, and DataReductionProxyPingbackClientImpl to
distinguish between child process crashes and kills on Android.
Previously, this was done by either passing a file descriptor to the
child process (AwBrowserTerminator) or by observing whether a crash
dump was successfully produced. This eliminates the need for the extra
descriptor for AwBrowserTerminator, as well as providing a more
accurate signal for classes that relied on checking for a crash dump.

Observers can use this interface in combination with observing child
process exits/disconnects. If an observer sees a child process
disconnect without having observed a crash signal for the child, it
can deduce that the child was killed.

Bug: crashpad:30
Change-Id: I6213d31f30d15b30d2a3987fe95229042371938b
Reviewed-on: https://chromium-review.googlesource.com/1026085
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553715}

components/crash/content/app/crashpad_linux.cc

@@ -50,7 +50,7 @@ class SandboxedHandler {
   SandboxedHandler() = default;
   ~SandboxedHandler() = delete;
 
-  int ConnectToHandler(base::ScopedFD* connection) {
+  int ConnectToHandler(int signo, base::ScopedFD* connection) {
     int fds[2];
     if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) {
       return errno;
@@ -58,11 +58,15 @@ class SandboxedHandler {
     base::ScopedFD local_connection(fds[0]);
     base::ScopedFD handlers_socket(fds[1]);
 
+    iovec iov;
+    iov.iov_base = &signo;
+    iov.iov_len = sizeof(signo);
+
     msghdr msg;
     msg.msg_name = nullptr;
     msg.msg_namelen = 0;
-    msg.msg_iov = nullptr;
-    msg.msg_iovlen = 0;
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
 
     char cmsg_buf[CMSG_SPACE(sizeof(int))];
     msg.msg_control = cmsg_buf;
@@ -86,7 +90,7 @@ class SandboxedHandler {
     SandboxedHandler* state = Get();
 
     base::ScopedFD connection;
-    if (state->ConnectToHandler(&connection) == 0) {
+    if (state->ConnectToHandler(signo, &connection) == 0) {
       ExceptionInformation exception_information;
       exception_information.siginfo_address =
           FromPointerCast<decltype(exception_information.siginfo_address)>(

components/crash/content/browser/crash_handler_host_linux.cc

@@ -35,7 +35,6 @@
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-#include "components/crash/content/app/breakpad_linux_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/breakpad/breakpad/src/client/linux/handler/exception_handler.h"
 #include "third_party/breakpad/breakpad/src/client/linux/minidump_writer/linux_dumper.h"
@@ -499,8 +498,30 @@ bool CrashHandlerHostLinux::IsShuttingDown() const {
 
 namespace crashpad {
 
+void CrashHandlerHost::AddObserver(Observer* observer) {
+  base::AutoLock lock(observers_lock_);
+  bool inserted = observers_.insert(observer).second;
+  DCHECK(inserted);
+}
+
+void CrashHandlerHost::RemoveObserver(Observer* observer) {
+  base::AutoLock lock(observers_lock_);
+  size_t removed = observers_.erase(observer);
+  DCHECK(removed);
+}
+
+// static
+CrashHandlerHost* CrashHandlerHost::Get() {
+  static CrashHandlerHost* instance = new CrashHandlerHost();
+  return instance;
+}
+
+CrashHandlerHost::~CrashHandlerHost() = default;
+
 CrashHandlerHost::CrashHandlerHost()
-    : file_descriptor_watcher_(FROM_HERE),
+    : observers_lock_(),
+      observers_(),
+      file_descriptor_watcher_(FROM_HERE),
       process_socket_(),
       browser_socket_() {
   int fds[2];
@@ -514,13 +535,15 @@ CrashHandlerHost::CrashHandlerHost()
   process_socket_.reset(fds[0]);
   browser_socket_.reset(fds[1]);
 
+  static const int on = 1;
+  CHECK_EQ(0, setsockopt(browser_socket_.get(), SOL_SOCKET, SO_PASSCRED, &on,
+                         sizeof(on)));
+
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&CrashHandlerHost::Init, base::Unretained(this)));
 }
 
-CrashHandlerHost::~CrashHandlerHost() = default;
-
 void CrashHandlerHost::Init() {
   base::MessageLoopCurrentForIO ml = base::MessageLoopCurrentForIO::Get();
   CHECK(ml->WatchFileDescriptor(browser_socket_.get(), /* persistent= */ true,
@@ -531,13 +554,18 @@ void CrashHandlerHost::Init() {
 
 bool CrashHandlerHost::ReceiveClientMessage(int client_fd,
                                             base::ScopedFD* handler_fd) {
+  int signo;
+  iovec iov;
+  iov.iov_base = &signo;
+  iov.iov_len = sizeof(signo);
+
   msghdr msg;
   msg.msg_name = nullptr;
   msg.msg_namelen = 0;
-  msg.msg_iov = nullptr;
-  msg.msg_iovlen = 0;
+  msg.msg_iov = &iov;
+  msg.msg_iovlen = 1;
 
-  char cmsg_buf[CMSG_SPACE(sizeof(int))];
+  char cmsg_buf[CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(ucred))];
   msg.msg_control = cmsg_buf;
   msg.msg_controllen = sizeof(cmsg_buf);
   msg.msg_flags = 0;
@@ -548,19 +576,45 @@ bool CrashHandlerHost::ReceiveClientMessage(int client_fd,
     return false;
   }
 
-  cmsghdr* cmsg = CMSG_FIRSTHDR(&msg);
-  if (!cmsg || cmsg->cmsg_level != SOL_SOCKET ||
-      cmsg->cmsg_type != SCM_RIGHTS || cmsg->cmsg_len < CMSG_LEN(sizeof(int))) {
+  base::ScopedFD child_fd;
+  pid_t child_pid = -1;
+  for (cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); cmsg;
+       cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+    if (cmsg->cmsg_level != SOL_SOCKET) {
+      continue;
+    }
+
+    if (cmsg->cmsg_type == SCM_RIGHTS) {
+      child_fd.reset(*reinterpret_cast<int*>(CMSG_DATA(cmsg)));
+    } else if (cmsg->cmsg_type == SCM_CREDENTIALS) {
+      child_pid = reinterpret_cast<ucred*>(CMSG_DATA(cmsg))->pid;
+    }
+  }
+
+  if (!child_fd.is_valid()) {
     LOG(ERROR) << "Death signal missing descriptor";
     return false;
   }
-  DCHECK(!CMSG_NXTHDR(&msg, cmsg));
 
-  handler_fd->reset(*reinterpret_cast<int*>(CMSG_DATA(cmsg)));
-  DCHECK(handler_fd->is_valid());
+  if (child_pid < 0) {
+    LOG(ERROR) << "Death signal missing pid";
+    return false;
+  }
+
+  NotifyCrashSignalObservers(child_pid, signo);
+
+  handler_fd->reset(child_fd.release());
   return true;
 }
 
+void CrashHandlerHost::NotifyCrashSignalObservers(base::ProcessId pid,
+                                                  int signo) {
+  base::AutoLock lock(observers_lock_);
+  for (Observer* observer : observers_) {
+    observer->ChildReceivedCrashSignal(pid, signo);
+  }
+}
+
 void CrashHandlerHost::OnFileCanWriteWithoutBlocking(int fd) {
   NOTREACHED();
 }

components/crash/content/browser/crash_handler_host_linux.h

@@ -8,6 +8,7 @@
 #include <sys/types.h>
 
 #include <memory>
+#include <set>
 #include <string>
 
 #include "base/files/file_path.h"
@@ -16,6 +17,8 @@
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_current.h"
 #include "base/message_loop/message_pump_for_io.h"
+#include "base/process/process_handle.h"
+#include "base/synchronization/lock.h"
 #include "build/build_config.h"
 #include "components/crash/content/app/breakpad_linux_impl.h"
 
@@ -117,16 +120,37 @@ namespace crashpad {
 class CrashHandlerHost : public base::MessagePumpForIO::FdWatcher,
                          public base::MessageLoopCurrent::DestructionObserver {
  public:
-  CrashHandlerHost();
-  ~CrashHandlerHost() override;
+  // An interface for observers to be notified when a child process is crashing.
+  class Observer {
+   public:
+    // Called when a child process is crashing. pid is the child's process ID in
+    // the CrashHandlerHost's PID namespace. signo is the signal the child
+    // received. Observers are notified synchronously while the child process
+    // is blocked in its signal handler. Observers may not call AddObserver()
+    // or RemoveObserver() in this method.
+    virtual void ChildReceivedCrashSignal(base::ProcessId pid, int signo) = 0;
+  };
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Return a pointer to the global CrashHandlerHost instance, which is created
+  // by the first call to this method.
+  static CrashHandlerHost* Get();
 
   // Get the file descriptor which processes should be given in order to signal
   // crashes to the browser.
   int GetDeathSignalSocket() const { return process_socket_.get(); }
 
+ protected:
+  ~CrashHandlerHost() override;
+
  private:
+  CrashHandlerHost();
+
   void Init();
   bool ReceiveClientMessage(int client_fd, base::ScopedFD* handler_fd);
+  void NotifyCrashSignalObservers(base::ProcessId pid, int signo);
 
   // MessagePumbLibevent::Watcher impl:
   void OnFileCanWriteWithoutBlocking(int fd) override;
@@ -135,6 +159,8 @@ class CrashHandlerHost : public base::MessagePumpForIO::FdWatcher,
   // MessageLoopCurrent::DestructionObserver impl:
   void WillDestroyCurrentMessageLoop() override;
 
+  base::Lock observers_lock_;
+  std::set<Observer*> observers_;
   base::MessagePumpForIO::FdWatchController file_descriptor_watcher_;
   base::ScopedFD process_socket_;
   base::ScopedFD browser_socket_;