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 0e387538e3033229dd378fc250e5fe04758f029d..48158b490903551f455ad6d00e78101519ba2c18 100644 (file)
--- 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 1bcc8263f4a6934df0c8d15fa78e22a6cc29e201..4f6fc22b9d68bee3e3b19e0579434dd5c25cc36a 100644 (file)
--- 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.