From 5fa6663458c8acd36b5bbcb861589809055dfef0 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Wed, 7 Feb 2024 16:42:23 -0800 Subject: [PATCH] Read data set by android_add_crash_detail into tombstone. Bug: 155462331 Bug: 309446525 Change-Id: I6d01aafca48e0e5e8cbd5ae87add6aec0c429503 --- debuggerd/crash_dump.cpp | 1 + debuggerd/debuggerd_test.cpp | 181 ++++++++++++++++++ debuggerd/handler/debuggerd_handler.cpp | 1 + debuggerd/include/debuggerd/handler.h | 3 + .../libdebuggerd/include/libdebuggerd/types.h | 1 + .../test/tombstone_proto_to_text_test.cpp | 16 ++ debuggerd/libdebuggerd/tombstone_proto.cpp | 44 ++++- .../libdebuggerd/tombstone_proto_to_text.cpp | 29 +++ debuggerd/proto/tombstone.proto | 8 +- debuggerd/protocol.h | 1 + 10 files changed, 283 insertions(+), 2 deletions(-) diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 0899111b2..a23a26925 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -322,6 +322,7 @@ static void ReadCrashInfo(unique_fd& fd, siginfo_t* siginfo, 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; + process_info->crash_detail_page = crash_info->data.d.crash_detail_page; FALLTHROUGH_INTENDED; case 1: case 2: diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index c0522aa9d..a308ffbc9 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -939,6 +939,187 @@ TEST_F(CrasherTest, abort_message) { ASSERT_MATCH(result, R"(Abort message: 'x{4045}')"); } +static char g_crash_detail_value_changes[] = "crash_detail_value"; +static char g_crash_detail_value[] = "crash_detail_value"; +static char g_crash_detail_value2[] = "crash_detail_value2"; + +inline crash_detail_t* _Nullable android_register_crash_detail_strs(const char* _Nonnull name, + const char* _Nonnull data) { + return android_register_crash_detail(name, strlen(name), data, strlen(data)); +} + +TEST_F(CrasherTest, crash_detail_single) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + android_register_crash_detail_strs("CRASH_DETAIL_NAME", g_crash_detail_value); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME: 'crash_detail_value')"); +} + +TEST_F(CrasherTest, crash_detail_single_byte_name) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + android_register_crash_detail_strs("CRASH_DETAIL_NAME\1", g_crash_detail_value); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME\\1: 'crash_detail_value')"); +} + + +TEST_F(CrasherTest, crash_detail_single_bytes) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + android_register_crash_detail("CRASH_DETAIL_NAME", strlen("CRASH_DETAIL_NAME"), "\1", + sizeof("\1")); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME: '\\1\\0')"); +} + +TEST_F(CrasherTest, crash_detail_mixed) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + const char data[] = "helloworld\1\255\3"; + android_register_crash_detail_strs("CRASH_DETAIL_NAME", data); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME: 'helloworld\\1\\255\\3')"); +} + +TEST_F(CrasherTest, crash_detail_many) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + for (int i = 0; i < 1000; ++i) { + std::string name = "CRASH_DETAIL_NAME" + std::to_string(i); + std::string value = "CRASH_DETAIL_VALUE" + std::to_string(i); + auto* h = android_register_crash_detail_strs(name.data(), value.data()); + android_unregister_crash_detail(h); + } + + android_register_crash_detail_strs("FINAL_NAME", "FINAL_VALUE"); + android_register_crash_detail_strs("FINAL_NAME2", "FINAL_VALUE2"); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_NOT_MATCH(result, "CRASH_DETAIL_NAME"); + ASSERT_NOT_MATCH(result, "CRASH_DETAIL_VALUE"); + ASSERT_MATCH(result, R"(FINAL_NAME: 'FINAL_VALUE')"); + ASSERT_MATCH(result, R"(FINAL_NAME2: 'FINAL_VALUE2')"); +} + +TEST_F(CrasherTest, crash_detail_single_changes) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + android_register_crash_detail_strs("CRASH_DETAIL_NAME", g_crash_detail_value_changes); + g_crash_detail_value_changes[0] = 'C'; + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME: 'Crash_detail_value')"); +} + +TEST_F(CrasherTest, crash_detail_multiple) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + android_register_crash_detail_strs("CRASH_DETAIL_NAME", g_crash_detail_value); + android_register_crash_detail_strs("CRASH_DETAIL_NAME2", g_crash_detail_value2); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME: 'crash_detail_value')"); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME2: 'crash_detail_value2')"); +} + +TEST_F(CrasherTest, crash_detail_remove) { + int intercept_result; + unique_fd output_fd; + StartProcess([]() { + auto* detail1 = android_register_crash_detail_strs("CRASH_DETAIL_NAME", g_crash_detail_value); + android_unregister_crash_detail(detail1); + android_register_crash_detail_strs("CRASH_DETAIL_NAME2", g_crash_detail_value2); + abort(); + }); + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_NOT_MATCH(result, R"(CRASH_DETAIL_NAME: 'crash_detail_value')"); + ASSERT_MATCH(result, R"(CRASH_DETAIL_NAME2: 'crash_detail_value2')"); +} + TEST_F(CrasherTest, abort_message_newline_trimmed) { int intercept_result; unique_fd output_fd; diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index ea07ce235..141723b03 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -397,6 +397,7 @@ static int debuggerd_dispatch_pseudothread(void* arg) { ASSERT_SAME_OFFSET(scudo_ring_buffer_size, scudo_ring_buffer_size); ASSERT_SAME_OFFSET(scudo_stack_depot_size, scudo_stack_depot_size); ASSERT_SAME_OFFSET(recoverable_gwp_asan_crash, recoverable_gwp_asan_crash); + ASSERT_SAME_OFFSET(crash_detail_page, crash_detail_page); #undef ASSERT_SAME_OFFSET iovs[3] = {.iov_base = &thread_info->process_info, diff --git a/debuggerd/include/debuggerd/handler.h b/debuggerd/include/debuggerd/handler.h index de12fc622..c18cf6e7a 100644 --- a/debuggerd/include/debuggerd/handler.h +++ b/debuggerd/include/debuggerd/handler.h @@ -33,6 +33,8 @@ struct AllocatorState; struct AllocationMetadata; }; // namespace gwp_asan +struct crash_detail_page_t; + // When updating this data structure, CrashInfoDataDynamic and the code in // ReadCrashInfo() must also be updated. struct __attribute__((packed)) debugger_process_info { @@ -46,6 +48,7 @@ struct __attribute__((packed)) debugger_process_info { size_t scudo_ring_buffer_size; size_t scudo_stack_depot_size; bool recoverable_gwp_asan_crash; + struct crash_detail_page_t* crash_detail_page; }; // GWP-ASan calbacks to support the recoverable mode. Separate from the diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/types.h b/debuggerd/libdebuggerd/include/libdebuggerd/types.h index 075b12cb0..4d69658e2 100644 --- a/debuggerd/libdebuggerd/include/libdebuggerd/types.h +++ b/debuggerd/libdebuggerd/include/libdebuggerd/types.h @@ -56,4 +56,5 @@ struct ProcessInfo { bool has_fault_address = false; uintptr_t untagged_fault_address = 0; uintptr_t maybe_tagged_fault_address = 0; + uintptr_t crash_detail_page = 0; }; diff --git a/debuggerd/libdebuggerd/test/tombstone_proto_to_text_test.cpp b/debuggerd/libdebuggerd/test/tombstone_proto_to_text_test.cpp index ac92ac064..a4c08a450 100644 --- a/debuggerd/libdebuggerd/test/tombstone_proto_to_text_test.cpp +++ b/debuggerd/libdebuggerd/test/tombstone_proto_to_text_test.cpp @@ -118,3 +118,19 @@ TEST_F(TombstoneProtoToTextTest, pac_enabled_keys) { "LOG pac_enabled_keys: 0000000000001009 \\(PR_PAC_APIAKEY, PR_PAC_APDBKEY, unknown " "0x1000\\)\\n"); } + +TEST_F(TombstoneProtoToTextTest, crash_detail_string) { + auto* crash_detail = tombstone_->add_crash_details(); + crash_detail->set_name("CRASH_DETAIL_NAME"); + crash_detail->set_data("crash_detail_value"); + ProtoToString(); + EXPECT_MATCH(text_, "(CRASH_DETAIL_NAME: 'crash_detail_value')"); +} + +TEST_F(TombstoneProtoToTextTest, crash_detail_bytes) { + auto* crash_detail = tombstone_->add_crash_details(); + crash_detail->set_name("CRASH_DETAIL_NAME"); + crash_detail->set_data("helloworld\1\255\3"); + ProtoToString(); + EXPECT_MATCH(text_, R"(CRASH_DETAIL_NAME: 'helloworld\\1\\255\\3')"); +} diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp index 744bfabf5..d014fa38b 100644 --- a/debuggerd/libdebuggerd/tombstone_proto.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto.cpp @@ -48,8 +48,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -251,6 +253,46 @@ static void dump_probable_cause(Tombstone* tombstone, unwindstack::AndroidUnwind } } +static void dump_crash_details(Tombstone* tombstone, + std::shared_ptr& process_memory, + const ProcessInfo& process_info) { + uintptr_t address = process_info.crash_detail_page; + while (address) { + struct crash_detail_page_t page; + if (!process_memory->ReadFully(address, &page, sizeof(page))) { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read crash detail page: %m"); + break; + } + if (page.used > kNumCrashDetails) { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "crash detail: page corrupted"); + break; + } + for (size_t i = 0; i < page.used; ++i) { + const crash_detail_t& crash_detail = page.crash_details[i]; + if (!crash_detail.data) { + continue; + } + std::string name(crash_detail.name_size, '\0'); + if (!process_memory->ReadFully(reinterpret_cast(crash_detail.name), name.data(), + crash_detail.name_size)) { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "crash detail: failed to read name: %m"); + continue; + } + std::string data(crash_detail.data_size, '\0'); + if (!process_memory->ReadFully(reinterpret_cast(crash_detail.data), data.data(), + crash_detail.data_size)) { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, + "crash detail: failed to read data for %s: %m", name.c_str()); + continue; + } + auto* proto_detail = tombstone->add_crash_details(); + proto_detail->set_name(name); + proto_detail->set_data(data); + } + address = reinterpret_cast(page.prev); + } +} + static void dump_abort_message(Tombstone* tombstone, std::shared_ptr& process_memory, const ProcessInfo& process_info) { @@ -698,7 +740,7 @@ void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::AndroidUnwinder* *result.mutable_signal_info() = sig; dump_abort_message(&result, unwinder->GetProcessMemory(), process_info); - + dump_crash_details(&result, unwinder->GetProcessMemory(), process_info); // Dump the main thread, but save the memory around the registers. dump_thread(&result, unwinder, main_thread, /* memory_dump */ true); diff --git a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp index ad9132087..cefa2d62e 100644 --- a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp @@ -18,7 +18,9 @@ #include +#include #include +#include #include #include #include @@ -425,6 +427,27 @@ static void print_memory_maps(CallbackType callback, const Tombstone& tombstone) } } +static std::string oct_encode(const std::string& data) { + std::string oct_encoded; + oct_encoded.reserve(data.size()); + + // N.B. the unsigned here is very important, otherwise e.g. \255 would render as + // \-123 (and overflow our buffer). + for (unsigned char c : data) { + if (isprint(c)) { + oct_encoded += c; + } else { + std::string oct_digits("\\\0\0\0", 4); + // char is encodable in 3 oct digits + static_assert(std::numeric_limits::max() <= 8 * 8 * 8); + auto [ptr, ec] = std::to_chars(oct_digits.data() + 1, oct_digits.data() + 4, c, 8); + oct_digits.resize(ptr - oct_digits.data()); + oct_encoded += oct_digits; + } + } + return oct_encoded; +} + static void print_main_thread(CallbackType callback, const Tombstone& tombstone, const Thread& thread) { print_thread_header(callback, tombstone, thread, true); @@ -468,6 +491,12 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, CBL("Abort message: '%s'", tombstone.abort_message().c_str()); } + for (const auto& crash_detail : tombstone.crash_details()) { + std::string oct_encoded_name = oct_encode(crash_detail.name()); + std::string oct_encoded_data = oct_encode(crash_detail.data()); + CBL("Extra crash detail: %s: '%s'", oct_encoded_name.c_str(), oct_encoded_data.c_str()); + } + print_thread_registers(callback, tombstone, thread, true); if (is_async_mte_crash) { CBL("Note: This crash is a delayed async MTE crash. Memory corruption has occurred"); diff --git a/debuggerd/proto/tombstone.proto b/debuggerd/proto/tombstone.proto index 49865a2bb..214cbfb46 100644 --- a/debuggerd/proto/tombstone.proto +++ b/debuggerd/proto/tombstone.proto @@ -15,6 +15,11 @@ option java_outer_classname = "TombstoneProtos"; // NOTE TO OEMS: // If you add custom fields to this proto, do not use numbers in the reserved range. +message CrashDetail { + bytes name = 1; + bytes data = 2; +} + message Tombstone { Architecture arch = 1; string build_fingerprint = 2; @@ -33,6 +38,7 @@ message Tombstone { Signal signal_info = 10; string abort_message = 14; + repeated CrashDetail crash_details = 21; repeated Cause causes = 15; map threads = 16; @@ -40,7 +46,7 @@ message Tombstone { repeated LogBuffer log_buffers = 18; repeated FD open_fds = 19; - reserved 21 to 999; + reserved 22 to 999; } enum Architecture { diff --git a/debuggerd/protocol.h b/debuggerd/protocol.h index 793428ae8..d3fc15edc 100644 --- a/debuggerd/protocol.h +++ b/debuggerd/protocol.h @@ -101,6 +101,7 @@ struct __attribute__((__packed__)) CrashInfoDataDynamic : public CrashInfoDataSt size_t scudo_ring_buffer_size; size_t scudo_stack_depot_size; bool recoverable_gwp_asan_crash; + uintptr_t crash_detail_page; }; struct __attribute__((__packed__)) CrashInfo {