From: Mike Stepanek (mstepane) Date: Thu, 28 Oct 2021 23:26:22 +0000 (+0000) Subject: Merge pull request #3134 in SNORT/snort3 from ~VHORBATO/snort3:uni_list_fix to master X-Git-Tag: 3.1.16.0~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96295240bf5e441a2d83724fdae3440fdad8a92c;p=thirdparty%2Fsnort3.git Merge pull request #3134 in SNORT/snort3 from ~VHORBATO/snort3:uni_list_fix to master Squashed commit of the following: commit 207aca5fe21b8c09ce9d0f5c0dfca3b571356e69 Author: Vitalii Date: Tue Oct 26 09:37:55 2021 +0300 flow: use the same pkt_type to link and unlink unidirectional flows Use Flow::key::pkt_type instead of Flow::pkt_type, which is set later and might not be available at link_uni time. Traces enabled for the 'stream' module. --- diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc index 3d62ad426..ab159ddf8 100644 --- a/src/flow/flow_cache.cc +++ b/src/flow/flow_cache.cc @@ -29,9 +29,11 @@ #include "hash/zhash.h" #include "helpers/flag_context.h" #include "ips_options/ips_flowbits.h" +#include "main/snort_debug.h" #include "memory/memory_cap.h" #include "packet_io/active.h" #include "packet_tracer/packet_tracer.h" +#include "stream/base/stream_module.h" #include "time/packet_time.h" #include "utils/stats.h" @@ -54,6 +56,7 @@ static const unsigned ALL_FLOWS = 3; //------------------------------------------------------------------------- THREAD_LOCAL bool FlowCache::pruning_in_progress = false; +extern THREAD_LOCAL const snort::Trace* stream_trace; FlowCache::FlowCache(const FlowCacheConfig& cfg) : config(cfg) { @@ -110,19 +113,39 @@ Flow* FlowCache::find(const FlowKey* key) // always prepend void FlowCache::link_uni(Flow* flow) { - if ( flow->pkt_type == PktType::IP ) + if ( flow->key->pkt_type == PktType::IP ) + { + debug_logf(stream_trace, TRACE_FLOW, nullptr, + "linking unidirectional flow (IP) to list of size: %u\n", + uni_ip_flows->get_count()); uni_ip_flows->link_uni(flow); + } else + { + debug_logf(stream_trace, TRACE_FLOW, nullptr, + "linking unidirectional flow (non-IP) to list of size: %u\n", + uni_flows->get_count()); uni_flows->link_uni(flow); + } } // but remove from any point void FlowCache::unlink_uni(Flow* flow) { - if ( flow->pkt_type == PktType::IP ) - uni_ip_flows->unlink_uni(flow); + if ( flow->key->pkt_type == PktType::IP ) + { + if ( uni_ip_flows->unlink_uni(flow) ) + debug_logf(stream_trace, TRACE_FLOW, nullptr, + "unlinked unidirectional flow (IP) from list, size: %u\n", + uni_ip_flows->get_count()); + } else - uni_flows->unlink_uni(flow); + { + if ( uni_flows->unlink_uni(flow) ) + debug_logf(stream_trace, TRACE_FLOW, nullptr, + "unlinked unidirectional flow (non-IP) from list, size: %u\n", + uni_flows->get_count()); + } } Flow* FlowCache::allocate(const FlowKey* key) @@ -164,8 +187,7 @@ Flow* FlowCache::allocate(const FlowKey* key) void FlowCache::remove(Flow* flow) { - if ( flow->next ) - unlink_uni(flow); + unlink_uni(flow); // FIXIT-M This check is added for offload case where both Flow::reset // and Flow::retire try remove the flow from hash. Flow::reset should @@ -423,8 +445,7 @@ unsigned FlowCache::delete_active_flows(unsigned mode, unsigned num_to_delete, u } // we have a winner... - if ( flow->next ) - unlink_uni(flow); + unlink_uni(flow); if ( flow->was_blocked() ) delete_stats.update(FlowDeleteState::BLOCKED); diff --git a/src/flow/flow_control.cc b/src/flow/flow_control.cc index 7abd95ae5..f51858faf 100644 --- a/src/flow/flow_control.cc +++ b/src/flow/flow_control.cc @@ -414,7 +414,7 @@ bool FlowControl::process(PktType type, Packet* p, bool* new_flow) // FIXIT-M refactor to unlink_uni immediately after session // is processed by inspector manager (all flows) - if ( flow->next && is_bidirectional(flow) ) + if ( is_bidirectional(flow) ) cache->unlink_uni(flow); return true; diff --git a/src/flow/flow_uni_list.h b/src/flow/flow_uni_list.h index 1c29e02f8..a566e5305 100644 --- a/src/flow/flow_uni_list.h +++ b/src/flow/flow_uni_list.h @@ -52,15 +52,16 @@ public: ++count; } - void unlink_uni(snort::Flow* flow) + bool unlink_uni(snort::Flow* flow) { if ( !flow->next ) - return; + return false; flow->next->prev = flow->prev; flow->prev->next = flow->next; flow->next = flow->prev = nullptr; --count; + return true; } snort::Flow* get_oldest_uni() diff --git a/src/flow/test/flow_cache_test.cc b/src/flow/test/flow_cache_test.cc index 9a74618ca..080faaa5b 100644 --- a/src/flow/test/flow_cache_test.cc +++ b/src/flow/test/flow_cache_test.cc @@ -28,6 +28,7 @@ #include "detection/detection_engine.h" #include "main/snort_config.h" +#include "main/snort_debug.h" #include "managers/inspector_manager.h" #include "memory/memory_cap.h" #include "packet_io/active.h" @@ -53,6 +54,7 @@ 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 const Trace* stream_trace = nullptr; void Active::drop_packet(snort::Packet const*, bool) { } PacketTracer::~PacketTracer() = default; @@ -93,6 +95,9 @@ bool ExpectCache::is_expected(Packet*) { return true; } Flow* HighAvailabilityManager::import(Packet&, FlowKey&) { return nullptr; } bool HighAvailabilityManager::in_standby(Flow*) { return true; } SfIpRet SfIp::set(void const*, int) { return SFIP_SUCCESS; } +void snort::trace_vprintf(const char*, TraceLevel, const char*, const Packet*, const char*, va_list) {} +uint8_t snort::TraceApi::get_constraints_generation() { return 0; } +void snort::TraceApi::filter(const Packet&) {} namespace memory { void MemoryCap::update_allocations(size_t) { } diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc index 7895e8865..72da087b9 100644 --- a/src/stream/base/stream_module.cc +++ b/src/stream/base/stream_module.cc @@ -36,6 +36,15 @@ using namespace snort; using namespace std; +#ifdef DEBUG_MSGS +static const TraceOption stream_trace_options[] = +{ + { "base", TRACE_BASE, "enable base stream trace logging" }, + { "flow", TRACE_FLOW, "enable flow trace logging" }, + { nullptr, 0, nullptr } +}; +#endif + THREAD_LOCAL const Trace* stream_trace = nullptr; static THREAD_LOCAL timeval reload_time { }; @@ -119,8 +128,7 @@ const TraceOption* StreamModule::get_trace_options() const #ifndef DEBUG_MSGS return nullptr; #else - static const TraceOption stream_trace_options(nullptr, 0, nullptr); - return &stream_trace_options; + return stream_trace_options; #endif } diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h index 4f4c1de4b..5b5c66031 100644 --- a/src/stream/base/stream_module.h +++ b/src/stream/base/stream_module.h @@ -37,6 +37,14 @@ extern THREAD_LOCAL snort::ProfileStats s5PerfStats; extern THREAD_LOCAL class FlowControl* flow_con; extern THREAD_LOCAL const snort::Trace* stream_trace; +#ifdef DEBUG_MSGS +enum +{ + TRACE_BASE = 0, + TRACE_FLOW +}; +#endif + //------------------------------------------------------------------------- // stream module //------------------------------------------------------------------------- diff --git a/src/stream/stream.cc b/src/stream/stream.cc index db0f0633b..70e715500 100644 --- a/src/stream/stream.cc +++ b/src/stream/stream.cc @@ -252,7 +252,7 @@ void Stream::stop_inspection( { assert(flow && flow->session); - debug_logf(stream_trace, p, "stop inspection on flow, dir %s \n", + debug_logf(stream_trace, TRACE_BASE, p, "stop inspection on flow, dir %s \n", dir == SSN_DIR_BOTH ? "BOTH" : ((dir == SSN_DIR_FROM_CLIENT) ? "FROM_CLIENT" : "FROM_SERVER"));