From 5f87bbdb0afebac6ad46a849e94460dbd0c41014 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 9 Jan 2019 17:01:49 -0800 Subject: [PATCH] debuggerd: switch to base::{Send,Receive}FileDescriptors. Bug: http://b/12204763 Test: debuggerd_test Change-Id: I0be40916214de51ab36fd6bd6d44090a84312e51 --- debuggerd/client/debuggerd_client.cpp | 9 ++- debuggerd/debuggerd_test.cpp | 9 ++- debuggerd/tombstoned/intercept_manager.cpp | 5 +- debuggerd/tombstoned/tombstoned.cpp | 7 ++- debuggerd/tombstoned/tombstoned_client.cpp | 4 +- debuggerd/util.cpp | 64 ---------------------- debuggerd/util.h | 24 -------- 7 files changed, 26 insertions(+), 96 deletions(-) diff --git a/debuggerd/client/debuggerd_client.cpp b/debuggerd/client/debuggerd_client.cpp index 610b96b22..60eb24131 100644 --- a/debuggerd/client/debuggerd_client.cpp +++ b/debuggerd/client/debuggerd_client.cpp @@ -26,6 +26,7 @@ #include +#include #include #include #include @@ -41,6 +42,7 @@ using namespace std::chrono_literals; +using android::base::SendFileDescriptors; using android::base::unique_fd; static bool send_signal(pid_t pid, const DebuggerdDumpType dump_type) { @@ -146,15 +148,16 @@ bool debuggerd_trigger_dump(pid_t tid, DebuggerdDumpType dump_type, unsigned int PLOG(ERROR) << "failed to set pipe buffer size"; } - if (send_fd(set_timeout(sockfd), &req, sizeof(req), std::move(pipe_write)) != sizeof(req)) { + ssize_t rc = SendFileDescriptors(set_timeout(sockfd), &req, sizeof(req), pipe_write.get()); + pipe_write.reset(); + if (rc != sizeof(req)) { PLOG(ERROR) << "libdebuggerd_client: failed to send output fd to tombstoned"; return false; } // Check to make sure we've successfully registered. InterceptResponse response; - ssize_t rc = - TEMP_FAILURE_RETRY(recv(set_timeout(sockfd.get()), &response, sizeof(response), MSG_TRUNC)); + rc = TEMP_FAILURE_RETRY(recv(set_timeout(sockfd.get()), &response, sizeof(response), MSG_TRUNC)); if (rc == 0) { LOG(ERROR) << "libdebuggerd_client: failed to read initial response from tombstoned: " << "timeout reached?"; diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index bea8b43ca..64df53e9e 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -53,6 +54,8 @@ #include "util.h" using namespace std::chrono_literals; + +using android::base::SendFileDescriptors; using android::base::unique_fd; #if defined(__LP64__) @@ -123,12 +126,14 @@ static void tombstoned_intercept(pid_t target_pid, unique_fd* intercept_fd, uniq ASSERT_GE(pipe_buffer_size, 1024 * 1024); - if (send_fd(intercept_fd->get(), &req, sizeof(req), std::move(output_pipe_write)) != sizeof(req)) { + ssize_t rc = SendFileDescriptors(intercept_fd->get(), &req, sizeof(req), output_pipe_write.get()); + output_pipe_write.reset(); + if (rc != sizeof(req)) { FAIL() << "failed to send output fd to tombstoned: " << strerror(errno); } InterceptResponse response; - ssize_t 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) { diff --git a/debuggerd/tombstoned/intercept_manager.cpp b/debuggerd/tombstoned/intercept_manager.cpp index c446dbba8..7d25c50a8 100644 --- a/debuggerd/tombstoned/intercept_manager.cpp +++ b/debuggerd/tombstoned/intercept_manager.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -31,6 +32,7 @@ #include "protocol.h" #include "util.h" +using android::base::ReceiveFileDescriptors; using android::base::unique_fd; static void intercept_close_cb(evutil_socket_t sockfd, short event, void* arg) { @@ -96,7 +98,8 @@ static void intercept_request_cb(evutil_socket_t sockfd, short ev, void* arg) { { unique_fd rcv_fd; InterceptRequest intercept_request; - ssize_t result = recv_fd(sockfd, &intercept_request, sizeof(intercept_request), &rcv_fd); + ssize_t result = + ReceiveFileDescriptors(sockfd, &intercept_request, sizeof(intercept_request), &rcv_fd); if (result == -1) { PLOG(WARNING) << "failed to read from intercept socket"; diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp index ad9206702..bbeb181c3 100644 --- a/debuggerd/tombstoned/tombstoned.cpp +++ b/debuggerd/tombstoned/tombstoned.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -45,6 +46,7 @@ #include "intercept_manager.h" using android::base::GetIntProperty; +using android::base::SendFileDescriptors; using android::base::StringPrintf; using android::base::unique_fd; @@ -224,7 +226,10 @@ static void perform_request(Crash* crash) { TombstonedCrashPacket response = { .packet_type = CrashPacketType::kPerformDump }; - ssize_t rc = send_fd(crash->crash_socket_fd, &response, sizeof(response), std::move(output_fd)); + ssize_t rc = + SendFileDescriptors(crash->crash_socket_fd, &response, sizeof(response), output_fd.get()); + output_fd.reset(); + if (rc == -1) { PLOG(WARNING) << "failed to send response to CrashRequest"; goto fail; diff --git a/debuggerd/tombstoned/tombstoned_client.cpp b/debuggerd/tombstoned/tombstoned_client.cpp index bdb4c1a6f..2c23c98f3 100644 --- a/debuggerd/tombstoned/tombstoned_client.cpp +++ b/debuggerd/tombstoned/tombstoned_client.cpp @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -28,6 +29,7 @@ #include "protocol.h" #include "util.h" +using android::base::ReceiveFileDescriptors; using android::base::unique_fd; bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* output_fd, @@ -53,7 +55,7 @@ bool tombstoned_connect(pid_t pid, unique_fd* tombstoned_socket, unique_fd* outp } unique_fd tmp_output_fd; - ssize_t rc = recv_fd(sockfd, &packet, sizeof(packet), &tmp_output_fd); + ssize_t rc = ReceiveFileDescriptors(sockfd, &packet, sizeof(packet), &tmp_output_fd); if (rc == -1) { async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to read response to DumpRequest packet: %s", strerror(errno)); diff --git a/debuggerd/util.cpp b/debuggerd/util.cpp index 50c5efc99..a37b3b93b 100644 --- a/debuggerd/util.cpp +++ b/debuggerd/util.cpp @@ -24,73 +24,9 @@ #include #include #include -#include #include #include "protocol.h" -using android::base::unique_fd; - -ssize_t send_fd(int sockfd, const void* data, size_t len, unique_fd fd) { - char cmsg_buf[CMSG_SPACE(sizeof(int))]; - - iovec iov = { .iov_base = const_cast(data), .iov_len = len }; - msghdr msg = { - .msg_iov = &iov, .msg_iovlen = 1, .msg_control = cmsg_buf, .msg_controllen = sizeof(cmsg_buf), - }; - auto cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - *reinterpret_cast(CMSG_DATA(cmsg)) = fd.get(); - - return TEMP_FAILURE_RETRY(sendmsg(sockfd, &msg, 0)); -} - -ssize_t recv_fd(int sockfd, void* _Nonnull data, size_t len, unique_fd* _Nullable out_fd) { - char cmsg_buf[CMSG_SPACE(sizeof(int))]; - - iovec iov = { .iov_base = const_cast(data), .iov_len = len }; - msghdr msg = { - .msg_iov = &iov, - .msg_iovlen = 1, - .msg_control = cmsg_buf, - .msg_controllen = sizeof(cmsg_buf), - .msg_flags = 0, - }; - auto cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - - ssize_t result = TEMP_FAILURE_RETRY(recvmsg(sockfd, &msg, 0)); - if (result == -1) { - return -1; - } - - unique_fd fd; - bool received_fd = msg.msg_controllen == sizeof(cmsg_buf); - if (received_fd) { - fd.reset(*reinterpret_cast(CMSG_DATA(cmsg))); - } - - if ((msg.msg_flags & MSG_TRUNC) != 0) { - errno = EFBIG; - return -1; - } else if ((msg.msg_flags & MSG_CTRUNC) != 0) { - errno = ERANGE; - return -1; - } - - if (out_fd) { - *out_fd = std::move(fd); - } else if (received_fd) { - errno = ERANGE; - return -1; - } - - return result; -} - std::string get_process_name(pid_t pid) { std::string result = ""; android::base::ReadFileToString(android::base::StringPrintf("/proc/%d/cmdline", pid), &result); diff --git a/debuggerd/util.h b/debuggerd/util.h index 8260b4496..e96442360 100644 --- a/debuggerd/util.h +++ b/debuggerd/util.h @@ -21,29 +21,5 @@ #include #include -#include - -// *** WARNING *** -// tombstoned's sockets are SOCK_SEQPACKET sockets. -// Short reads are treated as errors and short writes are assumed to not happen. - -// Sends a packet with an attached fd. -ssize_t send_fd(int sockfd, const void* _Nonnull data, size_t len, android::base::unique_fd fd); - -// Receives a packet and optionally, its attached fd. -// If out_fd is non-null, packets can optionally have an attached fd. -// If out_fd is null, received packets must not have an attached fd. -// -// Errors: -// EOVERFLOW: sockfd is SOCK_DGRAM or SOCK_SEQPACKET and buffer is too small. -// The first len bytes of the packet are stored in data, but the -// rest of the packet is dropped. -// ERANGE: too many file descriptors were attached to the packet. -// ENOMSG: not enough file descriptors were attached to the packet. -// -// plus any errors returned by the underlying recvmsg. -ssize_t recv_fd(int sockfd, void* _Nonnull data, size_t len, - android::base::unique_fd* _Nullable out_fd); - std::string get_process_name(pid_t pid); std::string get_thread_name(pid_t tid);