Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d705c2db authored by Florian Mayer's avatar Florian Mayer
Browse files

[MTE] only upgrade to SYNC mode for MTE crashes 

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

debuggerd/handler/debuggerd_handler.cpp

+12 −0
Original line number Diff line number Diff line
@@ -623,6 +623,18 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c 
    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

+2 −0
Original line number Diff line number Diff line
@@ -200,6 +200,7 @@ libinit_cc_defaults { 
        "libutils",

        "libziparchive",

    ],

    header_libs: ["bionic_libc_platform_headers"],

    bootstrap: true,

    visibility: [":__subpackages__"],

}
@@ -529,6 +530,7 @@ cc_library_static { 
        "libcap",

    ],

    export_include_dirs: ["test_utils/include"], // for tests

    header_libs: ["bionic_libc_platform_headers"],

}



// Host Verifier

init/service.cpp

+13 −4
Original line number Diff line number Diff line
@@ -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 @@ void Service::Reap(const siginfo_t& siginfo) { 
            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";