Bug 1962204 - Prepare the crash helper sockets before sending them to the Android service process r=afranchuk,geckoview-reviewers,ohall

The additional native methods are contained by the libcrashhelper shared
library instead of within libxul because they're needed before we load
libxul.

Differential Revision: https://phabricator.services.mozilla.com/D246521

mobile/android/geckoview/src/main/java/org/mozilla/gecko/crashhelper/CrashHelper.java

@@ -6,6 +6,7 @@ package org.mozilla.gecko.crashhelper;
 
 import android.app.Service;
 import android.content.ComponentName;
+import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
 import android.os.Binder;
@@ -129,7 +130,13 @@ public final class CrashHelper extends Service {
         throws IOException {
       mBreakpadClient = ParcelFileDescriptor.dup(breakpadClientFd);
       mBreakpadServer = ParcelFileDescriptor.dup(breakpadServerFd);
+      if (!CrashHelper.set_breakpad_opts(mBreakpadServer.getFd())) {
+        throw new IOException("Could not set the proper options on the Breakpad socket");
+      }
       mListener = ParcelFileDescriptor.dup(listenerFd);
+      if (!CrashHelper.bind_and_listen(mListener.getFd())) {
+        throw new IOException("Could not listen on incoming connections");
+      }
       mClient = ParcelFileDescriptor.dup(clientFd);
       mServer = ParcelFileDescriptor.dup(serverFd);
     }
@@ -141,13 +148,12 @@ public final class CrashHelper extends Service {
   // google_breakpad::CrashGenerationServer::CreateReportChannel(), so we can
   // use them Breakpad's crash generation server & clients. The rest are
   // specific to the crash helper process.
-  public static Pipes createCrashHelperPipes() {
-    // Ideally we'd set all the required options for the server socket, by using
-    // Os.setsockoptInt() to enable the SO_PASSCRED socket option and then a
-    // couple of calls to Os.fcntlInt() to read the socket file flags and then
-    // set it in non-blocking mode by setting O_NONBLOCK. Unfortunately both
-    // calls require Android API version more recent than we support, so this
-    // job is delegated to crashhelper_android.cpp after receiving the socket.
+  public static Pipes createCrashHelperPipes(final Context context) {
+    // We can't set the required socket options for the Breakpad server socket
+    // or our own listener from here, so we delegate those parts to native
+    // functions in crashhelper_android.cpp.
+    GeckoLoader.doLoadLibrary(context, "crashhelper");
+
     try {
       final FileDescriptor breakpad_client_fd = new FileDescriptor();
       final FileDescriptor breakpad_server_fd = new FileDescriptor();
@@ -188,4 +194,8 @@ public final class CrashHelper extends Service {
 
   protected static native void crash_generator(
       int clientPid, int breakpadFd, String minidumpPath, int listenFd, int serverFd);
+
+  protected static native boolean set_breakpad_opts(int breakpadFd);
+
+  protected static native boolean bind_and_listen(int listenFd);
 }

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java

@@ -399,14 +399,14 @@ public final class GeckoRuntime implements Parcelable {
   }
 
   private int[] startCrashHelper() {
-    final CrashHelper.Pipes pipes = CrashHelper.createCrashHelperPipes();
+    final Context context = GeckoAppShell.getApplicationContext();
+    final CrashHelper.Pipes pipes = CrashHelper.createCrashHelperPipes(context);
 
     if (pipes == null) {
       Log.e(LOGTAG, "Could not create the crash reporter IPC pipes");
       return new int[] {-1, -1};
     }
 
-    final Context context = GeckoAppShell.getApplicationContext();
     try {
       @SuppressWarnings("unchecked")
       final Class<? extends Service> cls =

toolkit/crashreporter/crash_helper/crashhelper_android.cpp

@@ -7,6 +7,7 @@
 #include <android/log.h>
 #include <fcntl.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 
 // For DirectAuxvDumpInfo
 #include "mozilla/toolkit/crashreporter/rust_minidump_writer_linux_ffi_generated.h"
@@ -14,21 +15,54 @@
 
 #define CRASH_HELPER_LOGTAG "GeckoCrashHelper"
 
+extern "C" JNIEXPORT jboolean JNICALL
+Java_org_mozilla_gecko_crashhelper_CrashHelper_set_1breakpad_1opts(
+    JNIEnv* jenv, jclass, jint breakpad_fd) {
+  // Enable passing credentials on the Breakpad server socket. We'd love to do
+  // it inside CrashHelper.java but the Java methods require an Android API
+  // version that's too recent for us.
+  const int val = 1;
+  int res = setsockopt(breakpad_fd, SOL_SOCKET, SO_PASSCRED, &val, sizeof(val));
+  if (res < 0) {
+    return false;
+  }
+
+  return true;
+}
+
+extern "C" JNIEXPORT jboolean JNICALL
+Java_org_mozilla_gecko_crashhelper_CrashHelper_bind_1and_1listen(
+    JNIEnv* jenv, jclass, jint listen_fd) {
+  struct sockaddr_un addr = {
+      .sun_family = AF_UNIX,
+      .sun_path = {},
+  };
+
+  // The address' path deliberately starts with a null byte to inform the
+  // kernel that this is an abstract address and not an actual file path.
+  snprintf(addr.sun_path + 1, sizeof(addr.sun_path) - 2,
+           "gecko-crash-helper-pipe.%d", getpid());
+
+  int res = bind(listen_fd, (const struct sockaddr*)&addr, sizeof(addr));
+  if (res < 0) {
+    return false;
+  }
+
+  res = listen(listen_fd, 1);
+  if (res < 0) {
+    return false;
+  }
+
+  return true;
+}
+
 extern "C" JNIEXPORT void JNICALL
 Java_org_mozilla_gecko_crashhelper_CrashHelper_crash_1generator(
     JNIEnv* jenv, jclass, jint client_pid, jint breakpad_fd,
     jstring minidump_path, jint listen_fd, jint server_fd) {
-  // Enable passing credentials on the server socket and set it in non-blocking
-  // mode. We'd love to do it inside CrashHelper.java but the Java methods
-  // require an Android API version that's too recent for us.
-  const int val = 1;
-  int res = setsockopt(breakpad_fd, SOL_SOCKET, SO_PASSCRED, &val, sizeof(val));
-  if (res < 0) {
-    __android_log_print(ANDROID_LOG_FATAL, CRASH_HELPER_LOGTAG,
-                        "Unable to set the Breakpad pipe socket options");
-    return;
-  }
-
+  // The breakpad server socket needs to be put in non-blocking mode, we do it
+  // here as the Rust code that picks it up won't touch it anymore and just
+  // pass it along to Breakpad.
   int flags = fcntl(breakpad_fd, F_GETFL);
   if (flags == -1) {
     __android_log_print(ANDROID_LOG_FATAL, CRASH_HELPER_LOGTAG,
@@ -36,7 +70,7 @@ Java_org_mozilla_gecko_crashhelper_CrashHelper_crash_1generator(
     return;
   }
 
-  res = fcntl(breakpad_fd, F_SETFL, flags | O_NONBLOCK);
+  int res = fcntl(breakpad_fd, F_SETFL, flags | O_NONBLOCK);
   if (res == -1) {
     __android_log_print(ANDROID_LOG_FATAL, CRASH_HELPER_LOGTAG,
                         "Unable to set the Breakpad pipe in non-blocking mode");

toolkit/crashreporter/crash_helper_common/src/ipc_poller/unix.rs

@@ -2,7 +2,10 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use nix::poll::{poll, PollFd, PollFlags, PollTimeout};
+use nix::{
+  errno::Errno,
+  poll::{poll, PollFd, PollFlags, PollTimeout},
+};
 
 use crate::{errors::IPCError, ignore_eintr, IPCConnector, IPCEvent, IPCListener};
 
@@ -48,8 +51,14 @@ pub fn wait_for_events(
             }
         }
 
-        if revents.contains(PollFlags::POLLHUP) && (index > 0) {
+        if revents.contains(PollFlags::POLLHUP) {
+            if index > 0 {
                 events.push(IPCEvent::Disconnect(index - 1));
+            } else {
+                // This should never happen, unless the listener socket was not
+                // set up properly or a failure happened during setup.
+                return Err(IPCError::System(Errno::EFAULT));
+            }
         }
 
         if !revents.is_empty() {