Merge "Add ability to handle multiple intercepts per pid." into main
diff --git a/debuggerd/common/include/dump_type.h b/debuggerd/common/include/dump_type.h
index a3e171b..82ef7b6 100644
--- a/debuggerd/common/include/dump_type.h
+++ b/debuggerd/common/include/dump_type.h
@@ -28,26 +28,24 @@
kDebuggerdTombstoneProto,
};
-inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& rhs) {
- switch (rhs) {
+inline const char* get_dump_type_name(const DebuggerdDumpType& dump_type) {
+ switch (dump_type) {
case kDebuggerdNativeBacktrace:
- stream << "kDebuggerdNativeBacktrace";
- break;
+ return "kDebuggerdNativeBacktrace";
case kDebuggerdTombstone:
- stream << "kDebuggerdTombstone";
- break;
+ return "kDebuggerdTombstone";
case kDebuggerdJavaBacktrace:
- stream << "kDebuggerdJavaBacktrace";
- break;
+ return "kDebuggerdJavaBacktrace";
case kDebuggerdAnyIntercept:
- stream << "kDebuggerdAnyIntercept";
- break;
+ return "kDebuggerdAnyIntercept";
case kDebuggerdTombstoneProto:
- stream << "kDebuggerdTombstoneProto";
- break;
+ return "kDebuggerdTombstoneProto";
default:
- stream << "[unknown]";
+ return "[unknown]";
}
+}
+inline std::ostream& operator<<(std::ostream& stream, const DebuggerdDumpType& rhs) {
+ stream << get_dump_type_name(rhs);
return stream;
}
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index 19ff7eb..c3e1302 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -114,7 +114,7 @@
R"(#\d\d pc [0-9a-f]+\s+ \S+ (\(offset 0x[0-9a-f]+\) )?\()" frame_name R"(\+)");
static void tombstoned_intercept(pid_t target_pid, unique_fd* intercept_fd, unique_fd* output_fd,
- InterceptStatus* status, DebuggerdDumpType intercept_type) {
+ InterceptResponse* response, DebuggerdDumpType intercept_type) {
intercept_fd->reset(socket_local_client(kTombstonedInterceptSocketName,
ANDROID_SOCKET_NAMESPACE_RESERVED, SOCK_SEQPACKET));
if (intercept_fd->get() == -1) {
@@ -155,18 +155,15 @@
FAIL() << "failed to send output fd to tombstoned: " << strerror(errno);
}
- InterceptResponse response;
- rc = TEMP_FAILURE_RETRY(read(intercept_fd->get(), &response, sizeof(response)));
+ rc = TEMP_FAILURE_RETRY(read(intercept_fd->get(), response, sizeof(*response)));
if (rc == -1) {
FAIL() << "failed to read response from tombstoned: " << strerror(errno);
} else if (rc == 0) {
FAIL() << "failed to read response from tombstoned (EOF)";
- } else if (rc != sizeof(response)) {
- FAIL() << "received packet of unexpected length from tombstoned: expected " << sizeof(response)
+ } else if (rc != sizeof(*response)) {
+ FAIL() << "received packet of unexpected length from tombstoned: expected " << sizeof(*response)
<< ", received " << rc;
}
-
- *status = response.status;
}
static bool pac_supported() {
@@ -225,9 +222,10 @@
FAIL() << "crasher hasn't been started";
}
- InterceptStatus status;
- tombstoned_intercept(crasher_pid, &this->intercept_fd, output_fd, &status, intercept_type);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(crasher_pid, &this->intercept_fd, output_fd, &response, intercept_type);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
}
void CrasherTest::FinishIntercept(int* result) {
@@ -1744,9 +1742,10 @@
pid_t pid = 123'456'789 + i;
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(pid, &intercept_fd, &output_fd, &status, kDebuggerdTombstone);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(pid, &intercept_fd, &output_fd, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
{
unique_fd tombstoned_socket, input_fd;
@@ -1778,9 +1777,10 @@
pid_t pid = pid_base + dump;
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(pid, &intercept_fd, &output_fd, &status, kDebuggerdTombstone);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(pid, &intercept_fd, &output_fd, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error messeage: " << response.error_message;
// Pretend to crash, and then immediately close the socket.
unique_fd sockfd(socket_local_client(kTombstonedCrashSocketName,
@@ -1811,9 +1811,10 @@
pid_t pid = pid_base + dump;
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(pid, &intercept_fd, &output_fd, &status, kDebuggerdTombstone);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(pid, &intercept_fd, &output_fd, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
{
unique_fd tombstoned_socket, input_fd;
@@ -1838,16 +1839,17 @@
}
}
-TEST(tombstoned, java_trace_intercept_smoke) {
+TEST(tombstoned, intercept_java_trace_smoke) {
// Using a "real" PID is a little dangerous here - if the test fails
// or crashes, we might end up getting a bogus / unreliable stack
// trace.
const pid_t self = getpid();
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(self, &intercept_fd, &output_fd, &status, kDebuggerdJavaBacktrace);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(self, &intercept_fd, &output_fd, &response, kDebuggerdJavaBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
// First connect to tombstoned requesting a native tombstone. This
// should result in a "regular" FD and not the installed intercept.
@@ -1869,25 +1871,96 @@
ASSERT_STREQ("java", outbuf);
}
-TEST(tombstoned, multiple_intercepts) {
+TEST(tombstoned, intercept_multiple_dump_types) {
const pid_t fake_pid = 1'234'567;
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &status, kDebuggerdJavaBacktrace);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdJavaBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
unique_fd intercept_fd_2, output_fd_2;
- tombstoned_intercept(fake_pid, &intercept_fd_2, &output_fd_2, &status, kDebuggerdNativeBacktrace);
- ASSERT_EQ(InterceptStatus::kFailedAlreadyRegistered, status);
+ tombstoned_intercept(fake_pid, &intercept_fd_2, &output_fd_2, &response,
+ kDebuggerdNativeBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+}
+
+TEST(tombstoned, intercept_bad_pid) {
+ const pid_t fake_pid = -1;
+ unique_fd intercept_fd, output_fd;
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdNativeBacktrace);
+ ASSERT_EQ(InterceptStatus::kFailed, response.status)
+ << "Error message: " << response.error_message;
+ ASSERT_MATCH(response.error_message, "bad pid");
+}
+
+TEST(tombstoned, intercept_bad_dump_types) {
+ const pid_t fake_pid = 1'234'567;
+ unique_fd intercept_fd, output_fd;
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response,
+ static_cast<DebuggerdDumpType>(20));
+ ASSERT_EQ(InterceptStatus::kFailed, response.status)
+ << "Error message: " << response.error_message;
+ ASSERT_MATCH(response.error_message, "bad dump type \\[unknown\\]");
+
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdAnyIntercept);
+ ASSERT_EQ(InterceptStatus::kFailed, response.status)
+ << "Error message: " << response.error_message;
+ ASSERT_MATCH(response.error_message, "bad dump type kDebuggerdAnyIntercept");
+
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdTombstoneProto);
+ ASSERT_EQ(InterceptStatus::kFailed, response.status)
+ << "Error message: " << response.error_message;
+ ASSERT_MATCH(response.error_message, "bad dump type kDebuggerdTombstoneProto");
+}
+
+TEST(tombstoned, intercept_already_registered) {
+ const pid_t fake_pid = 1'234'567;
+ unique_fd intercept_fd1, output_fd1;
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd1, &output_fd1, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ unique_fd intercept_fd2, output_fd2;
+ tombstoned_intercept(fake_pid, &intercept_fd2, &output_fd2, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kFailedAlreadyRegistered, response.status)
+ << "Error message: " << response.error_message;
+ ASSERT_MATCH(response.error_message, "already registered, type kDebuggerdTombstone");
+}
+
+TEST(tombstoned, intercept_tombstone_proto_matched_to_tombstone) {
+ const pid_t fake_pid = 1'234'567;
+
+ unique_fd intercept_fd, output_fd;
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ const char data[] = "tombstone_proto";
+ unique_fd tombstoned_socket, input_fd;
+ ASSERT_TRUE(
+ tombstoned_connect(fake_pid, &tombstoned_socket, &input_fd, kDebuggerdTombstoneProto));
+ ASSERT_TRUE(android::base::WriteFully(input_fd.get(), data, sizeof(data)));
+ tombstoned_notify_completion(tombstoned_socket.get());
+
+ char outbuf[sizeof(data)];
+ ASSERT_TRUE(android::base::ReadFully(output_fd.get(), outbuf, sizeof(outbuf)));
+ ASSERT_STREQ("tombstone_proto", outbuf);
}
TEST(tombstoned, intercept_any) {
const pid_t fake_pid = 1'234'567;
unique_fd intercept_fd, output_fd;
- InterceptStatus status;
- tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &status, kDebuggerdNativeBacktrace);
- ASSERT_EQ(InterceptStatus::kRegistered, status);
+ InterceptResponse response = {};
+ tombstoned_intercept(fake_pid, &intercept_fd, &output_fd, &response, kDebuggerdNativeBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
const char any[] = "any";
unique_fd tombstoned_socket, input_fd;
@@ -1900,6 +1973,77 @@
ASSERT_STREQ("any", outbuf);
}
+TEST(tombstoned, intercept_any_failed_with_multiple_intercepts) {
+ const pid_t fake_pid = 1'234'567;
+
+ InterceptResponse response = {};
+ unique_fd intercept_fd1, output_fd1;
+ tombstoned_intercept(fake_pid, &intercept_fd1, &output_fd1, &response, kDebuggerdNativeBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ unique_fd intercept_fd2, output_fd2;
+ tombstoned_intercept(fake_pid, &intercept_fd2, &output_fd2, &response, kDebuggerdJavaBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ unique_fd tombstoned_socket, input_fd;
+ ASSERT_FALSE(tombstoned_connect(fake_pid, &tombstoned_socket, &input_fd, kDebuggerdAnyIntercept));
+}
+
+TEST(tombstoned, intercept_multiple_verify_intercept) {
+ // Need to use our pid for java since that will verify the pid.
+ const pid_t fake_pid = getpid();
+
+ InterceptResponse response = {};
+ unique_fd intercept_fd1, output_fd1;
+ tombstoned_intercept(fake_pid, &intercept_fd1, &output_fd1, &response, kDebuggerdNativeBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ unique_fd intercept_fd2, output_fd2;
+ tombstoned_intercept(fake_pid, &intercept_fd2, &output_fd2, &response, kDebuggerdJavaBacktrace);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ unique_fd intercept_fd3, output_fd3;
+ tombstoned_intercept(fake_pid, &intercept_fd3, &output_fd3, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
+
+ const char native_data[] = "native";
+ unique_fd tombstoned_socket1, input_fd1;
+ ASSERT_TRUE(
+ tombstoned_connect(fake_pid, &tombstoned_socket1, &input_fd1, kDebuggerdNativeBacktrace));
+ ASSERT_TRUE(android::base::WriteFully(input_fd1.get(), native_data, sizeof(native_data)));
+ tombstoned_notify_completion(tombstoned_socket1.get());
+
+ char native_outbuf[sizeof(native_data)];
+ ASSERT_TRUE(android::base::ReadFully(output_fd1.get(), native_outbuf, sizeof(native_outbuf)));
+ ASSERT_STREQ("native", native_outbuf);
+
+ const char java_data[] = "java";
+ unique_fd tombstoned_socket2, input_fd2;
+ ASSERT_TRUE(
+ tombstoned_connect(fake_pid, &tombstoned_socket2, &input_fd2, kDebuggerdJavaBacktrace));
+ ASSERT_TRUE(android::base::WriteFully(input_fd2.get(), java_data, sizeof(java_data)));
+ tombstoned_notify_completion(tombstoned_socket2.get());
+
+ char java_outbuf[sizeof(java_data)];
+ ASSERT_TRUE(android::base::ReadFully(output_fd2.get(), java_outbuf, sizeof(java_outbuf)));
+ ASSERT_STREQ("java", java_outbuf);
+
+ const char tomb_data[] = "tombstone";
+ unique_fd tombstoned_socket3, input_fd3;
+ ASSERT_TRUE(tombstoned_connect(fake_pid, &tombstoned_socket3, &input_fd3, kDebuggerdTombstone));
+ ASSERT_TRUE(android::base::WriteFully(input_fd3.get(), tomb_data, sizeof(tomb_data)));
+ tombstoned_notify_completion(tombstoned_socket3.get());
+
+ char tomb_outbuf[sizeof(tomb_data)];
+ ASSERT_TRUE(android::base::ReadFully(output_fd3.get(), tomb_outbuf, sizeof(tomb_outbuf)));
+ ASSERT_STREQ("tombstone", tomb_outbuf);
+}
+
TEST(tombstoned, interceptless_backtrace) {
// Generate 50 backtraces, and then check to see that we haven't created 50 new tombstones.
auto get_tombstone_timestamps = []() -> std::map<int, time_t> {
@@ -2132,10 +2276,11 @@
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);
+ InterceptResponse response = {};
+ tombstoned_intercept(self, &intercept_fd, &output_fd, &response, kDebuggerdTombstone);
+ ASSERT_EQ(InterceptStatus::kRegistered, response.status)
+ << "Error message: " << response.error_message;
unique_fd tombstoned_socket, text_fd, proto_fd;
ASSERT_TRUE(
diff --git a/debuggerd/protocol.h b/debuggerd/protocol.h
index b60cf5b..212d6dc 100644
--- a/debuggerd/protocol.h
+++ b/debuggerd/protocol.h
@@ -65,7 +65,7 @@
};
enum class InterceptStatus : uint8_t {
- // Returned when an intercept of a different type has already been
+ // Returned when an intercept of the same type has already been
// registered (and is active) for a given PID.
kFailedAlreadyRegistered,
// Returned in all other failure cases.
diff --git a/debuggerd/tombstoned/intercept_manager.cpp b/debuggerd/tombstoned/intercept_manager.cpp
index 613e6f5..ac7b431 100644
--- a/debuggerd/tombstoned/intercept_manager.cpp
+++ b/debuggerd/tombstoned/intercept_manager.cpp
@@ -19,7 +19,10 @@
#include <inttypes.h>
#include <sys/types.h>
+#include <limits>
+#include <memory>
#include <unordered_map>
+#include <utility>
#include <event2/event.h>
#include <event2/listener.h>
@@ -36,8 +39,7 @@
using android::base::unique_fd;
static void intercept_close_cb(evutil_socket_t sockfd, short event, void* arg) {
- auto intercept = reinterpret_cast<Intercept*>(arg);
- InterceptManager* intercept_manager = intercept->intercept_manager;
+ std::unique_ptr<Intercept> intercept(reinterpret_cast<Intercept*>(arg));
CHECK_EQ(sockfd, intercept->sockfd.get());
@@ -46,131 +48,108 @@
// Ownership of intercept differs based on whether we've registered it with InterceptManager.
if (!intercept->registered) {
- delete intercept;
- } else {
- auto it = intercept_manager->intercepts.find(intercept->intercept_pid);
- if (it == intercept_manager->intercepts.end()) {
- LOG(FATAL) << "intercept close callback called after intercept was already removed?";
- }
- if (it->second.get() != intercept) {
- LOG(FATAL) << "intercept close callback has different Intercept from InterceptManager?";
- }
-
- const char* reason;
- if ((event & EV_TIMEOUT) != 0) {
- reason = "due to timeout";
- } else {
- reason = "due to input";
- }
-
- LOG(INFO) << "intercept for pid " << intercept->intercept_pid << " and type "
- << intercept->dump_type << " terminated: " << reason;
- intercept_manager->intercepts.erase(it);
+ LOG(WARNING) << "intercept for pid " << intercept->pid << " and type " << intercept->dump_type
+ << " closed before being registered.";
+ return;
}
+
+ const char* reason = (event & EV_TIMEOUT) ? "due to timeout" : "due to input";
+ LOG(INFO) << "intercept for pid " << intercept->pid << " and type " << intercept->dump_type
+ << " terminated: " << reason;
}
-static bool is_intercept_request_valid(const InterceptRequest& request) {
- if (request.pid <= 0 || request.pid > std::numeric_limits<pid_t>::max()) {
- return false;
+void InterceptManager::Unregister(Intercept* intercept) {
+ CHECK(intercept->registered);
+ auto pid_entry = intercepts.find(intercept->pid);
+ if (pid_entry == intercepts.end()) {
+ LOG(FATAL) << "No intercepts found for pid " << intercept->pid;
+ }
+ auto& dump_type_hash = pid_entry->second;
+ auto dump_type_entry = dump_type_hash.find(intercept->dump_type);
+ if (dump_type_entry == dump_type_hash.end()) {
+ LOG(FATAL) << "Unknown intercept " << intercept->pid << " " << intercept->dump_type;
+ }
+ if (intercept != dump_type_entry->second) {
+ LOG(FATAL) << "Mismatch pointer trying to unregister intercept " << intercept->pid << " "
+ << intercept->dump_type;
}
- if (request.dump_type < 0 || request.dump_type > kDebuggerdJavaBacktrace) {
- return false;
+ dump_type_hash.erase(dump_type_entry);
+ if (dump_type_hash.empty()) {
+ intercepts.erase(pid_entry);
}
-
- return true;
}
static void intercept_request_cb(evutil_socket_t sockfd, short ev, void* arg) {
- auto intercept = reinterpret_cast<Intercept*>(arg);
+ std::unique_ptr<Intercept> intercept(reinterpret_cast<Intercept*>(arg));
InterceptManager* intercept_manager = intercept->intercept_manager;
CHECK_EQ(sockfd, intercept->sockfd.get());
if ((ev & EV_TIMEOUT) != 0) {
LOG(WARNING) << "tombstoned didn't receive InterceptRequest before timeout";
- goto fail;
+ return;
} else if ((ev & EV_READ) == 0) {
LOG(WARNING) << "tombstoned received unexpected event on intercept socket";
- goto fail;
+ return;
}
- {
- unique_fd rcv_fd;
- InterceptRequest intercept_request;
- ssize_t result =
- ReceiveFileDescriptors(sockfd, &intercept_request, sizeof(intercept_request), &rcv_fd);
+ unique_fd rcv_fd;
+ InterceptRequest intercept_request;
+ ssize_t result =
+ ReceiveFileDescriptors(sockfd, &intercept_request, sizeof(intercept_request), &rcv_fd);
- if (result == -1) {
- PLOG(WARNING) << "failed to read from intercept socket";
- goto fail;
- } else if (result != sizeof(intercept_request)) {
- LOG(WARNING) << "intercept socket received short read of length " << result << " (expected "
- << sizeof(intercept_request) << ")";
- goto fail;
- }
-
- // Move the received FD to the upper half, in order to more easily notice FD leaks.
- int moved_fd = fcntl(rcv_fd.get(), F_DUPFD, 512);
- if (moved_fd == -1) {
- LOG(WARNING) << "failed to move received fd (" << rcv_fd.get() << ")";
- goto fail;
- }
- rcv_fd.reset(moved_fd);
-
- // We trust the other side, so only do minimal validity checking.
- if (!is_intercept_request_valid(intercept_request)) {
- InterceptResponse response = {};
- response.status = InterceptStatus::kFailed;
- 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::kFailedAlreadyRegistered;
- snprintf(response.error_message, sizeof(response.error_message),
- "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;
- }
-
- // Let the other side know that the intercept has been registered, now that we know we can't
- // fail. tombstoned is single threaded, so this isn't racy.
- InterceptResponse response = {};
- response.status = InterceptStatus::kRegistered;
- if (TEMP_FAILURE_RETRY(write(sockfd, &response, sizeof(response))) == -1) {
- PLOG(WARNING) << "failed to notify interceptor of registration";
- goto fail;
- }
-
- intercept->output_fd = std::move(rcv_fd);
- intercept_manager->intercepts[intercept_request.pid] = std::unique_ptr<Intercept>(intercept);
- intercept->registered = true;
-
- 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).
- event_assign(intercept->intercept_event, intercept_manager->base, sockfd, EV_READ | EV_TIMEOUT,
- intercept_close_cb, arg);
-
- struct timeval timeout = {.tv_sec = 10 * android::base::HwTimeoutMultiplier(), .tv_usec = 0};
- event_add(intercept->intercept_event, &timeout);
+ if (result == -1) {
+ PLOG(WARNING) << "failed to read from intercept socket";
+ return;
+ }
+ if (result != sizeof(intercept_request)) {
+ LOG(WARNING) << "intercept socket received short read of length " << result << " (expected "
+ << sizeof(intercept_request) << ")";
+ return;
}
- return;
+ // Move the received FD to the upper half, in order to more easily notice FD leaks.
+ int moved_fd = fcntl(rcv_fd.get(), F_DUPFD, 512);
+ if (moved_fd == -1) {
+ LOG(WARNING) << "failed to move received fd (" << rcv_fd.get() << ")";
+ return;
+ }
+ rcv_fd.reset(moved_fd);
-fail:
- delete intercept;
+ // See if we can properly register the intercept.
+ InterceptResponse response = {};
+ if (!intercept_manager->CanRegister(intercept_request, response)) {
+ TEMP_FAILURE_RETRY(write(sockfd, &response, sizeof(response)));
+ LOG(WARNING) << response.error_message;
+ return;
+ }
+
+ // Let the other side know that the intercept has been registered, now that we know we can't
+ // fail. tombstoned is single threaded, so this isn't racy.
+ response.status = InterceptStatus::kRegistered;
+ if (TEMP_FAILURE_RETRY(write(sockfd, &response, sizeof(response))) == -1) {
+ PLOG(WARNING) << "failed to notify interceptor of registration";
+ return;
+ }
+
+ intercept->pid = intercept_request.pid;
+ intercept->dump_type = intercept_request.dump_type;
+ intercept->output_fd = std::move(rcv_fd);
+ intercept_manager->Register(intercept.get());
+
+ 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).
+ event_assign(intercept->intercept_event, intercept_manager->base, sockfd, EV_READ | EV_TIMEOUT,
+ intercept_close_cb, arg);
+
+ // If no request comes in, then this will close the intercept and free the pointer.
+ struct timeval timeout = {.tv_sec = 10 * android::base::HwTimeoutMultiplier(), .tv_usec = 0};
+ event_add(intercept->intercept_event, &timeout);
+ intercept.release();
}
static void intercept_accept_cb(evconnlistener* listener, evutil_socket_t sockfd, sockaddr*, int,
@@ -187,41 +166,97 @@
event_add(intercept_event, &timeout);
}
+Intercept::~Intercept() {
+ event_free(intercept_event);
+ if (registered) {
+ CHECK(intercept_manager != nullptr);
+ intercept_manager->Unregister(this);
+ }
+}
+
InterceptManager::InterceptManager(event_base* base, int intercept_socket) : base(base) {
this->listener = evconnlistener_new(base, intercept_accept_cb, this, LEV_OPT_CLOSE_ON_FREE,
/* backlog */ -1, intercept_socket);
}
-bool dump_types_compatible(DebuggerdDumpType interceptor, DebuggerdDumpType dump) {
- if (interceptor == dump) {
- return true;
+static DebuggerdDumpType canonical_dump_type(const DebuggerdDumpType dump_type) {
+ // kDebuggerdTombstone and kDebuggerdTombstoneProto should be treated as
+ // a single dump_type for intercepts (kDebuggerdTombstone).
+ if (dump_type == kDebuggerdTombstoneProto) {
+ return kDebuggerdTombstone;
}
-
- if (interceptor == kDebuggerdTombstone && dump == kDebuggerdTombstoneProto) {
- return true;
- }
-
- return false;
+ return dump_type;
}
-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()) {
+Intercept* InterceptManager::Get(const pid_t pid, const DebuggerdDumpType dump_type) {
+ auto pid_entry = intercepts.find(pid);
+ if (pid_entry == intercepts.end()) {
+ return nullptr;
+ }
+
+ const auto& dump_type_hash = pid_entry->second;
+ auto dump_type_entry = dump_type_hash.find(canonical_dump_type(dump_type));
+ if (dump_type_entry == dump_type_hash.end()) {
+ if (dump_type != kDebuggerdAnyIntercept) {
+ return nullptr;
+ }
+ // If doing a dump with an any intercept, only allow an any to match
+ // a single intercept. If there are multiple dump types with intercepts
+ // then there would be no way to figure out which to use.
+ if (dump_type_hash.size() != 1) {
+ LOG(WARNING) << "Cannot intercept using kDebuggerdAnyIntercept: there is more than one "
+ "intercept registered for pid "
+ << pid;
+ return nullptr;
+ }
+ dump_type_entry = dump_type_hash.begin();
+ }
+ return dump_type_entry->second;
+}
+
+bool InterceptManager::CanRegister(const InterceptRequest& request, InterceptResponse& response) {
+ if (request.pid <= 0 || request.pid > std::numeric_limits<pid_t>::max()) {
+ response.status = InterceptStatus::kFailed;
+ snprintf(response.error_message, sizeof(response.error_message),
+ "invalid intercept request: bad pid %" PRId32, request.pid);
+ return false;
+ }
+ if (request.dump_type < 0 || request.dump_type > kDebuggerdJavaBacktrace) {
+ response.status = InterceptStatus::kFailed;
+ snprintf(response.error_message, sizeof(response.error_message),
+ "invalid intercept request: bad dump type %s", get_dump_type_name(request.dump_type));
return false;
}
- 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)) {
- LOG(WARNING) << "found non-matching intercept of type " << it->second->dump_type
- << " for requested type: " << dump_type;
+ if (Get(request.pid, request.dump_type) != nullptr) {
+ response.status = InterceptStatus::kFailedAlreadyRegistered;
+ snprintf(response.error_message, sizeof(response.error_message),
+ "pid %" PRId32 " already registered, type %s", request.pid,
+ get_dump_type_name(request.dump_type));
return false;
}
- auto intercept = std::move(it->second);
- this->intercepts.erase(it);
+ return true;
+}
+
+void InterceptManager::Register(Intercept* intercept) {
+ CHECK(!intercept->registered);
+ auto& dump_type_hash = intercepts[intercept->pid];
+ dump_type_hash[canonical_dump_type(intercept->dump_type)] = intercept;
+ intercept->registered = true;
+}
+
+bool InterceptManager::FindIntercept(pid_t pid, DebuggerdDumpType dump_type,
+ android::base::unique_fd* out_fd) {
+ Intercept* intercept = Get(pid, dump_type);
+ if (intercept == nullptr) {
+ return false;
+ }
+
+ if (dump_type != intercept->dump_type) {
+ LOG(INFO) << "found registered intercept of type " << intercept->dump_type
+ << " for requested type " << dump_type;
+ }
LOG(INFO) << "found intercept fd " << intercept->output_fd.get() << " for pid " << pid
<< " and type " << intercept->dump_type;
@@ -230,5 +265,8 @@
TEMP_FAILURE_RETRY(write(intercept->sockfd, &response, sizeof(response)));
*out_fd = std::move(intercept->output_fd);
+ // Delete the intercept data, which will unregister the intercept and remove the timeout event.
+ delete intercept;
+
return true;
}
diff --git a/debuggerd/tombstoned/intercept_manager.h b/debuggerd/tombstoned/intercept_manager.h
index a11d565..dc13aa9 100644
--- a/debuggerd/tombstoned/intercept_manager.h
+++ b/debuggerd/tombstoned/intercept_manager.h
@@ -28,17 +28,17 @@
#include "dump_type.h"
struct InterceptManager;
+struct InterceptRequest;
+struct InterceptResponse;
struct Intercept {
- ~Intercept() {
- event_free(intercept_event);
- }
+ ~Intercept();
InterceptManager* intercept_manager = nullptr;
event* intercept_event = nullptr;
android::base::unique_fd sockfd;
- pid_t intercept_pid = -1;
+ pid_t pid = -1;
android::base::unique_fd output_fd;
bool registered = false;
DebuggerdDumpType dump_type = kDebuggerdNativeBacktrace;
@@ -46,12 +46,17 @@
struct InterceptManager {
event_base* base;
- std::unordered_map<pid_t, std::unique_ptr<Intercept>> intercepts;
+ std::unordered_map<pid_t, std::unordered_map<DebuggerdDumpType, Intercept*>> intercepts;
evconnlistener* listener = nullptr;
InterceptManager(event_base* _Nonnull base, int intercept_socket);
InterceptManager(InterceptManager& copy) = delete;
InterceptManager(InterceptManager&& move) = delete;
- bool GetIntercept(pid_t pid, DebuggerdDumpType dump_type, android::base::unique_fd* out_fd);
+ bool CanRegister(const InterceptRequest& request, InterceptResponse& response);
+ Intercept* Get(const pid_t pid, const DebuggerdDumpType dump_type);
+ void Register(Intercept* intercept);
+ void Unregister(Intercept* intercept);
+
+ bool FindIntercept(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 50558f7..cf7904f 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -283,9 +283,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 (intercept_manager->FindIntercept(crash->crash_pid, crash->crash_type, &output_fd)) {
if (crash->crash_type == kDebuggerdTombstoneProto) {
crash->output.proto = CrashArtifact::devnull();
}