From 9ee76016f1abe825d5ebcaa472a2651e89f88171 Mon Sep 17 00:00:00 2001 From: "Russ Combs (rucombs)" Date: Fri, 28 Feb 2020 04:12:34 +0000 Subject: [PATCH] Merge pull request #2037 in SNORT/snort3 from ~DAVMCPHE/snort3:nuking_reload_errors to master Squashed commit of the following: commit 9d50248d9b1768509c9876ed3ed53a3c52cc8d91 Author: davis mcpherson Date: Thu Feb 27 21:12:31 2020 -0500 ghash: fix thread race condition with GHash member variables when a GHash instance is global commit 8b7b0bab38e9d567e81acf784b39fe4eae4d6534 Author: davis mcpherson Date: Mon Feb 10 17:24:28 2020 -0500 snort_config: footprint REG_TEST, no check for stream inspector add/rm, etc reload: add description of reload error to the response message of the reload_config command ips_base64: add logic to call ips option tinit method on reload if not previously called hash: delete unused sfmemcap.[h|cc] and remove unnecessary includes --- src/file_api/file_module.cc | 4 +- src/file_api/file_service.cc | 6 +- src/hash/ghash.cc | 22 +-- src/hash/ghash.h | 17 +- src/hash/hash_defs.h | 1 - src/hash/hash_key_operations.cc | 6 +- src/hash/test/CMakeLists.txt | 4 +- src/hash/xhash.cc | 2 +- src/log/messages.cc | 27 ++- src/log/messages.h | 3 + src/main.cc | 7 +- src/main/analyzer.cc | 1 + src/main/snort_config.cc | 167 ++++-------------- src/main/snort_config.h | 3 +- src/managers/ips_manager.cc | 11 +- src/managers/ips_manager.h | 1 + src/ports/port_object2.cc | 8 - .../http2_inspect/http2_dummy_packet.h | 2 +- src/stream/base/stream_base.cc | 4 + src/stream/base/stream_module.cc | 32 ++-- src/stream/base/stream_module.h | 3 + src/stream/flush_bucket.cc | 3 + src/stream/flush_bucket.h | 1 + src/utils/CMakeLists.txt | 2 - src/utils/sfmemcap.cc | 104 ----------- src/utils/sfmemcap.h | 39 ---- 26 files changed, 133 insertions(+), 347 deletions(-) delete mode 100644 src/utils/sfmemcap.cc delete mode 100644 src/utils/sfmemcap.h diff --git a/src/file_api/file_module.cc b/src/file_api/file_module.cc index 4819c388d..58be9bf45 100644 --- a/src/file_api/file_module.cc +++ b/src/file_api/file_module.cc @@ -290,7 +290,7 @@ bool FileIdModule::set(const char*, Value& v, SnortConfig*) { if (Snort::is_reloading() && !FileService::is_file_capture_enabled()) { - ReloadError("Enabling file capture requires a restart\n"); + ReloadError("Enabling file_id.enable_capture requires a restart\n"); return false; } fp.set_file_capture(true); @@ -388,7 +388,7 @@ bool FileIdModule::set(const char*, Value& v, SnortConfig*) if (file_rule.use.capture_enabled && Snort::is_reloading() && !FileService::is_file_capture_enabled()) { - ReloadError("Enabling file capture requires a restart\n"); + ReloadError("Enabling file_id.enable_file_capture requires a restart\n"); return false; } } diff --git a/src/file_api/file_service.cc b/src/file_api/file_service.cc index d8a53a57a..7aaaf7bdd 100644 --- a/src/file_api/file_service.cc +++ b/src/file_api/file_service.cc @@ -87,14 +87,14 @@ void FileService::verify_reload(SnortConfig* sc) return; if (max_files_cached != conf->max_files_cached) - ReloadError("Changing file_id:max_files_cached requires a restart\n"); + ReloadError("Changing file_id.max_files_cached requires a restart\n"); if (file_capture_enabled) { if (capture_memcap != conf->capture_memcap) - ReloadError("Changing file_id:capture_memcap requires a restart\n"); + ReloadError("Changing file_id.capture_memcap requires a restart\n"); if (capture_block_size != conf->capture_block_size) - ReloadError("Changing file_id:capture_block_size requires a restart\n"); + ReloadError("Changing file_id.capture_block_size requires a restart\n"); } } diff --git a/src/hash/ghash.cc b/src/hash/ghash.cc index d2261ed49..a9e20a8d8 100644 --- a/src/hash/ghash.cc +++ b/src/hash/ghash.cc @@ -107,20 +107,10 @@ GHash::~GHash() delete hashfcn; } -// set key length, hashkey, and index parameters required to find/add/remove a node -// the parameters set are valid until for the life of the initial method called -void GHash::set_node_parameters(const void* const key) -{ - klen = ( keysize > 0 ) ? keysize : strlen((const char*)key) + 1; - hashkey = hashfcn->do_hash((const unsigned char*)key, klen); - index = hashkey % nrows; -} - -GHashNode* GHash::find_node(const void* const key) +GHashNode* GHash::find_node(const void* const key, unsigned index) { assert(key); - set_node_parameters(key); for ( GHashNode* hnode = table[index]; hnode; hnode = hnode->next ) { if ( keysize == 0 ) @@ -142,7 +132,8 @@ int GHash::insert(const void* const key, void* const data) { assert(key && data); - if ( GHashNode* hnode = find_node(key) ) + unsigned index = get_index(key); + if ( GHashNode* hnode = find_node(key, index) ) { cnode = hnode; return HASH_INTABLE; @@ -155,6 +146,7 @@ int GHash::insert(const void* const key, void* const data) } else { + unsigned klen = get_key_length(key); hnode->key = snort_alloc(klen); memcpy(const_cast(hnode->key), key, klen); } @@ -184,7 +176,8 @@ void* GHash::find(const void* const key) { assert(key); - GHashNode* hnode = find_node(key); + unsigned index = get_index(key); + GHashNode* hnode = find_node(key, index); if ( hnode ) return hnode->data; @@ -226,7 +219,8 @@ int GHash::remove(const void* const key) { assert(key); - if ( GHashNode* hnode = find_node(key) ) + unsigned index = get_index(key); + if ( GHashNode* hnode = find_node(key, index) ) return free_node(index, hnode); else return HASH_NOT_FOUND; diff --git a/src/hash/ghash.h b/src/hash/ghash.h index ca210e69f..566a39a2d 100644 --- a/src/hash/ghash.h +++ b/src/hash/ghash.h @@ -24,6 +24,7 @@ // generic hash table - stores and maps key + data pairs +#include #include "hash_key_operations.h" #include "main/snort_types.h" @@ -56,11 +57,19 @@ public: { return count; } private: - void set_node_parameters(const void* const key); - GHashNode* find_node(const void* const key); + GHashNode* find_node(const void* const key, unsigned index); int free_node(unsigned index, GHashNode*); void next(); + unsigned get_key_length(const void* const key) + { return ( keysize > 0 ) ? keysize : strlen((const char*)key) + 1; } + + unsigned get_index(const void* const key) + { + unsigned hashkey = hashfcn->do_hash((const unsigned char*)key, get_key_length(key)); + return hashkey % nrows; + } + unsigned keysize; // bytes in key, if < 0 -> keys are strings bool userkey; // user owns the key */ gHashFree userfree; @@ -71,10 +80,6 @@ private: int crow; // findfirst/next row in table GHashNode* cnode; // findfirst/next node ptr - // node parameters for search/add/remove - unsigned klen = 0; - unsigned hashkey = 0; - int index = 0; }; diff --git a/src/hash/hash_defs.h b/src/hash/hash_defs.h index e7ab1d21c..673da8ecc 100644 --- a/src/hash/hash_defs.h +++ b/src/hash/hash_defs.h @@ -22,7 +22,6 @@ #include "hash_key_operations.h" #include "main/snort_types.h" -#include "utils/sfmemcap.h" namespace snort { diff --git a/src/hash/hash_key_operations.cc b/src/hash/hash_key_operations.cc index 72700c32d..c70ff91d5 100644 --- a/src/hash/hash_key_operations.cc +++ b/src/hash/hash_key_operations.cc @@ -70,10 +70,10 @@ unsigned HashKeyOperations::do_hash(const unsigned char* key, int len) bool HashKeyOperations::key_compare(const void* key1, const void* key2, size_t len) { - if ( memcmp(key1, key2, len ) == 0 ) - return true; - else + if ( memcmp(key1, key2, len) ) return false; + else + return true; } namespace snort diff --git a/src/hash/test/CMakeLists.txt b/src/hash/test/CMakeLists.txt index 7346627d1..20e176d8b 100644 --- a/src/hash/test/CMakeLists.txt +++ b/src/hash/test/CMakeLists.txt @@ -12,7 +12,6 @@ add_cpputest( xhash_test ../hash_lru_cache.cc ../primetable.cc ../xhash.cc - ../../utils/sfmemcap.cc ) add_cpputest( ghash_test @@ -30,5 +29,4 @@ add_cpputest( zhash_test ../primetable.cc ../xhash.cc ../zhash.cc - ../../utils/sfmemcap.cc -) \ No newline at end of file +) diff --git a/src/hash/xhash.cc b/src/hash/xhash.cc index 962e49e81..ee7d74152 100644 --- a/src/hash/xhash.cc +++ b/src/hash/xhash.cc @@ -516,7 +516,7 @@ bool XHash::delete_lru_node() { if ( HashNode* hnode = lru_cache->remove_lru_node() ) { - unlink_node(hnode); // remove from the hash table + unlink_node(hnode); free_user_data(hnode); mem_allocator->free(hnode); --num_nodes; diff --git a/src/log/messages.cc b/src/log/messages.cc index 6c5948f05..c4c0ad476 100644 --- a/src/log/messages.cc +++ b/src/log/messages.cc @@ -27,8 +27,6 @@ #include #include -#include -#include #include "main/snort_config.h" #include "parser/parser.h" @@ -43,6 +41,8 @@ static unsigned parse_errors = 0; static unsigned parse_warnings = 0; static unsigned reload_errors = 0; +static std::string reload_errors_description; + void reset_parse_errors() { parse_errors = 0; @@ -63,11 +63,22 @@ unsigned get_parse_warnings() return tmp; } +void reset_reload_errors() +{ + reload_errors = 0; + reload_errors_description.clear(); +} + unsigned get_reload_errors() { return reload_errors; } +std::string& get_reload_errors_description() +{ + return reload_errors_description; +} + static void log_message(FILE* file, const char* type, const char* msg) { const char* file_name; @@ -125,12 +136,20 @@ void ReloadError(const char* format, ...) va_list ap; va_start(ap, format); - vsnprintf(buf, STD_BUF, format, ap); + vsnprintf(buf, sizeof(buf), format, ap); va_end(ap); - buf[STD_BUF] = '\0'; + buf[sizeof(buf)-1] = '\0'; log_message(stderr, "ERROR", buf); + if ( reload_errors_description.empty() ) + reload_errors_description = buf; + else + { + reload_errors_description += ","; + reload_errors_description += buf; + } + reload_errors++; } diff --git a/src/log/messages.h b/src/log/messages.h index f830a4cbf..316895d6d 100644 --- a/src/log/messages.h +++ b/src/log/messages.h @@ -23,6 +23,7 @@ #include #include +#include #include #include "main/snort_types.h" @@ -49,7 +50,9 @@ enum WarningGroup void reset_parse_errors(); unsigned get_parse_errors(); unsigned get_parse_warnings(); +void reset_reload_errors(); unsigned get_reload_errors(); +std::string& get_reload_errors_description(); namespace snort { diff --git a/src/main.cc b/src/main.cc index fc447ace8..1b0ae01ed 100644 --- a/src/main.cc +++ b/src/main.cc @@ -343,7 +343,12 @@ int main_reload_config(lua_State* L) if ( !sc ) { if (get_reload_errors()) - current_request->respond("== reload failed - restart required\n"); + { + std::string response_message = "== reload failed - restart required - "; + response_message += get_reload_errors_description() + "\n"; + current_request->respond(response_message.c_str()); + reset_reload_errors(); + } else current_request->respond("== reload failed - bad config\n"); diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc index 922f54409..c825b2bfd 100644 --- a/src/main/analyzer.cc +++ b/src/main/analyzer.cc @@ -616,6 +616,7 @@ void Analyzer::reinit(SnortConfig* sc) { InspectorManager::thread_reinit(sc); ActionManager::thread_reinit(sc); + IpsManager::thread_reinit(sc); } void Analyzer::term() diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc index e60bc5a3c..6a736a2c9 100644 --- a/src/main/snort_config.cc +++ b/src/main/snort_config.cc @@ -493,160 +493,53 @@ void SnortConfig::merge(SnortConfig* cmd_line) state = new std::vector[num_slots]; } -// FIXIT-L this is a work around till snort supports adding/removing -// stream cache during reload -bool SnortConfig::verify_stream_inspectors() -{ - const std::vector inspector_names - { "stream_file", "stream_icmp", "stream_ip", "stream_tcp", "stream_udp", "stream_user" }; - static std::map orig_inspectors; - - // If wasn't initialized before try to initialize from current config - if (orig_inspectors.empty()) - { - const Inspector* const ptr = InspectorManager::get_inspector("stream", true); - if (ptr != nullptr) - { - for (auto name: inspector_names) - { - const bool in_orig = InspectorManager::inspector_exists_in_any_policy(name, get_conf()); - orig_inspectors[name] = in_orig; - } - } - } - - // If now available - compare - if (!orig_inspectors.empty()) - { - const Inspector* const ptr = InspectorManager::get_inspector("stream", true, this); - if (ptr != nullptr) - { - for (auto name: inspector_names) - { - const bool in_new = InspectorManager::inspector_exists_in_any_policy(name, this); - if (orig_inspectors[name] != in_new) - { - ReloadError("Snort Reload: Adding/removing %s requires a restart.\n", name); - return false; - } - } - } - } - - return true; -} - bool SnortConfig::verify() { + bool config_ok = false; + if (get_conf()->asn1_mem != asn1_mem) - { - ReloadError("Snort Reload: Changing the asn1 memory configuration " - "requires a restart.\n"); - return false; - } + ReloadError("Changing detection.asn1_mem requires a restart.\n"); - if ( bpf_filter != get_conf()->bpf_filter ) - { - ReloadError("Snort Reload: Changing the bpf filter configuration " - "requires a restart.\n"); - return false; - } + else if ( get_conf()->bpf_filter != bpf_filter ) + ReloadError("Changing packets.bfp_filter requires a restart.\n"); - if ( respond_attempts != get_conf()->respond_attempts || - respond_device != get_conf()->respond_device ) - { - ReloadError("Snort Reload: Changing config response requires a restart.\n"); - return false; - } + else if ( get_conf()->respond_attempts != respond_attempts ) + ReloadError("Changing active.attempts requires a restart.\n"); - if (get_conf()->chroot_dir != chroot_dir) - { - ReloadError("Snort Reload: Changing the chroot directory " - "configuration requires a restart.\n"); - return false; - } + else if ( get_conf()->respond_device != respond_device ) + ReloadError("Changing active.device requires a restart.\n"); - if ((get_conf()->run_flags & RUN_FLAG__DAEMON) != - (run_flags & RUN_FLAG__DAEMON)) - { - ReloadError("Snort Reload: Changing to or from daemon mode " - "requires a restart.\n"); - return false; - } + else if (get_conf()->chroot_dir != chroot_dir) + ReloadError("Changing process.chroot requires a restart.\n"); - /* Orig log dir because a chroot might have changed it */ - if (get_conf()->orig_log_dir != orig_log_dir) - { - ReloadError("Snort Reload: Changing the log directory " - "configuration requires a restart.\n"); - return false; - } + else if ((get_conf()->run_flags & RUN_FLAG__DAEMON) != (run_flags & RUN_FLAG__DAEMON)) + ReloadError("Changing process.daemon requires a restart.\n"); - if (get_conf()->max_attribute_hosts != max_attribute_hosts) - { - ReloadError("Snort Reload: Changing max_attribute_hosts " - "configuration requires a restart.\n"); - return false; - } - if (get_conf()->max_attribute_services_per_host != max_attribute_services_per_host) - { - ReloadError("Snort Reload: Changing max_attribute_services_per_host " - "configuration requires a restart.\n"); - return false; - } + else if (get_conf()->orig_log_dir != orig_log_dir) + ReloadError("Changing output.logdir requires a restart.\n"); - if ((get_conf()->run_flags & RUN_FLAG__NO_PROMISCUOUS) != - (run_flags & RUN_FLAG__NO_PROMISCUOUS)) - { - ReloadError("Snort Reload: Changing to or from promiscuous mode " - "requires a restart.\n"); - return false; - } + else if (get_conf()->group_id != group_id) + ReloadError("Changing process.setgid requires a restart.\n"); - if (get_conf()->group_id != group_id) - { - ReloadError("Snort Reload: Changing the group id configuration requires a restart.\n"); - return false; - } + else if (get_conf()->user_id != user_id) + ReloadError("Changing process.setuid requires a restart.\n"); - if (get_conf()->user_id != user_id) - { - ReloadError("Snort Reload: Changing the user id configuration requires a restart.\n"); - return false; - } + else if (get_conf()->daq_config->get_mru_size() != daq_config->get_mru_size()) + ReloadError("Changing daq.snaplen requires a restart.\n"); - if (get_conf()->daq_config->get_mru_size() != daq_config->get_mru_size()) - { - ReloadError("Snort Reload: Changing the packet snaplen " - "configuration requires a restart.\n"); - return false; - } + else if (get_conf()->threshold_config->memcap != threshold_config->memcap) + ReloadError("Changing alerts.event_filter_memcap requires a restart.\n"); - if (get_conf()->threshold_config->memcap != - threshold_config->memcap) - { - ReloadError("Snort Reload: Changing the threshold memcap " - "configuration requires a restart.\n"); - return false; - } + else if (get_conf()->rate_filter_config->memcap != rate_filter_config->memcap) + ReloadError("Changing alerts.rate_filter_memcap requires a restart.\n"); - if (get_conf()->rate_filter_config->memcap != - rate_filter_config->memcap) - { - ReloadError("Snort Reload: Changing the rate filter memcap " - "configuration requires a restart.\n"); - return false; - } + else if (get_conf()->detection_filter_config->memcap != detection_filter_config->memcap) + ReloadError("Changing alerts.detection_filter_memcap requires a restart.\n"); - if (get_conf()->detection_filter_config->memcap != - detection_filter_config->memcap) - { - ReloadError("Snort Reload: Changing the detection filter memcap " - "configuration requires a restart.\n"); - return false; - } + else + config_ok = true; - return verify_stream_inspectors(); + return config_ok; } void SnortConfig::set_alert_before_pass(bool enabled) diff --git a/src/main/snort_config.h b/src/main/snort_config.h index 9edad61b9..4c5dcdc1b 100644 --- a/src/main/snort_config.h +++ b/src/main/snort_config.h @@ -41,7 +41,7 @@ enum RunFlag { RUN_FLAG__READ = 0x00000001, RUN_FLAG__DAEMON = 0x00000002, - RUN_FLAG__NO_PROMISCUOUS = 0x00000004, + // unused = 0x00000004, // unused = 0x00000008, RUN_FLAG__INLINE = 0x00000010, @@ -179,7 +179,6 @@ struct SnortConfig { private: void init(const SnortConfig* const, ProtocolReference*); - bool verify_stream_inspectors(); public: SnortConfig(const SnortConfig* const other_conf = nullptr); diff --git a/src/managers/ips_manager.cc b/src/managers/ips_manager.cc index 089432347..9cfab779b 100644 --- a/src/managers/ips_manager.cc +++ b/src/managers/ips_manager.cc @@ -41,10 +41,11 @@ struct Option { const IpsApi* api; bool init; + bool thread_init; unsigned count; Option(const IpsApi* p) - { api = p; init = false; count = 0; } + { api = p; init = false; thread_init = false; count = 0; } }; typedef map OptionMap; @@ -359,8 +360,11 @@ void IpsManager::reset_options() void IpsManager::setup_options() { for ( auto& p : s_options ) - if ( p.second->init && p.second->api->tinit ) + if ( p.second->init && !p.second->thread_init && p.second->api->tinit ) + { p.second->api->tinit(SnortConfig::get_conf()); + p.second->thread_init = true; + } } void IpsManager::clear_options() @@ -379,6 +383,9 @@ bool IpsManager::verify(SnortConfig* sc) return true; } +void IpsManager::thread_reinit(snort::SnortConfig*) +{ IpsManager::setup_options(); } + #ifdef PIGLET static const IpsApi* find_api(const char* name) diff --git a/src/managers/ips_manager.h b/src/managers/ips_manager.h index 54a665c52..1ee3e1236 100644 --- a/src/managers/ips_manager.h +++ b/src/managers/ips_manager.h @@ -74,6 +74,7 @@ public: static void setup_options(); static void clear_options(); static bool verify(snort::SnortConfig*); + static void thread_reinit(snort::SnortConfig*); #ifdef PIGLET static IpsOptionWrapper* instantiate(const char*, snort::Module*, struct OptTreeNode*); diff --git a/src/ports/port_object2.cc b/src/ports/port_object2.cc index 110ec33dc..fd801521f 100644 --- a/src/ports/port_object2.cc +++ b/src/ports/port_object2.cc @@ -79,14 +79,6 @@ public: return HashKeyOperations::do_hash(key, len); } - - bool key_compare(const void* k1, const void* k2, size_t len) override - { - if ( memcmp(k1, k2, len ) ) - return false; - else - return true; - } }; static int* RuleHashToSortedArray(GHash* rh) diff --git a/src/service_inspectors/http2_inspect/http2_dummy_packet.h b/src/service_inspectors/http2_inspect/http2_dummy_packet.h index 1ab7e1676..043ff0eaa 100644 --- a/src/service_inspectors/http2_inspect/http2_dummy_packet.h +++ b/src/service_inspectors/http2_inspect/http2_dummy_packet.h @@ -20,7 +20,7 @@ /* * The purpose of this Packet subclass is to enable H2I to take direction from http_inspect on * whether or not to send a frame to detection. When http_inspect is processing normal HTTP/1.1 - * traffic it is dealing with a 'real' packet that has a context, the field on which disable_all() + * traffic it is dealing with a real packet that has a context, the field on which disable_all() * is called to disable detection on that packet. With HTTP/2 traffic, http_inspect is processing a * dummy packet that H2I created, which does not contain a context object. Rather than create an * entire new context object when we really only need a bool, http_inspect checks if the flow is diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index fc6c7df37..ddaa33885 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -192,7 +192,11 @@ void StreamBase::tinit() if ( config.flow_cache_cfg.max_flows > 0 ) flow_con->init_exp(config.flow_cache_cfg.max_flows); +#ifdef REG_TEST FlushBucket::set(config.footprint); +#else + FlushBucket::set(); +#endif } void StreamBase::tterm() diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc index c8ba5a7c9..56e5cf5d0 100644 --- a/src/stream/base/stream_module.cc +++ b/src/stream/base/stream_module.cc @@ -64,8 +64,10 @@ FLOW_TYPE_PARAMS(file_params, "180", "32"); static const Parameter s_params[] = { +#ifdef REG_TEST { "footprint", Parameter::PT_INT, "0:max32", "0", "use zero for production, non-zero for testing at given size (for TCP and user)" }, +#endif { "ip_frags_only", Parameter::PT_BOOL, nullptr, "false", "don't process non-frag flows" }, @@ -130,12 +132,15 @@ bool StreamModule::set(const char* fqn, Value& v, SnortConfig* c) { PktType type = PktType::NONE; +#ifdef REG_TEST if ( v.is("footprint") ) { config.footprint = v.get_uint32(); return true; } - else if ( v.is("ip_frags_only") ) +#endif + + if ( v.is("ip_frags_only") ) { if ( v.get_bool() ) c->set_run_flags(RUN_FLAG__IP_FRAGS_ONLY); @@ -176,8 +181,6 @@ bool StreamModule::set(const char* fqn, Value& v, SnortConfig* c) return true; } -// FIXIT-L the detection of stream.xxx_cache changes below is a temporary workaround -// remove this check when stream.xxx_cache params become reloadable bool StreamModule::end(const char*, int, SnortConfig* sc) { if ( reload_resource_manager.initialize(config) ) @@ -198,22 +201,23 @@ void StreamModule::reset_stats() // Stream handler to adjust allocated resources as needed on a config reload bool StreamReloadResourceManager::initialize(const StreamModuleConfig& config_) { - // FIXIT-L - saving config here to check footprint change is a bit of a hack, - if ( Snort::is_reloading() ) + // saving a copy of the config only works here because there is only + // one stream inspector per packet thread... + if ( !Snort::is_reloading() ) { - if ( config.footprint != config_.footprint ) - { - // FIXIT-M - reinit FlushBucket... - ReloadError("Changing of stream.footprint requires a restart\n"); - return false; - } - config = config_; - return true; + return false; } +#ifdef REG_TEST + if ( config.footprint != config_.footprint ) + { + ReloadError("Changing of stream.footprint requires a restart\n"); + return false; + } +#endif config = config_; - return false; + return true; } bool StreamReloadResourceManager::tinit() diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index f359673c9..3384ab30a 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -74,7 +74,10 @@ extern THREAD_LOCAL BaseStats stream_base_stats; struct StreamModuleConfig { FlowCacheConfig flow_cache_cfg; +#ifdef REG_TEST unsigned footprint = 0; +#endif + }; class StreamReloadResourceManager : public snort::ReloadResourceTuner diff --git a/src/stream/flush_bucket.cc b/src/stream/flush_bucket.cc index 3100ecd6a..a8ab9f627 100644 --- a/src/stream/flush_bucket.cc +++ b/src/stream/flush_bucket.cc @@ -51,6 +51,9 @@ void FlushBucket::set(unsigned sz) assert(s_flush_bucket); } +void FlushBucket::set() +{ set(0); } + void FlushBucket::clear() { delete s_flush_bucket; diff --git a/src/stream/flush_bucket.h b/src/stream/flush_bucket.h index 063870884..3ad3664b1 100644 --- a/src/stream/flush_bucket.h +++ b/src/stream/flush_bucket.h @@ -33,6 +33,7 @@ public: static uint16_t get_size(); static void set(unsigned sz); + static void set(); static void clear(); protected: diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index d83948583..c06eff91e 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -12,7 +12,6 @@ set( UTIL_INCLUDES safec.h segment_mem.h sflsq.h - sfmemcap.h stats.h util.h util_ber.h @@ -32,7 +31,6 @@ add_library ( utils OBJECT kmap.cc segment_mem.cc sflsq.cc - sfmemcap.cc snort_bounds.h stats.cc util.cc diff --git a/src/utils/sfmemcap.cc b/src/utils/sfmemcap.cc deleted file mode 100644 index 904d5cb19..000000000 --- a/src/utils/sfmemcap.cc +++ /dev/null @@ -1,104 +0,0 @@ -//-------------------------------------------------------------------------- -// Copyright (C) 2014-2020 Cisco and/or its affiliates. All rights reserved. -// Copyright (C) 2003-2013 Sourcefire, Inc. -// -// This program is free software; you can redistribute it and/or modify it -// under the terms of the GNU General Public License Version 2 as published -// by the Free Software Foundation. You may not use, modify or distribute -// this program under any other version of the GNU General Public License. -// -// This program is distributed in the hope that it will be useful, but -// WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -// General Public License for more details. -// -// You should have received a copy of the GNU General Public License along -// with this program; if not, write to the Free Software Foundation, Inc., -// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -//-------------------------------------------------------------------------- - -/* - sfmemcap.c - - These functions wrap the alloc & free functions. They enforce a memory cap using - the MEMCAP structure. The MEMCAP structure tracks memory usage. Each allocation - has 4 bytes added to it so we can store the allocation size. This allows us to - free a block and accurately track how much memory was recovered. - - Marc Norton -*/ - -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif - -#include "sfmemcap.h" - -#include "util.h" -#include "util_cstring.h" - -/* -* Set max # bytes & init other variables. -*/ -void sfmemcap_init(MEMCAP* mc, unsigned long nbytes) -{ - mc->memcap = nbytes; - mc->memused= 0; - mc->nblocks= 0; -} - -/* -* Allocate some memory -*/ -void* sfmemcap_alloc(MEMCAP* mc, unsigned long nbytes) -{ - long* data; - - //printf("sfmemcap_alloc: %d bytes requested, memcap=%d, - // used=%d\n",nbytes,mc->memcap,mc->memused); - - nbytes += sizeof(long); - - /* Check if we are limiting memory use */ - if ( mc->memcap > 0 ) - { - /* Check if we've maxed out our memory - if we are tracking memory */ - if ( (mc->memused + nbytes) > mc->memcap ) - return nullptr; - } - - //data = (long*)snort_alloc( nbytes ); - data = (long*)snort_calloc(nbytes); - *data++ = (long)nbytes; - - mc->memused += nbytes; - mc->nblocks++; - - return data; -} - -/* -* Free some memory -*/ -void sfmemcap_free(MEMCAP* mc, void* p) -{ - long* q; - - q = (long*)p; - q--; - mc->memused -= (unsigned)(*q); - mc->nblocks--; - - snort_free(q); -} - -/* -* For debugging. -*/ -void sfmemcap_showmem(MEMCAP* mc) -{ - fprintf(stderr, "memcap: memcap = %lu bytes,",mc->memcap); - fprintf(stderr, " memused= %lu bytes,",mc->memused); - fprintf(stderr, " nblocks= %d blocks\n",mc->nblocks); -} - diff --git a/src/utils/sfmemcap.h b/src/utils/sfmemcap.h deleted file mode 100644 index d2b80434f..000000000 --- a/src/utils/sfmemcap.h +++ /dev/null @@ -1,39 +0,0 @@ -//-------------------------------------------------------------------------- -// Copyright (C) 2014-2020 Cisco and/or its affiliates. All rights reserved. -// Copyright (C) 2003-2013 Sourcefire, Inc. -// -// This program is free software; you can redistribute it and/or modify it -// under the terms of the GNU General Public License Version 2 as published -// by the Free Software Foundation. You may not use, modify or distribute -// this program under any other version of the GNU General Public License. -// -// This program is distributed in the hope that it will be useful, but -// WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -// General Public License for more details. -// -// You should have received a copy of the GNU General Public License along -// with this program; if not, write to the Free Software Foundation, Inc., -// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -//-------------------------------------------------------------------------- - -#ifndef SFMEMCAP_H -#define SFMEMCAP_H - -// _alloc and _free wrappers that enforce a memory cap - -struct MEMCAP -{ - unsigned long memused; - unsigned long memcap; - int nblocks; -}; - -// FIXIT-L could be refactored as a class but should be deleted -void sfmemcap_init(MEMCAP* mc, unsigned long nbytes); -void* sfmemcap_alloc(MEMCAP* mc, unsigned long nbytes); -void sfmemcap_showmem(MEMCAP* mc); -void sfmemcap_free(MEMCAP* mc, void* memory); - -#endif - -- 2.47.3