From 095f29209508520b5958b037cb016d26820666d9 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Wed, 15 May 2024 18:34:29 -0700 Subject: [PATCH] Allow to re-enable MTE a specified time after a permissive fault The timeout has to be determined experimentally. Generally, it must be high enough to at least be the next instruction, and can be otherwise as low as performance reasons allow. This feature is for debugging only. Test: atest PermissiveMteTest Bug: 309604766 Change-Id: I54eff23374ebb239fd75b3b59ae72a7c33654454 --- debuggerd/handler/debuggerd_handler.cpp | 87 ++++++++++++++++++- debuggerd/test_permissive_mte/mte_crash.cpp | 9 ++ .../tests/debuggerd/PermissiveMteTest.java | 28 ++++++ 3 files changed, 122 insertions(+), 2 deletions(-) diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index e26746b607f5..42f0aa0d96da 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -36,10 +36,12 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -115,6 +117,59 @@ static bool is_permissive_mte() { (permissive_env && ParseBool(permissive_env) == ParseBoolResult::kTrue); } +static bool parse_uint_with_error_reporting(const char* s, const char* name, int* v) { + if (android::base::ParseInt(s, v) && *v >= 0) { + return true; + } + async_safe_format_log(ANDROID_LOG_ERROR, "libc", "invalid %s: %s", name, s); + return false; +} + +// We cannot use base::GetIntProperty, because that internally uses +// std::string, which allocates. +static bool property_parse_int(const char* name, int* out) { + const prop_info* pi = __system_property_find(name); + if (!pi) return false; + struct cookie_t { + int* out; + bool empty; + } cookie{out, true}; + __system_property_read_callback( + pi, + [](void* raw_cookie, const char* name, const char* value, uint32_t) { + // Property is set to empty value, ignoring. + if (!*value) return; + cookie_t* cookie = reinterpret_cast(raw_cookie); + if (parse_uint_with_error_reporting(value, name, cookie->out)) cookie->empty = false; + }, + &cookie); + return !cookie.empty; +} + +static int permissive_mte_renable_timer() { + if (char* env = getenv("MTE_PERMISSIVE_REENABLE_TIME_CPUMS")) { + int v; + if (parse_uint_with_error_reporting(env, "MTE_PERMISSIVE_REENABLE_TIME_CPUMS", &v)) return v; + } + + char process_sysprop_name[512]; + async_safe_format_buffer(process_sysprop_name, sizeof(process_sysprop_name), + "persist.sys.mte.permissive_reenable_timer.process.%s", getprogname()); + int v; + if (property_parse_int(process_sysprop_name, &v)) return v; + if (property_parse_int("persist.sys.mte.permissive_reenable_timer.default", &v)) return v; + char process_deviceconf_sysprop_name[512]; + async_safe_format_buffer( + process_deviceconf_sysprop_name, sizeof(process_deviceconf_sysprop_name), + "persist.device_config.memory_safety_native.permissive_reenable_timer.process.%s", + getprogname()); + if (property_parse_int(process_deviceconf_sysprop_name, &v)) return v; + if (property_parse_int( + "persist.device_config.memory_safety_native.permissive_reenable_timer.default", &v)) + return v; + return 0; +} + static inline void futex_wait(volatile void* ftx, int value) { syscall(__NR_futex, ftx, FUTEX_WAIT, value, nullptr, nullptr, 0); } @@ -599,12 +654,40 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c if (tagged_addr_ctrl < 0) { fatal_errno("failed to PR_GET_TAGGED_ADDR_CTRL"); } + int previous = tagged_addr_ctrl & PR_MTE_TCF_MASK; tagged_addr_ctrl = (tagged_addr_ctrl & ~PR_MTE_TCF_MASK) | PR_MTE_TCF_NONE; if (prctl(PR_SET_TAGGED_ADDR_CTRL, tagged_addr_ctrl, 0, 0, 0) < 0) { fatal_errno("failed to PR_SET_TAGGED_ADDR_CTRL"); } - async_safe_format_log(ANDROID_LOG_ERROR, "libc", - "MTE ERROR DETECTED BUT RUNNING IN PERMISSIVE MODE. CONTINUING."); + if (int reenable_timer = permissive_mte_renable_timer()) { + async_safe_format_log(ANDROID_LOG_ERROR, "libc", + "MTE ERROR DETECTED BUT RUNNING IN PERMISSIVE MODE. CONTINUING WITH " + "MTE DISABLED FOR %d MS OF CPU TIME.", + reenable_timer); + timer_t timerid{}; + struct sigevent sev {}; + sev.sigev_signo = BIONIC_ENABLE_MTE; + sev.sigev_notify = SIGEV_THREAD_ID; + sev.sigev_value.sival_int = previous; + sev.sigev_notify_thread_id = __gettid(); + // This MUST be CLOCK_THREAD_CPUTIME_ID. If we used CLOCK_MONOTONIC we could get stuck + // in an endless loop of re-running the same instruction, calling this signal handler, + // and re-enabling MTE before we had a chance to re-run the instruction. + if (timer_create(CLOCK_THREAD_CPUTIME_ID, &sev, &timerid) == -1) { + fatal_errno("timer_create() failed"); + } + struct itimerspec its {}; + its.it_value.tv_sec = reenable_timer / 1000; + its.it_value.tv_nsec = (reenable_timer % 1000) * 1000000; + + if (timer_settime(timerid, 0, &its, nullptr) == -1) { + fatal_errno("timer_settime() failed"); + } + } else { + async_safe_format_log( + ANDROID_LOG_ERROR, "libc", + "MTE ERROR DETECTED BUT RUNNING IN PERMISSIVE MODE. CONTINUING WITH MTE DISABLED."); + } pthread_mutex_unlock(&crash_mutex); } diff --git a/debuggerd/test_permissive_mte/mte_crash.cpp b/debuggerd/test_permissive_mte/mte_crash.cpp index 97ad73fbc381..75b70edb8fbb 100644 --- a/debuggerd/test_permissive_mte/mte_crash.cpp +++ b/debuggerd/test_permissive_mte/mte_crash.cpp @@ -20,5 +20,14 @@ int main(int, char**) { volatile char* f = (char*)malloc(1); printf("%c\n", f[17]); +#ifdef __aarch64__ + if (getenv("MTE_PERMISSIVE_REENABLE_TIME_CPUMS")) { + // Burn some cycles because the MTE_PERMISSIVE_REENABLE_TIME_CPUMS is based on CPU clock. + for (int i = 0; i < 1000000000; ++i) { + asm("isb"); + } + printf("%c\n", f[17]); + } +#endif return 0; } diff --git a/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java b/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java index 0203adcd4ac7..544299dd1a31 100644 --- a/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java +++ b/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java @@ -97,6 +97,34 @@ public void testCrash() throws Exception { } assertThat(numberTombstones).isEqualTo(1); } + + @Test + public void testReenableCrash() throws Exception { + CommandResult result = + getDevice().executeShellV2Command("MTE_PERMISSIVE=1 MTE_PERMISSIVE_REENABLE_TIME_CPUMS=1 " + + "/data/local/tmp/mte_crash testReenableCrash " + + mUUID); + assertThat(result.getExitCode()).isEqualTo(0); + int numberTombstones = 0; + String[] tombstones = getDevice().getChildren("/data/tombstones"); + for (String tombstone : tombstones) { + if (!tombstone.endsWith(".pb")) { + continue; + } + String tombstonePath = "/data/tombstones/" + tombstone; + Tombstone tombstoneProto = parseTombstone(tombstonePath); + if (!tombstoneProto.getCommandLineList().stream().anyMatch(x -> x.contains(mUUID))) { + continue; + } + if (!tombstoneProto.getCommandLineList().stream().anyMatch( + x -> x.contains("testReenableCrash"))) { + continue; + } + numberTombstones++; + } + assertThat(numberTombstones).isEqualTo(2); + } + @Test public void testCrashProperty() throws Exception { String prevValue = getDevice().getProperty("persist.sys.mte.permissive");