commit d705c2dbcde2561b4aead40d6b977772a760e92d
author Florian Mayer <fmayer@google.com> Mon Sep 12 18:19:47 2022 -0700
committer Florian Mayer <fmayer@google.com> Tue Sep 13 15:35:07 2022 -0700
tree 8a47a76bb21cd66f46f11df06c60d492e337c1ab
parent 61c82abe24ae265e41b1176077696814dceef137

[MTE] only upgrade to SYNC mode for MTE crashes

Bug: 244471804
Test: atest mte_ugprade_test on emulator
Change-Id: Ie974cf2dec96267012f1b01b9a40dad86551b1be

debuggerd/handler/debuggerd_handler.cpp

diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
index 92e7675..fa556bb 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -623,6 +623,18 @@
     async_safe_format_log(ANDROID_LOG_ERROR, "libc",
                           "MTE ERROR DETECTED BUT RUNNING IN PERMISSIVE MODE. CONTINUING.");
     pthread_mutex_unlock(&crash_mutex);
+  } else if (info->si_signo == SIGSEGV && info->si_code == SEGV_MTEAERR && getppid() == 1) {
+    // Back channel to init (see system/core/init/service.cpp) to signal that
+    // this process crashed due to an ASYNC MTE fault and should be considered
+    // for upgrade to SYNC mode. We are re-using the ART profiler signal, which
+    // is always handled (ignored in native processes, handled for generating a
+    // dump in ART processes), so a process will never crash from this signal
+    // except from here.
+    // The kernel is not particularly receptive to adding this information:
+    // https://lore.kernel.org/all/20220909180617.374238-1-fmayer@google.com/, so we work around
+    // like this.
+    info->si_signo = BIONIC_SIGNAL_ART_PROFILER;
+    resend_signal(info);
   }
 #endif
   else {

init/Android.bp

diff --git a/init/Android.bp b/init/Android.bp
index dfc90da..20d622d 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -200,6 +200,7 @@
         "libutils",
         "libziparchive",
     ],
+    header_libs: ["bionic_libc_platform_headers"],
     bootstrap: true,
     visibility: [":__subpackages__"],
 }
@@ -529,6 +530,7 @@
         "libcap",
     ],
     export_include_dirs: ["test_utils/include"], // for tests
+    header_libs: ["bionic_libc_platform_headers"],
 }
 
 // Host Verifier

init/service.cpp

diff --git a/init/service.cpp b/init/service.cpp
index 4cf409c..8334732 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -42,6 +42,10 @@
 #include "service_list.h"
 #include "util.h"
 
+#if defined(__BIONIC__)
+#include <bionic/reserved_signals.h>
+#endif
+
 #ifdef INIT_FULL_SOURCES
 #include <ApexProperties.sysprop.h>
 #include <android/api-level.h>
@@ -323,12 +327,17 @@
             mount_namespace_.has_value() && *mount_namespace_ == NS_DEFAULT;
     const bool is_process_updatable = use_default_mount_ns && is_apex_updatable;
 
-#ifdef SEGV_MTEAERR
+#if defined(__BIONIC__) && defined(SEGV_MTEAERR)
     // As a precaution, we only upgrade a service once per reboot, to limit
     // the potential impact.
-    // TODO(b/244471804): Once we have a kernel API to get sicode, compare it to MTEAERR here.
-    bool should_upgrade_mte = siginfo.si_code != CLD_EXITED && siginfo.si_status == SIGSEGV &&
-                              !upgraded_mte_;
+    //
+    // BIONIC_SIGNAL_ART_PROFILER is a magic value used by deuggerd to signal
+    // that the process crashed with SIGSEGV and SEGV_MTEAERR. This signal will
+    // never be seen otherwise in a crash, because it always gets handled by the
+    // profiling signal handlers in bionic. See also
+    // debuggerd/handler/debuggerd_handler.cpp.
+    bool should_upgrade_mte = siginfo.si_code != CLD_EXITED &&
+                              siginfo.si_status == BIONIC_SIGNAL_ART_PROFILER && !upgraded_mte_;
 
     if (should_upgrade_mte) {
         LOG(INFO) << "Upgrading service " << name_ << " to sync MTE";