summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Gao2018-02-22 13:38:33 -0600
committerJosh Gao2018-02-22 18:31:38 -0600
commit70adac6a8a8469b6a4c248417bba33fa8381b9ad (patch)
tree7474d0f38e7acf8291be530c7b1dd9bdf8179583 /debuggerd
parentcdf778f5d92ab8748897a4d95693524d3273c23a (diff)
downloadplatform-system-core-70adac6a8a8469b6a4c248417bba33fa8381b9ad.tar.gz
platform-system-core-70adac6a8a8469b6a4c248417bba33fa8381b9ad.tar.xz
platform-system-core-70adac6a8a8469b6a4c248417bba33fa8381b9ad.zip
debuggerd_fallback: don't recursively abort.
Calls to abort() will always result in our signal handler being called, because abort will manually unblock SIGABRT before raising it. This can lead to deadlock when handling address space exhaustion in the fallback handler. To fix this, switch our mutex to a recursive mutex, and manually keep track of our lock count. Bug: http://b/72929749 Test: debuggerd_test --gtest_filter="CrasherTest.seccomp_crash_oom" Change-Id: I609f263ce93550350b17757189326b627129d4a7
Diffstat (limited to 'debuggerd')
-rw-r--r--debuggerd/debuggerd_test.cpp51
-rw-r--r--debuggerd/handler/debuggerd_fallback.cpp16
2 files changed, 60 insertions, 7 deletions
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index f8b4bad6e..397ff2f11 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -20,6 +20,7 @@
20#include <sys/capability.h> 20#include <sys/capability.h>
21#include <sys/prctl.h> 21#include <sys/prctl.h>
22#include <sys/ptrace.h> 22#include <sys/ptrace.h>
23#include <sys/resource.h>
23#include <sys/syscall.h> 24#include <sys/syscall.h>
24#include <sys/types.h> 25#include <sys/types.h>
25#include <unistd.h> 26#include <unistd.h>
@@ -570,7 +571,7 @@ TEST_F(CrasherTest, fake_pid) {
570static const char* const kDebuggerdSeccompPolicy = 571static const char* const kDebuggerdSeccompPolicy =
571 "/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy"; 572 "/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";
572 573
573pid_t seccomp_fork() { 574static pid_t seccomp_fork_impl(void (*prejail)()) {
574 unique_fd policy_fd(open(kDebuggerdSeccompPolicy, O_RDONLY | O_CLOEXEC)); 575 unique_fd policy_fd(open(kDebuggerdSeccompPolicy, O_RDONLY | O_CLOEXEC));
575 if (policy_fd == -1) { 576 if (policy_fd == -1) {
576 LOG(FATAL) << "failed to open policy " << kDebuggerdSeccompPolicy; 577 LOG(FATAL) << "failed to open policy " << kDebuggerdSeccompPolicy;
@@ -607,10 +608,18 @@ pid_t seccomp_fork() {
607 continue; 608 continue;
608 } 609 }
609 610
611 if (prejail) {
612 prejail();
613 }
614
610 minijail_enter(jail.get()); 615 minijail_enter(jail.get());
611 return result; 616 return result;
612} 617}
613 618
619static pid_t seccomp_fork() {
620 return seccomp_fork_impl(nullptr);
621}
622
614TEST_F(CrasherTest, seccomp_crash) { 623TEST_F(CrasherTest, seccomp_crash) {
615 int intercept_result; 624 int intercept_result;
616 unique_fd output_fd; 625 unique_fd output_fd;
@@ -628,6 +637,46 @@ TEST_F(CrasherTest, seccomp_crash) {
628 ASSERT_BACKTRACE_FRAME(result, "abort"); 637 ASSERT_BACKTRACE_FRAME(result, "abort");
629} 638}
630 639
640static pid_t seccomp_fork_rlimit() {
641 return seccomp_fork_impl([]() {
642 struct rlimit rlim = {
643 .rlim_cur = 512 * 1024 * 1024,
644 .rlim_max = 512 * 1024 * 1024,
645 };
646
647 if (setrlimit(RLIMIT_AS, &rlim) != 0) {
648 raise(SIGINT);
649 }
650 });
651}
652
653TEST_F(CrasherTest, seccomp_crash_oom) {
654 int intercept_result;
655 unique_fd output_fd;
656
657 StartProcess(
658 []() {
659 std::vector<void*> vec;
660 for (int i = 0; i < 512; ++i) {
661 char* buf = static_cast<char*>(malloc(1024 * 1024));
662 if (!buf) {
663 abort();
664 }
665 memset(buf, 0xff, 1024 * 1024);
666 vec.push_back(buf);
667 }
668 },
669 &seccomp_fork_rlimit);
670
671 StartIntercept(&output_fd);
672 FinishCrasher();
673 AssertDeath(SIGABRT);
674 FinishIntercept(&intercept_result);
675 ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
676
677 // We can't actually generate a backtrace, just make sure that the process terminates.
678}
679
631__attribute__((noinline)) extern "C" bool raise_debugger_signal(DebuggerdDumpType dump_type) { 680__attribute__((noinline)) extern "C" bool raise_debugger_signal(DebuggerdDumpType dump_type) {
632 siginfo_t siginfo; 681 siginfo_t siginfo;
633 siginfo.si_code = SI_QUEUE; 682 siginfo.si_code = SI_QUEUE;
diff --git a/debuggerd/handler/debuggerd_fallback.cpp b/debuggerd/handler/debuggerd_fallback.cpp
index 364fca5e8..dea2e17eb 100644
--- a/debuggerd/handler/debuggerd_fallback.cpp
+++ b/debuggerd/handler/debuggerd_fallback.cpp
@@ -37,6 +37,7 @@
37 37
38#include <atomic> 38#include <atomic>
39#include <memory> 39#include <memory>
40#include <mutex>
40 41
41#include <android-base/file.h> 42#include <android-base/file.h>
42#include <android-base/unique_fd.h> 43#include <android-base/unique_fd.h>
@@ -298,11 +299,13 @@ exit:
298static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_message) { 299static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_message) {
299 // Only allow one thread to handle a crash at a time (this can happen multiple times without 300 // Only allow one thread to handle a crash at a time (this can happen multiple times without
300 // exit, since tombstones can be requested without a real crash happening.) 301 // exit, since tombstones can be requested without a real crash happening.)
301 static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER; 302 static std::recursive_mutex crash_mutex;
302 int ret = pthread_mutex_lock(&crash_mutex); 303 static int lock_count;
303 if (ret != 0) { 304
304 async_safe_format_log(ANDROID_LOG_INFO, "libc", "pthread_mutex_lock failed: %s", strerror(ret)); 305 crash_mutex.lock();
305 return; 306 if (lock_count++ > 0) {
307 async_safe_format_log(ANDROID_LOG_ERROR, "libc", "recursed signal handler call, exiting");
308 _exit(1);
306 } 309 }
307 310
308 unique_fd tombstone_socket, output_fd; 311 unique_fd tombstone_socket, output_fd;
@@ -313,7 +316,8 @@ static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_mes
313 tombstoned_notify_completion(tombstone_socket.get()); 316 tombstoned_notify_completion(tombstone_socket.get());
314 } 317 }
315 318
316 pthread_mutex_unlock(&crash_mutex); 319 --lock_count;
320 crash_mutex.unlock();
317} 321}
318 322
319extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext, 323extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext,