From: Russ Combs (rucombs) Date: Fri, 18 Nov 2016 20:53:54 +0000 (-0500) Subject: Merge pull request #710 in SNORT/snort3 from more_misc_fixes to master X-Git-Tag: 3.0.0-233~185 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7ef7843d9c89a8ecc53e649cb6b9359e25b314ca;p=thirdparty%2Fsnort3.git Merge pull request #710 in SNORT/snort3 from more_misc_fixes to master Squashed commit of the following: commit a0941af98b9d31cbed6dde9eb1cad27404ce76d4 Author: Russ Combs Date: Fri Nov 18 13:21:57 2016 -0500 fix race with multiple packet threads commit 53d440735a497d58a5ae7c7bbdd9f7443d61d158 Author: Russ Combs Date: Fri Nov 18 08:10:21 2016 -0500 create pid file after dropping privs commit f4e784f86cd395ef70710eec17c32726d7047b62 Author: Russ Combs Date: Fri Nov 18 08:08:19 2016 -0500 proper cleanup of port objects in failure cases commit a6b45aa3d2ddc6ca5de9d1df8fde5148f8b2fdd6 Author: Russ Combs 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 79d6cba90..e73f5f21d 100644 --- 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 ec9c7d085..27fd803a5 100644 --- 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 5d257475b..58476c426 100644 --- 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 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 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 feeb82113..341688859 100644 --- 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 979169754..6b2fe6004 100644 --- 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 7dec9e82e..ab6da0c55 100644 --- 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");