Merge pull request #2233 in SNORT/snort3 from ~SMINUT/snort3:force_finalize_hp to...

Squashed commit of the following:

commit e10265faf6232b58a32581033ca380a3e6d6b171
Author: Silviu Minut <sminut@cisco.com>
Date:   Fri May 29 20:16:09 2020 -0400

    stream_tcp: unconditionally release held packets that have timed out, regardless of flushing

diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index 4cfcd07447f6365e3ab0a7dc8e123dc1737b3dcf..eafbf167859fc300e0905b74027e2d8fcad38511 100644 (file)
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -82,6 +82,7 @@ const PegInfo tcp_pegs[] =
     { CountType::SUM, "held_packets_dropped", "number of held packets dropped" },
     { CountType::SUM, "held_packets_passed", "number of held packets passed" },
     { CountType::SUM, "held_packet_timeouts", "number of held packets that timed out" },
+    { CountType::SUM, "held_packet_purges", "number of held packets that were purged without flushing" },
     { CountType::NOW, "cur_packets_held", "number of packets currently held" },
     { CountType::MAX, "max_packets_held", "maximum number of packets held simultaneously" },
     { CountType::SUM, "partial_flushes", "number of partial flushes initiated" },

diff --git a/src/stream/tcp/tcp_module.h b/src/stream/tcp/tcp_module.h
index fd84df72f8911c31773c46a61483207b19c2bf03..96a6eb73cf53d49c0b33c9176a48420e63e47999 100644 (file)
--- a/src/stream/tcp/tcp_module.h
+++ b/src/stream/tcp/tcp_module.h
@@ -96,6 +96,7 @@ struct TcpStats
     PegCount held_packets_dropped;
     PegCount held_packets_passed;
     PegCount held_packet_timeouts;
+    PegCount held_packet_purges;
     PegCount current_packets_held;
     PegCount max_packets_held;
     PegCount partial_flushes;

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index 3ea0de62f3ec7985684d15c407856f0c9b4b2c6f..65310fed3983213b9ab37a362c78df5b4665dd6e 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -1340,7 +1340,17 @@ uint32_t TcpReassembler::perform_partial_flush(TcpReassemblerState& trs, Flow* f
     DetectionEngine de;
 
     Packet* p = set_packet(flow, trs.packet_dir, trs.server_side);
-    return perform_partial_flush(trs, p);
+    uint32_t result = perform_partial_flush(trs, p);
+
+    // If the held_packet hasn't been released by perform_partial_flush(),
+    // call finalize directly.
+    if ( trs.tracker->is_holding_packet() )
+    {
+        trs.tracker->finalize_held_packet(p);
+        tcpStats.held_packet_purges++;
+    }
+
+    return result;
 }
 
 // No error checking here, so the caller must ensure that p, p->flow and context

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index cd6f86566cb3b3409da11b811cee01c258d88855..0a07f464a1ee3df8bf61489faef54cd2339bc827 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -43,7 +43,7 @@ using namespace snort;
 
 THREAD_LOCAL HeldPacketQueue* hpq = nullptr;
 
-static const HeldPacketQueue::iter_t null_iterator { };
+const std::list<HeldPacket>::iterator TcpStreamTracker::null_iterator { };
 
 const char* tcp_state_names[] =
 {

diff --git a/src/stream/tcp/tcp_stream_tracker.h b/src/stream/tcp/tcp_stream_tracker.h
index 7ccab1a721910ca8d2f96ba695737eede844a746..c9128c23e258d8a5bc2edb2c20885845900e9905 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -291,6 +291,7 @@ public:
     void finalize_held_packet(snort::Packet*);
     void finalize_held_packet(snort::Flow*);
     uint32_t perform_partial_flush();
+    bool is_holding_packet() const { return held_packet != null_iterator; }
 
     // max_remove < 0 means time out all eligible packets.
     // Return whether there are more packets that need to be released.
@@ -366,6 +367,8 @@ protected:
     uint8_t tcp_options_len = 0;
     bool mac_addr_valid = false;
     bool fin_seq_set = false;  // FIXIT-M should be obviated by tcp state
+
+    static const std::list<HeldPacket>::iterator null_iterator;
 };
 
 // <--- note -- the 'state' parameter must be a reference