Pull request #3395: wizard: update glob storage due to shared memory

Merge in SNORT/snort3 from ~YVELYKOZ/snort3:wizard_mt_fix to master

Squashed commit of the following:

commit f9159d44d44a9def929b308cc9167bfd354bd99f
Author: Yehor <egor1velikogon@gmail.com>
Date:   Tue Apr 19 17:44:47 2022 +0300

    wizard: update glob storage due to shared memory

diff --git a/src/service_inspectors/wizard/hexes.cc b/src/service_inspectors/wizard/hexes.cc
index 3a1cebf3391c62ec7e1fcf6e8896dbc8d0451548..327d952195f49d8b47925a8abd7d7a28744a479c 100644 (file)
--- a/src/service_inspectors/wizard/hexes.cc
+++ b/src/service_inspectors/wizard/hexes.cc
@@ -136,7 +136,7 @@ bool HexBook::add_spell(const char* key, const char*& val)
 }
 
 const MagicPage* HexBook::find_spell(
-    const uint8_t* s, unsigned n, const MagicPage* p, unsigned i) const
+    const uint8_t* s, unsigned n, const MagicPage* p, unsigned i, const MagicPage*& bookmark) const
 {
     while ( i < n )
     {
@@ -146,7 +146,7 @@ const MagicPage* HexBook::find_spell(
         {
             if ( p->any )
             {
-                if ( const MagicPage* q = find_spell(s, n, p->next[c], i+1) )
+                if ( const MagicPage* q = find_spell(s, n, p->next[c], i+1, bookmark) )
                     return q;
             }
             else
@@ -158,7 +158,7 @@ const MagicPage* HexBook::find_spell(
         }
         if ( p->any )
         {
-            if ( const MagicPage* q = find_spell(s, n, p->any, i+1) )
+            if ( const MagicPage* q = find_spell(s, n, p->any, i+1, bookmark) )
                 return q;
         }
         return p->value ? p : nullptr;

diff --git a/src/service_inspectors/wizard/magic.cc b/src/service_inspectors/wizard/magic.cc
index dd56d2546470e9179b2ece094a617188d477b58b..13397fedc429572a026eca97cc8440fcb69c8301 100644 (file)
--- a/src/service_inspectors/wizard/magic.cc
+++ b/src/service_inspectors/wizard/magic.cc
@@ -43,10 +43,10 @@ MagicPage::~MagicPage()
 }
 
 const char* MagicBook::find_spell(const uint8_t* data, unsigned len,
-    const MagicPage*& p) const
+    const MagicPage*& p, const MagicPage*& bookmark) const
 {
     assert(p);
-    p = find_spell(data, len, p, 0);
+    p = find_spell(data, len, p, 0, bookmark);
     return p ? p->value : nullptr;
 }
 

diff --git a/src/service_inspectors/wizard/magic.h b/src/service_inspectors/wizard/magic.h
index 3d3ef76134d041e5c7b1fcb42ef0f14aaaa2bf69..c2341e2aca3951d7167a7d0505c800d4b66b33ae 100644 (file)
--- a/src/service_inspectors/wizard/magic.h
+++ b/src/service_inspectors/wizard/magic.h
@@ -52,22 +52,18 @@ public:
     MagicBook& operator=(const MagicBook&) = delete;
 
     virtual bool add_spell(const char* key, const char*& val) = 0;
-    virtual const char* find_spell(const uint8_t* data, unsigned len, const MagicPage*&) const;
+    virtual const char* find_spell(const uint8_t* data, unsigned len, const MagicPage*& p,
+        const MagicPage*& bookmark) const;
 
     const MagicPage* page1() const
     { return root; }
 
-    virtual void set_bookmark(const MagicPage* page = nullptr) const
-    { (void)page; }
-    virtual const MagicPage* get_bookmark() const
-    { return nullptr; }
-
 protected:
     MagicBook();
     MagicPage* root;
 
     virtual const MagicPage* find_spell(const uint8_t*, unsigned,
-        const MagicPage*, unsigned) const = 0;
+        const MagicPage*, unsigned, const MagicPage*&) const = 0;
 };
 
 //-------------------------------------------------------------------------
@@ -82,18 +78,11 @@ public:
 
     bool add_spell(const char*, const char*&) override;
 
-    void set_bookmark(const MagicPage* page = nullptr) const override
-    { glob = page; }
-
-    const MagicPage* get_bookmark() const override
-    { return glob; }
-
 private:
     bool translate(const char*, HexVector&);
     void add_spell(const char*, const char*, HexVector&, unsigned, MagicPage*);
-    const MagicPage* find_spell(const uint8_t*, unsigned, const MagicPage*, unsigned) const override;
-
-    mutable const MagicPage* glob;
+    const MagicPage* find_spell(const uint8_t*, unsigned, const MagicPage*, unsigned,
+        const MagicPage*&) const override;
 };
 
 //-------------------------------------------------------------------------
@@ -111,7 +100,8 @@ public:
 private:
     bool translate(const char*, HexVector&);
     void add_spell(const char*, const char*, HexVector&, unsigned, MagicPage*);
-    const MagicPage* find_spell(const uint8_t*, unsigned, const MagicPage*, unsigned) const override;
+    const MagicPage* find_spell(const uint8_t*, unsigned, const MagicPage*, unsigned,
+        const MagicPage*&) const override;
 };
 
 #endif

diff --git a/src/service_inspectors/wizard/spells.cc b/src/service_inspectors/wizard/spells.cc
index 997876133450dc3c1142c78aa770d306320fadff..28ef4e290140cecd315a3b09877c29900771a26b 100644 (file)
--- a/src/service_inspectors/wizard/spells.cc
+++ b/src/service_inspectors/wizard/spells.cc
@@ -27,12 +27,11 @@
 
 #include "magic.h"
 
-using namespace snort;
 using namespace std;
 
 #define WILD 0x100
 
-SpellBook::SpellBook() : glob(nullptr)
+SpellBook::SpellBook()
 {
     // allows skipping leading whitespace only
     root->next[(int)' '] = root;
@@ -87,7 +86,7 @@ void SpellBook::add_spell(
         ++i;
     }
     p->key = key;
-    p->value = SnortConfig::get_static_name(val);
+    p->value = snort::SnortConfig::get_static_name(val);
 }
 
 bool SpellBook::add_spell(const char* key, const char*& val)
@@ -130,7 +129,7 @@ bool SpellBook::add_spell(const char* key, const char*& val)
 }
 
 const MagicPage* SpellBook::find_spell(
-    const uint8_t* s, unsigned n, const MagicPage* p, unsigned i) const
+    const uint8_t* s, unsigned n, const MagicPage* p, unsigned i, const MagicPage*& bookmark) const
 {
     while ( i < n )
     {
@@ -140,7 +139,7 @@ const MagicPage* SpellBook::find_spell(
         {
             if ( p->any )
             {
-                if ( const MagicPage* q = find_spell(s, n, p->next[c], i+1) )
+                if ( const MagicPage* q = find_spell(s, n, p->next[c], i+1, bookmark) )
                     return q;
             }
             else
@@ -154,9 +153,9 @@ const MagicPage* SpellBook::find_spell(
         {
             while ( i < n )
             {
-                if ( const MagicPage* q = find_spell(s, n, p->any, i) )
+                if ( const MagicPage* q = find_spell(s, n, p->any, i, bookmark) )
                 {
-                    glob = q->any ? q : p;
+                    bookmark = q->any ? q : p;
                     return q;
                 }
                 ++i;
@@ -164,13 +163,13 @@ const MagicPage* SpellBook::find_spell(
             return p;
         }
 
-        // If no match but has glob, continue lookup from glob
-        if ( !p->value && glob )
+        // If no match but has bookmark, continue lookup from bookmark
+        if ( !p->value && bookmark )
         {
-            p = glob;
-            glob = nullptr;
+            p = bookmark;
+            bookmark = nullptr;
 
-            return find_spell(s, n, p, i);
+            return find_spell(s, n, p, i, bookmark);
         }
 
         return p->value ? p : nullptr;

diff --git a/src/service_inspectors/wizard/wizard.cc b/src/service_inspectors/wizard/wizard.cc
index 46de82103d859cf718ce161575e95e6540deb06d..f1d8bc60a38d8f277facb7727355489e49172406 100644 (file)
--- a/src/service_inspectors/wizard/wizard.cc
+++ b/src/service_inspectors/wizard/wizard.cc
@@ -80,6 +80,7 @@ struct Wand
 {
     const MagicPage* hex;
     const MagicPage* spell;
+    const MagicPage* bookmark;
     vector<CurseServiceTracker> curse_tracker;
 };
 
@@ -125,7 +126,6 @@ private:
     Wizard* wizard;
     Wand wand;
     uint16_t wizard_processed_bytes;
-    const MagicPage* bookmark;         // pointer to last glob
 };
 
 class Wizard : public Inspector
@@ -142,7 +142,7 @@ public:
     { return !w.hex && !w.spell && w.curse_tracker.empty(); }
     void reset(Wand&, bool tcp, bool c2s);
     bool cast_spell(Wand&, Flow*, const uint8_t*, unsigned, uint16_t&);
-    bool spellbind(const MagicPage*&, Flow*, const uint8_t*, unsigned);
+    bool spellbind(const MagicPage*&, Flow*, const uint8_t*, unsigned, const MagicPage*&);
     bool cursebind(const vector<CurseServiceTracker>&, Flow*, const uint8_t*, unsigned);
 
 public:
@@ -164,7 +164,7 @@ public:
 //-------------------------------------------------------------------------
 
 MagicSplitter::MagicSplitter(bool c2s, class Wizard* w) :
-    StreamSplitter(c2s), wizard_processed_bytes(0), bookmark(nullptr)
+    StreamSplitter(c2s), wizard_processed_bytes(0)
 {
     wizard = w;
     w->add_ref();
@@ -187,9 +187,6 @@ StreamSplitter::Status MagicSplitter::scan(
     Profile profile(wizPerfStats);
     count_scan(pkt->flow);
 
-    // setting last glob from current flow
-    if ( wand.spell )
-        wand.spell->book.set_bookmark(bookmark);
     bytes_scanned += len;
 
     if ( wizard->cast_spell(wand, pkt->flow, data, len, wizard_processed_bytes) )
@@ -200,7 +197,6 @@ StreamSplitter::Status MagicSplitter::scan(
         wizard_processed_bytes = 0;
         return STOP;
     }
-
     else if ( wizard->finished(wand) || bytes_scanned >= max(pkt->flow) )
     {
         count_miss(pkt->flow);
@@ -214,10 +210,6 @@ StreamSplitter::Status MagicSplitter::scan(
         return ABORT;
     }
 
-    // saving new last glob from current flow
-    if ( wand.spell )
-        bookmark = wand.spell->book.get_bookmark();
-
     // FIXIT-L Ideally, this event should be raised after wizard aborts its search. However, this
     // could take multiple packets because wizard needs wizard.max_search_depth payload bytes before
     // it aborts. This is an issue for AppId which consumes this event. AppId is required to declare
@@ -264,17 +256,17 @@ Wizard::~Wizard()
 
 void Wizard::reset(Wand& w, bool tcp, bool c2s)
 {
+    w.bookmark = nullptr;
+
     if ( c2s )
     {
         w.hex = c2s_hexes->page1();
         w.spell = c2s_spells->page1();
-        c2s_spells->set_bookmark();
     }
     else
     {
         w.hex = s2c_hexes->page1();
         w.spell = s2c_spells->page1();
-        s2c_spells->set_bookmark();
     }
 
     if (w.curse_tracker.empty())
@@ -326,9 +318,9 @@ StreamSplitter* Wizard::get_splitter(bool c2s)
 }
 
 bool Wizard::spellbind(
-    const MagicPage*& m, Flow* f, const uint8_t* data, unsigned len)
+    const MagicPage*& m, Flow* f, const uint8_t* data, unsigned len, const MagicPage*& bookmark)
 {
-    f->service = m->book.find_spell(data, len, m);
+    f->service = m->book.find_spell(data, len, m, bookmark);
     return f->service != nullptr;
 }
 
@@ -357,10 +349,10 @@ bool Wizard::cast_spell(
 
     wizard_processed_bytes += len;
 
-    if ( w.hex && spellbind(w.hex, f, data, len) )
+    if ( w.hex && spellbind(w.hex, f, data, len, w.bookmark) )
         return true;
 
-    if ( w.spell && spellbind(w.spell, f, data, len) )
+    if ( w.spell && spellbind(w.spell, f, data, len, w.bookmark) )
         return true;
 
     if (cursebind(w.curse_tracker, f, data, curse_len))