From: Ron Dempster (rdempste) Date: Wed, 17 May 2023 20:54:44 +0000 (+0000) Subject: Pull request #3804: flow: do not recycle flow cache entries X-Git-Tag: 3.1.62.0~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=06b0571f75ef454ece82ea7f9ee3c90274dd9ea0;p=thirdparty%2Fsnort3.git Pull request #3804: flow: do not recycle flow cache entries Merge in SNORT/snort3 from ~RDEMPSTE/snort3:free_flow to master Squashed commit of the following: commit 36cc202818b9d2d7eefd918943ee2c2739d2a414 Author: Ron Dempster (rdempste) Date: Tue Apr 25 09:49:46 2023 -0400 decompress, detetion, file_api, framework: cppcheck fixes commit 281da6ad7f3ad3b8aecfb363fd0895132ff6e301 Author: Ron Dempster (rdempste) Date: Tue Apr 25 09:51:25 2023 -0400 flow: clean up flow termination commit dc4f6ee866c7aefab7964eb4e5682c9af9d5d2db Author: Ron Dempster (rdempste) Date: Mon Apr 10 10:12:23 2023 -0400 flow: do not recycle flow cache entries --- diff --git a/src/decompress/file_decomp.cc b/src/decompress/file_decomp.cc index 679319c93..340d432af 100644 --- a/src/decompress/file_decomp.cc +++ b/src/decompress/file_decomp.cc @@ -290,22 +290,7 @@ fd_status_t File_Decomp_Init(fd_session_t* SessionPtr) /* Returns a new session object from the MemPool */ fd_session_t* File_Decomp_New() { - fd_session_t* New_Session = new fd_session_t; - - New_Session->State = STATE_NEW; - New_Session->Sig_State = 0; - New_Session->Total_In = 0; - New_Session->Total_Out = 0; - New_Session->Avail_In = 0; - New_Session->Next_In = nullptr; - New_Session->Avail_Out = 0; - New_Session->Next_Out = nullptr; - New_Session->File_Type = FILE_TYPE_NONE; - New_Session->vba_analysis = false; - New_Session->ole_data_ptr = nullptr; - New_Session->ole_data_len = 0; - - return New_Session; + return new fd_session_t{}; } /* Process Decompression. The session Next_In, Avail_In, Next_Out, Avail_Out MUST have been @@ -313,8 +298,6 @@ fd_session_t* File_Decomp_New() */ fd_status_t File_Decomp(fd_session_t* SessionPtr) { - fd_status_t Return_Code; - if ( (SessionPtr == nullptr) || (SessionPtr->State == STATE_NEW) || (SessionPtr->Next_In == nullptr) || (SessionPtr->Next_Out == nullptr) ) return( File_Decomp_Error ); @@ -322,6 +305,8 @@ fd_status_t File_Decomp(fd_session_t* SessionPtr) /* STATE_NEW: Look for one of the configured file signatures. */ if ( SessionPtr->State == STATE_READY ) { + fd_status_t Return_Code; + /* Look for the signature at the beginning of the payload stream. */ if ( (Return_Code = Locate_Sig_Here(SessionPtr)) == File_Decomp_OK ) { diff --git a/src/decompress/file_olefile.h b/src/decompress/file_olefile.h index 589f05115..d1584221a 100644 --- a/src/decompress/file_olefile.h +++ b/src/decompress/file_olefile.h @@ -180,21 +180,18 @@ public: return stream_size; } - FileProperty() - { - name = nullptr; - } + FileProperty() = default; private: - char* name; - object_type file_type; - color_flag color; - int32_t lef_sib_id; - int32_t rig_sib_id; - int32_t root_node_id; - char cls_id[16]; - int32_t starting_sector; - int64_t stream_size; + char* name = nullptr; + object_type file_type = EMPTY; + color_flag color = RED; + int32_t lef_sib_id = 0; + int32_t rig_sib_id = 0; + int32_t root_node_id = 0; + char cls_id[16] = {}; + int32_t starting_sector = 0; + int64_t stream_size = 0; }; class DirectoryList diff --git a/src/decompress/file_oleheader.h b/src/decompress/file_oleheader.h index dafc51075..b32c14d42 100644 --- a/src/decompress/file_oleheader.h +++ b/src/decompress/file_oleheader.h @@ -82,31 +82,26 @@ public: int32_t get_first_difat(); void set_difat_array(const uint8_t* buf); int32_t get_difat_array(int num); - OleHeader() - { - first_dir = -1; - fat_sector_count = 0; - first_minifat = 0; - } + OleHeader() = default; private: - unsigned char sig[8]; //0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1 - uint16_t minor_version; - uint16_t major_version; - uint16_t byte_order; - uint16_t sector_size; - uint16_t mini_sector_size; - int32_t dir_sector_count; - int32_t first_dir; - int32_t difat_count; - int32_t fat_sector_count; - uint32_t minifat_cutoff; - int32_t first_minifat; - int32_t minifat_count; - int32_t first_difat; - int32_t difat_array[MAX_DIFAT_SECTORS]; + unsigned char sig[8] = {}; //0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1 + uint16_t minor_version = 0; + uint16_t major_version = 0; + uint16_t byte_order = 0; + uint16_t sector_size = 0; + uint16_t mini_sector_size = 0; + int32_t dir_sector_count = 0; + int32_t first_dir = -1; + int32_t difat_count = 0; + int32_t fat_sector_count = 0; + uint32_t minifat_cutoff = 0; + int32_t first_minifat = 0; + int32_t minifat_count = 0; + int32_t first_difat = 0; + int32_t difat_array[MAX_DIFAT_SECTORS] = {}; - byte_order_endianess byte_order_endian; + byte_order_endianess byte_order_endian = LITL_END; }; #endif diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc index 41ae1d517..8b2c786a6 100644 --- a/src/file_api/file_cache.cc +++ b/src/file_api/file_cache.cc @@ -435,7 +435,7 @@ bool FileCache::apply_verdict(Packet* p, FileContext* file_ctx, FileVerdict verd FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL, p, "File signature lookup adding packet to retry queue" - "Resume=%d, Waited %" PRIi64 "ms.\n", resume, + "Resume=%d, Waited %" PRIi64 "ms.\n", resume, time_elapsed_ms(&now, &file_ctx->pending_expire_time, lookup_timeout)); } } @@ -457,15 +457,10 @@ bool FileCache::apply_verdict(Packet* p, FileContext* file_ctx, FileVerdict verd "apply_verdict:FILE_VERDICT_PENDING with action drop\n"); act->set_delayed_action(Active::ACT_DROP, true); } - else + else if (files) { - FileFlows* files = FileFlows::get_file_flows(flow); - if (files) - { - files->add_pending_file(file_ctx->get_file_id()); - FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p, - "apply_verdict:Adding file id to pending\n"); - } + FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p, + "apply_verdict:Adding file id to pending\n"); } } return true; diff --git a/src/file_api/file_capture.cc b/src/file_api/file_capture.cc index a5c87e68c..90147a4c0 100644 --- a/src/file_api/file_capture.cc +++ b/src/file_api/file_capture.cc @@ -475,7 +475,7 @@ void FileCapture::store_file() if (!file_info) return; - std::string& file_full_name = file_info->get_file_name(); + const std::string& file_full_name = file_info->get_file_name(); /*Check whether the file exists*/ struct stat buffer; diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc index cc81fea44..f72f6adb2 100644 --- a/src/file_api/file_lib.cc +++ b/src/file_api/file_lib.cc @@ -638,10 +638,9 @@ void FileContext::find_file_type_from_ips(Packet* pkt, const uint8_t* file_data, depth_exhausted = true; } const FileConfig* const conf = get_file_config(); + Packet *p = DetectionEngine::set_next_packet(pkt); DetectionEngine de; - Packet* p = DetectionEngine::get_current_packet(); p->flow = pkt->flow; - p->pkth = pkt->pkth; p->context->file_data = { file_data, (unsigned int)data_size }; p->context->file_pos = processed_bytes; @@ -805,16 +804,23 @@ uint64_t FileContext::get_processed_bytes() void FileContext::print_file_data(FILE* fp, const uint8_t* data, int len, int max_depth) { - char str[18]; - int i, pos; - if (max_depth < len) len = max_depth; fprintf(fp,"Show length: %d \n", len); - for (i=0, pos=0; irem_ref(); - ssn_client = nullptr; - } if ( ssn_server ) - { ssn_server->rem_ref(); - ssn_server = nullptr; - } if ( clouseau ) clouseau->rem_ref(); @@ -113,19 +70,25 @@ void Flow::term() if ( data ) clear_data(); - if ( ha_state ) - delete ha_state; + delete ha_state; + delete stash; + delete ips_cont; +} + +void Flow::init(PktType type) +{ + pkt_type = type; + bitop = nullptr; - if (stash) + if ( HighAvailabilityManager::active() ) { - delete stash; - stash = nullptr; + ha_state = new FlowHAState; + previous_ssn_state = ssn_state; } + mpls_client.length = 0; + mpls_server.length = 0; - delete ips_cont; - ips_cont = nullptr; - - service = nullptr; + stash = new FlowStash; } inline void Flow::clean() @@ -140,11 +103,8 @@ inline void Flow::clean() delete[] mpls_server.start; mpls_server.length = 0; } - if ( bitop ) - { - delete bitop; - bitop = nullptr; - } + delete bitop; + bitop = nullptr; filtering_state.clear(); } @@ -191,44 +151,6 @@ void Flow::reset(bool do_cleanup) session->cleanup(); } } - - free_flow_data(); - clean(); - - // FIXIT-M cleanup() winds up calling clear() - if ( ssn_client ) - { - ssn_client->rem_ref(); - ssn_client = nullptr; - } - if ( ssn_server ) - { - ssn_server->rem_ref(); - ssn_server = nullptr; - } - if ( clouseau ) - clear_clouseau(); - - if ( gadget ) - clear_gadget(); - - if ( data ) - clear_data(); - - if ( ha_state ) - ha_state->reset(); - - if ( stash ) - stash->reset(); - - deferred_trust.clear(); - - delete ips_cont; - ips_cont = nullptr; - - constexpr size_t offset = offsetof(Flow, context_chain); - // FIXIT-L need a struct to zero here to make future proof - memset((uint8_t*)this+offset, 0, sizeof(Flow)-offset); } void Flow::restart(bool dump_flow_data) @@ -496,7 +418,7 @@ void Flow::set_direction(Packet* p) } } -void Flow::set_expire(const Packet* p, uint32_t timeout) +void Flow::set_expire(const Packet* p, uint64_t timeout) { expire_time = (uint64_t)p->pkth->ts.tv_sec + timeout; } diff --git a/src/flow/flow.h b/src/flow/flow.h index 0b3ec2b19..8307bbf49 100644 --- a/src/flow/flow.h +++ b/src/flow/flow.h @@ -182,14 +182,13 @@ public: RESET, ALLOW }; - Flow(); + Flow() = default; ~Flow(); Flow(const Flow&) = delete; Flow& operator=(const Flow&) = delete; void init(PktType); - void term(); void flush(bool do_cleanup = true); void reset(bool do_cleanup = true); @@ -206,7 +205,7 @@ public: void markup_packet_flags(Packet*); void set_client_initiate(Packet*); void set_direction(Packet*); - void set_expire(const Packet*, uint32_t timeout); + void set_expire(const Packet*, uint64_t timeout); bool expired(const Packet*); void set_ttl(Packet*, bool client); void set_mpls_layer_per_dir(Packet*); @@ -412,76 +411,76 @@ public: public: // FIXIT-M privatize if possible // fields are organized by initialization and size to minimize - // void space and allow for memset of tail end of struct + // void space DeferredTrust deferred_trust; - // Anything before this comment is not zeroed during construction - const FlowKey* key; - BitOp* bitop; - FlowHAState* ha_state; - FlowStash* stash; + const FlowKey* key = nullptr; + BitOp* bitop = nullptr; + FlowHAState* ha_state = nullptr; + FlowStash* stash = nullptr; - uint8_t ip_proto; - PktType pkt_type; // ^^ + uint8_t ip_proto = 0; + PktType pkt_type = PktType::NONE; // ^^ // these fields are always set; not zeroed - Flow* prev, * next; - Session* session; - Inspector* ssn_client; - Inspector* ssn_server; - Continuation* ips_cont; + Flow* prev = nullptr; + Flow* next = nullptr; + Session* session = nullptr; + Inspector* ssn_client = nullptr; + Inspector* ssn_server = nullptr; + Continuation* ips_cont = nullptr; - long last_data_seen; - Layer mpls_client, mpls_server; + long last_data_seen = 0; + Layer mpls_client = {}; + Layer mpls_server = {}; - // everything from here down is zeroed IpsContextChain context_chain; - FlowData* flow_data; - FlowStats flowstats; - StreamFlowIntf* stream_intf; + FlowData* flow_data = nullptr; + FlowStats flowstats = {}; + StreamFlowIntf* stream_intf = nullptr; - SfIp client_ip; - SfIp server_ip; + SfIp client_ip = {}; + SfIp server_ip = {}; - LwState ssn_state; - LwState previous_ssn_state; + LwState ssn_state = {}; + LwState previous_ssn_state = {}; - Inspector* clouseau; // service identifier - Inspector* gadget; // service handler - Inspector* assistant_gadget; - Inspector* data; - const char* service; + Inspector* clouseau = nullptr; // service identifier + Inspector* gadget = nullptr; // service handler + Inspector* assistant_gadget = nullptr; + Inspector* data = nullptr; + const char* service = nullptr; - uint64_t expire_time; + uint64_t expire_time = 0; - unsigned network_policy_id; - unsigned inspection_policy_id; - unsigned ips_policy_id; - unsigned reload_id; + unsigned network_policy_id = 0; + unsigned inspection_policy_id = 0; + unsigned ips_policy_id = 0; + unsigned reload_id = 0; - uint32_t tenant; + uint32_t tenant = 0; - uint32_t default_session_timeout; + uint32_t default_session_timeout = 0; - int32_t client_intf; - int32_t server_intf; + int32_t client_intf = 0; + int32_t server_intf = 0; - int16_t client_group; - int16_t server_group; + int16_t client_group = 0; + int16_t server_group = 0; - uint16_t client_port; - uint16_t server_port; + uint16_t client_port = 0; + uint16_t server_port = 0; - uint16_t ssn_policy; - uint16_t session_state; + uint16_t ssn_policy = 0; + uint16_t session_state = 0; - uint8_t inner_client_ttl; - uint8_t inner_server_ttl; - uint8_t outer_client_ttl; - uint8_t outer_server_ttl; + uint8_t inner_client_ttl = 0; + uint8_t inner_server_ttl = 0; + uint8_t outer_client_ttl = 0; + uint8_t outer_server_ttl = 0; - uint8_t response_count; + uint8_t response_count = 0; struct { @@ -497,9 +496,9 @@ public: // FIXIT-M privatize if possible bool efd_flow : 1; // Indicate that current flow is an elephant flow bool svc_event_generated : 1; // Set if FLOW_NO_SERVICE_EVENT was generated for this flow bool retry_queued : 1; // Set if a packet was queued for retry for this flow - } flags; + } flags = {}; - FlowState flow_state; + FlowState flow_state = FlowState::SETUP; FilteringState filtering_state; diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc index 238cd49c3..3bab4cb94 100644 --- a/src/flow/flow_cache.cc +++ b/src/flow/flow_cache.cc @@ -55,12 +55,11 @@ static const unsigned WDT_MASK = 7; // kick watchdog once for every 8 flows dele // FlowCache stuff //------------------------------------------------------------------------- -THREAD_LOCAL bool FlowCache::pruning_in_progress = false; extern THREAD_LOCAL const snort::Trace* stream_trace; FlowCache::FlowCache(const FlowCacheConfig& cfg) : config(cfg) { - hash_table = new ZHash(config.max_flows, sizeof(FlowKey)); + hash_table = new ZHash(config.max_flows, sizeof(FlowKey), false); uni_flows = new FlowUniList; uni_ip_flows = new FlowUniList; flags = 0x0; @@ -74,6 +73,11 @@ FlowCache::~FlowCache() delete_uni(); } +unsigned FlowCache::get_flows_allocated() const +{ + return hash_table->get_num_nodes(); +} + void FlowCache::delete_uni() { delete uni_flows; @@ -87,7 +91,6 @@ void FlowCache::push(Flow* flow) { void* key = hash_table->push(flow); flow->key = (FlowKey*)key; - ++flows_allocated; } unsigned FlowCache::get_count() @@ -154,58 +157,45 @@ void FlowCache::unlink_uni(Flow* flow) Flow* FlowCache::allocate(const FlowKey* key) { + // This is called by packet processing and HA consume. This method is only called after a + // failed attempt to find a flow with this key. time_t timestamp = packet_time(); - Flow* flow = (Flow*)hash_table->get(key); - if ( !flow ) + if ( hash_table->get_num_nodes() >= config.max_flows ) { - if ( flows_allocated < config.max_flows ) - { - Flow* new_flow = new Flow(); - push(new_flow); - } - else if ( !prune_stale(timestamp, nullptr) ) + if ( !prune_stale(timestamp, nullptr) ) { if ( !prune_unis(key->pkt_type) ) prune_excess(nullptr); } - - flow = (Flow*)hash_table->get(key); - assert(flow); - - if ( flow->session && flow->pkt_type != key->pkt_type ) - flow->term(); - else - flow->reset(); } - link_uni(flow); - if ( flow->session && flow->pkt_type != key->pkt_type ) - flow->term(); + Flow* flow = new Flow; + push(flow); + flow = (Flow*)hash_table->get(key); + assert(flow); + link_uni(flow); flow->last_data_seen = timestamp; - return flow; } void FlowCache::remove(Flow* flow) { unlink_uni(flow); - - hash_table->release_node(flow->key); + const snort::FlowKey* key = flow->key; + // Delete before releasing the node, so that the key is valid until the flow is completely freed + delete flow; + hash_table->release_node(key); } bool FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup) { - assert(!pruning_in_progress); - pruning_in_progress = true; - if ( !flow->was_blocked() ) { flow->flush(do_cleanup); if ( flow->ssn_state.session_flags & SSNFLAG_KEEP_FLOW ) { flow->ssn_state.session_flags &= ~SSNFLAG_KEEP_FLOW; - pruning_in_progress = false; return false; } } @@ -213,14 +203,12 @@ bool FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup) flow->reset(do_cleanup); prune_stats.update(reason); remove(flow); - pruning_in_progress = false; return true; } void FlowCache::retire(Flow* flow) { flow->reset(true); - flow->term(); prune_stats.update(PruneReason::NONE); remove(flow); } @@ -469,10 +457,10 @@ unsigned FlowCache::delete_active_flows(unsigned mode, unsigned num_to_delete, u delete_stats.update(FlowDeleteState::ALLOWED); flow->reset(true); + // Delete before removing the node, so that the key is valid until the flow is completely freed + delete flow; //The flow should not be removed from the hash before reset hash_table->remove(); - delete flow; - --flows_allocated; ++deleted; --num_to_delete; } @@ -489,27 +477,8 @@ unsigned FlowCache::delete_flows(unsigned num_to_delete) { PacketTracerSuspend pt_susp; - // delete from the free list first... - while ( num_to_delete ) - { - if ( (deleted & WDT_MASK) == 0 ) - ThreadConfig::preemptive_kick(); - - Flow* flow = (Flow*)hash_table->pop(); - if ( !flow ) - break; - - delete flow; - delete_stats.update(FlowDeleteState::FREELIST); - - --flows_allocated; - ++deleted; - --num_to_delete; - } - - unsigned mode = ALLOWED_FLOWS_ONLY; - while ( num_to_delete && mode <= ALL_FLOWS ) - num_to_delete = delete_active_flows(mode++, num_to_delete, deleted); + for ( unsigned mode = ALLOWED_FLOWS_ONLY; num_to_delete && mode <= ALL_FLOWS; ++mode ) + num_to_delete = delete_active_flows(mode, num_to_delete, deleted); } if ( PacketTracer::is_active() and deleted ) @@ -525,20 +494,11 @@ unsigned FlowCache::purge() FlagContext(flags, SESSION_CACHE_FLAG_PURGING); unsigned retired = 0; - assert(!pruning_in_progress); - pruning_in_progress = true; while ( auto flow = static_cast(hash_table->lru_first()) ) { retire(flow); ++retired; } - pruning_in_progress = false; - - while ( Flow* flow = (Flow*)hash_table->pop() ) - { - delete flow; - --flows_allocated; - } // Remove these here so alloc/dealloc counts are right when Memory::get_pegs is called delete_uni(); @@ -560,8 +520,3 @@ size_t FlowCache::flows_size() const { return hash_table->get_num_nodes(); } - -size_t FlowCache::free_flows_size() const -{ - return hash_table->get_num_free_nodes(); -} diff --git a/src/flow/flow_cache.h b/src/flow/flow_cache.h index 0c725c42b..b23342f0d 100644 --- a/src/flow/flow_cache.h +++ b/src/flow/flow_cache.h @@ -95,16 +95,11 @@ public: const FlowCacheConfig& get_flow_cache_config() const { return config; } - unsigned get_flows_allocated() const - { return flows_allocated; } - - static bool is_pruning_in_progress() - { return pruning_in_progress; } + unsigned get_flows_allocated() const; size_t uni_flows_size() const; size_t uni_ip_flows_size() const; size_t flows_size() const; - size_t free_flows_size() const; private: void delete_uni(); @@ -117,13 +112,11 @@ private: (unsigned mode, unsigned num_to_delete, unsigned &deleted); private: - static THREAD_LOCAL bool pruning_in_progress; static const unsigned cleanup_flows = 1; FlowCacheConfig config; uint32_t flags; class ZHash* hash_table; - unsigned flows_allocated = 0; FlowUniList* uni_flows; FlowUniList* uni_ip_flows; diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index 77e898ae9..fe975b0c5 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -88,9 +88,6 @@ PegCount FlowControl::get_uni_ip_flows() const PegCount FlowControl::get_num_flows() const { return cache->flows_size(); } -PegCount FlowControl::get_num_free_flows() const -{ return cache->free_flows_size(); } - //------------------------------------------------------------------------- // cache foo @@ -150,8 +147,8 @@ Flow* FlowControl::stale_flow_cleanup(FlowCache* cache, Flow* flow, Packet* p) { PacketTracerSuspend pt_susp; - cache->release(flow, PruneReason::STALE); - flow = nullptr; + if ( cache->release(flow, PruneReason::STALE) ) + flow = nullptr; } } diff --git a/src/flow/flow_control.h b/src/flow/flow_control.h index fbe9a3f5c..500d9f5a8 100644 --- a/src/flow/flow_control.h +++ b/src/flow/flow_control.h @@ -98,7 +98,6 @@ public: PegCount get_uni_flows() const; PegCount get_uni_ip_flows() const; PegCount get_num_flows() const; - PegCount get_num_free_flows() const; private: void set_key(snort::FlowKey*, snort::Packet*); diff --git a/src/flow/test/deferred_trust_test.cc b/src/flow/test/deferred_trust_test.cc index 4c91f7894..132d1c838 100644 --- a/src/flow/test/deferred_trust_test.cc +++ b/src/flow/test/deferred_trust_test.cc @@ -37,11 +37,11 @@ using namespace snort; TEST_GROUP(deferred_trust_test) { - DeferredTrust deferred_trust; }; TEST(deferred_trust_test, set_deferred_trust) { + DeferredTrust deferred_trust; // Disable non-existent module_id deferred_trust.set_deferred_trust(1, false); CHECK_TEXT(!deferred_trust.is_active(), "Deferred trust should not be active"); @@ -89,6 +89,7 @@ TEST(deferred_trust_test, set_deferred_trust) TEST(deferred_trust_test, finalize) { + DeferredTrust deferred_trust; Active active{}; active.block_again(); @@ -158,6 +159,7 @@ void Active::drop_packet(const Packet*, bool) TEST(deferred_trust_test, finalize_clear) { + DeferredTrust deferred_trust; Active active{}; deferred_trust.clear(); diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 6fc0d0e2a..777ed99dc 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -69,18 +69,11 @@ void Active::set_drop_reason(char const*) { } Packet::Packet(bool) { } Packet::~Packet() = default; uint32_t Packet::get_flow_geneve_vni() const { return 0; } -Flow::Flow() -{ - constexpr size_t offset = offsetof(Flow, key); - // FIXIT-L need a struct to zero here to make future proof - memset((uint8_t*)this+offset, 0, sizeof(*this)-offset); -} Flow::~Flow() = default; DetectionEngine::DetectionEngine() = default; ExpectCache::~ExpectCache() = default; DetectionEngine::~DetectionEngine() = default; void Flow::init(PktType) { } -void Flow::term() { } void Flow::flush(bool) { } void Flow::reset(bool) { } void Flow::free_flow_data() { } diff --git a/src/flow/test/flow_control_test.cc b/src/flow/test/flow_control_test.cc index e00c02de2..267eaeb27 100644 --- a/src/flow/test/flow_control_test.cc +++ b/src/flow/test/flow_control_test.cc @@ -53,7 +53,6 @@ THREAD_LOCAL bool Active::s_suspend = false; THREAD_LOCAL Active::ActiveSuspendReason Active::s_suspend_reason = Active::ASP_NONE; THREAD_LOCAL PacketTracer* snort::s_pkt_trace = nullptr; -THREAD_LOCAL bool FlowCache::pruning_in_progress = false; void Active::drop_packet(snort::Packet const*, bool) { } PacketTracer::~PacketTracer() = default; @@ -69,12 +68,12 @@ Packet::~Packet() = default; uint32_t Packet::get_flow_geneve_vni() const { return 0; } FlowCache::FlowCache(const FlowCacheConfig& cfg) : config(cfg) { } FlowCache::~FlowCache() = default; -Flow::Flow() = default; Flow::~Flow() = default; DetectionEngine::DetectionEngine() = default; DetectionEngine::~DetectionEngine() = default; ExpectCache::~ExpectCache() = default; unsigned FlowCache::purge() { return 1; } +unsigned FlowCache::get_flows_allocated() const { return 0; } Flow* FlowCache::find(const FlowKey*) { return nullptr; } Flow* FlowCache::allocate(const FlowKey*) { return nullptr; } void FlowCache::push(Flow*) { } @@ -85,7 +84,6 @@ unsigned FlowCache::timeout(unsigned, time_t) { return 1; } size_t FlowCache::uni_flows_size() const { return 0; } size_t FlowCache::uni_ip_flows_size() const { return 0; } size_t FlowCache::flows_size() const { return 0; } -size_t FlowCache::free_flows_size() const { return 0; } void Flow::init(PktType) { } void DataBus::publish(unsigned, unsigned, DataEvent&, Flow*) { } void DataBus::publish(unsigned, unsigned, const uint8_t*, unsigned, Flow*) { } diff --git a/src/flow/test/flow_test.cc b/src/flow/test/flow_test.cc index c1255afb4..72502b065 100644 --- a/src/flow/test/flow_test.cc +++ b/src/flow/test/flow_test.cc @@ -112,7 +112,7 @@ TEST(nondefault_timeout, hard_expiration) { uint64_t validate = 100; Packet pkt(false); - Flow *flow = new Flow(); + Flow *flow = new Flow; DAQ_PktHdr_t pkthdr; pkt.pkth = &pkthdr; diff --git a/src/flow/test/ha_test.cc b/src/flow/test/ha_test.cc index e9a2712ae..c78836592 100644 --- a/src/flow/test/ha_test.cc +++ b/src/flow/test/ha_test.cc @@ -27,6 +27,7 @@ #include #include +#include using namespace snort; @@ -72,7 +73,6 @@ static struct __attribute__((__packed__)) TestUpdateMessage { HAMessageHeader mhdr; FlowKey key; HAClientHeader schdr; - // cppcheck-suppress unusedStructMember uint8_t scmsg[10]; } s_update_stream_message = { @@ -91,31 +91,6 @@ static struct __attribute__((__packed__)) TestUpdateMessage { }; -static struct timeval s_packet_time = { 0, 0 }; -static uint8_t s_message[MSG_SIZE]; -static SideChannel s_side_channel; -static SCMessage s_sc_message; -static SCMessage s_rec_sc_message; -static bool s_stream_consume_called = false; -static uint8_t s_stream_consume_size = 0; -static bool s_other_consume_called = false; -static uint8_t s_other_consume_size = 0; -static bool s_get_session_called = false; -static bool s_delete_session_called = false; -static bool s_transmit_message_called = false; -static bool s_stream_update_required = false; -static bool s_other_update_required = false; -static uint8_t* s_message_content = nullptr; -static uint8_t s_message_length = 0; -static Flow s_flow; -static FlowKey s_flowkey; -static Packet s_pkt; -static Active active; -static StreamHAClient* s_ha_client; -static FlowHAClient* s_other_ha_client; -static std::function s_handler = nullptr; -static SCMsgHdr s_sc_header = { 0, 1, 0, 0, }; - class StreamHAClient : public FlowHAClient { public: @@ -123,8 +98,8 @@ public: ~StreamHAClient() override = default; bool consume(Flow*&, const FlowKey*, HAMessage& msg, uint8_t size) override { - s_stream_consume_called = true; - s_stream_consume_size = size; + mock().actualCall("consume"); + mock().setData("stream_consume_size", (int)size); for ( uint8_t i = 0; i < 10; i++ ) { @@ -147,7 +122,10 @@ public: } return true; } - bool is_update_required(Flow*) override { return s_stream_update_required; } + bool is_update_required(Flow*) override + { + return (bool)mock().getData("stream_update_required").getIntValue(); + } }; class OtherHAClient : public FlowHAClient @@ -157,8 +135,8 @@ public: ~OtherHAClient() override = default; bool consume(Flow*&, const FlowKey*, HAMessage& msg, uint8_t size) override { - s_other_consume_called = true; - s_other_consume_size = size; + mock().actualCall("other_consume"); + mock().setData("other_consume_size", (int)size); for ( uint8_t i = 0; i < 5; i++ ) { @@ -181,7 +159,10 @@ public: } return true; } - bool is_update_required(Flow*) override { return s_other_update_required; } + bool is_update_required(Flow*) override + { + return (bool)mock().getData("other_update_required").getIntValue(); + } }; //------------------------------------------------------------------------- @@ -192,9 +173,9 @@ THREAD_LOCAL HAStats ha_stats = { }; Flow* Stream::get_flow(const FlowKey* flowkey) { - s_flowkey = *flowkey; - s_get_session_called = true; - return &s_flow; + mock().actualCall("get_flow"); + mock().setDataObject("flowkey", "FlowKey", (void*)flowkey); + return (Flow*)mock().getData("flow").getObjectPointer(); } Packet::Packet(bool) { } @@ -202,8 +183,8 @@ Packet::~Packet() = default; void Stream::delete_flow(const FlowKey* flowkey) { - s_flowkey = *flowkey; - s_delete_session_called = true; + mock().actualCall("delete_flow"); + mock().setDataObject("flowkey", "FlowKey", (void*)flowkey); } namespace snort @@ -212,21 +193,23 @@ void ErrorMessage(const char*,...) { } void LogMessage(const char*,...) { } void packet_gettimeofday(struct timeval* tv) -{ *tv = s_packet_time; } +{ + *tv = *(struct timeval*)mock().getData("packet_tv").getObjectPointer(); +} } bool FlowKey::is_equal(const void*, const void*, size_t) { return false; } int SFDAQInstance::ioctl(DAQ_IoctlCmd, void*, size_t) { return DAQ_SUCCESS; } -Flow::Flow() { ha_state = new FlowHAState; key = new FlowKey; } -Flow::~Flow() { delete key; delete ha_state; } +Flow::~Flow() { } FlowStash::~FlowStash() = default; void Flow::set_client_initiate(Packet*) { } void Flow::set_direction(Packet*) { } +static SideChannel s_side_channel; SideChannel* SideChannelManager::get_side_channel(SCPort) { return &s_side_channel; } @@ -237,6 +220,7 @@ Connector::Direction SideChannel::get_direction() void SideChannel::set_default_port(SCPort) { } +static std::function s_handler = nullptr; void SideChannel::register_receive_handler(const std::function& handler) { s_handler = handler; @@ -247,12 +231,15 @@ void SideChannel::unregister_receive_handler() { } bool SideChannel::discard_message(SCMessage*) { return true; } +static SCMsgHdr s_sc_header = { 0, 1, 0, 0 }; bool SideChannel::process(int) { - if ( s_handler && s_message_content && (s_message_length != 0)) + SCMessage* msg = (SCMessage*)mock().getData("message_content").getObjectPointer(); + if (s_handler && msg && msg->content && msg->content_length != 0) { - s_rec_sc_message.content = s_message_content; - s_rec_sc_message.content_length = s_message_length; + SCMessage s_rec_sc_message = {}; + s_rec_sc_message.content = msg->content; + s_rec_sc_message.content_length = msg->content_length; s_rec_sc_message.hdr = &s_sc_header; s_rec_sc_message.sc = &s_side_channel; s_handler(&s_rec_sc_message); @@ -264,9 +251,8 @@ bool SideChannel::process(int) bool SideChannel::transmit_message(SCMessage* msg) { - s_transmit_message_called = true; - s_message_content = msg->content; - s_message_length = msg->content_length; + mock().actualCall("transmit_message"); + mock().setDataObject("message", "SCMessage", msg); return true; } @@ -275,9 +261,9 @@ SCMessage* SideChannel::alloc_transmit_message(uint32_t len) if ( len > MSG_SIZE ) return nullptr; - s_sc_message.content = s_message; - s_sc_message.content_length = len; - return &s_sc_message; + SCMessage* message = (SCMessage*)mock().getData("message_content").getObjectPointer(); + message->content_length = len; + return message; } //------------------------------------------------------------------------- @@ -289,6 +275,7 @@ TEST_GROUP(high_availability_manager_test) void teardown() override { HighAvailabilityManager::term(); + mock().clear(); } }; @@ -307,22 +294,32 @@ TEST(high_availability_manager_test, inst_init_term) HighAvailabilityConfig hac; hac.enabled = true; hac.daq_channel = false; - hac.ports = new PortBitSet(); + hac.ports = new PortBitSet; hac.ports->set(1); hac.min_session_lifetime = { 1, 0 }; hac.min_sync_interval = { 0, 500000 }; HighAvailabilityManager::configure(&hac); HighAvailabilityManager::thread_init(); - s_ha_client = new StreamHAClient; CHECK(HighAvailabilityManager::active()==true); - delete s_ha_client; HighAvailabilityManager::thread_term(); CHECK(HighAvailabilityManager::active()==false); } TEST_GROUP(flow_ha_state_test) { + struct timeval s_packet_time; + + void setup() override + { + s_packet_time = {}; + mock().setDataObject("packet_tv", "struct timeval", &s_packet_time); + } + + void teardown() override + { + mock().clear(); + } }; TEST(flow_ha_state_test, timing_test) @@ -332,19 +329,17 @@ TEST(flow_ha_state_test, timing_test) FlowHAState::config_timers(min_age, min_age); // one-time config s_packet_time.tv_sec = 1; - FlowHAState* state = new FlowHAState; - state->set_next_update(); // set the time for next update + FlowHAState state; + state.set_next_update(); // set the time for next update s_packet_time.tv_sec = 2; // advance the clock to 2 seconds - CHECK(state->sync_interval_elapsed() == false); - delete state; + CHECK(state.sync_interval_elapsed() == false); s_packet_time.tv_sec = 1; - state = new FlowHAState; - CHECK(state->sync_interval_elapsed() == false); + FlowHAState state2; + CHECK(state2.sync_interval_elapsed() == false); s_packet_time.tv_sec = 22; // advance the clock to 22 seconds - state->set_next_update(); // set the time for next update - CHECK(state->sync_interval_elapsed() == true); - delete state; + state2.set_next_update(); // set the time for next update + CHECK(state2.sync_interval_elapsed() == true); } @@ -384,99 +379,123 @@ TEST(flow_ha_state_test, state_test) TEST_GROUP(high_availability_test) { + Flow s_flow; + Active active; + StreamHAClient* s_ha_client; // cppcheck-suppress variableScope + FlowHAClient* s_other_ha_client; // cppcheck-suppress variableScope + uint8_t s_message[MSG_SIZE]; + SCMessage s_sc_message; + Packet s_pkt; + struct timeval s_packet_time; + HighAvailabilityConfig hac; + FlowHAState* ha_state; + FlowKey flow_key; + void setup() override { + s_packet_time = {}; + mock().setDataObject("packet_tv", "struct timeval", &s_packet_time); + mock().setData("stream_update_required", false); + mock().setData("other_update_required", false); + ha_state = new FlowHAState; + s_flow.ha_state = ha_state; + flow_key = {}; + s_flow.key = &flow_key; + mock().setDataObject("flow", "Flow", &s_flow); + active = {}; + memset(s_message, 0, sizeof(s_message)); + s_sc_message = {}; + s_sc_message.content = s_message; + mock().setDataObject("message_content", "SCMessage", &s_sc_message); + s_pkt.active = &active; // cppcheck-suppress unreadVariable + memset(&ha_stats, 0, sizeof(ha_stats)); - HighAvailabilityConfig hac; hac.enabled = true; hac.daq_channel = false; - hac.ports = new PortBitSet(); + hac.ports = new PortBitSet; hac.ports->set(1); hac.min_session_lifetime = { 1, 0 }; hac.min_sync_interval = { 0, 500000 }; HighAvailabilityManager::configure(&hac); HighAvailabilityManager::thread_init(); - s_ha_client = new StreamHAClient; - s_other_ha_client = new OtherHAClient; + s_ha_client = new StreamHAClient; // cppcheck-suppress unreadVariable + s_other_ha_client = new OtherHAClient; // cppcheck-suppress unreadVariable } void teardown() override { delete s_other_ha_client; delete s_ha_client; + delete ha_state; HighAvailabilityManager::thread_term(); HighAvailabilityManager::term(); + mock().clear(); } }; TEST(high_availability_test, receive_deletion) { - s_delete_session_called = false; - s_message_content = (uint8_t*) &s_delete_message; - s_message_length = sizeof(s_delete_message); + s_sc_message.content = (uint8_t*) &s_delete_message; + s_sc_message.content_length = sizeof(s_delete_message); + mock().expectNCalls(1, "delete_flow"); HighAvailabilityManager::process_receive(); - CHECK(s_delete_session_called == true); - CHECK(memcmp((const void*)&s_flowkey, (const void*)&s_test_key, sizeof(s_test_key)) == 0); + mock().checkExpectations(); + MEMCMP_EQUAL_TEXT((const void*)mock().getData("flowkey").getObjectPointer(), + (const void*)&s_test_key, sizeof(s_test_key), "flow key should be s_test_key"); } TEST(high_availability_test, receive_update_stream_only) { - s_stream_consume_called = false; - s_stream_consume_size = 0; - s_message_content = (uint8_t*) &s_update_stream_message; - s_message_length = sizeof(s_update_stream_message); + s_sc_message.content = (uint8_t*) &s_update_stream_message; + s_sc_message.content_length = sizeof(s_update_stream_message); + mock().expectNCalls(1, "get_flow"); + mock().expectNCalls(1, "consume"); HighAvailabilityManager::process_receive(); - CHECK(s_stream_consume_called == true); - CHECK(s_stream_consume_size == 10); - CHECK(memcmp((const void*)&s_flowkey, (const void*)&s_test_key, sizeof(s_test_key)) == 0); + mock().checkExpectations(); + CHECK(mock().getData("stream_consume_size").getIntValue() == 10); + MEMCMP_EQUAL_TEXT((const void*)mock().getData("flowkey").getObjectPointer(), + (const void*)&s_test_key, sizeof(s_test_key), "flow key should be s_test_key"); } TEST(high_availability_test, transmit_deletion) { - s_transmit_message_called = false; + mock().expectNCalls(1, "transmit_message"); HighAvailabilityManager::process_deletion(s_flow); - CHECK(s_transmit_message_called == true); } TEST(high_availability_test, transmit_update_no_update) { - s_transmit_message_called = false; - s_stream_update_required = false; - s_other_update_required = false; - s_pkt.active = &active; + mock().setData("stream_update_required", (int)false); + mock().setData("other_update_required", (int)false); + mock().expectNCalls(1, "transmit_message"); HighAvailabilityManager::process_update(&s_flow, &s_pkt); - CHECK(s_transmit_message_called == false); } TEST(high_availability_test, transmit_update_stream_only) { - s_transmit_message_called = false; - s_stream_update_required = true; - s_other_update_required = false; - s_pkt.active = &active; + mock().setData("stream_update_required", (int)true); + mock().setData("other_update_required", (int)false); + mock().expectNCalls(1, "transmit_message"); HighAvailabilityManager::process_update(&s_flow, &s_pkt); - CHECK(s_transmit_message_called == true); } TEST(high_availability_test, transmit_update_both_update) { - s_transmit_message_called = false; - s_stream_update_required = true; - s_other_update_required = true; - s_pkt.active = &active; + mock().setData("stream_update_required", (int)true); + mock().setData("other_update_required", (int)true); CHECK(s_other_ha_client->handle == 1); s_flow.ha_state->set_pending(s_other_ha_client->handle); + mock().expectNCalls(1, "transmit_message"); HighAvailabilityManager::process_update(&s_flow, &s_pkt); - CHECK(s_transmit_message_called == true); } TEST(high_availability_test, read_flow_key_error_v4) { HAMessageHeader hdr = { 0, 0, 0, KEY_TYPE_IP4 }; HAMessage msg((uint8_t*) &s_test_key, KEY_SIZE_IP4 / 2); - FlowKey key; + FlowKey key{}; CHECK(read_flow_key(msg, &hdr, key) == 0); CHECK(ha_stats.truncated_msgs == 1); @@ -486,7 +505,7 @@ TEST(high_availability_test, read_flow_key_error_v6) { HAMessageHeader hdr = { 0, 0, 0, KEY_TYPE_IP6 }; HAMessage msg((uint8_t*) &s_test_key, KEY_SIZE_IP6 / 2); - FlowKey key; + FlowKey key{}; CHECK(read_flow_key(msg, &hdr, key) == 0); CHECK(ha_stats.truncated_msgs == 1); @@ -496,7 +515,7 @@ TEST(high_availability_test, read_flow_key_error_unknown) { HAMessageHeader hdr = { 0, 0, 0, 0x42 }; HAMessage msg((uint8_t*) &s_test_key, sizeof(s_test_key)); - FlowKey key; + FlowKey key{}; CHECK(read_flow_key(msg, &hdr, key) == 0); CHECK(ha_stats.unknown_key_type == 1); @@ -506,8 +525,9 @@ TEST(high_availability_test, consume_error_truncated_client_hdr) { HAClientHeader chdr = { 0, 0 }; HAMessage msg((uint8_t*) &chdr, sizeof(chdr) / 2); - FlowKey key; + FlowKey key{}; + mock().expectNCalls(1, "get_flow"); consume_ha_update_message(msg, key, &s_pkt); CHECK(ha_stats.update_msgs_consumed == 0); CHECK(ha_stats.truncated_msgs == 1); @@ -517,8 +537,9 @@ TEST(high_availability_test, consume_error_invalid_client_idx) { HAClientHeader chdr = { 0x42, 0 }; HAMessage msg((uint8_t*) &chdr, sizeof(chdr)); - FlowKey key; + FlowKey key{}; + mock().expectNCalls(1, "get_flow"); consume_ha_update_message(msg, key, &s_pkt); CHECK(ha_stats.update_msgs_consumed == 0); CHECK(ha_stats.unknown_client_idx == 1); @@ -529,12 +550,12 @@ TEST(high_availability_test, consume_error_truncated_client_msg) struct __attribute__((__packed__)) { HAClientHeader chdr = { 0, 0x42 }; - // cppcheck-suppress unusedStructMember uint8_t cmsg[0x42 / 2] = { }; } input; HAMessage msg((uint8_t*) &input, sizeof(input)); - FlowKey key; + FlowKey key{}; + mock().expectNCalls(1, "get_flow"); consume_ha_update_message(msg, key, &s_pkt); CHECK(ha_stats.update_msgs_consumed == 0); CHECK(ha_stats.truncated_msgs == 1); @@ -545,12 +566,13 @@ TEST(high_availability_test, consume_error_client_consume) struct __attribute__((__packed__)) { HAClientHeader chdr = { 0, 10 }; - // cppcheck-suppress unusedStructMember uint8_t cmsg[0x42 / 2] = { }; } input; HAMessage msg((uint8_t*) &input, sizeof(input)); - FlowKey key; + FlowKey key{}; + mock().expectNCalls(1, "get_flow"); + mock().expectNCalls(1, "consume"); consume_ha_update_message(msg, key, &s_pkt); CHECK(ha_stats.update_msgs_consumed == 0); CHECK(ha_stats.client_consume_errors == 1); @@ -561,7 +583,7 @@ TEST(high_availability_test, consume_error_key_mismatch) HAMessageHeader hdr[10] = { 0, HA_MESSAGE_VERSION, 0x32, KEY_TYPE_IP4 }; HAMessage msg((uint8_t*) &hdr, sizeof(hdr)); - FlowKey packet_key; + FlowKey packet_key{}; FlowKey* key = &packet_key; CHECK(consume_ha_message(msg, key, &s_pkt) == nullptr); CHECK(ha_stats.key_mismatch == 1); @@ -601,9 +623,7 @@ TEST(high_availability_test, produce_error_client_hdr_overflow) { uint8_t buffer[sizeof(HAClientHeader) / 2]; HAMessage msg(buffer, sizeof(buffer)); - Flow flow; - - write_update_msg_client(s_ha_client, flow, msg); + write_update_msg_client(s_ha_client, s_flow, msg); CHECK(msg.cursor == msg.buffer); } @@ -611,9 +631,7 @@ TEST(high_availability_test, produce_error_client_produce) { uint8_t buffer[sizeof(HAClientHeader)]; HAMessage msg(buffer, sizeof(buffer)); - Flow flow; - - write_update_msg_client(s_ha_client, flow, msg); + write_update_msg_client(s_ha_client, s_flow, msg); CHECK(msg.cursor == msg.buffer); } diff --git a/src/framework/data_bus.cc b/src/framework/data_bus.cc index ef3eed1a7..f9d0e7ae0 100644 --- a/src/framework/data_bus.cc +++ b/src/framework/data_bus.cc @@ -221,7 +221,7 @@ void DataBus::_subscribe(const PubKey& key, unsigned eid, DataHandler* h) _subscribe(pid, eid, h); } -void DataBus::_unsubscribe(const PubKey& key, unsigned eid, DataHandler* h) +void DataBus::_unsubscribe(const PubKey& key, unsigned eid, const DataHandler* h) { unsigned pid = get_id(key); unsigned idx = pid + eid; diff --git a/src/framework/data_bus.h b/src/framework/data_bus.h index 915fecf18..5b3e6c197 100644 --- a/src/framework/data_bus.h +++ b/src/framework/data_bus.h @@ -128,7 +128,7 @@ public: private: void _subscribe(unsigned pub_id, unsigned evt_id, DataHandler*); void _subscribe(const PubKey&, unsigned evt_id, DataHandler*); - void _unsubscribe(const PubKey&, unsigned evt_id, DataHandler*); + void _unsubscribe(const PubKey&, unsigned evt_id, const DataHandler*); void _publish(unsigned pub_id, unsigned evt_id, DataEvent&, Flow*) const; private: diff --git a/src/framework/test/data_bus_test.cc b/src/framework/test/data_bus_test.cc index 85d036f2d..7de08bf09 100644 --- a/src/framework/test/data_bus_test.cc +++ b/src/framework/test/data_bus_test.cc @@ -120,7 +120,7 @@ TEST_GROUP(data_bus) { InspectionPolicy my_inspection_policy; NetworkPolicy my_network_policy; - unsigned pub_id = 0; // cppcheck-suppress variableScope + unsigned pub_id = 0; void setup() override { diff --git a/src/hash/test/zhash_test.cc b/src/hash/test/zhash_test.cc index 25bad2d5f..ed5ddd47b 100644 --- a/src/hash/test/zhash_test.cc +++ b/src/hash/test/zhash_test.cc @@ -75,7 +75,7 @@ SnortConfig::~SnortConfig() = default; const SnortConfig* SnortConfig::get_conf() { return snort_conf; } -const unsigned ZHASH_ROWS = 1000; +const unsigned ZHASH_ROWS = 50; const unsigned ZHASH_KEY_SIZE = 100; const unsigned MAX_ZHASH_NODES = 100; char key_buf[ZHASH_KEY_SIZE]; @@ -153,6 +153,68 @@ TEST(zhash, create_zhash_test) } } +TEST(zhash, zhash_pop_test) +{ + unsigned* pop_data = (unsigned*)zh->pop(); + CHECK_TEXT(nullptr == pop_data, "Empty pop should return nullptr"); + unsigned* data = (unsigned*)snort_calloc(sizeof(unsigned)); + zh->push(data); + pop_data = (unsigned*)zh->pop(); + CHECK_TEXT(pop_data == data, "Pop from free list should return pushed data"); + snort_free(pop_data); + pop_data = (unsigned*)zh->pop(); + CHECK_TEXT(nullptr == pop_data, "Pop after pop should return nullptr"); +} + +TEST(zhash, zhash_get_test) +{ + unsigned* data = (unsigned*)snort_calloc(sizeof(unsigned)); + zh->push(data); + key_buf[0] = 'a'; + unsigned* get_data = (unsigned*)zh->get(key_buf); + CHECK_TEXT(get_data == data, "Get should return pushed data"); + get_data = (unsigned*)zh->get(key_buf); + CHECK_TEXT(get_data == data, "Second get should return data"); + key_buf[0] = 'b'; + get_data = (unsigned*)zh->get(key_buf); + CHECK_TEXT(nullptr == get_data, "Get with nonexistent key should return nullptr"); + get_data = (unsigned*)zh->lru_first(); + CHECK_TEXT(data == get_data, "Lru first should return data"); + get_data = (unsigned*)zh->remove(); + CHECK_TEXT(get_data == data, "Remove node should return data"); + snort_free(get_data); +} + +TEST(zhash, zhash_lru_test) +{ + unsigned* data1 = (unsigned*)snort_calloc(sizeof(unsigned)); + zh->push(data1); + key_buf[0] = '1'; + unsigned* get_data = (unsigned*)zh->get(key_buf); + CHECK_TEXT(get_data == data1, "Get should return pushed data1"); + unsigned* data2 = (unsigned*)snort_calloc(sizeof(unsigned)); + zh->push(data2); + key_buf[0] = '2'; + get_data = (unsigned*)zh->get(key_buf); + CHECK_TEXT(get_data == data2, "Get should return pushed data2"); + + get_data = (unsigned*)zh->lru_first(); + CHECK_TEXT(get_data == data1, "Lru first should return data1"); + + zh->lru_touch(); + get_data = (unsigned*)zh->lru_first(); + CHECK_TEXT(get_data == data2, "Lru first should return data2 after touch"); + get_data = (unsigned*)zh->remove(); + CHECK_TEXT(get_data == data2, "Remove node should return data2"); + snort_free(get_data); + + get_data = (unsigned*)zh->lru_first(); + CHECK_TEXT(get_data == data1, "Lru first should return data1"); + get_data = (unsigned*)zh->remove(); + CHECK_TEXT(get_data == data1, "Remove node should return data1"); + snort_free(get_data); +} + int main(int argc, char** argv) { return CommandLineTestRunner::RunAllTests(argc, argv); diff --git a/src/hash/zhash.cc b/src/hash/zhash.cc index e0ac2a72b..ed928e2dc 100644 --- a/src/hash/zhash.cc +++ b/src/hash/zhash.cc @@ -41,11 +41,12 @@ using namespace snort; //------------------------------------------------------------------------- -ZHash::ZHash(int rows, int key_len) +ZHash::ZHash(int rows, int key_len, bool recycle) : XHash(rows, key_len) { initialize(new FlowHashKeyOps(nrows)); anr_enabled = false; + recycle_nodes = recycle; } void* ZHash::get(const void* key) diff --git a/src/hash/zhash.h b/src/hash/zhash.h index e4a066118..45c2df455 100644 --- a/src/hash/zhash.h +++ b/src/hash/zhash.h @@ -27,7 +27,7 @@ class ZHash : public snort::XHash { public: - ZHash(int nrows, int keysize); + ZHash(int nrows, int keysize, bool recycle = true); ZHash(const ZHash&) = delete; ZHash& operator=(const ZHash&) = delete; diff --git a/src/main/test/distill_verdict_stubs.h b/src/main/test/distill_verdict_stubs.h index e7ea809c5..0150916aa 100644 --- a/src/main/test/distill_verdict_stubs.h +++ b/src/main/test/distill_verdict_stubs.h @@ -222,7 +222,6 @@ void Stream::block_flow(const Packet*) { } IpsContext::IpsContext(unsigned) { } NetworkPolicy* get_network_policy() { return nullptr; } InspectionPolicy* get_inspection_policy() { return nullptr; } -Flow::Flow() = default; Flow::~Flow() = default; void ThreadConfig::implement_thread_affinity(SThreadType, unsigned) { } void ThreadConfig::set_instance_tid(int) { } diff --git a/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc b/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc index 7eee00bf7..0eed093ce 100644 --- a/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc +++ b/src/network_inspectors/appid/detector_plugins/test/detector_sip_test.cc @@ -61,7 +61,6 @@ namespace snort AppIdApi appid_api; AppIdSessionApi::AppIdSessionApi(const AppIdSession*, const SfIp&) : StashGenericObject(STASH_GENERIC_OBJECT_APPID) { } -Flow::Flow() = default; Flow::~Flow() = default; AppIdSession* AppIdApi::get_appid_session(snort::Flow const&) { return nullptr; } @@ -209,7 +208,7 @@ TEST(detector_sip_tests, sip_event_handler) odpctxt->initialize(appid_inspector); SipEvent event(&pkt, nullptr, nullptr); SipEventHandler event_handler(appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); delete sip_data; delete session; diff --git a/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc b/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc index 4b84fdf93..ee268b2a2 100644 --- a/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc +++ b/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc @@ -137,7 +137,7 @@ TEST(appid_eve_process_event_handler_tests, eve_process_event_handler) EveProcessEvent event(p, "firefox", 90); event.set_client_process_mapping(true); AppIdEveProcessEventHandler event_handler(dummy_appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); CHECK(session->get_eve_client_app_id() == APPID_UT_ID); delete flow; @@ -149,7 +149,7 @@ TEST(appid_eve_process_event_handler_tests, eve_user_agent_event_handler) EveProcessEvent event(p, "firefox", 90); event.set_user_agent("chrome"); AppIdEveProcessEventHandler event_handler(dummy_appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); CHECK(session->get_client_id() == APPID_UT_ID); delete flow; @@ -161,7 +161,7 @@ TEST(appid_eve_process_event_handler_tests, eve_server_name_event_handler) EveProcessEvent event(p, "firefox", 90); event.set_server_name("www.google.com"); AppIdEveProcessEventHandler event_handler(dummy_appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); CHECK(session->get_payload_id() == APPID_UT_ID + 1); delete flow; @@ -175,7 +175,7 @@ TEST(appid_eve_process_event_handler_tests, eve_alpn_event_handler) event.set_alpn(alpn); event.set_quic(true); AppIdEveProcessEventHandler event_handler(dummy_appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); CHECK(session->get_alpn_service_app_id() == APPID_UT_ID + 2); delete flow; @@ -189,7 +189,7 @@ TEST(appid_eve_process_event_handler_tests, eve_unknown_alpn_event_handler) event.set_alpn(alpn); event.set_quic(true); AppIdEveProcessEventHandler event_handler(dummy_appid_inspector); - Flow* flow = new Flow(); + Flow* flow = new Flow; event_handler.handle(event, flow); CHECK(session->get_alpn_service_app_id() == APP_ID_NONE); delete flow; diff --git a/src/network_inspectors/appid/test/appid_mock_flow.h b/src/network_inspectors/appid/test/appid_mock_flow.h index eb74c23a7..ed1e1ee43 100644 --- a/src/network_inspectors/appid/test/appid_mock_flow.h +++ b/src/network_inspectors/appid/test/appid_mock_flow.h @@ -33,7 +33,6 @@ FlowData::~FlowData() = default; FlowData* mock_flow_data = nullptr; typedef int32_t AppId; -Flow::Flow() = default; Flow::~Flow() = default; class FakeFlow : public Flow diff --git a/src/payload_injector/test/payload_injector_test.cc b/src/payload_injector/test/payload_injector_test.cc index 448174f45..94f2e60a2 100644 --- a/src/payload_injector/test/payload_injector_test.cc +++ b/src/payload_injector/test/payload_injector_test.cc @@ -54,12 +54,6 @@ uint32_t Active::send_data(snort::Packet*, EncodeFlags, unsigned char const*, un void Active::block_session(snort::Packet*, bool) { } void DetectionEngine::disable_all(snort::Packet*) { } -Flow::Flow() -{ - gadget = nullptr; - flow_state = Flow::FlowState::SETUP; -} - Flow::~Flow() = default; IpsContext::IpsContext(unsigned int) { } IpsContext::~IpsContext() = default; diff --git a/src/service_inspectors/dce_rpc/dce_co.cc b/src/service_inspectors/dce_rpc/dce_co.cc index f110c6bba..b02d14cdf 100644 --- a/src/service_inspectors/dce_rpc/dce_co.cc +++ b/src/service_inspectors/dce_rpc/dce_co.cc @@ -818,7 +818,6 @@ static void dce_co_process_ctx_result(DCE2_SsnData*, DCE2_CoTracker* cot, const Uuid* transport) { DCE2_CoCtxIdNode* ctx_node, * existing_ctx_node; - DCE2_Ret status; /* Dequeue context item in pending queue - this will get put in the permanent * context id list or freed */ @@ -898,8 +897,7 @@ static void dce_co_process_ctx_result(DCE2_SsnData*, DCE2_CoTracker* cot, } else { - status = DCE2_ListInsert(cot->ctx_ids, (void*)(uintptr_t)ctx_node->ctx_id, - (void*)ctx_node); + DCE2_Ret status = DCE2_ListInsert(cot->ctx_ids, (void*)(uintptr_t)ctx_node->ctx_id, (void*)ctx_node); if (status != DCE2_RET__SUCCESS) { snort_free((void*)ctx_node); @@ -1468,7 +1466,6 @@ static DCE2_Ret dce_co_handle_frag(DCE2_SsnData* sd, DCE2_CoTracker* cot, { uint32_t size = (frag_len < DCE2_CO__MIN_ALLOC_SIZE) ? DCE2_CO__MIN_ALLOC_SIZE : frag_len; DCE2_BufferMinAddFlag mflag = DCE2_BUFFER_MIN_ADD_FLAG__USE; - DCE2_Ret status; dce2CommonStats* dce_common_stats = dce_get_proto_stats_ptr(sd); Packet* p = DetectionEngine::get_current_packet(); if (p == nullptr) @@ -1537,7 +1534,7 @@ static DCE2_Ret dce_co_handle_frag(DCE2_SsnData* sd, DCE2_CoTracker* cot, if (DceRpcCoLastFrag(co_hdr) || (DCE2_BufferLength(frag_buf) == max_frag_data)) mflag = DCE2_BUFFER_MIN_ADD_FLAG__IGNORE; - status = DCE2_BufferAddData(frag_buf, frag_ptr, + DCE2_Ret status = DCE2_BufferAddData(frag_buf, frag_ptr, frag_len, DCE2_BufferLength(frag_buf), mflag); if (status != DCE2_RET__SUCCESS) diff --git a/src/service_inspectors/dce_rpc/dce_common.cc b/src/service_inspectors/dce_rpc/dce_common.cc index 114db146f..e0ded056c 100644 --- a/src/service_inspectors/dce_rpc/dce_common.cc +++ b/src/service_inspectors/dce_rpc/dce_common.cc @@ -141,9 +141,14 @@ bool dce2_paf_abort(DCE2_SsnData* sd) return false; } +void reset_using_rpkt() +{ + using_rpkt = false; +} + void DCE2_Detect(DCE2_SsnData* sd) { - if (!sd) return ; + if (!sd) return; DceContextData::set_current_ropts(sd); if ( using_rpkt ) { diff --git a/src/service_inspectors/dce_rpc/dce_common.h b/src/service_inspectors/dce_rpc/dce_common.h index 969326d20..7cc7eb89a 100644 --- a/src/service_inspectors/dce_rpc/dce_common.h +++ b/src/service_inspectors/dce_rpc/dce_common.h @@ -407,6 +407,7 @@ snort::Packet* DCE2_GetRpkt(snort::Packet*, DCE2_RpktType, const uint8_t*, uint3 uint16_t DCE2_GetRpktMaxData(DCE2_RpktType); DCE2_Ret DCE2_AddDataToRpkt(snort::Packet*, const uint8_t*, uint32_t); DCE2_TransType get_dce2_trans_type(const snort::Packet* p); +void reset_using_rpkt(); #endif diff --git a/src/service_inspectors/dce_rpc/dce_smb2_session.cc b/src/service_inspectors/dce_rpc/dce_smb2_session.cc index f535be1b9..5bd0dd872 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2_session.cc +++ b/src/service_inspectors/dce_rpc/dce_smb2_session.cc @@ -73,8 +73,7 @@ uint32_t Dce2Smb2SessionTracker::fill_map(const uint64_t msg_id, const uint8_t c msgid_state* mid_ptr; if (it == mid_map.end()) { - mid_ptr = new msgid_state { 0, 0, { 0 }, { 0 } - }; + mid_ptr = new msgid_state; mid_map.insert(std::make_pair(current_flow_key, mid_ptr)); } else diff --git a/src/service_inspectors/dce_rpc/dce_smb2_session.h b/src/service_inspectors/dce_rpc/dce_smb2_session.h index fe319947a..f8bc618cf 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2_session.h +++ b/src/service_inspectors/dce_rpc/dce_smb2_session.h @@ -30,8 +30,8 @@ uint32_t Smb2Tid(const Smb2Hdr* hdr); typedef struct _msgid_state { - uint64_t max_req_msg_id; - uint64_t max_resp_msg_id; + uint64_t max_req_msg_id = 0; + uint64_t max_resp_msg_id = 0; std::unordered_set missing_req_msg_ids; std::unordered_set missing_resp_msg_ids; } msgid_state; diff --git a/src/service_inspectors/dce_rpc/dce_smb2_session_cache.h b/src/service_inspectors/dce_rpc/dce_smb2_session_cache.h index c352fb29f..771382645 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2_session_cache.h +++ b/src/service_inspectors/dce_rpc/dce_smb2_session_cache.h @@ -41,19 +41,19 @@ public: using Data = std::shared_ptr; - Data find_id(Key key) + Data find_id(const Key& key) { Data session = this->find(key); return session; } - Data find_session(Key key) + Data find_session(const Key& key) { Data session = this->find(key); return session; } - Data find_else_create_session(Key& key, Dce2Smb2SessionData* ssd) + Data find_else_create_session(const Key& key, Dce2Smb2SessionData* ssd) { Data new_session = Data(new Value(key)); Data session = this->find_else_insert(key, new_session, nullptr,false); diff --git a/src/service_inspectors/dce_rpc/dce_smb_inspector.cc b/src/service_inspectors/dce_rpc/dce_smb_inspector.cc index ac3b42107..69474cf40 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_inspector.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_inspector.cc @@ -68,6 +68,8 @@ void Dce2Smb::eval(Packet* p) assert(p->has_tcp_data() || p->has_udp_quic_data()); assert(p->flow); + reset_using_rpkt(); + Dce2SmbFlowData* smb_flowdata = (Dce2SmbFlowData*)p->flow->get_flow_data(Dce2SmbFlowData::inspector_id); diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.cc b/src/service_inspectors/dce_rpc/dce_smb_utils.cc index cbb7262d9..f92ab9ebe 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_utils.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_utils.cc @@ -658,7 +658,7 @@ void DCE2_SmbRemoveRequestTracker(DCE2_SmbSsnData* ssd, } void DCE2_SmbRemoveFileTrackerFromRequestTrackers(DCE2_SmbSsnData* ssd, - DCE2_SmbFileTracker* ftracker) + const DCE2_SmbFileTracker* ftracker) { if (ftracker == nullptr) return; diff --git a/src/service_inspectors/dce_rpc/dce_smb_utils.h b/src/service_inspectors/dce_rpc/dce_smb_utils.h index 42c539018..0a2fdb432 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_utils.h +++ b/src/service_inspectors/dce_rpc/dce_smb_utils.h @@ -141,7 +141,7 @@ void DCE2_SmbCleanFileTracker(DCE2_SmbFileTracker*); void DCE2_SmbFileTrackerDataFree(void*); void DCE2_SmbCleanSessionFileTracker(DCE2_SmbSsnData*, DCE2_SmbFileTracker*); void DCE2_SmbRemoveFileTrackerFromRequestTrackers(DCE2_SmbSsnData*, - DCE2_SmbFileTracker*); + const DCE2_SmbFileTracker*); DCE2_SmbFileTracker* DCE2_SmbDequeueTmpFileTracker(DCE2_SmbSsnData*, DCE2_SmbRequestTracker*, const uint16_t); DCE2_SmbFileTracker* DCE2_SmbNewFileTracker(DCE2_SmbSsnData*, diff --git a/src/service_inspectors/dce_rpc/smb_message.cc b/src/service_inspectors/dce_rpc/smb_message.cc index 728572f1b..419677881 100644 --- a/src/service_inspectors/dce_rpc/smb_message.cc +++ b/src/service_inspectors/dce_rpc/smb_message.cc @@ -1030,13 +1030,11 @@ static void DCE2_SmbProcessCommand(DCE2_SmbSsnData* ssd, const SmbNtHdr* smb_hdr ********************************************************************/ static DCE2_SmbRequestTracker* DCE2_SmbInspect(DCE2_SmbSsnData* ssd, const SmbNtHdr* smb_hdr) { - int smb_com = SmbCom(smb_hdr); - - if (smb_com < 0 or smb_com > 255) return nullptr; + uint8_t smb_com = SmbCom(smb_hdr); SMB_DEBUG(dce_smb_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL, DetectionEngine::get_current_packet(), - "SMB command: %s (0x%02X)\n", get_smb_com_string(smb_com), smb_com); + "SMB command: %s (0x%02X)\n", get_smb_com_string(smb_com), (unsigned)smb_com); if (smb_com_funcs[smb_com] == nullptr) { diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc index adf9a1695..5d41ba020 100644 --- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc +++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc @@ -49,7 +49,6 @@ fd_status_t File_Decomp_StopFree(fd_session_t*) { return File_Decomp_OK; } uint32_t str_to_hash(const uint8_t *, size_t) { return 0; } FlowData* Flow::get_flow_data(uint32_t) const { return nullptr; } int Flow::set_flow_data(FlowData*) { return 0;} -Flow::Flow() { stream_intf = nullptr; } Flow::~Flow() = default; } @@ -73,7 +72,7 @@ public: TEST_GROUP(http_transaction_test) { - Flow* const flow = new Flow(); + Flow* const flow = new Flow; HttpParaList params; HttpFlowData* flow_data = new HttpFlowData(flow, ¶ms); SectionType* const section_type = HttpUnitTestSetup::get_section_type(flow_data); diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc index 7ec06882b..1041162dc 100644 --- a/src/service_inspectors/smtp/smtp.cc +++ b/src/service_inspectors/smtp/smtp.cc @@ -1743,6 +1743,7 @@ TEST_CASE("handle_header_line", "[smtp]") // Cleanup delete p.context; + p.flow->stash = nullptr; } TEST_CASE("normalize_data", "[smtp]") @@ -1774,5 +1775,6 @@ TEST_CASE("normalize_data", "[smtp]") // Cleanup delete p.context; + p.flow->stash = nullptr; } #endif diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc index 0f12ff7e3..15abe460f 100644 --- a/src/stream/base/stream_base.cc +++ b/src/stream/base/stream_base.cc @@ -58,7 +58,6 @@ static std::mutex crash_dump_flow_control_mutex; static BaseStats g_stats; THREAD_LOCAL BaseStats stream_base_stats; THREAD_LOCAL PegCount current_flows_prev; -THREAD_LOCAL PegCount current_free_flows_prev; THREAD_LOCAL PegCount uni_flows_prev; THREAD_LOCAL PegCount uni_ip_flows_prev; @@ -88,13 +87,12 @@ const PegInfo base_pegs[] = // Keep the NOW stats at the bottom as it requires special sum_stats logic { CountType::NOW, "current_flows", "current number of flows in cache" }, - { CountType::NOW, "current_free_flows", "current number of free flows in cache" }, { CountType::NOW, "uni_flows", "number of uni flows in cache" }, { CountType::NOW, "uni_ip_flows", "number of uni ip flows in cache" }, { CountType::END, nullptr, nullptr } }; -#define NOW_PEGS_NUM 4 +#define NOW_PEGS_NUM 3 // FIXIT-L dependency on stats define in another file void base_prep() @@ -116,7 +114,6 @@ void base_prep() stream_base_stats.reload_blocked_flow_deletes= flow_con->get_deletes(FlowDeleteState::BLOCKED); stream_base_stats.current_flows = flow_con->get_num_flows(); - stream_base_stats.current_free_flows = flow_con->get_num_free_flows(); stream_base_stats.uni_flows = flow_con->get_uni_flows(); stream_base_stats.uni_ip_flows = flow_con->get_uni_ip_flows(); @@ -137,7 +134,6 @@ void base_sum() array_size(base_pegs) - 1 - NOW_PEGS_NUM); g_stats.current_flows += (int64_t)stream_base_stats.current_flows - (int64_t)current_flows_prev; - g_stats.current_free_flows += (int64_t)stream_base_stats.current_free_flows - (int64_t)current_free_flows_prev; g_stats.uni_flows += (int64_t)stream_base_stats.uni_flows - (int64_t)uni_flows_prev; g_stats.uni_ip_flows += (int64_t)stream_base_stats.uni_ip_flows - (int64_t)uni_ip_flows_prev; @@ -152,7 +148,6 @@ void base_stats() void base_reset(bool reset_all) { current_flows_prev = stream_base_stats.current_flows; - current_free_flows_prev = stream_base_stats.current_free_flows; uni_flows_prev = stream_base_stats.uni_flows; uni_ip_flows_prev = stream_base_stats.uni_ip_flows; diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index 58e73c5cc..96a8f6e19 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -77,7 +77,6 @@ struct BaseStats // Keep the NOW stats at the bottom as it requires special sum_stats logic PegCount current_flows; - PegCount current_free_flows; PegCount uni_flows; PegCount uni_ip_flows; diff --git a/src/stream/ip/ip_session.cc b/src/stream/ip/ip_session.cc index a16bd962c..2bf51536c 100644 --- a/src/stream/ip/ip_session.cc +++ b/src/stream/ip/ip_session.cc @@ -277,6 +277,7 @@ TEST_CASE("IP Session", "[ip_session]") update_session(&p, &lws); CHECK(lws.expire_time == 360); + lws.ssn_server = nullptr; } } #endif diff --git a/src/stream/stream.cc b/src/stream/stream.cc index 9d5a828f5..7bffc920c 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -213,12 +213,15 @@ void Stream::check_flow_closed(Packet* p) if ( flow->flags.use_direct_inject ) p->packet_flags |= PKT_USE_DIRECT_INJECT; + p->flow = nullptr; + // this will get called on each onload // eventually all onloads will occur and delete will be called if ( not flow->is_suspended() ) + { flow_con->release_flow(flow, PruneReason::NONE); - - p->flow = nullptr; + return; + } } else if (flow->session_state & STREAM_STATE_BLOCK_PENDING) { diff --git a/src/stream/tcp/test/tcp_normalizer_test.cc b/src/stream/tcp/test/tcp_normalizer_test.cc index 1e0b5ea75..8905bc076 100644 --- a/src/stream/tcp/test/tcp_normalizer_test.cc +++ b/src/stream/tcp/test/tcp_normalizer_test.cc @@ -39,8 +39,6 @@ bool norm_enabled = true; THREAD_LOCAL TcpStats tcpStats; THREAD_LOCAL SnortConfig* snort_conf = nullptr; -Flow::Flow( void ) {} - class FlowMock : public Flow { public: diff --git a/src/stream/test/stream_splitter_test.cc b/src/stream/test/stream_splitter_test.cc index c4c9ad6f6..a70366332 100644 --- a/src/stream/test/stream_splitter_test.cc +++ b/src/stream/test/stream_splitter_test.cc @@ -44,7 +44,6 @@ const SnortConfig* SnortConfig::get_conf() static StreamSplitter* next_splitter = nullptr; -Flow::Flow() = default; Packet::Packet(bool) { } Packet::~Packet() = default;