From: Shravan Rangarajuvenkata (shrarang) Date: Thu, 25 Jun 2020 15:00:14 +0000 (+0000) Subject: Merge pull request #2282 in SNORT/snort3 from ~SATHIRKA/snort3:navl_reload_memleak... X-Git-Tag: 3.0.2-1~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b7eb37df4738758557b513d6535e206c7630e75f;p=thirdparty%2Fsnort3.git Merge pull request #2282 in SNORT/snort3 from ~SATHIRKA/snort3:navl_reload_memleak to master Squashed commit of the following: commit 18178095f98e17af698d84080a37915241b6a71f Author: Sreeja Athirkandathil Narayanan 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 425958c1a..59374f720 100644 --- 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 2c1585f6e..e6d3a1d9f 100644 --- 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 39ab2bb89..0f7c040a2 100644 --- 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 d46f8bbf7..bdc94ebdc 100644 --- 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 2dee62d28..c70ceedad 100644 --- 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 #include +#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