Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5f1ce5b8 authored by Josh Gao's avatar Josh Gao Committed by android-build-merger
Browse files

Merge "Fix multithreaded backtraces for seccomp processes."

am: 4cd979fe

Change-Id: I74b64ba64645e6c638e8f5d558f8eda7fc4aec49
parents 165121c0 4cd979fe
Loading
Loading
Loading
Loading
+40 −4
Original line number Original line Diff line number Diff line
@@ -587,9 +587,28 @@ static const char* const kDebuggerdSeccompPolicy =
    "/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";
    "/system/etc/seccomp_policy/crash_dump." ABI_STRING ".policy";


static pid_t seccomp_fork_impl(void (*prejail)()) {
static pid_t seccomp_fork_impl(void (*prejail)()) {
  unique_fd policy_fd(open(kDebuggerdSeccompPolicy, O_RDONLY | O_CLOEXEC));
  std::string policy;
  if (policy_fd == -1) {
  if (!android::base::ReadFileToString(kDebuggerdSeccompPolicy, &policy)) {
    LOG(FATAL) << "failed to open policy " << kDebuggerdSeccompPolicy;
    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()};
  ScopedMinijail jail{minijail_new()};
@@ -600,7 +619,7 @@ static pid_t seccomp_fork_impl(void (*prejail)()) {
  minijail_no_new_privs(jail.get());
  minijail_no_new_privs(jail.get());
  minijail_log_seccomp_filter_failures(jail.get());
  minijail_log_seccomp_filter_failures(jail.get());
  minijail_use_seccomp_filter(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();
  pid_t result = fork();
  if (result == -1) {
  if (result == -1) {
@@ -735,6 +754,16 @@ TEST_F(CrasherTest, seccomp_tombstone) {
  ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
  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) {
TEST_F(CrasherTest, seccomp_backtrace) {
  int intercept_result;
  int intercept_result;
  unique_fd output_fd;
  unique_fd output_fd;
@@ -742,6 +771,11 @@ TEST_F(CrasherTest, seccomp_backtrace) {
  static const auto dump_type = kDebuggerdNativeBacktrace;
  static const auto dump_type = kDebuggerdNativeBacktrace;
  StartProcess(
  StartProcess(
      []() {
      []() {
        std::thread a(foo);
        std::thread b(bar);

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

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


TEST_F(CrasherTest, seccomp_crash_logcat) {
TEST_F(CrasherTest, seccomp_crash_logcat) {
+4 −3
Original line number Original line Diff line number Diff line
@@ -47,6 +47,7 @@
#include <unwindstack/Regs.h>
#include <unwindstack/Regs.h>


#include "debuggerd/handler.h"
#include "debuggerd/handler.h"
#include "handler/fallback.h"
#include "tombstoned/tombstoned.h"
#include "tombstoned/tombstoned.h"
#include "util.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 void trace_handler(siginfo_t* info, ucontext_t* ucontext) {
  static std::atomic<uint64_t> trace_output(pack_thread_fd(-1, -1));
  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.
    // Asked to dump by the original signal recipient.
    uint64_t val = trace_output.load();
    uint64_t val = trace_output.load();
    auto [tid, fd] = unpack_thread_fd(val);
    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_t siginfo = {};
        siginfo.si_code = SI_QUEUE;
        siginfo.si_code = SI_QUEUE;
        siginfo.si_value.sival_int = ~0;
        siginfo.si_value.sival_ptr = kDebuggerdFallbackSivalPtrRequestDump;
        siginfo.si_pid = getpid();
        siginfo.si_pid = getpid();
        siginfo.si_uid = getuid();
        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,
extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext,
                                           void* abort_message) {
                                           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);
    return trace_handler(info, ucontext);
  } else {
  } else {
    return crash_handler(info, ucontext, abort_message);
    return crash_handler(info, ucontext, abort_message);
+9 −4
Original line number Original line Diff line number Diff line
@@ -58,6 +58,8 @@
#include "dump_type.h"
#include "dump_type.h"
#include "protocol.h"
#include "protocol.h"


#include "handler/fallback.h"

using android::base::Pipe;
using android::base::Pipe;


// We muck with our fds in a 'thread' that doesn't share the same fd table.
// 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;
  void* abort_message = nullptr;
  uintptr_t si_val = reinterpret_cast<uintptr_t>(info->si_ptr);
  if (signal_number == DEBUGGER_SIGNAL) {
  if (signal_number == DEBUGGER_SIGNAL) {
    if (info->si_code == SI_QUEUE && info->si_pid == __getpid()) {
    if (info->si_code == SI_QUEUE && info->si_pid == __getpid()) {
      // Allow for the abort message to be explicitly specified via the sigqueue value.
      // 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.
      // 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);
      if (si_val != kDebuggerdFallbackSivalUintptrRequestDump) {
      abort_message = reinterpret_cast<void*>(value & ~1);
        abort_message = reinterpret_cast<void*>(si_val & ~1);
      info->si_ptr = reinterpret_cast<void*>(value & 1);
        info->si_ptr = reinterpret_cast<void*>(si_val & 1);
      }
    }
    }
  } else {
  } else {
    if (g_callbacks.get_abort_message) {
    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,
  // 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
  // then return 0 in subsequent calls, so check the sival_int to determine if
  // the fallback handler should be called first.
  // 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,
    // 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
    // you can only set NO_NEW_PRIVS to 1, and the effect should be at worst a single missing
    // ANR trace.
    // ANR trace.
+22 −0
Original line number Original line 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;