Merge pull request #2637 in SNORT/snort3 from ~MIALTIZE/snort3:metabegone to master

Squashed commit of the following:

commit 60f61048379b17f9a577bfaa78cd90e51dd75153
Author: Michael Altizer <mialtize@cisco.com>
Date:   Mon Nov 23 12:17:22 2020 -0500

    snort: Add OopsHandlerSuspend for suspending Snort's crash handler

    This is an RAII-style mechanism that will uninstall Snort's "oops"
    handler when created and reinstall it when it goes out of scope.

commit f4f202749f27de376b63f6cc353dbe45c1a4661b
Author: Michael Altizer <mialtize@cisco.com>
Date:   Mon Nov 23 11:47:06 2020 -0500

    helpers: Handle SIGILL and SIGFPE with the oops handler

commit 49ba9014e5df70bc3c78be25569e092aad38b642
Author: Michael Altizer <mialtize@cisco.com>
Date:   Fri Nov 20 15:58:41 2020 -0500

    inspector_manager: Remove unused inspector_exists_in_any_policy() function

commit 731ee59c29b04ee0baaa903860a7596d4c5ea046
Author: Michael Altizer <mialtize@cisco.com>
Date:   Fri Nov 20 15:56:58 2020 -0500

    inspector: Remove obsolete metapacket processing functionality

diff --git a/src/framework/inspector.h b/src/framework/inspector.h
index 52c4ddcd8ff80fa7bc1f5483a497a4e82d845487..9641c3806744d2f51732f98c28519f7f89eb464f 100644 (file)
--- a/src/framework/inspector.h
+++ b/src/framework/inspector.h
@@ -97,8 +97,6 @@ public:
     virtual void eval(Packet*) = 0;
     virtual void clear(Packet*) { }
 
-    virtual void meta(int, const uint8_t*) { }
-
     // framework support
     unsigned get_ref(unsigned i) { return ref_count[i]; }
     void set_ref(unsigned i, unsigned r) { ref_count[i] = r; }

diff --git a/src/helpers/process.cc b/src/helpers/process.cc
index 0a8d7a00a74a9f677c27d9965fb7ad56d587c056..c3cd080c110cef9fc59cf6e9b145d80a4a00a006 100644 (file)
--- a/src/helpers/process.cc
+++ b/src/helpers/process.cc
@@ -91,8 +91,10 @@ static struct
 }
 original_sigactions[] =
 {
+    { SIGILL, { } },
     { SIGABRT, { } },
     { SIGBUS,  { } },
+    { SIGFPE,  { } },
     { SIGSEGV, { } },
     { 0, { } },
 };
@@ -104,8 +106,10 @@ static struct
 }
 original_sighandlers[] =
 {
+    { SIGILL,  SIG_DFL },
     { SIGABRT, SIG_DFL },
     { SIGBUS,  SIG_DFL },
+    { SIGFPE,  SIG_DFL },
     { SIGSEGV, SIG_DFL },
     { 0, SIG_DFL },
 };
@@ -254,12 +258,18 @@ static void oops_handler(int signal)
     const char* sigstr = "???\n";
     switch (signal)
     {
+        case SIGILL:
+            sigstr = STRINGIFY(SIGILL) " (" STRINGIFY_MX(SIGILL) ")";
+            break;
         case SIGABRT:
             sigstr = STRINGIFY(SIGABRT) " (" STRINGIFY_MX(SIGABRT) ")";
             break;
         case SIGBUS:
             sigstr = STRINGIFY(SIGBUS) " (" STRINGIFY_MX(SIGBUS) ")";
             break;
+        case SIGFPE:
+            sigstr = STRINGIFY(SIGFPE) " (" STRINGIFY_MX(SIGFPE) ")";
+            break;
         case SIGSEGV:
             sigstr = STRINGIFY(SIGSEGV) " (" STRINGIFY_MX(SIGSEGV) ")";
             break;
@@ -400,6 +410,15 @@ static bool restore_signal(int sig, bool silent)
     return true;
 }
 
+void install_oops_handler()
+{
+    add_signal(SIGILL, oops_handler);
+    add_signal(SIGABRT, oops_handler);
+    add_signal(SIGBUS, oops_handler);
+    add_signal(SIGFPE, oops_handler);
+    add_signal(SIGSEGV, oops_handler);
+}
+
 void init_signals()
 {
     sigset_t set;
@@ -408,39 +427,50 @@ void init_signals()
     // FIXIT-L this is undefined for multithreaded apps
     sigprocmask(SIG_SETMASK, &set, nullptr);
 
-    // Make this program behave nicely when signals come along.
+    // First things first, install the crash handler
+    install_oops_handler();
+
+    // Ignore SIGPIPE for now (it's not particularly actionable in a multithreaded program)
+    add_signal(SIGPIPE, SIG_IGN);
+
+    // Set up a clean exit when expected shutdown signals come along
     add_signal(SIGTERM, exit_handler);
     add_signal(SIGINT, exit_handler);
     add_signal(SIGQUIT, dirty_handler);
 
+    // Finally, set up signal handlers for custom Snort actions
     add_signal(SIGNAL_SNORT_DUMP_STATS, dump_stats_handler);
     add_signal(SIGNAL_SNORT_ROTATE_STATS, rotate_stats_handler);
     add_signal(SIGNAL_SNORT_RELOAD, reload_config_handler);
     add_signal(SIGNAL_SNORT_READ_ATTR_TBL, reload_attrib_handler);
 
-    add_signal(SIGPIPE, SIG_IGN);
-    add_signal(SIGABRT, oops_handler);
-    add_signal(SIGSEGV, oops_handler);
-    add_signal(SIGBUS, oops_handler);
-
+    // Errno will have potentially been left set from a failed handler installation
     errno = 0;
 }
 
-void term_signals()
+void remove_oops_handler()
 {
-    restore_signal(SIGTERM);
-    restore_signal(SIGINT);
-    restore_signal(SIGQUIT);
+    restore_signal(SIGILL);
+    restore_signal(SIGABRT);
+    restore_signal(SIGBUS);
+    restore_signal(SIGFPE);
+    restore_signal(SIGSEGV);
+}
 
+void term_signals()
+{
     restore_signal(SIGNAL_SNORT_DUMP_STATS);
     restore_signal(SIGNAL_SNORT_ROTATE_STATS);
     restore_signal(SIGNAL_SNORT_RELOAD);
     restore_signal(SIGNAL_SNORT_READ_ATTR_TBL);
 
+    restore_signal(SIGTERM);
+    restore_signal(SIGINT);
+    restore_signal(SIGQUIT);
+
     restore_signal(SIGPIPE);
-    restore_signal(SIGABRT);
-    restore_signal(SIGSEGV);
-    restore_signal(SIGBUS);
+
+    remove_oops_handler();
 }
 
 static void help_signal(unsigned n, const char* name, const char* h)

diff --git a/src/helpers/process.h b/src/helpers/process.h
index 078ecdea7330a68221b52544e34fae684376f5a4..b8468c6fdca22f79394e4ccba32d79a9f072e319 100644 (file)
--- a/src/helpers/process.h
+++ b/src/helpers/process.h
@@ -39,6 +39,8 @@ const char* get_signal_name(PigSignal);
 
 void init_signals();
 void term_signals();
+void install_oops_handler();
+void remove_oops_handler();
 void help_signals();
 
 void daemonize();

diff --git a/src/main/snort.cc b/src/main/snort.cc
index 9367c870f2cce5454eae4e4e152c09502f4cf16d..87aa5ad60960de065066b3905156e714555a7f62 100644 (file)
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -641,3 +641,13 @@ SnortConfig* Snort::get_updated_module(SnortConfig* other_conf, const char* name
     reloading = false;
     return sc;
 }
+
+OopsHandlerSuspend::OopsHandlerSuspend()
+{
+    remove_oops_handler();
+}
+
+OopsHandlerSuspend::~OopsHandlerSuspend()
+{
+    install_oops_handler();
+}

diff --git a/src/main/snort.h b/src/main/snort.h
index 3efeb7fc4636cc140d3182baa0cd7d6f66607a60..82c930e9983826c1812edc61ce7d2770058a3d78 100644 (file)
--- a/src/main/snort.h
+++ b/src/main/snort.h
@@ -62,6 +62,13 @@ private:
     static bool privileges_dropped;
 };
 
+// RAII-style mechanism for removal and reinstallation of Snort's crash handler
+class SO_PUBLIC OopsHandlerSuspend
+{
+public:
+    OopsHandlerSuspend();
+    ~OopsHandlerSuspend();
+};
 }
 
 #endif

diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc
index 7be6c3f058d08bad9d7c9d3dd661ed02294e714d..0f9432bd89ec087dc6605c436a5de233b4109e2e 100644 (file)
--- a/src/managers/inspector_manager.cc
+++ b/src/managers/inspector_manager.cc
@@ -423,18 +423,6 @@ static bool get_instance(
     return false;
 }
 
-static PHInstance* get_instance_by_type(FrameworkPolicy* fp, const char* keyword)
-{
-    std::vector<PHInstance*>::iterator it;
-
-    for ( it = fp->ilist.begin(); it != fp->ilist.end(); ++it )
-    {
-        if ( !strcmp((*it)->pp_class.api.base.name, keyword) )
-            return *it;
-    }
-    return nullptr;
-}
-
 static PHInstance* get_instance_by_service(FrameworkPolicy* fp, const char* keyword)
 {
     std::vector<PHInstance*>::iterator it;
@@ -532,13 +520,6 @@ void InspectorManager::update_policy(SnortConfig* sc)
         p->set_reloaded(RELOAD_TYPE_NONE);
 }
 
-// FIXIT-M create a separate list for meta handlers?  is there really more than one?
-void InspectorManager::dispatch_meta(FrameworkPolicy* fp, int type, const uint8_t* data)
-{
-    for ( auto* p : fp->ilist )
-        p->handler->meta(type, data);
-}
-
 Binder* InspectorManager::get_binder()
 {
     InspectionPolicy* pi = get_inspection_policy();
@@ -549,29 +530,6 @@ Binder* InspectorManager::get_binder()
     return (Binder*)pi->framework_policy->binder;
 }
 
-bool InspectorManager::inspector_exists_in_any_policy(const char* key, SnortConfig* sc)
-{
-    PolicyMap* pm = sc->policy_map;
-
-    if (pm == nullptr)
-        return false;
-
-    for (unsigned i=0; i<pm->inspection_policy_count(); i++)
-    {
-        const InspectionPolicy* const pi = pm->get_inspection_policy(i);
-
-        if ( !pi || !pi->framework_policy )
-            continue;
-
-        const PHInstance* const p = get_instance_by_type(pi->framework_policy, key);
-
-        if ( p )
-            return true;
-    }
-
-    return false;
-}
-
 // FIXIT-P cache get_inspector() returns or provide indexed lookup
 Inspector* InspectorManager::get_inspector(const char* key, bool dflt_only, const SnortConfig* sc)
 {

diff --git a/src/managers/inspector_manager.h b/src/managers/inspector_manager.h
index d86dd633f27bb678c94a65d3447e5e4482acd2cb..a3d6b572818cf684b0f452bdeea8f6b261daa4b4 100644 (file)
--- a/src/managers/inspector_manager.h
+++ b/src/managers/inspector_manager.h
@@ -82,14 +82,12 @@ public:
     static void thread_term();
 
     static void release_policy(FrameworkPolicy*);
-    static void dispatch_meta(FrameworkPolicy*, int type, const uint8_t* data);
 
     static void execute(Packet*);
     static void probe(Packet*);
 
     static void clear(Packet*);
     static void empty_trash();
-    static bool inspector_exists_in_any_policy(const char* key, SnortConfig* sc);
 
 #ifdef PIGLET
     static Inspector* instantiate(const char*, Module*, SnortConfig*);

diff --git a/src/piglet_plugins/pp_inspector_iface.cc b/src/piglet_plugins/pp_inspector_iface.cc
index b20add54411445374b216d7e9e4d4ba02a00480f..d3475d0fc9420039054f7f5b2250af1844031f0d 100644 (file)
--- a/src/piglet_plugins/pp_inspector_iface.cc
+++ b/src/piglet_plugins/pp_inspector_iface.cc
@@ -111,7 +111,6 @@ static const luaL_Reg methods[] =
             return 0;
         }
     },
-    // FIXIT-M add meta() method
     // FIXIT-M add exec() method
     {
         "get_buf_from_key",