From: Maya Dagon (mdagon) Date: Fri, 9 Feb 2024 13:52:08 +0000 (+0000) Subject: Pull request #4175: CSCwi44108 - snort_calloc is used on non-trivial structures X-Git-Tag: 3.1.81.0~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eb148f1a6804e1eb81f4bb4fe08ec346eda0d6ed;p=thirdparty%2Fsnort3.git Pull request #4175: CSCwi44108 - snort_calloc is used on non-trivial structures Merge in SNORT/snort3 from ~MDAGON/snort3:calloc_cleanup to master Squashed commit of the following: commit 17f3fc999811be731ba414fb9d7cfc999044542e Author: maya dagon Date: Thu Jan 25 08:51:13 2024 -0500 detection: add c'tors, use new instead of snort_calloc --- diff --git a/src/detection/detection_continuation.h b/src/detection/detection_continuation.h index b3ca57c51..1df6d2687 100644 --- a/src/detection/detection_continuation.h +++ b/src/detection/detection_continuation.h @@ -72,7 +72,7 @@ private: State(const detection_option_tree_node_t& n, const detection_option_eval_data_t& d, snort::IpsOption* s, unsigned wp, uint64_t id, bool p) : data(d), - root(1, nullptr, d.otn, new RuleLatencyState[snort::ThreadConfig::get_instance_max()]()), + root(1, d.otn), selector(s), node(const_cast(&n)), waypoint(wp), original_waypoint(wp), sid(id), packet_number(d.p->context->packet_number), opt_parent(p) @@ -83,9 +83,6 @@ private: root.children = &node; } - ~State() - { delete[] root.latency_state; } - inline bool eval(snort::Packet&); detection_option_eval_data_t data; diff --git a/src/detection/detection_options.cc b/src/detection/detection_options.cc index fb4723ed3..88f56aa5d 100644 --- a/src/detection/detection_options.cc +++ b/src/detection/detection_options.cc @@ -46,12 +46,10 @@ #include "latency/rule_latency_state.h" #include "log/messages.h" #include "main/snort_config.h" -#include "main/thread_config.h" #include "managers/ips_manager.h" #include "parser/parser.h" #include "profiler/rule_profiler_defs.h" #include "protocols/packet_manager.h" -#include "utils/util.h" #include "utils/util_cstring.h" #include "detection_continuation.h" @@ -210,16 +208,6 @@ static bool detection_option_tree_compare( return true; } -void free_detection_option_tree(detection_option_tree_node_t* node) -{ - for (int i = 0; i < node->num_children; i++) - free_detection_option_tree(node->children[i]); - - snort_free(node->children); - snort_free(node->state); - snort_free(node); -} - class DetectionOptionTreeHashKeyOps : public HashKeyOperations { public: @@ -269,7 +257,7 @@ public: void free_user_data(HashNode* hnode) override { - free_detection_option_tree((detection_option_tree_node_t*)hnode->data); + delete (detection_option_tree_node_t*)hnode->data; } }; @@ -836,17 +824,6 @@ void detection_option_tree_reset_otn_stats(std::vector& nodes, unsign } } -detection_option_tree_root_t* new_root(OptTreeNode* otn) -{ - detection_option_tree_root_t* p = (detection_option_tree_root_t*) - snort_calloc(sizeof(detection_option_tree_root_t)); - - p->latency_state = new RuleLatencyState[ThreadConfig::get_instance_max()](); - p->otn = otn; - - return p; -} - void free_detection_option_root(void** existing_tree) { detection_option_tree_root_t* root; @@ -857,21 +834,7 @@ void free_detection_option_root(void** existing_tree) root = (detection_option_tree_root_t*)*existing_tree; snort_free(root->children); - delete[] root->latency_state; - snort_free(root); + delete root; *existing_tree = nullptr; } -detection_option_tree_node_t* new_node(option_type_t type, void* data) -{ - detection_option_tree_node_t* p = - (detection_option_tree_node_t*)snort_calloc(sizeof(*p)); - - p->option_type = type; - p->option_data = data; - - p->state = (dot_node_state_t*) - snort_calloc(ThreadConfig::get_instance_max(), sizeof(*p->state)); - - return p; -} diff --git a/src/detection/detection_options.h b/src/detection/detection_options.h index 3423d5436..93b75268f 100644 --- a/src/detection/detection_options.h +++ b/src/detection/detection_options.h @@ -34,8 +34,11 @@ #include #include "detection/rule_option_types.h" +#include "latency/rule_latency_state.h" +#include "main/thread_config.h" #include "time/clock_defs.h" #include "trace/trace_api.h" +#include "utils/util.h" namespace snort { @@ -78,6 +81,15 @@ struct dot_node_state_t unsigned latency_timeouts; unsigned latency_suspends; + dot_node_state_t() + { + result = 0; + conts = nullptr; + memset(&last_check, 0, sizeof(last_check)); + context_num = run_num = 0; + reset_profiling(); + } + // FIXIT-L perf profiler stuff should be factored of the node state struct void update(hr_duration delta, bool match) { @@ -111,8 +123,11 @@ struct detection_option_tree_bud_t detection_option_tree_bud_t() : relative_children(0), num_children(0), children(nullptr), otn(nullptr) {} - detection_option_tree_bud_t(int num, detection_option_tree_node_t** d_otn, const OptTreeNode* r_otn) - : relative_children(0), num_children(num), children(d_otn), otn(r_otn) {} + detection_option_tree_bud_t(int num, const OptTreeNode* _otn) + : relative_children(0), num_children(num), children(nullptr), otn(_otn) {} + +protected: + ~detection_option_tree_bud_t() {} }; struct detection_option_tree_node_t : public detection_option_tree_bud_t @@ -122,18 +137,45 @@ struct detection_option_tree_node_t : public detection_option_tree_bud_t dot_node_state_t* state; int is_relative; option_type_t option_type; + + detection_option_tree_node_t(option_type_t type, void* data) : + evaluate(nullptr), option_data(data), is_relative(0), option_type(type) + { + state = new dot_node_state_t[snort::ThreadConfig::get_instance_max()]; + } + + ~detection_option_tree_node_t() + { + for (int i = 0; i < num_children; i++) + delete children[i]; + + snort_free(children); + delete[] state; + } }; struct detection_option_tree_root_t : public detection_option_tree_bud_t { RuleLatencyState* latency_state; - detection_option_tree_root_t() - : detection_option_tree_bud_t(), latency_state(nullptr) {} + detection_option_tree_root_t() : latency_state(nullptr) {} - detection_option_tree_root_t(int num, detection_option_tree_node_t** d_otn, const OptTreeNode* r_otn, - RuleLatencyState* lat) - : detection_option_tree_bud_t(num, d_otn, r_otn), latency_state(lat) {} + detection_option_tree_root_t(OptTreeNode* _otn) + : detection_option_tree_bud_t(0, _otn) + { + latency_state = new RuleLatencyState[snort::ThreadConfig::get_instance_max()](); + } + + detection_option_tree_root_t(int num, const OptTreeNode* _otn) + : detection_option_tree_bud_t(num, _otn) + { + latency_state = new RuleLatencyState[snort::ThreadConfig::get_instance_max()](); + } + + ~detection_option_tree_root_t() + { + delete[] latency_state; + } }; struct detection_option_eval_data_t @@ -171,11 +213,7 @@ void detection_option_tree_update_otn_stats(std::vector&, std::unordered_map&, unsigned); void detection_option_tree_reset_otn_stats(std::vector&, unsigned); -detection_option_tree_root_t* new_root(OptTreeNode*); void free_detection_option_root(void** existing_tree); -detection_option_tree_node_t* new_node(option_type_t, void*); -void free_detection_option_tree(detection_option_tree_node_t*); - #endif diff --git a/src/detection/fp_create.cc b/src/detection/fp_create.cc index 804a8e372..b6b76c7b1 100644 --- a/src/detection/fp_create.cc +++ b/src/detection/fp_create.cc @@ -118,7 +118,7 @@ static int finalize_detection_option_tree(SnortConfig* sc, detection_option_tree if ( void* dup_node = add_detection_option_tree(sc, node) ) { // FIXIT-L delete dup_node and keep original? - free_detection_option_tree(node); + delete node; root->children[i] = (detection_option_tree_node_t*)dup_node; } else @@ -160,7 +160,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp return -1; if (!*existing_tree) - *existing_tree = new_root(otn); + *existing_tree = new detection_option_tree_root_t(otn); detection_option_tree_root_t* const root = (detection_option_tree_root_t*)*existing_tree; detection_option_tree_bud_t* bud = root; @@ -211,7 +211,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp if (!child) { /* No children at this node */ - child = new_node(opt_fp->type, option_data); + child = new detection_option_tree_node_t(opt_fp->type, option_data); child->evaluate = opt_fp->OptTestFunc; bud->children[i] = child; @@ -241,7 +241,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp { /* No matching child node, create a new and add to array */ detection_option_tree_node_t** tmp_children; - child = new_node(opt_fp->type, option_data); + child = new detection_option_tree_node_t(opt_fp->type, option_data); child->evaluate = opt_fp->OptTestFunc; child->num_children++; child->children = (detection_option_tree_node_t**) @@ -277,7 +277,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp return 0; /* Append a leaf node that has option data of the SigInfo/otn pointer */ - child = new_node(RULE_OPTION_TYPE_LEAF_NODE, otn); + child = new detection_option_tree_node_t(RULE_OPTION_TYPE_LEAF_NODE, otn); if (bud->children[0]) { @@ -318,7 +318,7 @@ static int otn_create_tree(OptTreeNode* otn, void** existing_tree, Mpse::MpseTyp } void* option_data = fbs->ips_opt; - child = new_node(fbs->type, option_data); + child = new detection_option_tree_node_t(fbs->type, option_data); child->evaluate = fbs->OptTestFunc; child->is_relative = fbs->isRelative; bud->children[i++] = child; @@ -379,7 +379,7 @@ static int pmx_create_tree(SnortConfig* sc, void* id, void** existing_tree, Mpse OptTreeNode* otn = (OptTreeNode*)pmx->rule_node.rnRuleData; if (!*existing_tree) - *existing_tree = new_root(otn); + *existing_tree = new detection_option_tree_root_t(otn); return otn_create_tree(otn, existing_tree, mpse_type); } diff --git a/src/latency/rule_latency.cc b/src/latency/rule_latency.cc index 21d7a72b3..fffe97a96 100644 --- a/src/latency/rule_latency.cc +++ b/src/latency/rule_latency.cc @@ -590,19 +590,14 @@ TEST_CASE ( "default latency rule interface", "[latency]" ) if ( !instances ) instances = 1; - std::unique_ptr latency_state(new RuleLatencyState[instances]()); - std::unique_ptr children( new detection_option_tree_node_t*[1]()); - detection_option_tree_node_t child; + detection_option_tree_node_t child(RULE_OPTION_TYPE_LEAF_NODE, nullptr); children[0] = &child; - std::unique_ptr child_state(new dot_node_state_t[instances]()); - child.state = child_state.get(); - detection_option_tree_root_t root; - root.latency_state = latency_state.get(); + root.latency_state = new RuleLatencyState[instances](); root.num_children = 1; root.children = children.get(); @@ -644,23 +639,23 @@ TEST_CASE ( "default latency rule interface", "[latency]" ) SECTION( "timeouts under threshold" ) { CHECK( false == RuleInterface::timeout_and_suspend(root, 2, hr_time(0_ticks), true) ); - CHECK( child_state[0].latency_timeouts == 1 ); - CHECK( child_state[0].latency_suspends == 0 ); + CHECK( child.state[0].latency_timeouts == 1 ); + CHECK( child.state[0].latency_suspends == 0 ); } SECTION( "timeouts exceed threshold" ) { CHECK( true == RuleInterface::timeout_and_suspend(root, 1, hr_time(0_ticks), true) ); - CHECK( child_state[0].latency_timeouts == 1 ); - CHECK( child_state[0].latency_suspends == 1 ); + CHECK( child.state[0].latency_timeouts == 1 ); + CHECK( child.state[0].latency_suspends == 1 ); } } SECTION( "suspend disabled" ) { CHECK( false == RuleInterface::timeout_and_suspend(root, 0, hr_time(0_ticks), false) ); - CHECK( child_state[0].latency_timeouts == 1 ); - CHECK( child_state[0].latency_suspends == 0 ); + CHECK( child.state[0].latency_timeouts == 1 ); + CHECK( child.state[0].latency_suspends == 0 ); } } }