From 05d5610a3f92a2a31a31ad5f1e45e33a3b46a7da Mon Sep 17 00:00:00 2001 From: Gabriele Svelto Date: Thu, 24 Apr 2025 21:47:31 +0000 Subject: [PATCH] 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 --- .../gecko/crashhelper/CrashHelper.java | 24 +++++--- .../org/mozilla/geckoview/GeckoRuntime.java | 4 +- .../crash_helper/crashhelper_android.cpp | 58 +++++++++++++++---- .../src/ipc_poller/unix.rs | 15 ++++- 4 files changed, 77 insertions(+), 24 deletions(-) diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/crashhelper/CrashHelper.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/crashhelper/CrashHelper.java index 3e89675c6a3c..92f2bf61dfa9 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/crashhelper/CrashHelper.java +++ b/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); } diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java index f6aaeeebfaa7..7489a2078508 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java +++ b/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 cls = diff --git a/toolkit/crashreporter/crash_helper/crashhelper_android.cpp b/toolkit/crashreporter/crash_helper/crashhelper_android.cpp index e0e79f60c2c6..b9e66a5c5bc7 100644 --- a/toolkit/crashreporter/crash_helper/crashhelper_android.cpp +++ b/toolkit/crashreporter/crash_helper/crashhelper_android.cpp @@ -7,6 +7,7 @@ #include #include #include +#include // 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"); diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_poller/unix.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_poller/unix.rs index 07758291b073..2abb7f9f39b5 100644 --- a/toolkit/crashreporter/crash_helper_common/src/ipc_poller/unix.rs +++ b/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) { - events.push(IPCEvent::Disconnect(index - 1)); + 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() {