]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
threading fixes plus misc
authorRuss Combs <rucombs@cisco.com>
Mon, 25 Aug 2014 16:15:20 +0000 (12:15 -0400)
committerRuss Combs <rucombs@cisco.com>
Mon, 25 Aug 2014 16:15:20 +0000 (12:15 -0400)
19 files changed:
ChangeLog
doc/style.txt
src/detection/fpcreate.cc
src/flow/flow.cc
src/flow/flow.h
src/framework/inspector.cc
src/ips_options/ips_http_header.cc
src/main.cc
src/main/snort.cc
src/main/snort_config.cc
src/main/snort_config.h
src/main/snort_module.cc
src/main/thread.cc
src/main/thread.h
src/managers/inspector_manager.cc
src/managers/inspector_manager.h
src/network_inspectors/binder/binder.cc
src/protocols/packet.h
src/stream/base/stream_base.cc

index 1f8604f628360bbfae656d1411f4f7cc9eab5b67..848fbc90489f38fc92ea5d697437791a9711b066 100644 (file)
--- 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
index 1d0ff80ce4cf7e51a1ea8ea947f7ec6c1399734a..d442b44e7537d46a167f56053910d532c73db08c 100644 (file)
@@ -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
 
index 1e1fc932ca686ea39022253690b276355f378a7e..28d9191cf82da70ccf993768f13ac46f584fe51b 100644 (file)
@@ -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;
 
index df9c671bf4d3cc05681b4af0e598f90485fca551..499f47aa0f02a7b3a117242cd13ee6d940f35dda 100644 (file)
@@ -111,8 +111,6 @@ void Flow::reset()
 
 void Flow::clear(bool freeAppData)
 {
-    assert(flow_state < 3);
-
     if ( freeAppData )
         free_application_data();
 
index ffacf2cf65cbecba652209ddbc3f92a720a4e173..53c21f4cc1c73cbbc62760e7ae3d12049270660b 100644 (file)
@@ -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
index 036de926553c70139c5565f58cbd0c0826f30cde..7319b9fb942185401594f83b47cd7ff6a8aebac1 100644 (file)
 // 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;
     
index 38d84f2ad5cf6e07d5b046b0a596fe97f4830647..f9f12258c4185a77dcfa2009b6ba81be91b2c1d9 100644 (file)
@@ -96,7 +96,7 @@ public:
     { name = s; };
 
     CursorActionType get_cursor_type() const
-    { return CAT_SET_OTHER; };
+    { return CAT_SET_HEADER; };
 
     int eval(Cursor&, Packet*);
 
index 9821d47b45b1f4281163a5490167d16c63908553..60822ea356dabd75462e65ffcbf61806d59a32a3 100644 (file)
@@ -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];
index 66d6eeb6d69676e4a8c4556a6f6809f666001890..39ec27565e66d0085c6f19877090c63538eb8673 100644 (file)
@@ -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) )
index 81ba4bd6b7becb4166574654ab3dff5e151a65d3..f0aaa40281b4e88acc5052fd714c52775ef51813 100644 (file)
@@ -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;
 }
index a7963b0fb467d2a63a3ca4ddcee073c4f0df4f98..3a3e9213ca146aee67e30bfd568fa0de1bf467d8 100644 (file)
@@ -251,7 +251,6 @@ struct SnortConfig
 
     struct VarNode* var_list;
 
-    int max_threads;
     unsigned remote_control;
 
     SnortState* state;
index 81abda16f4644870906d9f566babfe086b4cd790..fa4f1525ad3acd65dc32e929d4f30e269e9a6200 100644 (file)
@@ -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());
 
index d56496465a90ddd0f524d8346c658d9630961378..8e08549878ac07db4921d8712f615cdffade2055 100644 (file)
@@ -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;
 }
 
 //-------------------------------------------------------------------------
index 01ca5f5c5fa5e7f20c59fbd8dc3c93199142975a..3b9023761ef9a0adb78ae979c25825b2329af188 100644 (file)
 //#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
index e9f82bcd306bed08b5ecc970bfc16aeda301ce9b..0a73db21a645d02e7f95c3089505fd4cf7165b8d 100644 (file)
@@ -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);
index a70d03091cbbf3080ff67673e68b840f57d471e2..4a66cc30e9fd80094d7c98aa90525608745bc79c 100644 (file)
@@ -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*);
 
index cbd780819ebfff909da18b75f33d0574db369a6c..c0d2048695e615d5632689805451c19517dacca1 100644 (file)
@@ -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
index fc63dc01b994d780fa4b1485f6ee231dc6b49dd8..9090cb945e4098e1b9a485f07ac36d104260a8b1 100644 (file)
@@ -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
index a71086d5f4a4ca7c1a25e64fb3485f1088f83109..8441bd16b2886cefd25fe831b3a5d9b4cea4898a 100644 (file)
@@ -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;