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 (file)
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -274,7 +274,7 @@ struct JdwpState {
 
  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 (file)
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -1659,7 +1659,7 @@ static std::string DescribeCommand(Request& request) {
  *
  * 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 @@ void JdwpState::ProcessRequest(Request& request, ExpandBuf* pReply) {
    * 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 @@ void JdwpState::ProcessRequest(Request& request, ExpandBuf* pReply) {
 
   /* 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 (file)
--- a/runtime/jdwp/jdwp_main.cc
+++ b/runtime/jdwp/jdwp_main.cc
@@ -122,11 +122,12 @@ void JdwpNetStateBase::Close() {
 }
 
 /*
- * 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 @@ void JdwpState::SendRequest(ExpandBuf* pReq) {
   }
 
   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 @@ bool JdwpState::HandlePacket() {
   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 @@ bool JdwpState::HandlePacket() {
    */
   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 (file)
--- a/runtime/jdwp/jdwp_priv.h
+++ b/runtime/jdwp/jdwp_priv.h
@@ -69,8 +69,8 @@ class JdwpNetStateBase {
 
   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.