Merge "Fix multithreaded backtraces for seccomp processes." am: 4cd979fe67 am: 5f1ce5b8a2 (78816afa) · Commits · e / os / android_system_core

debuggerd/debuggerd_test.cpp

+40 −4

Original line number	Diff line number	Diff line
		@@ -587,9 +587,28 @@ static const char* const kDebuggerdSeccompPolicy =
		"/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";

		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;
		std::string policy;
		if (!android::base::ReadFileToString(kDebuggerdSeccompPolicy, &policy)) {
		PLOG(FATAL) << "failed to read policy file";
		}

		// Allow a bunch of syscalls used by the tests.
		policy += "\nclone: 1";
		policy += "\nsigaltstack: 1";
		policy += "\nnanosleep: 1";

		FILE* tmp_file = tmpfile();
		if (!tmp_file) {
		PLOG(FATAL) << "tmpfile failed";
		}

		unique_fd tmp_fd(dup(fileno(tmp_file)));
		if (!android::base::WriteStringToFd(policy, tmp_fd.get())) {
		PLOG(FATAL) << "failed to write policy to tmpfile";
		}

		if (lseek(tmp_fd.get(), 0, SEEK_SET) != 0) {
		PLOG(FATAL) << "failed to seek tmp_fd";
		}

		ScopedMinijail jail{minijail_new()};
		@@ -600,7 +619,7 @@ static pid_t seccomp_fork_impl(void (*prejail)()) {
		minijail_no_new_privs(jail.get());
		minijail_log_seccomp_filter_failures(jail.get());
		minijail_use_seccomp_filter(jail.get());
		minijail_parse_seccomp_filters_from_fd(jail.get(), policy_fd.release());
		minijail_parse_seccomp_filters_from_fd(jail.get(), tmp_fd.release());

		pid_t result = fork();
		if (result == -1) {
		@@ -735,6 +754,16 @@ TEST_F(CrasherTest, seccomp_tombstone) {
		ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
		}

		extern "C" void foo() {
		LOG(INFO) << "foo";
		std::this_thread::sleep_for(1s);
		}

		extern "C" void bar() {
		LOG(INFO) << "bar";
		std::this_thread::sleep_for(1s);
		}

		TEST_F(CrasherTest, seccomp_backtrace) {
		int intercept_result;
		unique_fd output_fd;
		@@ -742,6 +771,11 @@ TEST_F(CrasherTest, seccomp_backtrace) {
		static const auto dump_type = kDebuggerdNativeBacktrace;
		StartProcess(
		[]() {
		std::thread a(foo);
		std::thread b(bar);

		std::this_thread::sleep_for(100ms);

		raise_debugger_signal(dump_type);
		_exit(0);
		},
		@@ -756,6 +790,8 @@ TEST_F(CrasherTest, seccomp_backtrace) {
		std::string result;
		ConsumeFd(std::move(output_fd), &result);
		ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
		ASSERT_BACKTRACE_FRAME(result, "foo");
		ASSERT_BACKTRACE_FRAME(result, "bar");
		}

		TEST_F(CrasherTest, seccomp_crash_logcat) {

debuggerd/handler/debuggerd_fallback.cpp

+4 −3

Original line number	Diff line number	Diff line
		@@ -47,6 +47,7 @@
		#include <unwindstack/Regs.h>

		#include "debuggerd/handler.h"
		#include "handler/fallback.h"
		#include "tombstoned/tombstoned.h"
		#include "util.h"

		@@ -187,7 +188,7 @@ static std::pair<pid_t, int> unpack_thread_fd(uint64_t value) {
		static void trace_handler(siginfo_t* info, ucontext_t* ucontext) {
		static std::atomic<uint64_t> trace_output(pack_thread_fd(-1, -1));

		if (info->si_value.sival_int == ~0) {
		if (info->si_value.sival_ptr == kDebuggerdFallbackSivalPtrRequestDump) {
		// Asked to dump by the original signal recipient.
		uint64_t val = trace_output.load();
		auto [tid, fd] = unpack_thread_fd(val);
		@@ -259,7 +260,7 @@ static void trace_handler(siginfo_t* info, ucontext_t* ucontext) {

		siginfo_t siginfo = {};
		siginfo.si_code = SI_QUEUE;
		siginfo.si_value.sival_int = ~0;
		siginfo.si_value.sival_ptr = kDebuggerdFallbackSivalPtrRequestDump;
		siginfo.si_pid = getpid();
		siginfo.si_uid = getuid();

		@@ -331,7 +332,7 @@ static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_mes

		extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext,
		void* abort_message) {
		if (info->si_signo == DEBUGGER_SIGNAL && info->si_value.sival_int != 0) {
		if (info->si_signo == DEBUGGER_SIGNAL && info->si_value.sival_ptr != nullptr) {
		return trace_handler(info, ucontext);
		} else {
		return crash_handler(info, ucontext, abort_message);

debuggerd/handler/debuggerd_handler.cpp

+9 −4

Original line number	Diff line number	Diff line
		@@ -58,6 +58,8 @@
		#include "dump_type.h"
		#include "protocol.h"

		#include "handler/fallback.h"

		using android::base::Pipe;

		// We muck with our fds in a 'thread' that doesn't share the same fd table.
		@@ -473,13 +475,15 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c
		}

		void* abort_message = nullptr;
		uintptr_t si_val = reinterpret_cast<uintptr_t>(info->si_ptr);
		if (signal_number == DEBUGGER_SIGNAL) {
		if (info->si_code == SI_QUEUE && info->si_pid == __getpid()) {
		// Allow for the abort message to be explicitly specified via the sigqueue value.
		// Keep the bottom bit intact for representing whether we want a backtrace or a tombstone.
		uintptr_t value = reinterpret_cast<uintptr_t>(info->si_ptr);
		abort_message = reinterpret_cast<void*>(value & ~1);
		info->si_ptr = reinterpret_cast<void*>(value & 1);
		if (si_val != kDebuggerdFallbackSivalUintptrRequestDump) {
		abort_message = reinterpret_cast<void*>(si_val & ~1);
		info->si_ptr = reinterpret_cast<void*>(si_val & 1);
		}
		}
		} else {
		if (g_callbacks.get_abort_message) {
		@@ -492,7 +496,8 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c
		// of a specific thread. It is possible that the prctl call might return 1,
		// then return 0 in subsequent calls, so check the sival_int to determine if
		// the fallback handler should be called first.
		if (info->si_value.sival_int == ~0 \|\| prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0) == 1) {
		if (si_val == kDebuggerdFallbackSivalUintptrRequestDump \|\|
		prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0) == 1) {
		// This check might be racy if another thread sets NO_NEW_PRIVS, but this should be unlikely,
		// you can only set NO_NEW_PRIVS to 1, and the effect should be at worst a single missing
		// ANR trace.

debuggerd/handler/fallback.h

0 → 100644

+22 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright 2018 The Android Open Source Project
		*
		* Licensed under the Apache License, Version 2.0 (the "License");
		* you may not use this file except in compliance with the License.
		* You may obtain a copy of the License at
		*
		* http://www.apache.org/licenses/LICENSE-2.0
		*
		* Unless required by applicable law or agreed to in writing, software
		* distributed under the License is distributed on an "AS IS" BASIS,
		* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/

		#pragma once

		#include <stdint.h>

		static void* const kDebuggerdFallbackSivalPtrRequestDump = reinterpret_cast<void*>(~0UL);
		static const uintptr_t kDebuggerdFallbackSivalUintptrRequestDump = ~0UL;