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
This commit is contained in:
Josh Gao 2018-02-22 11:38:33 -08:00
parent cdf778f5d9
commit 70adac6a8a
2 changed files with 60 additions and 7 deletions

View file

@ -20,6 +20,7 @@
#include <sys/capability.h>
#include <sys/prctl.h>
#include <sys/ptrace.h>
#include <sys/resource.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
@ -570,7 +571,7 @@ TEST_F(CrasherTest, fake_pid) {
static const char* const kDebuggerdSeccompPolicy =
"/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";
pid_t seccomp_fork() {
static pid_t seccomp_fork_impl(void (*prejail)()) {
unique_fd policy_fd(open(kDebuggerdSeccompPolicy, O_RDONLY | O_CLOEXEC));
if (policy_fd == -1) {
LOG(FATAL) << "failed to open policy " << kDebuggerdSeccompPolicy;
@ -607,10 +608,18 @@ pid_t seccomp_fork() {
continue;
}
if (prejail) {
prejail();
}
minijail_enter(jail.get());
return result;
}
static pid_t seccomp_fork() {
return seccomp_fork_impl(nullptr);
}
TEST_F(CrasherTest, seccomp_crash) {
int intercept_result;
unique_fd output_fd;
@ -628,6 +637,46 @@ TEST_F(CrasherTest, seccomp_crash) {
ASSERT_BACKTRACE_FRAME(result, "abort");
}
static pid_t seccomp_fork_rlimit() {
return seccomp_fork_impl([]() {
struct rlimit rlim = {
.rlim_cur = 512 * 1024 * 1024,
.rlim_max = 512 * 1024 * 1024,
};
if (setrlimit(RLIMIT_AS, &rlim) != 0) {
raise(SIGINT);
}
});
}
TEST_F(CrasherTest, seccomp_crash_oom) {
int intercept_result;
unique_fd output_fd;
StartProcess(
[]() {
std::vector<void*> vec;
for (int i = 0; i < 512; ++i) {
char* buf = static_cast<char*>(malloc(1024 * 1024));
if (!buf) {
abort();
}
memset(buf, 0xff, 1024 * 1024);
vec.push_back(buf);
}
},
&seccomp_fork_rlimit);
StartIntercept(&output_fd);
FinishCrasher();
AssertDeath(SIGABRT);
FinishIntercept(&intercept_result);
ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
// We can't actually generate a backtrace, just make sure that the process terminates.
}
__attribute__((noinline)) extern "C" bool raise_debugger_signal(DebuggerdDumpType dump_type) {
siginfo_t siginfo;
siginfo.si_code = SI_QUEUE;

View file

@ -37,6 +37,7 @@
#include <atomic>
#include <memory>
#include <mutex>
#include <android-base/file.h>
#include <android-base/unique_fd.h>
@ -298,11 +299,13 @@ exit:
static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_message) {
// Only allow one thread to handle a crash at a time (this can happen multiple times without
// exit, since tombstones can be requested without a real crash happening.)
static pthread_mutex_t crash_mutex = PTHREAD_MUTEX_INITIALIZER;
int ret = pthread_mutex_lock(&crash_mutex);
if (ret != 0) {
async_safe_format_log(ANDROID_LOG_INFO, "libc", "pthread_mutex_lock failed: %s", strerror(ret));
return;
static std::recursive_mutex crash_mutex;
static int lock_count;
crash_mutex.lock();
if (lock_count++ > 0) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "recursed signal handler call, exiting");
_exit(1);
}
unique_fd tombstone_socket, output_fd;
@ -313,7 +316,8 @@ static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_mes
tombstoned_notify_completion(tombstone_socket.get());
}
pthread_mutex_unlock(&crash_mutex);
--lock_count;
crash_mutex.unlock();
}
extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext,