Do not send JDWP data in case of error.
When sending JDWP reply packet with an error, we want to ignore any data in the
buffer. While we reflect that in the packet's length (first 4 bytes), we used
to send all data in the buffer. This causes the receiver to read malformed JDWP
packet.
This CL adds "length" argument to JdwpNetStateBase::WritePacket so we control
the amount of data we send. If the reply has no error, it is set to the length
of the buffer. Otherwise it is the length of JDWP packet header indicating no
data is available.
Bug: 13366758
Change-Id: Ib72c10d5faf065fb44901c34b2732496b6667efa
diff --git a/runtime/jdwp/jdwp.h b/runtime/jdwp/jdwp.h
index fec0e31..4c17c96 100644
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -274,7 +274,7 @@
private:
explicit JdwpState(const JdwpOptions* options);
- void ProcessRequest(Request& request, ExpandBuf* pReply);
+ size_t ProcessRequest(Request& request, ExpandBuf* pReply);
bool InvokeInProgress();
bool IsConnected();
void SuspendByPolicy(JdwpSuspendPolicy suspend_policy, JDWP::ObjectId thread_self_id)
diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc
index 5f21098..c2a2b54 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -1659,7 +1659,7 @@
*
* On entry, the JDWP thread is in VMWAIT.
*/
-void JdwpState::ProcessRequest(Request& request, ExpandBuf* pReply) {
+size_t JdwpState::ProcessRequest(Request& request, ExpandBuf* pReply) {
JdwpError result = ERR_NONE;
if (request.GetCommandSet() != kJDWPDdmCmdSet) {
@@ -1728,14 +1728,11 @@
* If we encountered an error, only send the header back.
*/
uint8_t* replyBuf = expandBufGetBuffer(pReply);
+ size_t replyLength = (result == ERR_NONE) ? expandBufGetLength(pReply) : kJDWPHeaderLen;
+ Set4BE(replyBuf + 0, replyLength);
Set4BE(replyBuf + 4, request.GetId());
Set1(replyBuf + 8, kJDWPFlagReply);
Set2BE(replyBuf + 9, result);
- if (result == ERR_NONE) {
- Set4BE(replyBuf + 0, expandBufGetLength(pReply));
- } else {
- Set4BE(replyBuf + 0, kJDWPHeaderLen);
- }
CHECK_GT(expandBufGetLength(pReply), 0U) << GetCommandName(request) << " " << request.GetId();
@@ -1757,6 +1754,8 @@
/* tell the VM that GC is okay again */
self->TransitionFromRunnableToSuspended(old_state);
+
+ return replyLength;
}
/*
diff --git a/runtime/jdwp/jdwp_main.cc b/runtime/jdwp/jdwp_main.cc
index 500585d..77c963f 100644
--- a/runtime/jdwp/jdwp_main.cc
+++ b/runtime/jdwp/jdwp_main.cc
@@ -122,11 +122,12 @@
}
/*
- * Write a packet. Grabs a mutex to assure atomicity.
+ * Write a packet of "length" bytes. Grabs a mutex to assure atomicity.
*/
-ssize_t JdwpNetStateBase::WritePacket(ExpandBuf* pReply) {
+ssize_t JdwpNetStateBase::WritePacket(ExpandBuf* pReply, size_t length) {
MutexLock mu(Thread::Current(), socket_lock_);
- return TEMP_FAILURE_RETRY(write(clientSock, expandBufGetBuffer(pReply), expandBufGetLength(pReply)));
+ DCHECK_LE(length, expandBufGetLength(pReply));
+ return TEMP_FAILURE_RETRY(write(clientSock, expandBufGetBuffer(pReply), length));
}
/*
@@ -173,7 +174,7 @@
}
errno = 0;
- ssize_t actual = netState->WritePacket(pReq);
+ ssize_t actual = netState->WritePacket(pReq, expandBufGetLength(pReq));
if (static_cast<size_t>(actual) != expandBufGetLength(pReq)) {
PLOG(ERROR) << StringPrintf("Failed to send JDWP packet to debugger (%zd of %zu)",
actual, expandBufGetLength(pReq));
@@ -387,8 +388,8 @@
JDWP::Request request(netStateBase->input_buffer_, netStateBase->input_count_);
ExpandBuf* pReply = expandBufAlloc();
- ProcessRequest(request, pReply);
- ssize_t cc = netStateBase->WritePacket(pReply);
+ size_t replyLength = ProcessRequest(request, pReply);
+ ssize_t cc = netStateBase->WritePacket(pReply, replyLength);
/*
* We processed this request and sent its reply. Notify other threads waiting for us they can now
@@ -396,7 +397,7 @@
*/
EndProcessingRequest();
- if (cc != (ssize_t) expandBufGetLength(pReply)) {
+ if (cc != static_cast<ssize_t>(replyLength)) {
PLOG(ERROR) << "Failed sending reply to debugger";
expandBufFree(pReply);
return false;
diff --git a/runtime/jdwp/jdwp_priv.h b/runtime/jdwp/jdwp_priv.h
index 4e6aada..29ad185 100644
--- a/runtime/jdwp/jdwp_priv.h
+++ b/runtime/jdwp/jdwp_priv.h
@@ -69,8 +69,8 @@
void Close();
- ssize_t WritePacket(ExpandBuf* pReply);
- ssize_t WriteBufferedPacket(const std::vector<iovec>& iov);
+ ssize_t WritePacket(ExpandBuf* pReply, size_t length) LOCKS_EXCLUDED(socket_lock_);
+ ssize_t WriteBufferedPacket(const std::vector<iovec>& iov) LOCKS_EXCLUDED(socket_lock_);
int clientSock; // Active connection to debugger.