From 5ca97cd358b93c0cdca3509b232350ae5893fe69 Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxincheng@gmail.com>
Date: Mon, 29 Apr 2019 17:03:00 +0800
Subject: [PATCH] crimson/net: misc fixes in v1 read path

* abort when decoding/signature-check failed;
* change log-level to warn;
* free the decoded message when failed;
* add unlikely to read path;

Signed-off-by: Yingxin Cheng <yingxincheng@gmail.com>
---
 src/crimson/net/Errors.cc     |  2 ++
 src/crimson/net/Errors.h      |  1 +
 src/crimson/net/ProtocolV1.cc | 23 ++++++++++++-----------
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/crimson/net/Errors.cc b/src/crimson/net/Errors.cc
index 62d60ce1c7b..76a32527d14 100644
--- a/src/crimson/net/Errors.cc
+++ b/src/crimson/net/Errors.cc
@@ -41,6 +41,8 @@ const std::error_category& net_category()
           return "connection refused";
         case error::connection_reset:
           return "connection reset";
+        case error::corrupted_message:
+          return "corrupted message";
         default:
           return "unknown";
       }
diff --git a/src/crimson/net/Errors.h b/src/crimson/net/Errors.h
index d75082fd864..c5c17dc0e3b 100644
--- a/src/crimson/net/Errors.h
+++ b/src/crimson/net/Errors.h
@@ -28,6 +28,7 @@ enum class error {
   connection_aborted,
   connection_refused,
   connection_reset,
+  corrupted_message,
 };
 
 /// net error category
diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc
index 408f7f0d0fe..22242e95a35 100644
--- a/src/crimson/net/ProtocolV1.cc
+++ b/src/crimson/net/ProtocolV1.cc
@@ -777,27 +777,28 @@ seastar::future<> ProtocolV1::read_message()
       ::decode(m.footer, p);
       auto msg = ::decode_message(nullptr, 0, m.header, m.footer,
                                   m.front, m.middle, m.data, nullptr);
-      if (!msg) {
-	logger().debug("decode message failed");
-	return;
+      if (unlikely(!msg)) {
+        logger().warn("{} decode message failed", conn);
+        throw std::system_error{make_error_code(error::corrupted_message)};
       }
+      constexpr bool add_ref = false; // Message starts with 1 ref
+      // TODO: change MessageRef with foreign_ptr
+      auto msg_ref = MessageRef{msg, add_ref};
+
       if (session_security) {
-	if (session_security->check_message_signature(msg)) {
-	  logger().debug("signature check failed");
-	  return;
-	}
+        if (unlikely(session_security->check_message_signature(msg))) {
+          logger().warn("{} message signature check failed", conn);
+          throw std::system_error{make_error_code(error::corrupted_message)};
+        }
       }
       // TODO: set time stamps
       msg->set_byte_throttler(conn.policy.throttler_bytes);
 
-      if (!conn.update_rx_seq(msg->get_seq())) {
+      if (unlikely(!conn.update_rx_seq(msg->get_seq()))) {
         // skip this message
         return;
       }
 
-      constexpr bool add_ref = false; // Message starts with 1 ref
-      // TODO: change MessageRef with foreign_ptr
-      auto msg_ref = MessageRef{msg, add_ref};
       // start dispatch, ignoring exceptions from the application layer
       seastar::with_gate(pending_dispatch, [this, msg = std::move(msg_ref)] {
           logger().debug("{} <= {}@{} === {}", messenger,
-- 
2.39.5