Have adbconnection handle DDMS packets if agent not loaded
If DDMS was being used on a userdebug or eng build it would cause the
libjdwp agent to be loaded on various system processes, most notably
system_server. This would cause a massive performance hit as it tries
to force system_server to jit but, due to security policies, it is
forced to interpreter instead.
To fix this we make it so that (so long as no debugger commands are
issued) DDMS commands are handled without loading the JDWP agent. When
a non-DDMS command is issued we will load and initialize the JDWP
agent which will take over handling DDMS traffic from then on.
This will ensure that in the common (for userdebug) use-case where
processes only encounter DDMS commands the process will not need to
load the full debugger.
Test: ./test.py --host -j50
Test: ./art/tools/run-libjdwp-tests.sh --mode=device
Test: Run ddms monitor on host,
adb shell stop &&
adb shell setprop dalvik.vm.jdwp-provider adbconnection
adb shell start;
Ensure that device does not start to jank
Test: Run ddms monitor on host,
adb shell stop &&
adb shell setprop dalvik.vm.jdwp-provider adbconnection
adb shell start;
Turn off ddms monitor.
Ensure that device does not start to jank
Test: Build and run
Test: use ddms monitor.
Test: Use Android Studio.
Test: Build and debug debuggable app (bandhook-kotlin)
Test: Build and debug non-debuggable app on userdebug build
(bandhook-kotlin)
Test: Debug running system process on userdebug build
(com.android.packageinstaller)
Bug: 62821960
Bug: 72456312
Bug: 72457427
Change-Id: Ib8d2af6c76bd2fac5a4b27e730695b2d016d3583
diff --git a/adbconnection/adbconnection.cc b/adbconnection/adbconnection.cc
index 80cfc83..56677c9 100644
--- a/adbconnection/adbconnection.cc
+++ b/adbconnection/adbconnection.cc
@@ -42,6 +42,7 @@
#include "cutils/sockets.h"
#endif
+#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/eventfd.h>
@@ -49,17 +50,35 @@
namespace adbconnection {
+// Messages sent from the transport
using dt_fd_forward::kListenStartMessage;
using dt_fd_forward::kListenEndMessage;
using dt_fd_forward::kAcceptMessage;
using dt_fd_forward::kCloseMessage;
+// Messages sent to the transport
+using dt_fd_forward::kPerformHandshakeMessage;
+using dt_fd_forward::kSkipHandshakeMessage;
+
using android::base::StringPrintf;
+static constexpr const char kJdwpHandshake[14] = {
+ 'J', 'D', 'W', 'P', '-', 'H', 'a', 'n', 'd', 's', 'h', 'a', 'k', 'e'
+};
+
static constexpr int kEventfdLocked = 0;
static constexpr int kEventfdUnlocked = 1;
static constexpr int kControlSockSendTimeout = 10;
+static constexpr size_t kPacketHeaderLen = 11;
+static constexpr off_t kPacketSizeOff = 0;
+static constexpr off_t kPacketIdOff = 4;
+static constexpr off_t kPacketCommandSetOff = 9;
+static constexpr off_t kPacketCommandOff = 10;
+
+static constexpr uint8_t kDdmCommandSet = 199;
+static constexpr uint8_t kDdmChunkCommand = 1;
+
static AdbConnectionState* gState;
static bool IsDebuggingPossible() {
@@ -116,6 +135,10 @@
shutting_down_(false),
agent_loaded_(false),
agent_listening_(false),
+ agent_has_socket_(false),
+ sent_agent_fds_(false),
+ performed_handshake_(false),
+ notified_ddm_active_(false),
next_ddm_id_(1) {
// Setup the addr.
control_addr_.controlAddrUn.sun_family = AF_UNIX;
@@ -245,10 +268,32 @@
}
void AdbConnectionState::CloseFds() {
- // Lock the write_event_fd so that concurrent PublishDdms will see that the connection is closed.
- ScopedEventFdLock lk(adb_write_event_fd_);
- // shutdown(adb_connection_socket_, SHUT_RDWR);
- adb_connection_socket_.reset();
+ {
+ // Lock the write_event_fd so that concurrent PublishDdms will see that the connection is
+ // closed.
+ ScopedEventFdLock lk(adb_write_event_fd_);
+ // shutdown(adb_connection_socket_, SHUT_RDWR);
+ adb_connection_socket_.reset();
+ }
+
+ // If we didn't load anything we will need to do the handshake again.
+ performed_handshake_ = false;
+
+ // If the agent isn't loaded we might need to tell ddms code the connection is closed.
+ if (!agent_loaded_ && notified_ddm_active_) {
+ NotifyDdms(/*active*/false);
+ }
+}
+
+void AdbConnectionState::NotifyDdms(bool active) {
+ art::ScopedObjectAccess soa(art::Thread::Current());
+ DCHECK_NE(notified_ddm_active_, active);
+ notified_ddm_active_ = active;
+ if (active) {
+ art::Dbg::DdmConnected();
+ } else {
+ art::Dbg::DdmDisconnected();
+ }
}
uint32_t AdbConnectionState::NextDdmId() {
@@ -257,6 +302,13 @@
}
void AdbConnectionState::PublishDdmData(uint32_t type, const art::ArrayRef<const uint8_t>& data) {
+ SendDdmPacket(NextDdmId(), DdmPacketType::kCmd, type, data);
+}
+
+void AdbConnectionState::SendDdmPacket(uint32_t id,
+ DdmPacketType packet_type,
+ uint32_t type,
+ art::ArrayRef<const uint8_t> data) {
// Get the write_event early to fail fast.
ScopedEventFdLock lk(adb_write_event_fd_);
if (adb_connection_socket_ == -1) {
@@ -276,7 +328,7 @@
kJDWPHeaderLen // jdwp command packet size
+ sizeof(uint32_t) // Type
+ sizeof(uint32_t); // length
- std::array<uint8_t, kDdmPacketHeaderSize> pkt;
+ alignas(sizeof(uint32_t)) std::array<uint8_t, kDdmPacketHeaderSize> pkt;
uint8_t* pkt_data = pkt.data();
// Write the length first.
@@ -284,22 +336,35 @@
pkt_data += sizeof(uint32_t);
// Write the id next;
- *reinterpret_cast<uint32_t*>(pkt_data) = htonl(NextDdmId());
+ *reinterpret_cast<uint32_t*>(pkt_data) = htonl(id);
pkt_data += sizeof(uint32_t);
// next the flags. (0 for cmd packet because DDMS).
- *(pkt_data++) = 0;
- // Now the cmd-set
- *(pkt_data++) = kJDWPDdmCmdSet;
- // Now the command
- *(pkt_data++) = kJDWPDdmCmd;
+ *(pkt_data++) = static_cast<uint8_t>(packet_type);
+ switch (packet_type) {
+ case DdmPacketType::kCmd: {
+ // Now the cmd-set
+ *(pkt_data++) = kJDWPDdmCmdSet;
+ // Now the command
+ *(pkt_data++) = kJDWPDdmCmd;
+ break;
+ }
+ case DdmPacketType::kReply: {
+ // This is the error code bytes which are all 0
+ *(pkt_data++) = 0;
+ *(pkt_data++) = 0;
+ }
+ }
+ // These are at unaligned addresses so we need to do them manually.
// now the type.
- *reinterpret_cast<uint32_t*>(pkt_data) = htonl(type);
+ uint32_t net_type = htonl(type);
+ memcpy(pkt_data, &net_type, sizeof(net_type));
pkt_data += sizeof(uint32_t);
// Now the data.size()
- *reinterpret_cast<uint32_t*>(pkt_data) = htonl(data.size());
+ uint32_t net_len = htonl(data.size());
+ memcpy(pkt_data, &net_len, sizeof(net_len));
pkt_data += sizeof(uint32_t);
static uint32_t constexpr kIovSize = 2;
@@ -327,17 +392,16 @@
}
}
-void AdbConnectionState::SendAgentFds() {
- // TODO
+void AdbConnectionState::SendAgentFds(bool require_handshake) {
DCHECK(!sent_agent_fds_);
- char dummy = '!';
+ const char* message = require_handshake ? kPerformHandshakeMessage : kSkipHandshakeMessage;
union {
cmsghdr cm;
char buffer[CMSG_SPACE(dt_fd_forward::FdSet::kDataLength)];
} cm_un;
iovec iov;
- iov.iov_base = &dummy;
- iov.iov_len = 1;
+ iov.iov_base = const_cast<char*>(message);
+ iov.iov_len = strlen(message) + 1;
msghdr msg;
msg.msg_name = nullptr;
@@ -492,6 +556,7 @@
return;
}
while (!shutting_down_ && control_sock_ != -1) {
+ bool should_listen_on_connection = !agent_has_socket_ && !sent_agent_fds_;
struct pollfd pollfds[4] = {
{ sleep_event_fd_, POLLIN, 0 },
// -1 as an fd causes it to be ignored by poll
@@ -501,8 +566,8 @@
{ (adb_connection_socket_ == -1 ? control_sock_ : -1), POLLIN | POLLRDHUP, 0 },
// if we have not loaded the agent either the adb_connection_socket_ is -1 meaning we don't
// have a real connection yet or the socket through adb needs to be listened to for incoming
- // data that the agent can handle.
- { ((!agent_has_socket_ && !sent_agent_fds_) ? adb_connection_socket_ : -1), POLLIN, 0 }
+ // data that the agent or this plugin can handle.
+ { should_listen_on_connection ? adb_connection_socket_ : -1, POLLIN | POLLRDHUP, 0 }
};
int res = TEMP_FAILURE_RETRY(poll(pollfds, 4, -1));
if (res < 0) {
@@ -528,7 +593,7 @@
if (memcmp(kListenStartMessage, buf, sizeof(kListenStartMessage)) == 0) {
agent_listening_ = true;
if (adb_connection_socket_ != -1) {
- SendAgentFds();
+ SendAgentFds(/*require_handshake*/ !performed_handshake_);
}
} else if (memcmp(kListenEndMessage, buf, sizeof(kListenEndMessage)) == 0) {
agent_listening_ = false;
@@ -538,6 +603,8 @@
} else if (memcmp(kAcceptMessage, buf, sizeof(kAcceptMessage)) == 0) {
agent_has_socket_ = true;
sent_agent_fds_ = false;
+ // We will only ever do the handshake once so reset this.
+ performed_handshake_ = false;
} else {
LOG(ERROR) << "Unknown message received from debugger! '" << std::string(buf) << "'";
}
@@ -566,7 +633,9 @@
}
if (maybe_send_fds && agent_loaded_ && agent_listening_) {
VLOG(jdwp) << "Sending fds as soon as we received them.";
- SendAgentFds();
+ // The agent was already loaded so this must be after a disconnection. Therefore have the
+ // transport perform the handshake.
+ SendAgentFds(/*require_handshake*/ true);
}
} else if (FlagsSet(control_sock_poll.revents, POLLRDHUP)) {
// The other end of the adb connection just dropped it.
@@ -578,27 +647,15 @@
} else if (FlagsSet(adb_socket_poll.revents, POLLIN)) {
DCHECK(!agent_has_socket_);
if (!agent_loaded_) {
- DCHECK(!agent_listening_);
- // TODO Should we check in some other way if we are userdebug/eng?
- CHECK(art::Dbg::IsJdwpAllowed());
- // Load the agent now!
- self->AssertNoPendingException();
- art::Runtime::Current()->AttachAgent(/* JNIEnv* */ nullptr,
- MakeAgentArg(),
- /* classloader */ nullptr,
- /*allow_non_debuggable_tooling*/ true);
- if (self->IsExceptionPending()) {
- LOG(ERROR) << "Failed to load agent " << agent_name_;
- art::ScopedObjectAccess soa(self);
- self->GetException()->Dump();
- self->ClearException();
- return;
- }
- agent_loaded_ = true;
+ HandleDataWithoutAgent(self);
} else if (agent_listening_ && !sent_agent_fds_) {
VLOG(jdwp) << "Sending agent fds again on data.";
- SendAgentFds();
+ // Agent was already loaded so it can deal with the handshake.
+ SendAgentFds(/*require_handshake*/ true);
}
+ } else if (FlagsSet(adb_socket_poll.revents, POLLRDHUP)) {
+ DCHECK(!agent_has_socket_);
+ CloseFds();
} else {
VLOG(jdwp) << "Woke up poll without anything to do!";
}
@@ -606,10 +663,175 @@
}
}
+static uint32_t ReadUint32AndAdvance(/*in-out*/uint8_t** in) {
+ uint32_t res;
+ memcpy(&res, *in, sizeof(uint32_t));
+ *in = (*in) + sizeof(uint32_t);
+ return ntohl(res);
+}
+
+void AdbConnectionState::HandleDataWithoutAgent(art::Thread* self) {
+ DCHECK(!agent_loaded_);
+ DCHECK(!agent_listening_);
+ // TODO Should we check in some other way if we are userdebug/eng?
+ CHECK(art::Dbg::IsJdwpAllowed());
+ // We try to avoid loading the agent which is expensive. First lets just perform the handshake.
+ if (!performed_handshake_) {
+ PerformHandshake();
+ return;
+ }
+ // Read the packet header to figure out if it is one we can handle. We only 'peek' into the stream
+ // to see if it's one we can handle. This doesn't change the state of the socket.
+ alignas(sizeof(uint32_t)) uint8_t packet_header[kPacketHeaderLen];
+ ssize_t res = TEMP_FAILURE_RETRY(recv(adb_connection_socket_.get(),
+ packet_header,
+ sizeof(packet_header),
+ MSG_PEEK));
+ // We want to be very careful not to change the socket state until we know we succeeded. This will
+ // let us fall-back to just loading the agent and letting it deal with everything.
+ if (res <= 0) {
+ // Close the socket. We either hit EOF or an error.
+ if (res < 0) {
+ PLOG(ERROR) << "Unable to peek into adb socket due to error. Closing socket.";
+ }
+ CloseFds();
+ return;
+ } else if (res < static_cast<int>(kPacketHeaderLen)) {
+ LOG(ERROR) << "Unable to peek into adb socket. Loading agent to handle this. Only read " << res;
+ AttachJdwpAgent(self);
+ return;
+ }
+ uint32_t full_len = ntohl(*reinterpret_cast<uint32_t*>(packet_header + kPacketSizeOff));
+ uint32_t pkt_id = ntohl(*reinterpret_cast<uint32_t*>(packet_header + kPacketIdOff));
+ uint8_t pkt_cmd_set = packet_header[kPacketCommandSetOff];
+ uint8_t pkt_cmd = packet_header[kPacketCommandOff];
+ if (pkt_cmd_set != kDdmCommandSet ||
+ pkt_cmd != kDdmChunkCommand ||
+ full_len < kPacketHeaderLen) {
+ VLOG(jdwp) << "Loading agent due to jdwp packet that cannot be handled by adbconnection.";
+ AttachJdwpAgent(self);
+ return;
+ }
+ uint32_t avail = -1;
+ res = TEMP_FAILURE_RETRY(ioctl(adb_connection_socket_.get(), FIONREAD, &avail));
+ if (res < 0) {
+ PLOG(ERROR) << "Failed to determine amount of readable data in socket! Closing connection";
+ CloseFds();
+ return;
+ } else if (avail < full_len) {
+ LOG(WARNING) << "Unable to handle ddm command in adbconnection due to insufficent data. "
+ << "Expected " << full_len << " bytes but only " << avail << " are readable. "
+ << "Loading jdwp agent to deal with this.";
+ AttachJdwpAgent(self);
+ return;
+ }
+ // Actually read the data.
+ std::vector<uint8_t> full_pkt;
+ full_pkt.resize(full_len);
+ res = TEMP_FAILURE_RETRY(recv(adb_connection_socket_.get(), full_pkt.data(), full_len, 0));
+ if (res < 0) {
+ PLOG(ERROR) << "Failed to recv data from adb connection. Closing connection";
+ CloseFds();
+ return;
+ }
+ DCHECK_EQ(memcmp(full_pkt.data(), packet_header, sizeof(packet_header)), 0);
+ size_t data_size = full_len - kPacketHeaderLen;
+ if (data_size < (sizeof(uint32_t) * 2)) {
+ // This is an error (the data isn't long enough) but to match historical behavior we need to
+ // ignore it.
+ return;
+ }
+ uint8_t* ddm_data = full_pkt.data() + kPacketHeaderLen;
+ uint32_t ddm_type = ReadUint32AndAdvance(&ddm_data);
+ uint32_t ddm_len = ReadUint32AndAdvance(&ddm_data);
+ if (ddm_len > data_size - (2 * sizeof(uint32_t))) {
+ // This is an error (the data isn't long enough) but to match historical behavior we need to
+ // ignore it.
+ return;
+ }
+
+ if (!notified_ddm_active_) {
+ NotifyDdms(/*active*/ true);
+ }
+ uint32_t reply_type;
+ std::vector<uint8_t> reply;
+ if (!art::Dbg::DdmHandleChunk(self->GetJniEnv(),
+ ddm_type,
+ art::ArrayRef<const jbyte>(reinterpret_cast<const jbyte*>(ddm_data),
+ ddm_len),
+ /*out*/&reply_type,
+ /*out*/&reply)) {
+ // To match historical behavior we don't send any response when there is no data to reply with.
+ return;
+ }
+ SendDdmPacket(pkt_id,
+ DdmPacketType::kReply,
+ reply_type,
+ art::ArrayRef<const uint8_t>(reply));
+}
+
+void AdbConnectionState::PerformHandshake() {
+ CHECK(!performed_handshake_);
+ // Check to make sure we are able to read the whole handshake.
+ uint32_t avail = -1;
+ int res = TEMP_FAILURE_RETRY(ioctl(adb_connection_socket_.get(), FIONREAD, &avail));
+ if (res < 0 || avail < sizeof(kJdwpHandshake)) {
+ if (res < 0) {
+ PLOG(ERROR) << "Failed to determine amount of readable data for handshake!";
+ }
+ LOG(WARNING) << "Closing connection to broken client.";
+ CloseFds();
+ return;
+ }
+ // Perform the handshake.
+ char handshake_msg[sizeof(kJdwpHandshake)];
+ res = TEMP_FAILURE_RETRY(recv(adb_connection_socket_.get(),
+ handshake_msg,
+ sizeof(handshake_msg),
+ MSG_DONTWAIT));
+ if (res < static_cast<int>(sizeof(kJdwpHandshake)) ||
+ strncmp(handshake_msg, kJdwpHandshake, sizeof(kJdwpHandshake)) != 0) {
+ if (res < 0) {
+ PLOG(ERROR) << "Failed to read handshake!";
+ }
+ LOG(WARNING) << "Handshake failed!";
+ CloseFds();
+ return;
+ }
+ // Send the handshake back.
+ res = TEMP_FAILURE_RETRY(send(adb_connection_socket_.get(),
+ kJdwpHandshake,
+ sizeof(kJdwpHandshake),
+ 0));
+ if (res < static_cast<int>(sizeof(kJdwpHandshake))) {
+ PLOG(ERROR) << "Failed to send jdwp-handshake response.";
+ CloseFds();
+ return;
+ }
+ performed_handshake_ = true;
+}
+
+void AdbConnectionState::AttachJdwpAgent(art::Thread* self) {
+ self->AssertNoPendingException();
+ art::Runtime::Current()->AttachAgent(/* JNIEnv */ nullptr,
+ MakeAgentArg(),
+ /* classloader */ nullptr,
+ /*allow_non_debuggable_tooling*/ true);
+ if (self->IsExceptionPending()) {
+ LOG(ERROR) << "Failed to load agent " << agent_name_;
+ art::ScopedObjectAccess soa(self);
+ self->GetException()->Dump();
+ self->ClearException();
+ return;
+ }
+ agent_loaded_ = true;
+}
+
std::string AdbConnectionState::MakeAgentArg() {
// TODO Get this from something user settable?
const std::string& opts = art::Runtime::Current()->GetJdwpOptions();
return agent_name_ + "=" + opts + (opts.empty() ? "" : ",")
+ + "ddm_already_active=" + (notified_ddm_active_ ? "y" : "n") + ","
+ "transport=dt_fd_forward,address=" + std::to_string(remote_agent_control_sock_);
}