Merge pull request #2282 in SNORT/snort3 from ~SATHIRKA/snort3:navl_reload_memleak...

Squashed commit of the following:

commit 18178095f98e17af698d84080a37915241b6a71f
Author: Sreeja Athirkandathil Narayanan <sathirka@cisco.com>
Date:   Mon Jun 15 13:48:15 2020 -0400

    appid: Delete stale third-party connections when reloading third-party on midstream

diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index 425958c1ae2168c2c273d488dbce8dcc9eeb4598..59374f72042ba8b4208ab288f3cd46e023dd4718 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -750,7 +750,7 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto
                 is_discovery_done = do_tp_discovery(*tp_appid_ctxt, asd, protocol, p,
                     direction, change_bits);
         }
-        else
+        else if (!tp_appid_ctxt->get_tp_reload_in_progress())
             is_discovery_done = do_tp_discovery(*tp_appid_ctxt, asd, protocol, p,
                 direction, change_bits);
     }

diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index 2c1585f6eb2a41694f5c5467a37bb54225f1d91b..e6d3a1d9f4401f1799faca4f04b7ae92d61c4f7e 100644 (file)
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -56,6 +56,7 @@ THREAD_LOCAL const Trace* appid_trace = nullptr;
 
 THREAD_LOCAL ProfileStats appid_perf_stats;
 THREAD_LOCAL AppIdStats appid_stats;
+THREAD_LOCAL bool ThirdPartyAppIdContext::tp_reload_in_progress = false;
 
 static const Parameter s_params[] =
 {
@@ -128,12 +129,13 @@ class ACThirdPartyAppIdContextSwap : public AnalyzerCommand
 {
 public:
     bool execute(Analyzer&, void**) override;
-    ACThirdPartyAppIdContextSwap(ThirdPartyAppIdContext* tp_ctxt, Request& current_request,
-        bool from_shell): tp_ctxt(tp_ctxt),request(current_request),
-        from_shell(from_shell) { }
+    ACThirdPartyAppIdContextSwap(const AppIdInspector& inspector, ThirdPartyAppIdContext* tp_ctxt,
+        Request& current_request, bool from_shell): inspector(inspector),
+        tp_ctxt(tp_ctxt),request(current_request), from_shell(from_shell) { }
     ~ACThirdPartyAppIdContextSwap() override;
     const char* stringify() override { return "THIRD-PARTY_CONTEXT_SWAP"; }
 private:
+    const AppIdInspector& inspector;
     ThirdPartyAppIdContext* tp_ctxt =  nullptr;
     Request& request;
     bool from_shell;
@@ -142,12 +144,13 @@ private:
 bool ACThirdPartyAppIdContextSwap::execute(Analyzer&, void**)
 {
     assert(tp_appid_thread_ctxt);
-    AppIdInspector* inspector = (AppIdInspector*) InspectorManager::get_inspector(MOD_NAME);
-    assert(inspector);
-    ThirdPartyAppIdContext* tp_appid_ctxt = inspector->get_ctxt().get_tp_appid_ctxt();
+    ThirdPartyAppIdContext* tp_appid_ctxt = inspector.get_ctxt().get_tp_appid_ctxt();
     assert(tp_appid_thread_ctxt != tp_appid_ctxt);
+    bool reload_in_progress = tp_appid_thread_ctxt->tfini(true);
+    tp_appid_thread_ctxt->set_tp_reload_in_progress(reload_in_progress);
+    if (reload_in_progress)
+        return false;
     request.respond("== swapping third-party configuration\n", from_shell);
-    tp_appid_thread_ctxt->tfini();
     tp_appid_ctxt->tinit();
     tp_appid_thread_ctxt = tp_appid_ctxt;
 
@@ -213,7 +216,6 @@ static int reload_third_party(lua_State* L)
         current_request.respond("== reload pending; retry\n", from_shell);
         return 0;
     }
-    Swapper::set_reload_in_progress(true);
     current_request.respond(".. reloading third-party\n", from_shell);
     AppIdInspector* inspector = (AppIdInspector*) InspectorManager::get_inspector(MOD_NAME);
     if (!inspector)
@@ -228,8 +230,10 @@ static int reload_third_party(lua_State* L)
         current_request.respond("== reload third-party failed - third-party module doesn't exist\n", from_shell);
         return 0;
     }
+    Swapper::set_reload_in_progress(true);
     ctxt.create_tp_appid_ctxt();
-    main_broadcast_command(new ACThirdPartyAppIdContextSwap(old_ctxt, current_request, from_shell), from_shell);
+    main_broadcast_command(new ACThirdPartyAppIdContextSwap(*inspector, old_ctxt,
+        current_request, from_shell), from_shell);
     return 0;
 }
 

diff --git a/src/network_inspectors/appid/test/appid_discovery_test.cc b/src/network_inspectors/appid/test/appid_discovery_test.cc
index 39ab2bb890ade61306ca8066ecd6ab7059b3b5d8..0f7c040a2afd2222f2d1ae032be869645ad21a93 100644 (file)
--- a/src/network_inspectors/appid/test/appid_discovery_test.cc
+++ b/src/network_inspectors/appid/test/appid_discovery_test.cc
@@ -150,6 +150,7 @@ PegCount* AppIdModule::get_counts() const { return nullptr; }
 ProfileStats* AppIdModule::get_profile() const { return nullptr; }
 void AppIdModule::set_trace(const Trace*) const { }
 const TraceOption* AppIdModule::get_trace_options() const { return nullptr; }
+THREAD_LOCAL bool ThirdPartyAppIdContext::tp_reload_in_progress = false;
 
 // Stubs for config
 static AppIdConfig app_config;

diff --git a/src/network_inspectors/appid/test/tp_mock.cc b/src/network_inspectors/appid/test/tp_mock.cc
index d46f8bbf75d8e07ffeb53d5251079b1098dc0103..bdc94ebdc3d0e28244059fa9bc606c1700eaf4a6 100644 (file)
--- a/src/network_inspectors/appid/test/tp_mock.cc
+++ b/src/network_inspectors/appid/test/tp_mock.cc
@@ -54,7 +54,7 @@ public:
     }
 
     int tinit() override { return 0; }
-    int tfini() override { return 0; }
+    bool tfini(bool) override { return false; }
 };
 
 class ThirdPartyAppIdSessionImpl : public ThirdPartyAppIdSession

diff --git a/src/network_inspectors/appid/tp_appid_module_api.h b/src/network_inspectors/appid/tp_appid_module_api.h
index 2dee62d286c2cad3a55edbc611d19180d73b80e9..c70ceedadc7d950ff9e550256749193005011093 100644 (file)
--- a/src/network_inspectors/appid/tp_appid_module_api.h
+++ b/src/network_inspectors/appid/tp_appid_module_api.h
@@ -23,6 +23,7 @@
 
 #include <vector>
 #include <string>
+#include "main/thread.h"
 #include "tp_appid_types.h"
 
 #define THIRD_PARTY_APPID_API_VERSION 5
@@ -51,10 +52,13 @@ public:
     const std::string& module_name() const { return name; }
 
     virtual int tinit() = 0;
-    virtual int tfini() = 0;
+    virtual bool tfini(bool reload = false) = 0;
 
     virtual const ThirdPartyConfig& get_config() const { return cfg; }
 
+    void set_tp_reload_in_progress(bool value) { tp_reload_in_progress = value; }
+    bool get_tp_reload_in_progress() { return tp_reload_in_progress; }
+
 protected:
     const uint32_t version;
     const std::string name;
@@ -64,6 +68,7 @@ private:
     // No implicit constructor as derived classes need to provide
     // version and name.
     ThirdPartyAppIdContext() : version(0), name("") { }
+    static THREAD_LOCAL bool tp_reload_in_progress;
 };
 
 #endif