diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 70f333e13..cbb118151 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -265,10 +265,12 @@ static void ParseArgs(int argc, char** argv, pid_t* pseudothread_tid, DebuggerdD } static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo, - std::unique_ptr* regs, ProcessInfo* process_info) { + std::unique_ptr* regs, ProcessInfo* process_info, + bool* recoverable_gwp_asan_crash) { std::aligned_storage::type buf; CrashInfo* crash_info = reinterpret_cast(&buf); ssize_t rc = TEMP_FAILURE_RETRY(read(fd.get(), &buf, sizeof(buf))); + *recoverable_gwp_asan_crash = false; if (rc == -1) { PLOG(FATAL) << "failed to read target ucontext"; } else { @@ -304,6 +306,7 @@ static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo, process_info->scudo_region_info = crash_info->data.d.scudo_region_info; process_info->scudo_ring_buffer = crash_info->data.d.scudo_ring_buffer; process_info->scudo_ring_buffer_size = crash_info->data.d.scudo_ring_buffer_size; + *recoverable_gwp_asan_crash = crash_info->data.d.recoverable_gwp_asan_crash; FALLTHROUGH_INTENDED; case 1: case 2: @@ -468,6 +471,7 @@ int main(int argc, char** argv) { std::map thread_info; siginfo_t siginfo; std::string error; + bool recoverable_gwp_asan_crash = false; { ATRACE_NAME("ptrace"); @@ -519,7 +523,8 @@ int main(int argc, char** argv) { if (thread == g_target_thread) { // Read the thread's registers along with the rest of the crash info out of the pipe. - ReadCrashInfo(input_pipe, &siginfo, &info.registers, &process_info); + ReadCrashInfo(input_pipe, &siginfo, &info.registers, &process_info, + &recoverable_gwp_asan_crash); info.siginfo = &siginfo; info.signo = info.siginfo->si_signo; @@ -646,7 +651,7 @@ int main(int argc, char** argv) { } } - if (fatal_signal) { + if (fatal_signal && !recoverable_gwp_asan_crash) { // Don't try to notify ActivityManager if it just crashed, or we might hang until timeout. if (thread_info[target_process].thread_name != "system_server") { activity_manager_notify(target_process, signo, amfd_data); diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index d2bf0d705..baa5bfb50 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -395,6 +395,7 @@ static int debuggerd_dispatch_pseudothread(void* arg) { ASSERT_SAME_OFFSET(scudo_region_info, scudo_region_info); ASSERT_SAME_OFFSET(scudo_ring_buffer, scudo_ring_buffer); ASSERT_SAME_OFFSET(scudo_ring_buffer_size, scudo_ring_buffer_size); + ASSERT_SAME_OFFSET(recoverable_gwp_asan_crash, recoverable_gwp_asan_crash); #undef ASSERT_SAME_OFFSET iovs[3] = {.iov_base = &thread_info->process_info, @@ -572,14 +573,13 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c // In order to do that, we need to disable GWP-ASan's guard pages. The // following callbacks handle this case. gwp_asan_callbacks_t gwp_asan_callbacks = g_callbacks.get_gwp_asan_callbacks(); - bool gwp_asan_recoverable = false; if (signal_number == SIGSEGV && signal_has_si_addr(info) && gwp_asan_callbacks.debuggerd_needs_gwp_asan_recovery && gwp_asan_callbacks.debuggerd_gwp_asan_pre_crash_report && gwp_asan_callbacks.debuggerd_gwp_asan_post_crash_report && gwp_asan_callbacks.debuggerd_needs_gwp_asan_recovery(info->si_addr)) { gwp_asan_callbacks.debuggerd_gwp_asan_pre_crash_report(info->si_addr); - gwp_asan_recoverable = true; + process_info.recoverable_gwp_asan_crash = true; } // If sival_int is ~0, it means that the fallback handler has been called @@ -593,7 +593,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c // you can only set NO_NEW_PRIVS to 1, and the effect should be at worst a single missing // ANR trace. debuggerd_fallback_handler(info, ucontext, process_info.abort_msg); - if (no_new_privs && gwp_asan_recoverable) { + if (no_new_privs && process_info.recoverable_gwp_asan_crash) { gwp_asan_callbacks.debuggerd_gwp_asan_post_crash_report(info->si_addr); return; } @@ -670,7 +670,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c // If the signal is fatal, don't unlock the mutex to prevent other crashing threads from // starting to dump right before our death. pthread_mutex_unlock(&crash_mutex); - } else if (gwp_asan_recoverable) { + } else if (process_info.recoverable_gwp_asan_crash) { gwp_asan_callbacks.debuggerd_gwp_asan_post_crash_report(info->si_addr); pthread_mutex_unlock(&crash_mutex); } diff --git a/debuggerd/include/debuggerd/handler.h b/debuggerd/include/debuggerd/handler.h index de88be5d9..ebb537236 100644 --- a/debuggerd/include/debuggerd/handler.h +++ b/debuggerd/include/debuggerd/handler.h @@ -35,7 +35,7 @@ struct AllocationMetadata; // When updating this data structure, CrashInfoDataDynamic and the code in // ReadCrashInfo() must also be updated. -struct debugger_process_info { +struct __attribute__((packed)) debugger_process_info { void* abort_msg; void* fdsan_table; const gwp_asan::AllocatorState* gwp_asan_state; @@ -44,6 +44,7 @@ struct debugger_process_info { const char* scudo_region_info; const char* scudo_ring_buffer; size_t scudo_ring_buffer_size; + bool recoverable_gwp_asan_crash; }; // GWP-ASan calbacks to support the recoverable mode. Separate from the diff --git a/debuggerd/protocol.h b/debuggerd/protocol.h index e7cb21896..b60cf5bb6 100644 --- a/debuggerd/protocol.h +++ b/debuggerd/protocol.h @@ -99,6 +99,7 @@ struct __attribute__((__packed__)) CrashInfoDataDynamic : public CrashInfoDataSt uintptr_t scudo_region_info; uintptr_t scudo_ring_buffer; size_t scudo_ring_buffer_size; + bool recoverable_gwp_asan_crash; }; struct __attribute__((__packed__)) CrashInfo {