From: Michael Altizer (mialtize) <mialtize@cisco.com>
Date: Tue, 2 Jun 2020 16:48:48 +0000 (+0000)
Subject: Merge pull request #2233 in SNORT/snort3 from ~SMINUT/snort3:force_finalize_hp to... 
X-Git-Tag: 3.0.1-5~35
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b9c5523be74da538d09b764c41712dc815a272e3;p=thirdparty%2Fsnort3.git

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

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 4cfcd0744..eafbf1678 100644
--- 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 fd84df72f..96a6eb73c 100644
--- 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 3ea0de62f..65310fed3 100644
--- 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 cd6f86566..0a07f464a 100644
--- 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 7ccab1a72..c9128c23e 100644
--- 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