From 618cea3ebda342b39db103ac85e96a541d67b1f7 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 26 Jan 2021 17:45:43 -0800 Subject: [PATCH] Remove use of libbase logging in libdebuggerd. libbase logging uses getprogname() to get the default tag, which breaks for the fallback handler which is statically linked into the dynamic linker. Switch to libasync_safe for logging. Test: atest -c CtsSeccompHostTestCases:android.seccomp.cts.SeccompHostJUnit4DeviceTest#testAppZygoteSyscalls Change-Id: Ieeaf33fb26cff4ba7e1589d1d883ac2fcc74cf47 --- debuggerd/Android.bp | 1 + debuggerd/libdebuggerd/tombstone.cpp | 13 +++++--- debuggerd/libdebuggerd/tombstone_proto.cpp | 32 ++++++++++++------- .../libdebuggerd/tombstone_proto_to_text.cpp | 7 ++-- debuggerd/libdebuggerd/utility.cpp | 6 ++-- 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp index 2390d7979..53f85bf32 100644 --- a/debuggerd/Android.bp +++ b/debuggerd/Android.bp @@ -206,6 +206,7 @@ cc_library_static { ], whole_static_libs: [ + "libasync_safe", "gwp_asan_crash_handler", "libscudo", "libtombstone_proto", diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp index 822b74aea..185bd6e73 100644 --- a/debuggerd/libdebuggerd/tombstone.cpp +++ b/debuggerd/libdebuggerd/tombstone.cpp @@ -36,12 +36,12 @@ #include #include -#include #include #include #include #include #include +#include #include #include #include @@ -588,7 +588,7 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m unwindstack::UnwinderFromPid unwinder(kMaxFrames, pid, unwindstack::Regs::CurrentArch()); if (!unwinder.Init()) { - LOG(FATAL) << "Failed to init unwinder object."; + async_safe_fatal("failed to init unwinder object"); } ProcessInfo process_info; @@ -606,8 +606,11 @@ void engrave_tombstone(unique_fd output_fd, unique_fd proto_fd, unwindstack::Unw Tombstone tombstone; engrave_tombstone_proto(&tombstone, unwinder, threads, target_thread, process_info, open_files); - if (!tombstone.SerializeToFileDescriptor(proto_fd.get())) { - PLOG(ERROR) << "Failed to write proto tombstone"; + if (proto_fd != -1) { + if (!tombstone.SerializeToFileDescriptor(proto_fd.get())) { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to write proto tombstone: %s", + strerror(errno)); + } } log_t log; @@ -631,7 +634,7 @@ void engrave_tombstone(unique_fd output_fd, unique_fd proto_fd, unwindstack::Unw auto it = threads.find(target_thread); if (it == threads.end()) { - LOG(FATAL) << "failed to find target thread"; + async_safe_fatal("failed to find target thread"); } dump_thread(&log, unwinder, it->second, process_info, true); diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp index 801b11242..bb3c7eae0 100644 --- a/debuggerd/libdebuggerd/tombstone_proto.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto.cpp @@ -31,7 +31,8 @@ #include #include -#include +#include + #include #include #include @@ -151,13 +152,15 @@ static void dump_abort_message(Tombstone* tombstone, unwindstack::Unwinder* unwi size_t length; if (!process_memory->ReadFully(address, &length, sizeof(length))) { - PLOG(ERROR) << "Failed to read abort message header"; + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read abort message header: %s", + strerror(errno)); return; } // The length field includes the length of the length field itself. if (length < sizeof(size_t)) { - LOG(ERROR) << "Abort message header malformed: claimed length = " << length; + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, + "abort message header malformed: claimed length = %zu", length); return; } @@ -168,7 +171,8 @@ static void dump_abort_message(Tombstone* tombstone, unwindstack::Unwinder* unwi msg.resize(length); if (!process_memory->ReadFully(address + sizeof(length), &msg[0], length)) { - PLOG(ERROR) << "Failed to read abort message header"; + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read abort message header: %s", + strerror(errno)); return; } @@ -236,7 +240,11 @@ static void dump_thread(Tombstone* tombstone, unwindstack::Unwinder* unwinder, dump.set_begin_address(value); - CHECK(start_offset + bytes <= sizeof(buf)); + if (start_offset + bytes > sizeof(buf)) { + async_safe_fatal("dump_memory overflowed? start offset = %zu, bytes read = %zd", + start_offset, bytes); + } + dump.set_memory(buf, start_offset + bytes); *thread.add_memory_dump() = std::move(dump); @@ -247,10 +255,12 @@ static void dump_thread(Tombstone* tombstone, unwindstack::Unwinder* unwinder, unwinder->SetRegs(regs_copy.get()); unwinder->Unwind(); if (unwinder->NumFrames() == 0) { - LOG(ERROR) << "Failed to unwind"; + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to unwind"); if (unwinder->LastErrorCode() != unwindstack::ERROR_NONE) { - LOG(ERROR) << " Error code: " << unwinder->LastErrorCodeString(); - LOG(ERROR) << " Error address: " << StringPrintf("0x%" PRIx64, unwinder->LastErrorAddress()); + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, " error code: %s", + unwinder->LastErrorCodeString()); + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, " error address: 0x%" PRIx64, + unwinder->LastErrorAddress()); } } else { unwinder->SetDisplayBuildID(true); @@ -351,11 +361,9 @@ static void dump_log_file(Tombstone* tombstone, const char* logger, pid_t pid) { // non-blocking EOF; we're done break; } else { - ALOGE("Error while reading log: %s\n", strerror(-actual)); break; } } else if (actual == 0) { - ALOGE("Got zero bytes while reading log: %s\n", strerror(errno)); break; } @@ -431,7 +439,9 @@ void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::Unwinder* unwind result.set_selinux_label(main_thread.selinux_label); result.set_process_name(main_thread.process_name); - CHECK(main_thread.siginfo != nullptr); + if (!main_thread.siginfo) { + async_safe_fatal("siginfo missing"); + } Signal sig; sig.set_number(main_thread.signo); diff --git a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp index 5ba0ad6e2..187379daf 100644 --- a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp @@ -25,9 +25,9 @@ #include #include -#include #include #include +#include #include "tombstone.pb.h" @@ -113,7 +113,7 @@ static void print_thread_registers(CallbackType callback, const Tombstone& tombs break; default: - LOG(FATAL) << "unknown architecture"; + async_safe_fatal("unknown architecture"); } for (const auto& reg : thread.registers()) { @@ -217,7 +217,6 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, } if (!tombstone.has_signal_info()) { - LOG(ERROR) << "signal info missing in tombstone"; CBL("signal information missing"); } else { std::string fault_addr_desc; @@ -319,7 +318,7 @@ bool tombstone_proto_to_text(const Tombstone& tombstone, CallbackType callback) const auto& threads = tombstone.threads(); auto main_thread_it = threads.find(tombstone.tid()); if (main_thread_it == threads.end()) { - LOG(ERROR) << "failed to find entry for main thread in tombstone"; + CBL("failed to find entry for main thread in tombstone"); return false; } diff --git a/debuggerd/libdebuggerd/utility.cpp b/debuggerd/libdebuggerd/utility.cpp index f406ad48e..f941990d0 100644 --- a/debuggerd/libdebuggerd/utility.cpp +++ b/debuggerd/libdebuggerd/utility.cpp @@ -30,11 +30,11 @@ #include -#include #include #include #include #include +#include #include #include #include @@ -259,11 +259,11 @@ void drop_capabilities() { memset(&capdata, 0, sizeof(capdata)); if (capset(&capheader, &capdata[0]) == -1) { - PLOG(FATAL) << "failed to drop capabilities"; + async_safe_fatal("failed to drop capabilities: %s", strerror(errno)); } if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) { - PLOG(FATAL) << "failed to set PR_SET_NO_NEW_PRIVS"; + async_safe_fatal("failed to set PR_SET_NO_NEW_PRIVS: %s", strerror(errno)); } }