tombstoned: allow intercepts for java traces.
All intercept requests and crash dump requests must now specify a
dump_type, which can be one of kDebuggerdNativeBacktrace,
kDebuggerdTombstone or kDebuggerdJavaBacktrace. Each process can have
only one outstanding intercept registered at a time.
There's only one non-trivial change in this changeset; and that is
to crash_dump. We now pass the type of dump via a command line
argument instead of inferring it from the (resent) signal, this allows
us to connect to tombstoned before we wait for the signal as the
protocol requires.
Test: debuggerd_test
Change-Id: I189b215acfecd08ac52ab29117e3465da00e3a37
diff --git a/debuggerd/tombstoned/include/tombstoned/tombstoned.h b/debuggerd/tombstoned/include/tombstoned/tombstoned.h
index 908517d..6403dbe 100644
--- a/debuggerd/tombstoned/include/tombstoned/tombstoned.h
+++ b/debuggerd/tombstoned/include/tombstoned/tombstoned.h
@@ -20,7 +20,9 @@
#include <android-base/unique_fd.h>
+#include "dump_type.h"
+
bool tombstoned_connect(pid_t pid, android::base::unique_fd* tombstoned_socket,
- android::base::unique_fd* output_fd, bool is_native_crash = true);
+ android::base::unique_fd* output_fd, DebuggerdDumpType dump_type);
bool tombstoned_notify_completion(int tombstoned_socket);
diff --git a/debuggerd/tombstoned/intercept_manager.cpp b/debuggerd/tombstoned/intercept_manager.cpp
index 4d4eb9e..24960bc 100644
--- a/debuggerd/tombstoned/intercept_manager.cpp
+++ b/debuggerd/tombstoned/intercept_manager.cpp
@@ -61,11 +61,24 @@
reason = "due to input";
}
- LOG(INFO) << "intercept for pid " << intercept->intercept_pid << " terminated " << reason;
+ LOG(INFO) << "intercept for pid " << intercept->intercept_pid << " and type "
+ << intercept->dump_type << " terminated: " << reason;
intercept_manager->intercepts.erase(it);
}
}
+static bool is_intercept_request_valid(const InterceptRequest& request) {
+ if (request.pid <= 0 || request.pid > std::numeric_limits<pid_t>::max()) {
+ return false;
+ }
+
+ if (request.dump_type < 0 || request.dump_type > kDebuggerdJavaBacktrace) {
+ return false;
+ }
+
+ return true;
+}
+
static void intercept_request_cb(evutil_socket_t sockfd, short ev, void* arg) {
auto intercept = reinterpret_cast<Intercept*>(arg);
InterceptManager* intercept_manager = intercept->intercept_manager;
@@ -103,23 +116,24 @@
rcv_fd.reset(moved_fd);
// We trust the other side, so only do minimal validity checking.
- if (intercept_request.pid <= 0 || intercept_request.pid > std::numeric_limits<pid_t>::max()) {
+ if (!is_intercept_request_valid(intercept_request)) {
InterceptResponse response = {};
response.status = InterceptStatus::kFailed;
- snprintf(response.error_message, sizeof(response.error_message), "invalid pid %" PRId32,
- intercept_request.pid);
+ snprintf(response.error_message, sizeof(response.error_message), "invalid intercept request");
TEMP_FAILURE_RETRY(write(sockfd, &response, sizeof(response)));
goto fail;
}
intercept->intercept_pid = intercept_request.pid;
+ intercept->dump_type = intercept_request.dump_type;
// Check if it's already registered.
if (intercept_manager->intercepts.count(intercept_request.pid) > 0) {
InterceptResponse response = {};
- response.status = InterceptStatus::kFailed;
+ response.status = InterceptStatus::kFailedAlreadyRegistered;
snprintf(response.error_message, sizeof(response.error_message),
- "pid %" PRId32 " already intercepted", intercept_request.pid);
+ "pid %" PRId32 " already intercepted, type %d", intercept_request.pid,
+ intercept_request.dump_type);
TEMP_FAILURE_RETRY(write(sockfd, &response, sizeof(response)));
LOG(WARNING) << response.error_message;
goto fail;
@@ -138,7 +152,8 @@
intercept_manager->intercepts[intercept_request.pid] = std::unique_ptr<Intercept>(intercept);
intercept->registered = true;
- LOG(INFO) << "tombstoned registered intercept for pid " << intercept_request.pid;
+ LOG(INFO) << "registered intercept for pid " << intercept_request.pid << " and type "
+ << intercept_request.dump_type;
// Register a different read event on the socket so that we can remove intercepts if the socket
// closes (e.g. if a user CTRL-C's the process that requested the intercept).
@@ -174,16 +189,27 @@
intercept_socket);
}
-bool InterceptManager::GetIntercept(pid_t pid, android::base::unique_fd* out_fd) {
+bool InterceptManager::GetIntercept(pid_t pid, DebuggerdDumpType dump_type,
+ android::base::unique_fd* out_fd) {
auto it = this->intercepts.find(pid);
if (it == this->intercepts.end()) {
return false;
}
+ if (dump_type == kDebuggerdAnyIntercept) {
+ LOG(INFO) << "found registered intercept of type " << it->second->dump_type
+ << " for requested type kDebuggerdAnyIntercept";
+ } 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;
+ }
+
auto intercept = std::move(it->second);
this->intercepts.erase(it);
- LOG(INFO) << "found intercept fd " << intercept->output_fd.get() << " for pid " << pid;
+ LOG(INFO) << "found intercept fd " << intercept->output_fd.get() << " for pid " << pid
+ << " and type " << intercept->dump_type;
InterceptResponse response = {};
response.status = InterceptStatus::kStarted;
TEMP_FAILURE_RETRY(write(intercept->sockfd, &response, sizeof(response)));
diff --git a/debuggerd/tombstoned/intercept_manager.h b/debuggerd/tombstoned/intercept_manager.h
index cb5db62..a11d565 100644
--- a/debuggerd/tombstoned/intercept_manager.h
+++ b/debuggerd/tombstoned/intercept_manager.h
@@ -25,6 +25,8 @@
#include <android-base/unique_fd.h>
+#include "dump_type.h"
+
struct InterceptManager;
struct Intercept {
@@ -39,6 +41,7 @@
pid_t intercept_pid = -1;
android::base::unique_fd output_fd;
bool registered = false;
+ DebuggerdDumpType dump_type = kDebuggerdNativeBacktrace;
};
struct InterceptManager {
@@ -50,5 +53,5 @@
InterceptManager(InterceptManager& copy) = delete;
InterceptManager(InterceptManager&& move) = delete;
- bool GetIntercept(pid_t pid, android::base::unique_fd* out_fd);
+ bool GetIntercept(pid_t pid, DebuggerdDumpType dump_type, android::base::unique_fd* out_fd);
};
diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp
index 05df9f2..0a9f000 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -35,6 +35,7 @@
#include <cutils/sockets.h>
#include "debuggerd/handler.h"
+#include "dump_type.h"
#include "protocol.h"
#include "util.h"
@@ -52,10 +53,10 @@
struct Crash;
-class CrashType {
+class CrashQueue {
public:
- CrashType(const std::string& dir_path, const std::string& file_name_prefix, size_t max_artifacts,
- size_t max_concurrent_dumps)
+ CrashQueue(const std::string& dir_path, const std::string& file_name_prefix, size_t max_artifacts,
+ 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)),
@@ -114,8 +115,8 @@
void on_crash_completed() { --num_concurrent_dumps_; }
- static CrashType* const tombstone;
- static CrashType* const java_trace;
+ static CrashQueue* const tombstone;
+ static CrashQueue* const java_trace;
private:
void find_oldest_artifact() {
@@ -158,19 +159,19 @@
std::deque<Crash*> queued_requests_;
- DISALLOW_COPY_AND_ASSIGN(CrashType);
+ DISALLOW_COPY_AND_ASSIGN(CrashQueue);
};
// Whether java trace dumps are produced via tombstoned.
static constexpr bool kJavaTraceDumpsEnabled = false;
-/* static */ CrashType* const CrashType::tombstone =
- new CrashType("/data/tombstones", "tombstone_" /* file_name_prefix */, 10 /* max_artifacts */,
- 1 /* max_concurrent_dumps */);
+/* static */ CrashQueue* const CrashQueue::tombstone =
+ new CrashQueue("/data/tombstones", "tombstone_" /* file_name_prefix */, 10 /* max_artifacts */,
+ 1 /* max_concurrent_dumps */);
-/* static */ CrashType* const CrashType::java_trace =
- (kJavaTraceDumpsEnabled ? new CrashType("/data/anr", "anr_" /* file_name_prefix */,
- 64 /* max_artifacts */, 4 /* max_concurrent_dumps */)
+/* static */ CrashQueue* const CrashQueue::java_trace =
+ (kJavaTraceDumpsEnabled ? new CrashQueue("/data/anr", "anr_" /* file_name_prefix */,
+ 64 /* max_artifacts */, 4 /* max_concurrent_dumps */)
: nullptr);
// Ownership of Crash is a bit messy.
@@ -183,10 +184,17 @@
pid_t crash_pid;
event* crash_event = nullptr;
- // Not owned by |Crash|.
- CrashType* crash_type = nullptr;
+ DebuggerdDumpType crash_type;
};
+static CrashQueue* get_crash_queue(const Crash* crash) {
+ if (crash->crash_type == kDebuggerdJavaBacktrace) {
+ return CrashQueue::java_trace;
+ }
+
+ return CrashQueue::tombstone;
+}
+
// Forward declare the callbacks so they can be placed in a sensible order.
static void crash_accept_cb(evconnlistener* listener, evutil_socket_t sockfd, sockaddr*, int, void*);
static void crash_request_cb(evutil_socket_t sockfd, short ev, void* arg);
@@ -194,10 +202,8 @@
static void perform_request(Crash* crash) {
unique_fd output_fd;
- // Note that java traces are not interceptible.
- if ((crash->crash_type == CrashType::java_trace) ||
- !intercept_manager->GetIntercept(crash->crash_pid, &output_fd)) {
- output_fd = crash->crash_type->get_output_fd();
+ if (!intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd)) {
+ output_fd = get_crash_queue(crash)->get_output_fd();
}
TombstonedCrashPacket response = {
@@ -220,7 +226,7 @@
event_add(crash->crash_event, &timeout);
}
- crash->crash_type->on_crash_started();
+ get_crash_queue(crash)->on_crash_started();
return;
fail:
@@ -228,22 +234,22 @@
}
static void crash_accept_cb(evconnlistener* listener, evutil_socket_t sockfd, sockaddr*, int,
- void* crash_type) {
+ void*) {
event_base* base = evconnlistener_get_base(listener);
Crash* crash = new Crash();
+ // TODO: Make sure that only java crashes come in on the java socket
+ // and only native crashes on the native socket.
struct timeval timeout = { 1, 0 };
event* crash_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, crash_request_cb, crash);
crash->crash_fd.reset(sockfd);
crash->crash_event = crash_event;
- crash->crash_type = static_cast<CrashType*>(crash_type);
event_add(crash_event, &timeout);
}
static void crash_request_cb(evutil_socket_t sockfd, short ev, void* arg) {
ssize_t rc;
Crash* crash = static_cast<Crash*>(arg);
- CrashType* type = crash->crash_type;
TombstonedCrashPacket request = {};
@@ -271,7 +277,13 @@
goto fail;
}
- if (type == CrashType::tombstone) {
+ crash->crash_type = request.packet.dump_request.dump_type;
+ if (crash->crash_type < 0 || crash->crash_type > kDebuggerdAnyIntercept) {
+ LOG(WARNING) << "unexpected crash dump type: " << crash->crash_type;
+ goto fail;
+ }
+
+ if (crash->crash_type != kDebuggerdJavaBacktrace) {
crash->crash_pid = request.packet.dump_request.pid;
} else {
// Requests for java traces are sent from untrusted processes, so we
@@ -290,7 +302,7 @@
LOG(INFO) << "received crash request for pid " << crash->crash_pid;
- if (type->maybe_enqueue_crash(crash)) {
+ if (get_crash_queue(crash)->maybe_enqueue_crash(crash)) {
LOG(INFO) << "enqueueing crash request for pid " << crash->crash_pid;
} else {
perform_request(crash);
@@ -307,7 +319,7 @@
Crash* crash = static_cast<Crash*>(arg);
TombstonedCrashPacket request = {};
- crash->crash_type->on_crash_completed();
+ get_crash_queue(crash)->on_crash_completed();
if ((ev & EV_READ) == 0) {
goto fail;
@@ -330,11 +342,11 @@
}
fail:
- CrashType* type = crash->crash_type;
+ CrashQueue* queue = get_crash_queue(crash);
delete crash;
// If there's something queued up, let them proceed.
- type->maybe_dequeue_crashes(perform_request);
+ queue->maybe_dequeue_crashes(perform_request);
}
int main(int, char* []) {
@@ -366,7 +378,7 @@
intercept_manager = new InterceptManager(base, intercept_socket);
evconnlistener* tombstone_listener = evconnlistener_new(
- base, crash_accept_cb, CrashType::tombstone, -1, LEV_OPT_CLOSE_ON_FREE, crash_socket);
+ base, crash_accept_cb, CrashQueue::tombstone, -1, LEV_OPT_CLOSE_ON_FREE, crash_socket);
if (!tombstone_listener) {
LOG(FATAL) << "failed to create evconnlistener for tombstones.";
}
@@ -379,7 +391,7 @@
evutil_make_socket_nonblocking(java_trace_socket);
evconnlistener* java_trace_listener = evconnlistener_new(
- base, crash_accept_cb, CrashType::java_trace, -1, LEV_OPT_CLOSE_ON_FREE, java_trace_socket);
+ base, crash_accept_cb, CrashQueue::java_trace, -1, LEV_OPT_CLOSE_ON_FREE, java_trace_socket);
if (!java_trace_listener) {
LOG(FATAL) << "failed to create evconnlistener for java traces.";
}
diff --git a/debuggerd/tombstoned/tombstoned_client.cpp b/debuggerd/tombstoned/tombstoned_client.cpp
index 39dc6eb..bdb4c1a 100644
--- a/debuggerd/tombstoned/tombstoned_client.cpp
+++ b/debuggerd/tombstoned/tombstoned_client.cpp
@@ -31,10 +31,11 @@
using android::base::unique_fd;
bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* output_fd,
- bool is_native_crash) {
- unique_fd sockfd(socket_local_client(
- (is_native_crash ? kTombstonedCrashSocketName : kTombstonedJavaTraceSocketName),
- ANDROID_SOCKET_NAMESPACE_RESERVED, SOCK_SEQPACKET));
+ DebuggerdDumpType dump_type) {
+ unique_fd sockfd(
+ socket_local_client((dump_type != kDebuggerdJavaBacktrace ? kTombstonedCrashSocketName
+ : kTombstonedJavaTraceSocketName),
+ ANDROID_SOCKET_NAMESPACE_RESERVED, SOCK_SEQPACKET));
if (sockfd == -1) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to connect to tombstoned: %s",
strerror(errno));
@@ -44,6 +45,7 @@
TombstonedCrashPacket packet = {};
packet.packet_type = CrashPacketType::kDumpRequest;
packet.packet.dump_request.pid = pid;
+ packet.packet.dump_request.dump_type = dump_type;
if (TEMP_FAILURE_RETRY(write(sockfd, &packet, sizeof(packet))) != sizeof(packet)) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to write DumpRequest packet: %s",
strerror(errno));