From: Sage Weil <sage@inktank.com>
Date: Sun, 6 Jan 2013 16:38:27 +0000 (-0800)
Subject: msg/Pipe: prepare Message data for wire under pipe_lock
X-Git-Tag: v0.56.1~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9aecacda7fbf07f12b210f87cf3dbb53021b068d;p=ceph.git

msg/Pipe: prepare Message data for wire under pipe_lock

We cannot trust the Message bufferlists or other structures to be
stable without pipe_lock, as another Pipe may claim and modify the sent
list items while we are writing to the socket.

Related to #3678.

Signed-off-by: Sage Weil <sage@inktank.com>
(cherry picked from commit d16ad9263d7b1d3c096f56c56e9631fae8509651)
---

diff --git a/src/msg/Pipe.cc b/src/msg/Pipe.cc
index 0e387538e303..48158b490903 100644
--- a/src/msg/Pipe.cc
+++ b/src/msg/Pipe.cc
@@ -1418,10 +1418,35 @@ void Pipe::writer()
 	// encode and copy out of *m
 	m->encode(connection_state->get_features(), !msgr->cct->_conf->ms_nocrc);
 
+	// prepare everything
+	ceph_msg_header& header = m->get_header();
+	ceph_msg_footer& footer = m->get_footer();
+
+	// Now that we have all the crcs calculated, handle the
+	// digital signature for the message, if the pipe has session
+	// security set up.  Some session security options do not
+	// actually calculate and check the signature, but they should
+	// handle the calls to sign_message and check_signature.  PLR
+	if (session_security == NULL) {
+	  ldout(msgr->cct, 20) << "writer no session security" << dendl;
+	} else {
+	  if (session_security->sign_message(m)) {
+	    ldout(msgr->cct, 20) << "writer failed to sign seq # " << header.seq
+				 << "): sig = " << footer.sig << dendl;
+	  } else {
+	    ldout(msgr->cct, 20) << "writer signed seq # " << header.seq
+				 << "): sig = " << footer.sig << dendl;
+	  }
+	}
+
+	bufferlist blist = m->get_payload();
+	blist.append(m->get_middle());
+	blist.append(m->get_data());
+
 	pipe_lock.Unlock();
 
         ldout(msgr->cct,20) << "writer sending " << m->get_seq() << " " << m << dendl;
-	int rc = write_message(m);
+	int rc = write_message(header, footer, blist);
 
 	pipe_lock.Lock();
 	if (rc < 0) {
@@ -1801,32 +1826,10 @@ int Pipe::write_keepalive()
 }
 
 
-int Pipe::write_message(Message *m)
+int Pipe::write_message(ceph_msg_header& header, ceph_msg_footer& footer, bufferlist& blist)
 {
-  const ceph_msg_header& header = m->get_header();
-  const ceph_msg_footer& footer = m->get_footer();
   int ret;
 
-  // Now that we have all the crcs calculated, handle the digital signature for the message, if the 
-  // pipe has session security set up.  Some session security options do not actually calculate and 
-  // check the signature, but they should handle the calls to sign_message and check_signature.  PLR
-
-  if (session_security == NULL) {
-    ldout(msgr->cct, 20) << "Pipe: write_message:  session security NULL for this pipe." << dendl;
-  } else {
-    if (session_security->sign_message(m)) {
-      ldout(msgr->cct, 20) << "Failed to put signature in client message (seq # " << header.seq << "): sig = " << footer.sig << dendl;
-    } else {
-      ldout(msgr->cct, 20) << "Put signature in client message (seq # " << header.seq << "): sig = " << footer.sig << dendl;
-    }
-  }
-
-  bufferlist blist = m->get_payload();
-  blist.append(m->get_middle());
-  blist.append(m->get_data());
-  
-  ldout(msgr->cct,20)  << "write_message " << m << dendl;
-  
   // set up msghdr and iovecs
   struct msghdr msg;
   memset(&msg, 0, sizeof(msg));
diff --git a/src/msg/Pipe.h b/src/msg/Pipe.h
index 1bcc8263f4a6..4f6fc22b9d68 100644
--- a/src/msg/Pipe.h
+++ b/src/msg/Pipe.h
@@ -177,7 +177,7 @@ class DispatchQueue;
     int randomize_out_seq();
 
     int read_message(Message **pm);
-    int write_message(Message *m);
+    int write_message(ceph_msg_header& h, ceph_msg_footer& f, bufferlist& body);
     /**
      * Write the given data (of length len) to the Pipe's socket. This function
      * will loop until all passed data has been written out.