From 32d3cdda22287584081a55eb3d646c0e8f53a92d Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 21 Sep 2021 12:20:53 -0700 Subject: [PATCH] libdebuggerd: fix process uptime. I was here because we have a case where timeout(1) kills logcat, but debuggerd alleges that the process that was killed had started less than a second ago. I'm not sure this is the problem there, but I did notice that far too many tombstones were claiming improbably short process uptimes. It turns out that the code was measuring the *thread* uptime, not the *process* uptime. Also simplify the code a bit by switching to sysinfo(2) rather than reading a file. Test: manual, plus the existing unit test Change-Id: Ie2810b1d5777ad9182be92bfb3f60795dc978b24 --- debuggerd/libdebuggerd/tombstone_proto.cpp | 37 +++++++------------ .../seccomp_policy/crash_dump.arm.policy | 1 + .../seccomp_policy/crash_dump.arm64.policy | 1 + .../seccomp_policy/crash_dump.policy.def | 1 + .../seccomp_policy/crash_dump.x86.policy | 1 + .../seccomp_policy/crash_dump.x86_64.policy | 1 + 6 files changed, 18 insertions(+), 24 deletions(-) diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp index 0c93c900b..99f636e64 100644 --- a/debuggerd/libdebuggerd/tombstone_proto.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -596,14 +597,6 @@ static void dump_tags_around_fault_addr(Signal* signal, const Tombstone& tombsto } } -static std::optional read_uptime_secs() { - std::string uptime; - if (!android::base::ReadFileToString("/proc/uptime", &uptime)) { - return {}; - } - return strtoll(uptime.c_str(), nullptr, 10); -} - void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::Unwinder* unwinder, const std::map& threads, pid_t target_thread, const ProcessInfo& process_info, const OpenFilesList* open_files) { @@ -614,28 +607,24 @@ void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::Unwinder* unwind result.set_revision(android::base::GetProperty("ro.revision", "unknown")); result.set_timestamp(get_timestamp()); - std::optional system_uptime = read_uptime_secs(); - if (system_uptime) { - android::procinfo::ProcessInfo proc_info; - std::string error; - if (android::procinfo::GetProcessInfo(target_thread, &proc_info, &error)) { - uint64_t starttime = proc_info.starttime / sysconf(_SC_CLK_TCK); - result.set_process_uptime(*system_uptime - starttime); - } else { - async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read process info: %s", - error.c_str()); - } - } else { - async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read /proc/uptime: %s", - strerror(errno)); - } - const ThreadInfo& main_thread = threads.at(target_thread); result.set_pid(main_thread.pid); result.set_tid(main_thread.tid); result.set_uid(main_thread.uid); result.set_selinux_label(main_thread.selinux_label); + struct sysinfo si; + sysinfo(&si); + android::procinfo::ProcessInfo proc_info; + std::string error; + if (android::procinfo::GetProcessInfo(main_thread.pid, &proc_info, &error)) { + uint64_t starttime = proc_info.starttime / sysconf(_SC_CLK_TCK); + result.set_process_uptime(si.uptime - starttime); + } else { + async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to read process info: %s", + error.c_str()); + } + auto cmd_line = result.mutable_command_line(); for (const auto& arg : main_thread.command_line) { *cmd_line->Add() = arg; diff --git a/debuggerd/seccomp_policy/crash_dump.arm.policy b/debuggerd/seccomp_policy/crash_dump.arm.policy index 4eac0e92a..8fd03c427 100644 --- a/debuggerd/seccomp_policy/crash_dump.arm.policy +++ b/debuggerd/seccomp_policy/crash_dump.arm.policy @@ -20,6 +20,7 @@ getdents64: 1 faccessat: 1 recvmsg: 1 recvfrom: 1 +sysinfo: 1 process_vm_readv: 1 tgkill: 1 rt_sigprocmask: 1 diff --git a/debuggerd/seccomp_policy/crash_dump.arm64.policy b/debuggerd/seccomp_policy/crash_dump.arm64.policy index 21887abe0..858a338bd 100644 --- a/debuggerd/seccomp_policy/crash_dump.arm64.policy +++ b/debuggerd/seccomp_policy/crash_dump.arm64.policy @@ -19,6 +19,7 @@ getdents64: 1 faccessat: 1 recvmsg: 1 recvfrom: 1 +sysinfo: 1 process_vm_readv: 1 tgkill: 1 rt_sigprocmask: 1 diff --git a/debuggerd/seccomp_policy/crash_dump.policy.def b/debuggerd/seccomp_policy/crash_dump.policy.def index 90843fcba..152697cf3 100644 --- a/debuggerd/seccomp_policy/crash_dump.policy.def +++ b/debuggerd/seccomp_policy/crash_dump.policy.def @@ -25,6 +25,7 @@ getdents64: 1 faccessat: 1 recvmsg: 1 recvfrom: 1 +sysinfo: 1 process_vm_readv: 1 diff --git a/debuggerd/seccomp_policy/crash_dump.x86.policy b/debuggerd/seccomp_policy/crash_dump.x86.policy index 4eac0e92a..8fd03c427 100644 --- a/debuggerd/seccomp_policy/crash_dump.x86.policy +++ b/debuggerd/seccomp_policy/crash_dump.x86.policy @@ -20,6 +20,7 @@ getdents64: 1 faccessat: 1 recvmsg: 1 recvfrom: 1 +sysinfo: 1 process_vm_readv: 1 tgkill: 1 rt_sigprocmask: 1 diff --git a/debuggerd/seccomp_policy/crash_dump.x86_64.policy b/debuggerd/seccomp_policy/crash_dump.x86_64.policy index 1585cc6ee..281e231b0 100644 --- a/debuggerd/seccomp_policy/crash_dump.x86_64.policy +++ b/debuggerd/seccomp_policy/crash_dump.x86_64.policy @@ -19,6 +19,7 @@ getdents64: 1 faccessat: 1 recvmsg: 1 recvfrom: 1 +sysinfo: 1 process_vm_readv: 1 tgkill: 1 rt_sigprocmask: 1