From: Russ Combs <rucombs@cisco.com>
Date: Mon, 25 Aug 2014 16:15:20 +0000 (-0400)
Subject: threading fixes plus misc
X-Git-Tag: 3.0.0-233~1419^2~10
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e93a2b7caed0563db3ed55823950eede8efb2d8d;p=thirdparty%2Fsnort3.git

threading fixes plus misc
---

diff --git a/ChangeLog b/ChangeLog
index 1f8604f62..848fbc904 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -12,6 +12,8 @@
 -- fixed Xcode warnings
 -- fixed sflsq cursor
 -- additional cleanup
+-- fixed --max-threads configuration
+-- fixed PKT_RESIZED bits
 
 115
 -- remove share.h
diff --git a/doc/style.txt b/doc/style.txt
index 1d0ff80ce..d442b44e7 100644
--- a/doc/style.txt
+++ b/doc/style.txt
@@ -37,9 +37,10 @@ yet firm so feedback is welcome to get something we can live with.
 
 === Logging
 
-* Don't output messages for the coder.  Output messages that make
-  sense to the user.  The function name doesn't belong in the error
-  message.
+* Messages intended for the user should not look like debug messages.  Eg,
+  the function name should not be included.
+
+* Most debug messages should just be deleted.
 
 === Types
 
diff --git a/src/detection/fpcreate.cc b/src/detection/fpcreate.cc
index 1e1fc932c..28d9191cf 100644
--- a/src/detection/fpcreate.cc
+++ b/src/detection/fpcreate.cc
@@ -862,9 +862,6 @@ static bool pmd_can_be_fp(PatternMatchData* pmd, CursorActionType cat)
     if ( !pmd->pattern_buf || !pmd->pattern_size )
         return false;
 
-    if ( pmd->relative )
-        return false;
-
     if ( cat <= CAT_SET_OTHER )
         return false;
 
diff --git a/src/flow/flow.cc b/src/flow/flow.cc
index df9c671bf..499f47aa0 100644
--- a/src/flow/flow.cc
+++ b/src/flow/flow.cc
@@ -111,8 +111,6 @@ void Flow::reset()
 
 void Flow::clear(bool freeAppData)
 {
-    assert(flow_state < 3);
-
     if ( freeAppData )
         free_application_data();
 
diff --git a/src/flow/flow.h b/src/flow/flow.h
index ffacf2cf6..53c21f4cc 100644
--- a/src/flow/flow.h
+++ b/src/flow/flow.h
@@ -214,7 +214,7 @@ public:  // FIXIT-M privatize if possible
     Inspector* gadget;
     const char* service;
 
-    int flow_state;
+    int flow_state;  // FIXIT-H wow - this is poorly encapsulated!  did i do that?  :(
     FlowState s5_state;  // FIXIT-L rename this (s5 not appropriate)
 
     sfip_t client_ip; // FIXIT-L family and bits should be changed to uint16_t
diff --git a/src/framework/inspector.cc b/src/framework/inspector.cc
index 036de9265..7319b9fb9 100644
--- a/src/framework/inspector.cc
+++ b/src/framework/inspector.cc
@@ -29,15 +29,16 @@
 // packet handler stuff
 //-------------------------------------------------------------------------
 
-SO_PUBLIC unsigned Inspector::max_slots = 1;
 SO_PUBLIC unsigned THREAD_LOCAL Inspector::slot = 0;
+unsigned Inspector::max_slots = 1;
 
 SO_PUBLIC Inspector::Inspector()
 {
-    assert(slot < max_slots);
-    ref_count = new unsigned[max_slots];
+    unsigned max = get_instance_max();
+    assert(slot < max);
+    ref_count = new unsigned[max];
 
-    for ( unsigned i = 0; i < max_slots; ++i )
+    for ( unsigned i = 0; i < max; ++i )
         ref_count[i] = 0;
 }
 
@@ -45,7 +46,7 @@ SO_PUBLIC Inspector::~Inspector()
 {
     unsigned total = 0;
 
-    for (unsigned i = 0; i < max_slots; ++i )
+    for (unsigned i = 0; i < get_instance_max(); ++i )
         total += ref_count[i];
 
     assert(!total);
@@ -55,7 +56,7 @@ SO_PUBLIC Inspector::~Inspector()
 
 SO_PUBLIC bool Inspector::is_inactive()
 {
-    for (unsigned i = 0; i < max_slots; ++i )
+    for (unsigned i = 0; i < get_instance_max(); ++i )
         if ( ref_count[i] )
             return false;
     
diff --git a/src/ips_options/ips_http_header.cc b/src/ips_options/ips_http_header.cc
index 38d84f2ad..f9f12258c 100644
--- a/src/ips_options/ips_http_header.cc
+++ b/src/ips_options/ips_http_header.cc
@@ -96,7 +96,7 @@ public:
     { name = s; };
 
     CursorActionType get_cursor_type() const
-    { return CAT_SET_OTHER; };
+    { return CAT_SET_HEADER; };
 
     int eval(Cursor&, Packet*);
 
diff --git a/src/main.cc b/src/main.cc
index 9821d47b4..60822ea35 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -693,7 +693,7 @@ static void snort_main()
     socket_init();
     TimeStart();
 
-    max_pigs = snort_conf->max_threads;
+    max_pigs = get_instance_max();
     assert(max_pigs > 0);
 
     pigs = new Pig[max_pigs];
diff --git a/src/main/snort.cc b/src/main/snort.cc
index 66d6eeb6d..39ec27565 100644
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -888,14 +888,16 @@ DAQ_Verdict packet_callback(
     }
     else
     {
+        Packet* p = &s_packet;
         if ( s_packet.packet_flags & PKT_MODIFIED )
         {
             // this packet was normalized and/or has replacements
             PacketManager::encode_update(&s_packet);
             verdict = DAQ_VERDICT_REPLACE;
         }
-        else if ( s_packet.packet_flags & PKT_RESIZED )
+        else if ( p->packet_flags & PKT_RESIZED )
         {
+            printf("packet flags = 0x%X\n", p->packet_flags);
             // we never increase, only trim, but
             // daq doesn't support resizing wire packet
             if ( !DAQ_Inject(s_packet.pkth, 0, s_packet.pkt, s_packet.pkth->pktlen) )
diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc
index 81ba4bd6b..f0aaa4028 100644
--- a/src/main/snort_config.cc
+++ b/src/main/snort_config.cc
@@ -24,8 +24,6 @@
 #include "config.h"
 #endif
 
-#include <thread>
-
 #include "snort_types.h"
 #include "detection/treenodes.h"
 #include "events/event_queue.h"
@@ -181,13 +179,11 @@ SnortConfig * SnortConfNew(void)
     sc->max_metadata_services = DEFAULT_MAX_METADATA_SERVICES;
     sc->mpls_stack_depth = DEFAULT_LABELCHAIN_LENGTH;
 
-    sc->max_threads = 1;
     InspectorManager::new_config(sc);
 
     sc->var_list = NULL;
 
-    sc->state = (SnortState*)SnortAlloc(
-        sizeof(SnortState)*sc->max_threads);
+    sc->state = (SnortState*)SnortAlloc(sizeof(SnortState)*get_instance_max());
 
     sc->policy_map = new PolicyMap();
 
@@ -450,12 +446,6 @@ SnortConfig* MergeSnortConfs(SnortConfig *cmd_line, SnortConfig *config_file)
     if (cmd_line->run_flags & RUN_FLAG__PROCESS_ALL_EVENTS)
         config_file->event_queue_config->process_all_events = 1;
 
-    if ( cmd_line->max_threads )
-        config_file->max_threads = cmd_line->max_threads;
-
-    if ( config_file->max_threads <= 0 )
-        config_file->max_threads = std::thread::hardware_concurrency();
-
     if ( cmd_line->remote_control )
         config_file->remote_control = cmd_line->remote_control;
 
@@ -468,7 +458,7 @@ SnortConfig* MergeSnortConfs(SnortConfig *cmd_line, SnortConfig *config_file)
 
     free(config_file->state);
     config_file->state = (SnortState*)SnortAlloc(
-        sizeof(SnortState)*config_file->max_threads);
+        sizeof(SnortState)*get_instance_max());
 
     return config_file;
 }
diff --git a/src/main/snort_config.h b/src/main/snort_config.h
index a7963b0fb..3a3e9213c 100644
--- a/src/main/snort_config.h
+++ b/src/main/snort_config.h
@@ -251,7 +251,6 @@ struct SnortConfig
 
     struct VarNode* var_list;
 
-    int max_threads;
     unsigned remote_control;
 
     SnortState* state;
diff --git a/src/main/snort_module.cc b/src/main/snort_module.cc
index 81abda16f..fa4f1525a 100644
--- a/src/main/snort_module.cc
+++ b/src/main/snort_module.cc
@@ -503,11 +503,8 @@ bool SnortModule::set(const char*, Value& v, SnortConfig* sc)
         ConfigShowYear(sc, v.get_string());
 
     else if ( v.is("-z") || v.is("--max-packet-threads") )
-    {
-        sc->max_threads = v.get_long();
-        if ( !sc->max_threads )
-            sc->max_threads = -1; // max
-    }
+        set_instance_max(v.get_long());
+
     else if ( v.is("--alert-before-pass") )
         ConfigAlertBeforePass(sc, v.get_string());
 
diff --git a/src/main/thread.cc b/src/main/thread.cc
index d56496465..8e0854987 100644
--- a/src/main/thread.cc
+++ b/src/main/thread.cc
@@ -21,6 +21,7 @@
 #include "thread.h"
 
 #include <sys/stat.h>
+#include <thread>
 #include "snort.h"
 
 //-------------------------------------------------------------------------
@@ -29,6 +30,7 @@
 // works for now.
 //-------------------------------------------------------------------------
 
+static unsigned instance_max = 1;
 static THREAD_LOCAL unsigned instance_id = 0;
 
 void set_instance_id(unsigned id)
@@ -36,6 +38,14 @@ void set_instance_id(unsigned id)
     instance_id = id;
 }
 
+void set_instance_max(unsigned max)
+{
+    if ( max )
+        instance_max = max;
+    else
+        instance_max = std::thread::hardware_concurrency();
+}
+
 unsigned get_instance_id()
 {
     return instance_id;
@@ -43,7 +53,7 @@ unsigned get_instance_id()
 
 unsigned get_instance_max()
 {
-    return snort_conf->max_threads;
+    return instance_max;
 }
 
 //-------------------------------------------------------------------------
diff --git a/src/main/thread.h b/src/main/thread.h
index 01ca5f5c5..3b9023761 100644
--- a/src/main/thread.h
+++ b/src/main/thread.h
@@ -35,8 +35,11 @@
 //#define THREAD_LOCAL thread_local
 
 void set_instance_id(unsigned);
+void set_instance_max(unsigned);
+
 unsigned get_instance_id();
 unsigned get_instance_max();
+
 const char* get_instance_file(std::string&, const char* name);
 
 #endif
diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc
index e9f82bcd3..0a73db21a 100644
--- a/src/managers/inspector_manager.cc
+++ b/src/managers/inspector_manager.cc
@@ -138,6 +138,9 @@ struct FrameworkPolicy
     PHVector generic;
     PHVector service;
 
+    Inspector* binder;
+    Inspector* wizard;
+
     void vectorize();
 };
 
@@ -171,7 +174,13 @@ void FrameworkPolicy::vectorize()
             break;
 
         case IT_BINDER:
+            binder = p->handler;
+            break;
+
         case IT_WIZARD:
+            wizard = p->handler;
+            break;
+
         case IT_MAX:
             break;
         }
@@ -258,6 +267,9 @@ void InspectorManager::empty_trash()
 void InspectorManager::new_policy (InspectionPolicy* pi)
 {
     pi->framework_policy = new FrameworkPolicy;
+
+    pi->framework_policy->binder = nullptr;
+    pi->framework_policy->wizard = nullptr;
 }
 
 void InspectorManager::delete_policy (InspectionPolicy* pi)
@@ -296,6 +308,7 @@ static PHInstance* get_new(
         delete p;
         return NULL;
     }
+
     fp->ilist.push_back(p);
     return p;
 }
@@ -307,6 +320,26 @@ void InspectorManager::dispatch_meta (FrameworkPolicy* fp, int type, const uint8
         p->handler->meta(type, data);
 }
 
+Inspector* InspectorManager::get_binder()
+{
+    InspectionPolicy* pi = get_inspection_policy();
+
+    if ( !pi || !pi->framework_policy )
+        return nullptr;
+
+    return pi->framework_policy->binder;
+} 
+
+Inspector* InspectorManager::get_wizard()
+{
+    InspectionPolicy* pi = get_inspection_policy();
+
+    if ( !pi || !pi->framework_policy )
+        return nullptr;
+
+    return pi->framework_policy->wizard;
+} 
+
 Inspector* InspectorManager::get_inspector(const char* key)
 {
     InspectionPolicy* pi = get_inspection_policy();
@@ -377,24 +410,28 @@ void InspectorManager::thread_init(SnortConfig* sc)
 {
     Inspector::slot = get_instance_id();
 
-    InspectionPolicy* pi = get_inspection_policy();
+    for ( auto* p : sc->framework_config->clist )
+        if ( p->api.tinit )
+            p->api.tinit();
 
-    if ( !pi || !pi->framework_policy )
-        return;
+    InspectionPolicy* pi = get_inspection_policy();
 
-    for ( auto* p : pi->framework_policy->ilist )
-        p->handler->tinit();
+    if ( pi && pi->framework_policy )
+    {
+        for ( auto* p : pi->framework_policy->ilist )
+            p->handler->tinit();
+    }
 }
 
 void InspectorManager::thread_term(SnortConfig* sc)
 {
     InspectionPolicy* pi = get_inspection_policy();
 
-    if ( !pi || !pi->framework_policy )
-        return;
-
-    for ( auto* p : pi->framework_policy->ilist )
-        p->handler->tterm();
+    if ( pi && pi->framework_policy )
+    {
+        for ( auto* p : pi->framework_policy->ilist )
+            p->handler->tterm();
+    }
 
     for ( auto* p : sc->framework_config->clist )
         if ( p->api.tterm )
@@ -434,7 +471,6 @@ void InspectorManager::instantiate(
 
 bool InspectorManager::configure(SnortConfig *sc)
 {
-    Inspector::max_slots = sc->max_threads;
     sort(s_handlers.begin(), s_handlers.end(), PHGlobal::comp);
 
     FrameworkPolicy* fp = sc->policy_map->inspection_policy[0]->framework_policy;
@@ -493,7 +529,7 @@ void InspectorManager::bumble(Packet* p)
     if ( !flow->service )
         return;
 
-    Inspector* ins = get_inspector("binder");
+    Inspector* ins = get_binder();
 
     if ( ins )
         ins->exec(0, flow);
diff --git a/src/managers/inspector_manager.h b/src/managers/inspector_manager.h
index a70d03091..4a66cc30e 100644
--- a/src/managers/inspector_manager.h
+++ b/src/managers/inspector_manager.h
@@ -51,6 +51,9 @@ public:
     static void free_inspector(Inspector*);
     static InspectSsnFunc get_session(const char* key);
 
+    static Inspector* get_binder();
+    static Inspector* get_wizard();
+
     static bool configure(SnortConfig*);
     static void print_config(SnortConfig*);
 
diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc
index cbd780819..c0d204869 100644
--- a/src/network_inspectors/binder/binder.cc
+++ b/src/network_inspectors/binder/binder.cc
@@ -213,7 +213,7 @@ int Binder::check_rules(Flow* flow, Packet* p)
 
     if ( !pb->type.size() || pb->type == "wizard" )
     {
-        ins = InspectorManager::get_inspector("wizard");
+        ins = InspectorManager::get_wizard();
         flow->set_clouseau(ins);
     }
     else
diff --git a/src/protocols/packet.h b/src/protocols/packet.h
index fc63dc01b..9090cb945 100644
--- a/src/protocols/packet.h
+++ b/src/protocols/packet.h
@@ -95,15 +95,15 @@ extern "C" {
 
 #define PKT_PSEUDO           0x00080000  /* is a pseudo packet */
 #define PKT_MODIFIED         0x00100000  /* packet had normalizations, etc. */
-#define PKT_RESIZED          0x00180000  /* packet has new size; must set modified too */
+#define PKT_RESIZED          0x00300000  /* packet has new size; must set modified too */
 
 // neither of these flags will be set for (full) retransmissions or non-data segments
 // a partial overlap results in out of sequence condition
 // out of sequence condition is sticky
-#define PKT_STREAM_ORDER_OK  0x00200000  /* this segment is in order, w/o gaps */
-#define PKT_STREAM_ORDER_BAD 0x00400000  /* this stream had at least one gap */
+#define PKT_STREAM_ORDER_OK  0x00400000  /* this segment is in order, w/o gaps */
+#define PKT_STREAM_ORDER_BAD 0x00800000  /* this stream had at least one gap */
 
-#define PKT_FILE_EVENT_SET   0x00800000
+#define PKT_FILE_EVENT_SET   0x01000000
 #define PKT_UNUSED_FLAGS     0xff000000
 
 // 0x40000000 are available
diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc
index a71086d5f..8441bd16b 100644
--- a/src/stream/base/stream_base.cc
+++ b/src/stream/base/stream_base.cc
@@ -153,7 +153,7 @@ StreamBase::StreamBase(const StreamConfig* c)
 void StreamBase::tinit()
 {
     assert(!flow_con);
-    Inspector* pi = InspectorManager::get_inspector("binder");
+    Inspector* pi = InspectorManager::get_binder();
     flow_con = new FlowControl(pi);
     InspectSsnFunc f;