From: davis mcpherson <davis.mcpherson@gmail.com>
Date: Tue, 19 Apr 2016 19:50:46 +0000 (-0400)
Subject: on session cleanup always release all allocated resources (e.g. splitter, reassembler...)
X-Git-Tag: 3.0.0-233~444^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cf2177004aadfd81f503b009543abfef7809ce21;p=thirdparty%2Fsnort3.git

on session cleanup always release all allocated resources (e.g. splitter, reassembler...)
---

diff --git a/src/stream/libtcp/tcp_stream_session.cc b/src/stream/libtcp/tcp_stream_session.cc
index e4abebe25..6773b6461 100644
--- a/src/stream/libtcp/tcp_stream_session.cc
+++ b/src/stream/libtcp/tcp_stream_session.cc
@@ -405,7 +405,7 @@ void TcpStreamSession::SwapPacketHeaderFoo(void)
 void TcpStreamSession::reset(void)
 {
     if (tcp_init)
-        clear_session(2);
+        clear_session(true);
 }
 
 bool TcpStreamSession::setup(Packet*)
@@ -413,8 +413,8 @@ bool TcpStreamSession::setup(Packet*)
     // FIXIT-L this it should not be necessary to reset here
     reset();
 
-    client->init_tracker( );
-    server->init_tracker( );
+    client->init_tcp_state();
+    server->init_tcp_state();
     lws_init = tcp_init = false;
     no_3whs = false;
     pkt_action_mask = ACTION_NOTHING;
@@ -430,14 +430,15 @@ bool TcpStreamSession::setup(Packet*)
 void TcpStreamSession::cleanup(void)
 {
     // this flushes data and then calls TcpSessionClear()
-    cleanup_session(1);
+    cleanup_session( true );
+
 }
 
 void TcpStreamSession::clear(void)
 {
     if ( tcp_init )
         // this does NOT flush data
-        clear_session(1);
+        clear_session( true );
 }
 
 void TcpStreamSession::set_splitter(bool to_server, StreamSplitter* ss)
@@ -502,15 +503,16 @@ void TcpStreamSession::print(void)
     server->print();
 }
 
-void TcpStreamSession::cleanup_session(int, Packet*)
+void TcpStreamSession::clear_session(bool freeAppData)
 {
 }
 
-void TcpStreamSession::set_os_policy(void)
+void TcpStreamSession::cleanup_session(bool freeAppData, Packet* p)
 {
 }
 
-void TcpStreamSession::clear_session(int)
+void TcpStreamSession::set_os_policy(void)
 {
 }
 
+
diff --git a/src/stream/libtcp/tcp_stream_session.h b/src/stream/libtcp/tcp_stream_session.h
index 99bc5f29c..e602f6ae0 100644
--- a/src/stream/libtcp/tcp_stream_session.h
+++ b/src/stream/libtcp/tcp_stream_session.h
@@ -71,7 +71,7 @@ public:
     void SwapPacketHeaderFoo(void);
 
     virtual void update_perf_base_state(char) { }
-    virtual void cleanup_session(int freeApplicationData, Packet* p = nullptr);
+    virtual void cleanup_session(bool freeAppData, Packet* p = nullptr);
 
     // FIXIT - these 2 function names convey no meaning afaict... figure out
     // why are they called and name appropriately...
@@ -148,7 +148,7 @@ public:
 
 protected:
     virtual void set_os_policy(void);
-    virtual void clear_session(int freeApplicationData);
+    virtual void clear_session(bool freeAppData);
 
     TcpStreamTracker* talker = nullptr;
     TcpStreamTracker* listener = nullptr;
diff --git a/src/stream/libtcp/tcp_stream_tracker.h b/src/stream/libtcp/tcp_stream_tracker.h
index 633ee2e4b..693caba5e 100644
--- a/src/stream/libtcp/tcp_stream_tracker.h
+++ b/src/stream/libtcp/tcp_stream_tracker.h
@@ -319,7 +319,9 @@ public:
         return rst_pkt_sent;
     }
 
-    virtual void init_tracker(void) =  0;
+    virtual void init_tcp_state(void) =  0;
+    virtual void init_toolbox(void) = 0;
+
     virtual void print(void) =  0;
     virtual void init_flush_policy(void) =  0;
     virtual void set_splitter(StreamSplitter* ss) =  0;
diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc
index c1bd940f0..c36f14f87 100644
--- a/src/stream/tcp/tcp_session.cc
+++ b/src/stream/tcp/tcp_session.cc
@@ -127,7 +127,7 @@ TcpSession::TcpSession(Flow* flow) : TcpStreamSession(flow)
 TcpSession::~TcpSession(void)
 {
     if (tcp_init)
-        clear_session(1);
+        clear_session(true);
 
     delete client;
     delete server;
@@ -137,6 +137,9 @@ bool TcpSession::setup(Packet* p)
 {
     TcpStreamSession::setup(p);
 
+    client->init_toolbox();
+    server->init_toolbox();
+
     SESSION_STATS_ADD(tcpStats);
     return true;
 }
@@ -181,7 +184,7 @@ void TcpSession::restart(Packet* p)
 // make sense of the code in this file.
 //-------------------------------------------------------------------------
 
-void TcpSession::clear_session(int freeApplicationData)
+void TcpSession::clear_session(bool freeAppData)
 {
     // update stats
     if ( tcp_init )
@@ -198,7 +201,6 @@ void TcpSession::clear_session(int freeApplicationData)
     else if (flow->get_session_flags() & SSNFLAG_TIMEDOUT)
         tcpStats.timeouts++;
 
-
     DebugFormat(DEBUG_STREAM_STATE, "In TcpSessionClear, %lu bytes in use\n", tcp_memcap->used());
 
     if ( client->reassembler )
@@ -215,25 +217,11 @@ void TcpSession::clear_session(int freeApplicationData)
         server->reassembler->purge_segment_list();
     }
 
-
-    // update light-weight state
-    if ( freeApplicationData == 2 )
-    {
-        flow->restart(true);
-
-        paf_reset(&client->paf_state);
-        paf_reset(&server->paf_state);
-        client->reset_splitter();
-        server->reset_splitter();
-    }
-    else
-    {
-        flow->clear(freeApplicationData);
-        paf_clear(&client->paf_state);
-        paf_clear(&server->paf_state);
-        set_splitter(true, nullptr);
-        set_splitter(false, nullptr);
-    }
+    flow->clear(freeAppData);
+    paf_clear(&client->paf_state);
+    paf_clear(&server->paf_state);
+    set_splitter(true, nullptr);
+    set_splitter(false, nullptr);
 
     // generate event for rate filtering
     tel.log_internal_event(INTERNAL_EVENT_SESSION_DEL);
@@ -243,16 +231,14 @@ void TcpSession::clear_session(int freeApplicationData)
     lws_init = tcp_init = false;
 }
 
-void TcpSession::cleanup_session(int freeApplicationData, Packet* p)
+void TcpSession::cleanup_session(bool freeAppData, Packet* p)
 {
-    // FIXIT - this function does both client & server sides...refactor to do one and
-    // call for each
     if ( client->reassembler != nullptr )
         client->reassembler->flush_queued_segments(flow, true, p);
     if ( server->reassembler != nullptr )
         server->reassembler->flush_queued_segments(flow, true, p);
 
-    clear_session(freeApplicationData);
+    clear_session(freeAppData);
 }
 
 void TcpSession::update_perf_base_state(char newState)
@@ -593,7 +579,7 @@ bool TcpSession::handle_syn_on_reset_session(TcpSegmentDescriptor& tsd)
         DebugMessage(DEBUG_STREAM_STATE, "Got SYN pkt on reset ssn, re-SYN-ing\n");
 
         // FIXIT-L this leads to bogus 129:20
-        cleanup_session(2);
+        cleanup_session(true);
 
         if ( tcph->is_rst() )
         {
@@ -1088,14 +1074,14 @@ void TcpSession::cleanup_session_if_expired(Packet* p)
         {
             /* If this one has been reset, delete the TCP
              * portion, and start a new. */
-            cleanup_session(2);
+            cleanup_session(true);
         }
         else
         {
             DebugMessage(DEBUG_STREAM_STATE, "Stream TCP session timedout!\n");
 
             /* Not reset, simply time'd out.  Clean it up */
-            cleanup_session(2);
+            cleanup_session(true);
         }
         tcpStats.timeouts++;
     }
diff --git a/src/stream/tcp/tcp_session.h b/src/stream/tcp/tcp_session.h
index 9c59f4eac..7a27cf51d 100644
--- a/src/stream/tcp/tcp_session.h
+++ b/src/stream/tcp/tcp_session.h
@@ -57,7 +57,7 @@ public:
     void set_extra_data(Packet*, uint32_t /*flag*/) override;
     void clear_extra_data(Packet*, uint32_t /*flag*/) override;
 
-    void cleanup_session(int freeApplicationData, Packet* = nullptr) override;
+    void cleanup_session(bool freeAppData, Packet* = nullptr) override;
 
     void update_perf_base_state(char newState) override;
     TcpStreamTracker::TcpState get_talker_state(void) override;
@@ -81,14 +81,13 @@ private:
     void process_tcp_stream(TcpSegmentDescriptor&);
     int process_tcp_data(TcpSegmentDescriptor&);
     void process_tcp_packet(TcpSegmentDescriptor&);
-    void FinishServerInit(TcpSegmentDescriptor&);
     void swap_trackers();
 
     void NewTcpSessionOnSyn(TcpSegmentDescriptor&);
     void NewTcpSessionOnSynAck(TcpSegmentDescriptor&);
     void set_os_policy() override;
 
-    void clear_session(int freeApplicationData) override;
+    void clear_session(bool freeAppData) override;
 
     int process_dis(Packet*);
     void update_on_3whs_complete(TcpSegmentDescriptor&);
diff --git a/src/stream/tcp/tcp_state_closed.cc b/src/stream/tcp/tcp_state_closed.cc
index 33f3d3c11..a7bff9caa 100644
--- a/src/stream/tcp/tcp_state_closed.cc
+++ b/src/stream/tcp/tcp_state_closed.cc
@@ -193,7 +193,7 @@ bool TcpStateClosed::do_post_sm_packet_actions(TcpSegmentDescriptor& tsd)
         {
             // The last ACK is a part of the session. Delete the session after processing is
             // complete.
-            session.cleanup_session(0, tsd.get_pkt() );
+            session.cleanup_session(false, tsd.get_pkt() );
             flow->session_state |= STREAM_STATE_CLOSED;
             session.set_pkt_action_flag(ACTION_LWSSN_CLOSED);
         }
diff --git a/src/stream/tcp/tcp_state_last_ack.cc b/src/stream/tcp/tcp_state_last_ack.cc
index b489d3ea8..e7f71c291 100644
--- a/src/stream/tcp/tcp_state_last_ack.cc
+++ b/src/stream/tcp/tcp_state_last_ack.cc
@@ -189,7 +189,7 @@ bool TcpStateLastAck::do_post_sm_packet_actions(TcpSegmentDescriptor& tsd)
         {
             // The last ACK is a part of the session. Delete the session after processing is
             // complete.
-            session.cleanup_session(0, tsd.get_pkt() );
+            session.cleanup_session(false, tsd.get_pkt() );
             flow->session_state |= STREAM_STATE_CLOSED;
             session.set_pkt_action_flag(ACTION_LWSSN_CLOSED);
         }
diff --git a/src/stream/tcp/tcp_state_time_wait.cc b/src/stream/tcp/tcp_state_time_wait.cc
index 6bc562008..d10c94daa 100644
--- a/src/stream/tcp/tcp_state_time_wait.cc
+++ b/src/stream/tcp/tcp_state_time_wait.cc
@@ -181,7 +181,7 @@ bool TcpStateTimeWait::do_post_sm_packet_actions(TcpSegmentDescriptor& tsd)
         {
             // The last ACK is a part of the session. Delete the session after processing is
             // complete.
-            session.cleanup_session(0, tsd.get_pkt() );
+            session.cleanup_session(false, tsd.get_pkt() );
             flow->session_state |= STREAM_STATE_CLOSED;
             session.set_pkt_action_flag(ACTION_LWSSN_CLOSED);
         }
diff --git a/src/stream/tcp/tcp_tracker.cc b/src/stream/tcp/tcp_tracker.cc
index 6b794a7bc..7ea12a7a9 100644
--- a/src/stream/tcp/tcp_tracker.cc
+++ b/src/stream/tcp/tcp_tracker.cc
@@ -39,7 +39,7 @@ TcpTracker::~TcpTracker(void)
     delete reassembler;
 }
 
-void TcpTracker::init_tracker(void)
+void TcpTracker::init_tcp_state(void )
 {
     tcp_state = ( client_tracker ) ?
         TcpStreamTracker::TCP_STATE_NONE : TcpStreamTracker::TCP_LISTEN;
@@ -54,7 +54,10 @@ void TcpTracker::init_tracker(void)
     memset(&mac_addr, 0, sizeof(mac_addr));
     mac_addr_valid = false;
     rst_pkt_sent = false;
+}
 
+void TcpTracker::init_toolbox(void)
+{
     delete splitter;
     splitter = nullptr;
     delete normalizer;
diff --git a/src/stream/tcp/tcp_tracker.h b/src/stream/tcp/tcp_tracker.h
index 842996d0a..66b10223e 100644
--- a/src/stream/tcp/tcp_tracker.h
+++ b/src/stream/tcp/tcp_tracker.h
@@ -57,7 +57,7 @@ public:
     TcpTracker(bool);
     virtual ~TcpTracker(void);
 
-    void init_tracker(void) override;
+    void init_tcp_state(void) override;
     void print(void) override;
     void init_flush_policy(void) override;
     void set_splitter(StreamSplitter* ss) override;
@@ -84,6 +84,9 @@ public:
     bool update_on_fin_sent(TcpSegmentDescriptor& tsd) override;
     bool is_segment_seq_valid(TcpSegmentDescriptor& tsd) override;
     void flush_data_on_fin_recv(TcpSegmentDescriptor& tsd) override;
+
+    void init_toolbox(void);
+
 };
 
 #endif