Merge pull request #1610 in SNORT/snort3 from ~SBAIGAL/snort3:perfmon_event_fix to...

Squashed commit of the following:

commit a3fcf0a70b39bf05ed8ed9f204fd88a42fd8ea81
Author: Steven Baigal (sbaigal) <sbaigal@cisco.com>
Date:   Wed May 15 13:51:26 2019 -0400

    perf_mon: add real timestamp to empty perf_stats data; updated dbus default subscription code and perf_mon event subscirption code to resolve memory leak and invalid event subscription from reloading; moved flow_ip_tracker to thread local

diff --git a/src/flow/test/flow_stash_test.cc b/src/flow/test/flow_stash_test.cc
index 983d9fafd2e529eb6aabb262b0085265a4d49293..5fde609deceedc94ff2102a9760508e7dd9672c1 100644 (file)
--- a/src/flow/test/flow_stash_test.cc
+++ b/src/flow/test/flow_stash_test.cc
@@ -94,13 +94,13 @@ void DataBus::subscribe(const char* key, DataHandler* h)
 {
     DB->_subscribe(key, h);
 }
-void DataBus::subscribe_default(const char* key, DataHandler* h)
+void DataBus::subscribe_default(const char* key, DataHandler* h, SnortConfig* sc)
 {
     DB->_subscribe(key, h);
 }
 
 void DataBus::unsubscribe(const char*, DataHandler*) {}
-void DataBus::unsubscribe_default(const char*, DataHandler*) {}
+void DataBus::unsubscribe_default(const char*, DataHandler*, SnortConfig* sc) {}
 
 void DataBus::publish(const char* key, DataEvent& e, Flow* f)
 {

diff --git a/src/framework/data_bus.cc b/src/framework/data_bus.cc
index ac6259fe67939f9cf62f91610b47fc81f3db78b3..d948cb662e034fe7f7511c0843585b04f828b3aa 100644 (file)
--- a/src/framework/data_bus.cc
+++ b/src/framework/data_bus.cc
@@ -106,9 +106,12 @@ void DataBus::subscribe(const char* key, DataHandler* h)
 }
 
 // for subscribers that need to receive events regardless of active inspection policy
-void DataBus::subscribe_default(const char* key, DataHandler* h)
+void DataBus::subscribe_default(const char* key, DataHandler* h, SnortConfig* sc)
 {
-    get_default_inspection_policy(SnortConfig::get_conf())->dbus._subscribe(key, h);
+    if (sc)
+        get_default_inspection_policy(sc)->dbus._subscribe(key, h);
+    else
+        get_default_inspection_policy(SnortConfig::get_conf())->dbus._subscribe(key, h);
 }
 
 void DataBus::unsubscribe(const char* key, DataHandler* h)
@@ -116,9 +119,12 @@ void DataBus::unsubscribe(const char* key, DataHandler* h)
     get_data_bus()._unsubscribe(key, h);
 }
 
-void DataBus::unsubscribe_default(const char* key, DataHandler* h)
+void DataBus::unsubscribe_default(const char* key, DataHandler* h, SnortConfig* sc)
 {
-    get_default_inspection_policy(SnortConfig::get_conf())->dbus._unsubscribe(key, h);
+    if (sc)
+        get_default_inspection_policy(sc)->dbus._unsubscribe(key, h);
+    else
+        get_default_inspection_policy(SnortConfig::get_conf())->dbus._unsubscribe(key, h);
 }
 
 // notify subscribers of event

diff --git a/src/framework/data_bus.h b/src/framework/data_bus.h
index 5444245b351e0cd01159f5458657ff5209fbdfaa..44eba7c95d134e13edfd8ad20a5a443acd8c0ad2 100644 (file)
--- a/src/framework/data_bus.h
+++ b/src/framework/data_bus.h
@@ -38,6 +38,7 @@ namespace snort
 {
 class Flow;
 struct Packet;
+struct SnortConfig;
 
 class DataEvent
 {
@@ -96,9 +97,9 @@ public:
     void add_mapped_module(const char*);
 
     static void subscribe(const char* key, DataHandler*);
-    static void subscribe_default(const char* key, DataHandler*);
+    static void subscribe_default(const char* key, DataHandler*, SnortConfig* = nullptr);
     static void unsubscribe(const char* key, DataHandler*);
-    static void unsubscribe_default(const char* key, DataHandler*);
+    static void unsubscribe_default(const char* key, DataHandler*, SnortConfig* = nullptr);
     static void publish(const char* key, DataEvent&, Flow* = nullptr);
 
     // convenience methods

diff --git a/src/network_inspectors/perf_monitor/perf_monitor.cc b/src/network_inspectors/perf_monitor/perf_monitor.cc
index 68a54a46fc5a2ebf36600be2171c9ed629d663ec..e53fbd139c698709affb6ff6afe0725b8bcf10de 100644 (file)
--- a/src/network_inspectors/perf_monitor/perf_monitor.cc
+++ b/src/network_inspectors/perf_monitor/perf_monitor.cc
@@ -50,34 +50,17 @@ THREAD_LOCAL SimpleStats pmstats;
 THREAD_LOCAL ProfileStats perfmonStats;
 
 static THREAD_LOCAL std::vector<PerfTracker*>* trackers;
-
+static THREAD_LOCAL FlowIPTracker* flow_ip_tracker = nullptr;
 //-------------------------------------------------------------------------
 // class stuff
 //-------------------------------------------------------------------------
 
+class FlowIPDataHandler;
 class PerfMonitor : public Inspector
 {
 public:
     PerfMonitor(PerfConfig*);
-    ~PerfMonitor() override
-    {
-        if ( perf_idle_handler )
-        {
-            DataBus::unsubscribe_default(THREAD_IDLE_EVENT, perf_idle_handler);
-            delete perf_idle_handler;
-        }
-        if ( perf_rotate_handler )
-        {
-            DataBus::unsubscribe_default(THREAD_ROTATE_EVENT, perf_rotate_handler);
-            delete perf_rotate_handler;
-        }
-        if ( flow_ip_handler )
-        {
-            DataBus::unsubscribe_default(FLOW_STATE_EVENT, flow_ip_handler);
-            delete flow_ip_handler;
-        }
-        delete config;
-    }
+    ~PerfMonitor() override { delete config;}
 
     bool configure(SnortConfig*) override;
     void show(SnortConfig*) override;
@@ -92,21 +75,9 @@ public:
 
     FlowIPTracker* get_flow_ip();
 
-    void reset_perf_idle_handler()
-    { perf_idle_handler = nullptr; }
-
-    void reset_perf_rotate_handler()
-    { perf_rotate_handler = nullptr; }
-
-    void reset_flow_ip_handler()
-    { flow_ip_handler = nullptr; }
-
 private:
     PerfConfig* const config;
-    FlowIPTracker* flow_ip_tracker = nullptr;
-    DataHandler* flow_ip_handler = nullptr;
-    DataHandler* perf_idle_handler;
-    DataHandler* perf_rotate_handler;
+    FlowIPDataHandler* flow_ip_handler = nullptr;
 
     void disable_tracker(size_t);
 };
@@ -114,11 +85,8 @@ private:
 class PerfIdleHandler : public DataHandler
 {
 public:
-    PerfIdleHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
-    { DataBus::subscribe_default(THREAD_IDLE_EVENT, this); }
-
-    ~PerfIdleHandler() override
-    { perf_monitor.reset_perf_idle_handler(); }
+    PerfIdleHandler(PerfMonitor& p, SnortConfig*& sc) : DataHandler(PERF_NAME), perf_monitor(p)
+    { DataBus::subscribe_default(THREAD_IDLE_EVENT, this, sc); }
 
     void handle(DataEvent&, Flow*) override
     { perf_monitor.eval(nullptr); }
@@ -130,11 +98,8 @@ private:
 class PerfRotateHandler : public DataHandler
 {
 public:
-    PerfRotateHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
-    { DataBus::subscribe_default(THREAD_ROTATE_EVENT, this); }
-
-    ~PerfRotateHandler() override
-    { perf_monitor.reset_perf_rotate_handler(); }
+    PerfRotateHandler(PerfMonitor& p, SnortConfig* sc) : DataHandler(PERF_NAME), perf_monitor(p)
+    { DataBus::subscribe_default(THREAD_ROTATE_EVENT, this, sc); }
 
     void handle(DataEvent&, Flow*) override
     { perf_monitor.rotate(); }
@@ -146,11 +111,8 @@ private:
 class FlowIPDataHandler : public DataHandler
 {
 public:
-    FlowIPDataHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
-    { DataBus::subscribe_default(FLOW_STATE_EVENT, this); }
-
-    ~FlowIPDataHandler() override
-    { perf_monitor.reset_flow_ip_handler(); }
+    FlowIPDataHandler(PerfMonitor& p, SnortConfig* sc) : DataHandler(PERF_NAME), perf_monitor(p)
+    { DataBus::subscribe_default(FLOW_STATE_EVENT, this, sc); }
 
     void handle(DataEvent&, Flow* flow) override
     {
@@ -172,7 +134,8 @@ public:
             return;
 
         FlowIPTracker* tracker = perf_monitor.get_flow_ip();
-        tracker->update_state(&flow->client_ip, &flow->server_ip, state);
+        if (tracker)
+            tracker->update_state(&flow->client_ip, &flow->server_ip, state);
     }
 
 private:
@@ -245,6 +208,7 @@ void PerfMonitor::disable_tracker(size_t i)
     if ( tracker == flow_ip_tracker )
     {
         DataBus::unsubscribe_default(FLOW_STATE_EVENT, flow_ip_handler);
+        delete flow_ip_handler;
         flow_ip_tracker = nullptr;
     }
 
@@ -257,15 +221,13 @@ void PerfMonitor::disable_tracker(size_t i)
 // type and version fields immediately after timestamp like seconds, usec,
 // type, version#, data1, data2, ...
 
-bool PerfMonitor::configure(SnortConfig*)
+bool PerfMonitor::configure(SnortConfig* sc)
 {
-    // DataBus deletes these during shutdown, but we may also need to
-    // delete manually those subscriptions came from failed reload
-    perf_idle_handler = new PerfIdleHandler(*this);
-    perf_rotate_handler = new PerfRotateHandler(*this);
+    new PerfIdleHandler(*this, sc);
+    new PerfRotateHandler(*this, sc);
 
     if ( config->perf_flags & PERF_FLOWIP )
-        flow_ip_handler = new FlowIPDataHandler(*this);
+        flow_ip_handler = new FlowIPDataHandler(*this, sc);
 
     return config->resolve();
 }
@@ -312,6 +274,7 @@ void PerfMonitor::tterm()
             trackers->pop_back();
         }
         delete trackers;
+        flow_ip_tracker = nullptr;
     }
 }
 
@@ -378,6 +341,10 @@ bool PerfMonitor::ready_to_process(Packet* p)
     {
         if ((cur_time - sample_time) >= config->sample_interval)
         {
+            if (cnt == 0)
+                for (auto& tracker : *trackers)
+                    tracker->update_time(cur_time);
+
             cnt = 0;
             sample_time = cur_time;
             return true;
@@ -487,8 +454,5 @@ TEST_CASE("Process timing logic", "[perfmon]")
     REQUIRE((perfmon.ready_to_process(&p) == false));
     REQUIRE((perfmon.ready_to_process(&p) == false));
     REQUIRE((perfmon.ready_to_process(&p) == true));
-
-    perfmon.reset_perf_idle_handler();
-    perfmon.reset_perf_rotate_handler();
 }
 #endif

diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.cc b/src/service_inspectors/dce_rpc/dce_smb_utils.cc
index 4251fa9e7947d0a6181ef7e298aff5263d29c1a0..cd2d4b54207570384e56ab099c2e51c1a7c4d78a 100644 (file)
--- a/src/service_inspectors/dce_rpc/dce_smb_utils.cc
+++ b/src/service_inspectors/dce_rpc/dce_smb_utils.cc
@@ -1580,7 +1580,6 @@ static DCE2_Ret DCE2_SmbFileAPIProcess(DCE2_SmbSsnData* ssd,
 
     Profile profile(dce2_smb_pstat_smb_file_api);
     Packet* p = DetectionEngine::get_current_packet();
-    DetectionEngine::get_current_packet();
     FileFlows* file_flows = FileFlows::get_file_flows(p->flow);
     if (!file_flows->file_process(p, data_ptr, (int)data_len, position, upload,
         DCE2_SmbIsVerdictSuspend(upload, position)))