Merge pull request #2149 in SNORT/snort3 from ~SATHIRKA/snort3:tsan_appid to master

Squashed commit of the following:

commit 9b719e481c49f208ce7997edd841194d7010fc1e
Author: Sreeja Athirkandathil Narayanan <sathirka@cisco.com>
Date:   Thu Mar 26 17:27:58 2020 -0400

    appid: Fixing thread-safety issues in appid

diff --git a/src/network_inspectors/appid/appid_config.cc b/src/network_inspectors/appid/appid_config.cc
index 5e8baa64c75847e3d5aa6b1622fed60ac78cc01f..ce5b5ff8a2e3671e139513505f3a95e443e2908b 100644 (file)
--- a/src/network_inspectors/appid/appid_config.cc
+++ b/src/network_inspectors/appid/appid_config.cc
@@ -47,19 +47,15 @@
 
 using namespace snort;
 
-SnortProtocolId snortId_for_unsynchronized;
-SnortProtocolId snortId_for_ftp_data;
-SnortProtocolId snortId_for_http2;
 
 ThirdPartyAppIdContext* AppIdContext::tp_appid_ctxt = nullptr;
 OdpContext* AppIdContext::odp_ctxt = nullptr;
 
-static void map_app_names_to_snort_ids(SnortConfig* sc)
+static void map_app_names_to_snort_ids(SnortConfig* sc, AppIdConfig& config)
 {
-    /* init globals for snortId compares */
-    snortId_for_unsynchronized = sc->proto_ref->add("unsynchronized");
-    snortId_for_ftp_data = sc->proto_ref->add("ftp-data");
-    snortId_for_http2    = sc->proto_ref->add("http2");
+    config.snortId_for_unsynchronized = sc->proto_ref->add("unsynchronized");
+    config.snortId_for_ftp_data = sc->proto_ref->add("ftp-data");
+    config.snortId_for_http2    = sc->proto_ref->add("http2");
 
     // Have to create SnortProtocolIds during configuration initialization.
     sc->proto_ref->add("rexec");
@@ -116,14 +112,14 @@ bool AppIdContext::init_appid(SnortConfig* sc)
         odp_ctxt->get_service_disco_mgr().initialize();
         LuaDetectorManager::initialize(*this, 1);
         odp_ctxt->initialize();
+
+        // do not reload third party on reload_config()
+        if (!tp_appid_ctxt)
+            tp_appid_ctxt = TPLibHandler::create_tp_appid_ctxt(config, *odp_ctxt);
         once = true;
     }
 
-    // do not reload third party on reload_config()
-    if (!tp_appid_ctxt)
-        tp_appid_ctxt = TPLibHandler::create_tp_appid_ctxt(config, *odp_ctxt);
-
-    map_app_names_to_snort_ids(sc);
+    map_app_names_to_snort_ids(sc, config);
     return true;
 }
 

diff --git a/src/network_inspectors/appid/appid_config.h b/src/network_inspectors/appid/appid_config.h
index e044ef052a3cd5b30c5ebd1ff9ebd96099fd967a..859fc8c95d839d9c5cae5f614fc26b14baedef10 100644 (file)
--- a/src/network_inspectors/appid/appid_config.h
+++ b/src/network_inspectors/appid/appid_config.h
@@ -40,9 +40,6 @@
 
 #define APP_ID_PORT_ARRAY_SIZE  65536
 
-extern SnortProtocolId snortId_for_unsynchronized;
-extern SnortProtocolId snortId_for_ftp_data;
-extern SnortProtocolId snortId_for_http2;
 
 class PatternClientDetector;
 class PatternServiceDetector;
@@ -71,7 +68,9 @@ public:
     size_t memcap = 0;
     bool list_odp_detectors = false;
     bool log_all_sessions = false;
-
+    SnortProtocolId snortId_for_unsynchronized;
+    SnortProtocolId snortId_for_ftp_data;
+    SnortProtocolId snortId_for_http2;
     void show() const;
 };
 

diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc
index 78a0dab64a15659031a2da08da44d38860cbac9b..be44b8d22374f7b2289fff8af9a8dd1e3b207713 100644 (file)
--- a/src/network_inspectors/appid/appid_session.cc
+++ b/src/network_inspectors/appid/appid_session.cc
@@ -39,6 +39,7 @@
 
 #include "app_forecast.h"
 #include "app_info_table.h"
+#include "appid_config.h"
 #include "appid_debug.h"
 #include "appid_dns_session.h"
 #include "appid_http_session.h"
@@ -85,7 +86,7 @@ AppIdSession* AppIdSession::allocate_session(const Packet* p, IpProtocol proto,
     AppIdSession* asd = new AppIdSession(proto, ip, port, *inspector);
     asd->flow = p->flow;
     asd->stats.first_packet_second = p->pkth->ts.tv_sec;
-    asd->snort_protocol_id = snortId_for_unsynchronized;
+    asd->snort_protocol_id = asd->ctxt.config.snortId_for_unsynchronized;
     p->flow->set_flow_data(asd);
     return asd;
 }
@@ -306,13 +307,13 @@ void AppIdSession::sync_with_snort_protocol_id(AppId newAppId, Packet* p)
             // UNKNOWN_PROTOCOL_ID case.
             if (tmp_snort_protocol_id == UNKNOWN_PROTOCOL_ID &&
                 (newAppId == APP_ID_HTTP2))
-                tmp_snort_protocol_id = snortId_for_http2;
+                tmp_snort_protocol_id = ctxt.config.snortId_for_http2;
 
             if (tmp_snort_protocol_id != snort_protocol_id)
             {
                 snort_protocol_id = tmp_snort_protocol_id;
                 if (appidDebug->is_active() &&
-                    tmp_snort_protocol_id == snortId_for_http2)
+                    tmp_snort_protocol_id == ctxt.config.snortId_for_http2)
                     LogMessage("AppIdDbg %s Telling Snort that it's HTTP/2\n",
                         appidDebug->get_debug_session());
 

diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc
index 310b5fff2438870e0fcdcb5c1267cbfe25409b05..b26281ede75da9a33d91c0fff432a8eb23fdd3b3 100644 (file)
--- a/src/network_inspectors/appid/tp_appid_utils.cc
+++ b/src/network_inspectors/appid/tp_appid_utils.cc
@@ -737,7 +737,7 @@ bool do_tp_discovery(ThirdPartyAppIdContext& tp_appid_ctxt, AppIdSession& asd, I
                         tp_app_id = APP_ID_NONE;
                 }
                 if (tp_app_id == APP_ID_SSL &&
-                    (Stream::get_snort_protocol_id(p->flow) == snortId_for_ftp_data))
+                    (Stream::get_snort_protocol_id(p->flow) == asd.ctxt.config.snortId_for_ftp_data))
                 {
                     //  If we see SSL on an FTP data channel set tpAppId back
                     //  to APP_ID_NONE so the FTP preprocessor picks up the flow.