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

Commit 5ec54d1e authored by Jerome Gaillard's avatar Jerome Gaillard
Browse files

Revert "tombstoned: support for protobuf fds." 

Revert "Let crash_dump read /proc/$PID."

Revert submission 1556807-tombstone_proto

Reason for revert: b/178455196, Broken test: android.seccomp.cts.SeccompHostJUnit4DeviceTest#testAppZygoteSyscalls on git_master on cf_x86_64_phone-userdebug

Reverted Changes:
Ide6811297:tombstoned: switch from goto to RAII.
I8d285c4b4:tombstoned: make it easier to add more types of ou...
Id0f0fa285:tombstoned: support for protobuf fds.
I6be6082ab:Let crash_dump read /proc/$PID.
Id812ca390:Make protobuf vendor_ramdisk_available.
Ieeece6e6d:libdebuggerd: add protobuf implementation.

Change-Id: I0c4f3a17e8b06d6c65255388c571ebf11d371dbb
parent 1e45d3f2

debuggerd/common/include/dump_type.h

+1 −5
Original line number Diff line number Diff line
@@ -24,8 +24,7 @@ enum DebuggerdDumpType : uint8_t { 
  kDebuggerdNativeBacktrace,

  kDebuggerdTombstone,

  kDebuggerdJavaBacktrace,

  kDebuggerdAnyIntercept,

  kDebuggerdTombstoneProto,

  kDebuggerdAnyIntercept

};



inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& rhs) {
@@ -42,9 +41,6 @@ inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& r 
    case kDebuggerdAnyIntercept:

      stream << "kDebuggerdAnyIntercept";

      break;

    case kDebuggerdTombstoneProto:

      stream << "kDebuggerdTombstoneProto";

      break;

    default:

      stream << "[unknown]";

  }

debuggerd/debuggerd_test.cpp

+0 −67
Original line number Diff line number Diff line
@@ -1425,70 +1425,3 @@ TEST_F(CrasherTest, stack_overflow) { 
  ConsumeFd(std::move(output_fd), &result);

  ASSERT_MATCH(result, R"(Cause: stack pointer[^\n]*stack overflow.\n)");

}
 


TEST(tombstoned, proto) {

  const pid_t self = getpid();

  unique_fd tombstoned_socket, text_fd, proto_fd;

  ASSERT_TRUE(

      tombstoned_connect(self, &tombstoned_socket, &text_fd, &proto_fd, kDebuggerdTombstoneProto));



  tombstoned_notify_completion(tombstoned_socket.get());



  ASSERT_NE(-1, text_fd.get());

  ASSERT_NE(-1, proto_fd.get());



  struct stat text_st;

  ASSERT_EQ(0, fstat(text_fd.get(), &text_st));



  // Give tombstoned some time to link the files into place.

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



  // Find the tombstone.

  std::optional<int> tombstone_index;

  for (int i = 0; i < 50; ++i) {

    std::string path = android::base::StringPrintf("/data/tombstones/tombstone_%02d", i);



    struct stat st;

    if (TEMP_FAILURE_RETRY(stat(path.c_str(), &st)) != 0) {

      continue;

    }



    if (st.st_dev == text_st.st_dev && st.st_ino == text_st.st_ino) {

      tombstone_index = i;

      break;

    }

  }



  ASSERT_TRUE(tombstone_index);

  std::string proto_path =

      android::base::StringPrintf("/data/tombstones/tombstone_%02d.pb", *tombstone_index);



  struct stat proto_fd_st;

  struct stat proto_file_st;

  ASSERT_EQ(0, fstat(proto_fd.get(), &proto_fd_st));

  ASSERT_EQ(0, stat(proto_path.c_str(), &proto_file_st));



  ASSERT_EQ(proto_fd_st.st_dev, proto_file_st.st_dev);

  ASSERT_EQ(proto_fd_st.st_ino, proto_file_st.st_ino);

}



TEST(tombstoned, proto_intercept) {

  const pid_t self = getpid();

  unique_fd intercept_fd, output_fd;

  InterceptStatus status;



  tombstoned_intercept(self, &intercept_fd, &output_fd, &status, kDebuggerdTombstone);

  ASSERT_EQ(InterceptStatus::kRegistered, status);



  unique_fd tombstoned_socket, text_fd, proto_fd;

  ASSERT_TRUE(

      tombstoned_connect(self, &tombstoned_socket, &text_fd, &proto_fd, kDebuggerdTombstoneProto));

  ASSERT_TRUE(android::base::WriteStringToFd("foo", text_fd.get()));

  tombstoned_notify_completion(tombstoned_socket.get());



  text_fd.reset();



  std::string output;

  ASSERT_TRUE(android::base::ReadFdToString(output_fd, &output));

  ASSERT_EQ("foo", output);

}

debuggerd/tombstoned/include/tombstoned/tombstoned.h

+1 −5
Original line number Diff line number Diff line
@@ -23,10 +23,6 @@ 
#include "dump_type.h"



bool tombstoned_connect(pid_t pid, android::base::unique_fd* tombstoned_socket,

                        android::base::unique_fd* text_output_fd,

                        android::base::unique_fd* proto_output_fd, DebuggerdDumpType dump_type);



bool tombstoned_connect(pid_t pid, android::base::unique_fd* tombstoned_socket,

                        android::base::unique_fd* text_output_fd, DebuggerdDumpType dump_type);

                        android::base::unique_fd* output_fd, DebuggerdDumpType dump_type);



bool tombstoned_notify_completion(int tombstoned_socket);

debuggerd/tombstoned/intercept_manager.cpp

+1 −13
Original line number Diff line number Diff line
@@ -191,18 +191,6 @@ InterceptManager::InterceptManager(event_base* base, int intercept_socket) : bas 
                                      /* backlog */ -1, intercept_socket);

}



bool dump_types_compatible(DebuggerdDumpType interceptor, DebuggerdDumpType dump) {

  if (interceptor == dump) {

    return true;

  }



  if (interceptor == kDebuggerdTombstone && dump == kDebuggerdTombstoneProto) {

    return true;

  }



  return false;

}



bool InterceptManager::GetIntercept(pid_t pid, DebuggerdDumpType dump_type,

                                    android::base::unique_fd* out_fd) {

  auto it = this->intercepts.find(pid);
@@ -213,7 +201,7 @@ bool InterceptManager::GetIntercept(pid_t pid, DebuggerdDumpType dump_type, 
  if (dump_type == kDebuggerdAnyIntercept) {

    LOG(INFO) << "found registered intercept of type " << it->second->dump_type

              << " for requested type kDebuggerdAnyIntercept";

  } else if (!dump_types_compatible(it->second->dump_type, dump_type)) {

  } else if (it->second->dump_type != dump_type) {

    LOG(WARNING) << "found non-matching intercept of type " << it->second->dump_type

                 << " for requested type: " << dump_type;

    return false;

debuggerd/tombstoned/tombstoned.cpp

+8 −55
Original line number Diff line number Diff line
@@ -62,22 +62,14 @@ enum CrashStatus { 
struct CrashArtifact {

  unique_fd fd;

  std::optional<std::string> temporary_path;



  static CrashArtifact devnull() {

    CrashArtifact result;

    result.fd.reset(open("/dev/null", O_WRONLY | O_CLOEXEC));

    return result;

  }

};



struct CrashArtifactPaths {

  std::string text;

  std::optional<std::string> proto;

};



struct CrashOutput {

  CrashArtifact text;

  std::optional<CrashArtifact> proto;

};



// Ownership of Crash is a bit messy.
@@ -97,15 +89,14 @@ struct Crash { 
class CrashQueue {

 public:

  CrashQueue(const std::string& dir_path, const std::string& file_name_prefix, size_t max_artifacts,

             size_t max_concurrent_dumps, bool supports_proto)

             size_t max_concurrent_dumps)

      : file_name_prefix_(file_name_prefix),

        dir_path_(dir_path),

        dir_fd_(open(dir_path.c_str(), O_DIRECTORY | O_RDONLY | O_CLOEXEC)),

        max_artifacts_(max_artifacts),

        next_artifact_(0),

        max_concurrent_dumps_(max_concurrent_dumps),

        num_concurrent_dumps_(0),

        supports_proto_(supports_proto) {

        num_concurrent_dumps_(0) {

    if (dir_fd_ == -1) {

      PLOG(FATAL) << "failed to open directory: " << dir_path;

    }
@@ -128,14 +119,14 @@ class CrashQueue { 
  static CrashQueue* for_tombstones() {

    static CrashQueue queue("/data/tombstones", "tombstone_" /* file_name_prefix */,

                            GetIntProperty("tombstoned.max_tombstone_count", 32),

                            1 /* max_concurrent_dumps */, true /* supports_proto */);

                            1 /* max_concurrent_dumps */);

    return &queue;

  }



  static CrashQueue* for_anrs() {

    static CrashQueue queue("/data/anr", "trace_" /* file_name_prefix */,

                            GetIntProperty("tombstoned.max_anr_count", 64),

                            4 /* max_concurrent_dumps */, false /* supports_proto */);

                            4 /* max_concurrent_dumps */);

    return &queue;

  }
@@ -169,15 +160,6 @@ class CrashQueue { 
        // Don't generate tombstones for backtrace requests.

        return {};



      case kDebuggerdTombstoneProto:

        if (!supports_proto_) {

          LOG(ERROR) << "received kDebuggerdTombstoneProto on a queue that doesn't support proto";

          return {};

        }

        result.proto = create_temporary_file();

        result.text = create_temporary_file();

        break;



      case kDebuggerdTombstone:

        result.text = create_temporary_file();

        break;
@@ -196,10 +178,6 @@ class CrashQueue { 
    CrashArtifactPaths result;

    result.text = StringPrintf("%s%02d", file_name_prefix_.c_str(), next_artifact_);



    if (supports_proto_) {

      result.proto = StringPrintf("%s%02d.pb", file_name_prefix_.c_str(), next_artifact_);

    }



    next_artifact_ = (next_artifact_ + 1) % max_artifacts_;

    return result;

  }
@@ -265,8 +243,6 @@ class CrashQueue { 
  const size_t max_concurrent_dumps_;

  size_t num_concurrent_dumps_;



  bool supports_proto_;



  std::deque<std::unique_ptr<Crash>> queued_requests_;



  DISALLOW_COPY_AND_ASSIGN(CrashQueue);
@@ -285,11 +261,7 @@ static void perform_request(std::unique_ptr<Crash> crash) { 
  unique_fd output_fd;

  bool intercepted =

      intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd);

  if (intercepted) {

    if (crash->crash_type == kDebuggerdTombstoneProto) {

      crash->output.proto = CrashArtifact::devnull();

    }

  } else {

  if (!intercepted) {

    if (auto o = CrashQueue::for_crash(crash.get())->get_output(crash->crash_type); o) {

      crash->output = std::move(*o);

      output_fd.reset(dup(crash->output.text.fd));
@@ -301,13 +273,8 @@ static void perform_request(std::unique_ptr<Crash> crash) { 


  TombstonedCrashPacket response = {.packet_type = CrashPacketType::kPerformDump};



  ssize_t rc = -1;

  if (crash->output.proto) {

    rc = SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get(),

                             crash->output.proto->fd.get());

  } else {

    rc = SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get());

  }

  ssize_t rc =

      SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get());



  output_fd.reset();
@@ -376,7 +343,7 @@ static void crash_request_cb(evutil_socket_t sockfd, short ev, void* arg) { 
  }



  crash->crash_type = request.packet.dump_request.dump_type;

  if (crash->crash_type < 0 || crash->crash_type > kDebuggerdTombstoneProto) {

  if (crash->crash_type < 0 || crash->crash_type > kDebuggerdAnyIntercept) {

    LOG(WARNING) << "unexpected crash dump type: " << crash->crash_type;

    return;

  }
@@ -471,14 +438,6 @@ static void crash_completed(borrowed_fd sockfd, std::unique_ptr<Crash> crash) { 
    }

  }



  if (crash->output.proto && crash->output.proto->fd != -1) {

    if (!paths.proto) {

      LOG(ERROR) << "missing path for proto tombstone";

    } else if (!link_fd(crash->output.proto->fd, queue->dir_fd(), *paths.proto)) {

      LOG(ERROR) << "failed to link proto tombstone";

    }

  }



  // If we don't have O_TMPFILE, we need to clean up after ourselves.

  if (crash->output.text.temporary_path) {

    rc = unlinkat(queue->dir_fd().get(), crash->output.text.temporary_path->c_str(), 0);
@@ -486,12 +445,6 @@ static void crash_completed(borrowed_fd sockfd, std::unique_ptr<Crash> crash) { 
      PLOG(ERROR) << "failed to unlink temporary tombstone at " << paths.text;

    }

  }

  if (crash->output.proto && crash->output.proto->temporary_path) {

    rc = unlinkat(queue->dir_fd().get(), crash->output.proto->temporary_path->c_str(), 0);

    if (rc != 0) {

      PLOG(ERROR) << "failed to unlink temporary proto tombstone";

    }

  }

}



static void crash_completed_cb(evutil_socket_t sockfd, short ev, void* arg) {