From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Fri, 5 Jul 2019 09:32:41 +0000 (+0800)
Subject: crimson/net: prevent reusing the sent messages
X-Git-Tag: v15.1.0~2283^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2d6e77049259daf6521849ab8404b975965fa488;p=ceph.git

crimson/net: prevent reusing the sent messages

We are tracking sent messages by seq, so reusing messages will cause
undefined behavior because seq should not be shared between different
messages.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---

diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc
index 4601b3460b18..093cb1c3288d 100644
--- a/src/crimson/net/ProtocolV1.cc
+++ b/src/crimson/net/ProtocolV1.cc
@@ -691,6 +691,7 @@ ceph::bufferlist ProtocolV1::do_sweep_messages(
   }
 
   std::for_each(msgs.begin(), msgs.begin()+num_msgs, [this, &bl](const MessageRef& msg) {
+    ceph_assert(!msg->get_seq() && "message already has seq");
     msg->set_seq(++conn.out_seq);
     auto& header = msg->get_header();
     header.src = messenger.get_myname();
diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc
index d86f2bcd1d7c..e5bee37a5eba 100644
--- a/src/crimson/net/ProtocolV2.cc
+++ b/src/crimson/net/ProtocolV2.cc
@@ -1368,6 +1368,7 @@ ceph::bufferlist ProtocolV2::do_sweep_messages(
 
     msg->encode(conn.features, 0);
 
+    ceph_assert(!msg->get_seq() && "message already has seq");
     msg->set_seq(++conn.out_seq);
     uint64_t ack_seq = conn.in_seq;
     // ack_left = 0;
diff --git a/src/test/crimson/test_messenger.cc b/src/test/crimson/test_messenger.cc
index 3e22643b0329..f5eb576d2054 100644
--- a/src/test/crimson/test_messenger.cc
+++ b/src/test/crimson/test_messenger.cc
@@ -36,7 +36,6 @@ static seastar::future<> test_echo(unsigned rounds,
         : public ceph::net::Dispatcher,
           public seastar::peering_sharded_service<Server> {
       ceph::net::Messenger *msgr = nullptr;
-      MessageRef msg_pong{new MPing(), false};
       ceph::auth::DummyAuthClientServer dummy_auth;
 
       Dispatcher* get_local_shard() override {
@@ -51,7 +50,7 @@ static seastar::future<> test_echo(unsigned rounds,
           logger().info("server got {}", *m);
         }
         // reply with a pong
-        return c->send(msg_pong);
+        return c->send(make_message<MPing>());
       }
 
       seastar::future<> init(const entity_name_t& name,
@@ -95,7 +94,6 @@ static seastar::future<> test_echo(unsigned rounds,
       ceph::net::Messenger *msgr = nullptr;
       std::map<ceph::net::Connection*, seastar::promise<>> pending_conns;
       std::map<ceph::net::Connection*, PingSessionRef> sessions;
-      MessageRef msg_ping{new MPing(), false};
       ceph::auth::DummyAuthClientServer dummy_auth;
 
       Client(unsigned rounds, double keepalive_ratio)
@@ -230,7 +228,7 @@ static seastar::future<> test_echo(unsigned rounds,
                                 seastar::stop_iteration::no);
                             });
                         } else {
-                          return conn->send(msg_ping)
+                          return conn->send(make_message<MPing>())
                             .then([&count_ping] {
                               count_ping += 1;
                               return seastar::make_ready_future<seastar::stop_iteration>(