Merge pull request #710 in SNORT/snort3 from more_misc_fixes to master

Squashed commit of the following:

commit a0941af98b9d31cbed6dde9eb1cad27404ce76d4
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Nov 18 13:21:57 2016 -0500

    fix race with multiple packet threads

commit 53d440735a497d58a5ae7c7bbdd9f7443d61d158
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Nov 18 08:10:21 2016 -0500

    create pid file after dropping privs

commit f4e784f86cd395ef70710eec17c32726d7047b62
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Nov 18 08:08:19 2016 -0500

    proper cleanup of port objects in failure cases

commit a6b45aa3d2ddc6ca5de9d1df8fde5148f8b2fdd6
Author: Russ Combs <rucombs@cisco.com>
Date:   Fri Nov 18 07:14:05 2016 -0500

    only print file stats banner if stats available

diff --git a/src/file_api/file_stats.cc b/src/file_api/file_stats.cc
index 79d6cba905f2cac388e7756d4f270df2bd2aae5e..e73f5f21d0a3031e0cf4a7fdef1d368e22dbde58 100644 (file)
--- a/src/file_api/file_stats.cc
+++ b/src/file_api/file_stats.cc
@@ -83,6 +83,7 @@ void file_stats_print()
     if ( !check_total )
         return;
 
+    LogLabel("File Statistics");
     LogLabel("file type stats (files)");
 
     LogMessage("         Type              Download   Upload \n");

diff --git a/src/main/snort.cc b/src/main/snort.cc
index ec9c7d085ed73b4afe1b548e6017c5b8c00738ab..27fd803a5db7c86966a28c3f47d278ede564a6f6 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -323,14 +323,9 @@ void Snort::init(int argc, char** argv)
 // packet passing is done by the driver/hardware.  the goal then is to put as
 // much initialization stuff in Snort::init() as possible and to restrict this
 // function to those things that depend on DAQ startup or non-root user/group.
-//
-// FIXIT-L breaks DAQ_New()/Start() because packet threads won't be root when
-// opening iface
+
 void Snort::drop_privileges()
 {
-    if ( SnortConfig::create_pid_file() )
-        CreatePidFile(snort_main_thread_pid);
-
     /* FIXIT-M X - I have no idea if the chroot functionality actually works. */
     /* Drop the Chrooted Settings */
     if ( !snort_conf->chroot_dir.empty() )
@@ -339,6 +334,9 @@ void Snort::drop_privileges()
     /* Drop privileges if requested, when initialization is done */
     SetUidGid(SnortConfig::get_uid(), SnortConfig::get_gid());
 
+    if ( SnortConfig::create_pid_file() )
+        CreatePidFile(snort_main_thread_pid);
+
     initializing = false;
     privileges_dropped = true;
 }

diff --git a/src/packet_io/sfdaq.cc b/src/packet_io/sfdaq.cc
index 5d257475b2acec14aa91de817452a3902ea9bec9..58476c4261a798ba790e2d8cfee765055007d8f6 100644 (file)
--- a/src/packet_io/sfdaq.cc
+++ b/src/packet_io/sfdaq.cc
@@ -60,6 +60,7 @@ static const DAQ_Module_t* daq_mod = nullptr;
 static DAQ_Mode daq_mode = DAQ_MODE_PASSIVE;
 static uint32_t snap = DEFAULT_PKT_SNAPLEN;
 static bool loaded = false;
+static std::mutex bpf_gate;
 
 // specific for each thread / instance
 static THREAD_LOCAL SFDAQInstance *local_instance = nullptr;
@@ -422,15 +423,15 @@ bool SFDAQInstance::can_whitelist()
 bool SFDAQInstance::set_filter(const char* bpf)
 {
     int err = 0;
-    static std::mutex bpf_gate;
 
-    // doesn't look like the bpf flex scanner is reentrant
-    bpf_gate.lock();
+    // The BPF can be compiled either during daq_set_filter() or daq_start(),
+    // so protect the thread-unsafe BPF scanner/compiler in both places.
 
     if (bpf and *bpf)
+    {
+        std::lock_guard<std::mutex> lock(bpf_gate);
         err = daq_set_filter(daq_mod, daq_hand, bpf);
-
-    bpf_gate.unlock();
+    }
 
     if (err)
         FatalError("Can't set DAQ BPF filter to '%s' (%s)\n",
@@ -441,7 +442,14 @@ bool SFDAQInstance::set_filter(const char* bpf)
 
 bool SFDAQInstance::start()
 {
-    int err = daq_start(daq_mod, daq_hand);
+    int err;
+
+    // The BPF can be compiled either during daq_set_filter() or daq_start(),
+    // so protect the thread-unsafe BPF scanner/compiler in both places.
+    {
+        std::lock_guard<std::mutex> lock(bpf_gate);
+        err = daq_start(daq_mod, daq_hand);
+    }
 
     if (err)
         ErrorMessage("Can't start DAQ (%d) - %s\n", err, daq_get_error(daq_mod, daq_hand));

diff --git a/src/ports/port_object.cc b/src/ports/port_object.cc
index feeb821138b46fbbecf5f2e90efa169a22e465df..341688859e97bcbff3d3c2240040839247a056dc 100644 (file)
--- a/src/ports/port_object.cc
+++ b/src/ports/port_object.cc
@@ -233,7 +233,7 @@ PortObject* PortObjectDup(PortObject* po)
 
     ponew = PortObjectNew();
     if ( !ponew )
-        return 0;
+        return nullptr;
 
     /* Dup the Name */
     if ( po->name )
@@ -251,9 +251,8 @@ PortObject* PortObjectDup(PortObject* po)
             poinew = PortObjectItemDup(poi);
             if (!poinew)
             {
-                snort_free(ponew->name);
-                snort_free(ponew);
-                return 0;
+                PortObjectFree(ponew);
+                return nullptr;
             }
 
             PortObjectAddItem(ponew, poinew, NULL);
@@ -288,7 +287,7 @@ PortObject* PortObjectDupPorts(PortObject* po)
 
     ponew = PortObjectNew();
     if ( !ponew )
-        return 0;
+        return nullptr;
 
     /* Dup the Name */
     if ( po->name )
@@ -306,8 +305,8 @@ PortObject* PortObjectDupPorts(PortObject* po)
             poinew = PortObjectItemDup(poi);
             if (!poinew)
             {
-                snort_free(ponew);
-                return NULL;
+                PortObjectFree(ponew);
+                return nullptr;
             }
             PortObjectAddItem(ponew, poinew, NULL);
         }

diff --git a/src/ports/port_object2.cc b/src/ports/port_object2.cc
index 979169754f285c9afc09ae067306ce5d00e1c7fd..6b2fe60049f955a6033a38d773c497521c41c8c1 100644 (file)
--- a/src/ports/port_object2.cc
+++ b/src/ports/port_object2.cc
@@ -213,7 +213,7 @@ PortObject2* PortObject2Dup(PortObject* po)
 
             if (!poinew)
             {
-                snort_free(ponew);
+                PortObject2Free(ponew);
                 return NULL;
             }
 

diff --git a/src/utils/stats.cc b/src/utils/stats.cc
index 7dec9e82ee1f1ef0d1fbf7b489967bd41806ad81..ab6da0c55187ef53dcd40026c0b64c93f0af5ed2 100644 (file)
--- a/src/utils/stats.cc
+++ b/src/utils/stats.cc
@@ -291,7 +291,6 @@ void DropStats()
     const char* exclude = "daq snort";
     ModuleManager::dump_stats(snort_conf, exclude);
 
-    LogLabel("File Statistics");
     file_stats_print();
 
     LogLabel("Summary Statistics");