From bb7e1b6840da55fa95d79996fc543908a75fd933 Mon Sep 17 00:00:00 2001 From: "Michael Altizer (mialtize)" Date: Tue, 1 Sep 2020 01:45:47 +0000 Subject: [PATCH] Merge pull request #2427 in SNORT/snort3 from ~MIALTIZE/snort3:cpputest_leaks to master Squashed commit of the following: commit f1f70793fa2f9bfa46a9f48245372df2899bcc5a Author: Michael Altizer Date: Mon Aug 31 15:35:04 2020 -0400 host_tracker: Fix allocator unit test to work on 32-bit systems again commit 85a78101fe8bfb9d3055488bcba029ec4e430f02 Author: Michael Altizer Date: Thu Aug 27 13:43:28 2020 -0400 cmake: Restore accidentally removed caching of static DAQ modules commit a89ffd26d712ca56ee8fafe24e1d64f616fc9c8b Author: Michael Altizer Date: Thu Aug 27 11:15:02 2020 -0400 utils: Add sys/time.h to util.h for struct timeval definition This fixes the Alpine Linux build issue where the forward declaration of struct timval from ts_print() ended up in the snort namespace. commit d3c78c5f6ec5fbc21231043512a3d90e30488e64 Author: Michael Altizer Date: Thu Aug 27 11:12:32 2020 -0400 rna: Remove redefinition of USHRT_MAX commit 9393c5e3621b1900acf15d07e08020c98be763cd Author: Michael Altizer Date: Tue Aug 25 12:41:00 2020 -0400 tests: Fix the majority of memory leaks in CppUTest unit tests Additionally, this allows us to use the finally released CppUTest 4.0. commit 7d363fe48a6a11836bd9e44f2fd8d54f936acafc Author: Michael Altizer Date: Tue Aug 25 17:18:19 2020 -0400 style: Replace some tabs that snuck in with proper spaces --- cmake/FindDAQ.cmake | 2 +- .../tcp_connector/test/tcp_connector_test.cc | 3 - src/detection/detect_trace.cc | 2 +- src/helpers/test/hyper_search_test.cc | 7 +- .../test/host_cache_allocator_ht_test.cc | 34 +++--- .../test/host_cache_allocator_test.cc | 4 +- .../test/host_cache_module_test.cc | 22 +--- .../test/host_tracker_module_test.cc | 2 + src/ips_options/test/ips_regex_test.cc | 9 +- .../appid/test/app_info_table_test.cc | 6 - .../appid/test/appid_api_test.cc | 16 ++- .../appid/test/appid_detector_test.cc | 2 - .../appid/test/appid_http_event_test.cc | 2 - .../appid/test/appid_http_session_test.cc | 108 +++++++++--------- .../appid/test/appid_mock_definitions.h | 4 +- .../appid/test/appid_mock_http_session.h | 7 +- .../appid/test/appid_session_api_test.cc | 2 - src/network_inspectors/rna/rna_pnd.h | 4 +- .../rna/test/rna_module_mock.h | 5 - .../rna/test/rna_module_test.cc | 22 ++-- src/search_engines/test/hyperscan_test.cc | 16 +-- src/side_channel/side_channel.cc | 1 + src/side_channel/test/side_channel_test.cc | 5 - src/stream/ip/ip_defrag.cc | 6 +- src/stream/tcp/segment_overlap_editor.cc | 6 +- src/stream/tcp/tcp_segment_descriptor.cc | 6 +- src/stream/tcp/test/tcp_normalizer_test.cc | 1 - src/utils/util.h | 1 + 28 files changed, 139 insertions(+), 166 deletions(-) diff --git a/cmake/FindDAQ.cmake b/cmake/FindDAQ.cmake index a420ddcaf..907c65583 100644 --- a/cmake/FindDAQ.cmake +++ b/cmake/FindDAQ.cmake @@ -67,6 +67,6 @@ if (PKG_CONFIG_EXECUTABLE AND ENABLE_STATIC_DAQ) endif() if (DAQ_STATIC_MODULES) list(SORT DAQ_STATIC_MODULES) - # set(DAQ_STATIC_MODULES ${DAQ_STATIC_MODULES} CACHE INTERNAL "Static DAQ modules") + set(DAQ_STATIC_MODULES ${DAQ_STATIC_MODULES} CACHE INTERNAL "Static DAQ modules") endif() endif() diff --git a/src/connectors/tcp_connector/test/tcp_connector_test.cc b/src/connectors/tcp_connector/test/tcp_connector_test.cc index af9576284..a5581bc37 100644 --- a/src/connectors/tcp_connector/test/tcp_connector_test.cc +++ b/src/connectors/tcp_connector/test/tcp_connector_test.cc @@ -344,8 +344,6 @@ TEST_GROUP(tcp_connector_tinit_tterm_thread_call) { void setup() override { - // FIXIT-RC workaround for CppUTest mem leak detector issue - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); tcpc_api = (const ConnectorApi*) tcp_connector; s_instance = 0; set_normal_status(); @@ -370,7 +368,6 @@ TEST_GROUP(tcp_connector_tinit_tterm_thread_call) tcpc_api->tterm(connector); tcpc_api->dtor(connector_common); tcp_connector->mod_dtor(mod); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; diff --git a/src/detection/detect_trace.cc b/src/detection/detect_trace.cc index 16eaae4a2..5ae1ba132 100644 --- a/src/detection/detect_trace.cc +++ b/src/detection/detect_trace.cc @@ -130,7 +130,7 @@ void node_eval_trace(const detection_option_tree_node_t* node, const Cursor& cur return; if ( trace_enabled(detection_trace, TRACE_BUFFER, 5) ) - dump_buffer(cursor.buffer() + pos, cursor.length(), p); + dump_buffer(cursor.buffer() + pos, cursor.length(), p); else if ((pos != cursor_pos) || strcmp(cursor_name, name)) { cursor_pos = pos; diff --git a/src/helpers/test/hyper_search_test.cc b/src/helpers/test/hyper_search_test.cc index bf4e69254..41e20e52c 100644 --- a/src/helpers/test/hyper_search_test.cc +++ b/src/helpers/test/hyper_search_test.cc @@ -66,7 +66,11 @@ int SnortConfig::request_scratch(ScratchAllocator* s) } void SnortConfig::release_scratch(int) -{ s_state.clear(); } +{ + scratcher = nullptr; + s_state.clear(); + s_state.shrink_to_fit(); +} const SnortConfig* SnortConfig::get_conf() { return snort_conf; } @@ -253,7 +257,6 @@ TEST(hyper_search_test_group, not_found4) int main(int argc, char** argv) { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/host_tracker/test/host_cache_allocator_ht_test.cc b/src/host_tracker/test/host_cache_allocator_ht_test.cc index cf1f94c78..d7746ca36 100644 --- a/src/host_tracker/test/host_cache_allocator_ht_test.cc +++ b/src/host_tracker/test/host_cache_allocator_ht_test.cc @@ -33,7 +33,6 @@ #include #include -using namespace std; using namespace snort; namespace snort @@ -59,13 +58,18 @@ TEST(host_cache_allocator_ht, allocate) const size_t hc_item_sz = sizeof(HostCacheIp::Data) + sizeof(HostTracker); const size_t ht_item_sz = sizeof(HostApplication); - // room for n host trackers in the cache and m host applications in ht - const size_t max_size = n * hc_item_sz + m * ht_item_sz; + // room for n host trackers in the cache and 2^floor(log2(3))+2^ceil(log2(3))-1 host + // applications in ht + // FIXIT-L this makes a questionable assumption about the STL vector implementation + // that it will double the allocation each time it needs to increase its size, so + // going from 2 to 3 will allocate 4 and then release 2, meaning in order to exactly + // induce pruning, the max size should be just one short of holding 6 + const size_t max_size = n * hc_item_sz + 5 * ht_item_sz; host_cache.set_max_size(max_size); // insert n empty host trackers: - for (size_t i=0; iadd_service(port, IpProtocol::TCP, 676, true)); // Insert a new host tracker item. The sequence of operations is this: - // - host tracker vector is holding 2 * ht_item_sz = 24 and wants to - // double in size. - // - the allocator honors the vector's request, allocating 48 bytes and - // informs the host_cache about the new size, which exceeds max_size now + // - host tracker vector is holding 2 * bytes and wants to double in size. + // - the allocator honors the vector's request, allocating 4 * bytes and + // informs the host_cache about the new size, which exceeds max_size // - host_cache prunes, removing the least recent host tracker. - // Since this ht is empty, precisely hc_item_sz = 88 bytes are freed. - // - the host tracker vector destructor frees up an additional 24 bytes + // Since this ht is empty, precisely bytes are freed. + // - the host tracker vector destructor frees up an additional 2 * bytes // that it reallocated. // Hence, after the next insert, the math is this: - size_t sz = host_cache.mem_size() + 4*ht_item_sz - hc_item_sz - 2*ht_item_sz; + size_t sz = host_cache.mem_size() + 4 * ht_item_sz - hc_item_sz - 2 * ht_item_sz; CHECK(true == ht_ptr->add_service(m, IpProtocol::TCP, 676, true)); CHECK(sz == host_cache.mem_size()); @@ -138,13 +141,12 @@ TEST(host_cache_allocator_ht, allocate) res = host_cache.remove(ip, ht_ptr); CHECK(res == true); CHECK(ht_ptr != nullptr); - } int main(int argc, char** argv) { - // Use this if you want to turn off memory checks entirely: + // FIXIT-L There is currently no external way to fully release the memory from the global host + // cache unordered_map in host_cache.cc MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); - return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/host_tracker/test/host_cache_allocator_test.cc b/src/host_tracker/test/host_cache_allocator_test.cc index 49bf94aee..1497262c2 100644 --- a/src/host_tracker/test/host_cache_allocator_test.cc +++ b/src/host_tracker/test/host_cache_allocator_test.cc @@ -121,8 +121,8 @@ TEST(cache_allocator, allocate) int main(int argc, char** argv) { - // Use this if you want to turn off memory checks entirely: + // FIXIT-L There is currently no external way to fully release the memory from the global host + // cache unordered_map in host_cache.cc MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); - return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/host_tracker/test/host_cache_module_test.cc b/src/host_tracker/test/host_cache_module_test.cc index 62d145260..b7b8391e5 100644 --- a/src/host_tracker/test/host_cache_module_test.cc +++ b/src/host_tracker/test/host_cache_module_test.cc @@ -37,7 +37,6 @@ #include "sfip/sf_ip.h" using namespace snort; -using namespace std; // All tests here use the same module since host_cache is global. Creating a local module for each // test will cause host_cache PegCount testing to be dependent on the order of running these tests. @@ -84,15 +83,6 @@ HostCacheAllocIp::HostCacheAllocIp() TEST_GROUP(host_cache_module) { - void setup() override - { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); - } - - void teardown() override - { - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); - } }; static void try_reload_prune(bool is_not_locked) @@ -111,8 +101,6 @@ static void try_reload_prune(bool is_not_locked) // This method is a friend of LruCacheSharedMemcap class. TEST(host_cache_module, misc) { - Value size_val((double)2112); - Parameter size_param = { "size", Parameter::PT_INT, nullptr, nullptr, "cache size" }; const PegInfo* ht_pegs = module.get_pegs(); const PegCount* ht_stats = module.get_counts(); @@ -165,13 +153,6 @@ TEST(host_cache_module, misc) CHECK(ht_stats[4] == 2); // 2 reload_prunes CHECK(ht_stats[5] == 1); // 1 remove - size_val.set(&size_param); - - // Set up the host_cache max size. - module.begin("host_cache", 0, nullptr); - module.set(nullptr, size_val, nullptr); - module.end("host_cache", 0, nullptr); - ht_stats = module.get_counts(); CHECK(ht_stats[0] == 4); } @@ -194,5 +175,8 @@ TEST(host_cache_module, log_host_cache_messages) int main(int argc, char** argv) { + // FIXIT-L There is currently no external way to fully release the memory from the global host + // cache unordered_map in host_cache.cc + MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/host_tracker/test/host_tracker_module_test.cc b/src/host_tracker/test/host_tracker_module_test.cc index ad74160c9..3522af22c 100644 --- a/src/host_tracker/test/host_tracker_module_test.cc +++ b/src/host_tracker/test/host_tracker_module_test.cc @@ -100,6 +100,8 @@ TEST(host_tracker_module, host_tracker_module_test_basic) int main(int argc, char** argv) { + // FIXIT-L There is currently no external way to fully release the memory from the global host + // cache unordered_map in host_cache.cc MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/ips_options/test/ips_regex_test.cc b/src/ips_options/test/ips_regex_test.cc index e408e2fcb..13a82e20b 100644 --- a/src/ips_options/test/ips_regex_test.cc +++ b/src/ips_options/test/ips_regex_test.cc @@ -71,7 +71,12 @@ int SnortConfig::request_scratch(ScratchAllocator* s) return 0; } -void SnortConfig::release_scratch(int) { } +void SnortConfig::release_scratch(int) +{ + scratcher = nullptr; + s_state.clear(); + s_state.shrink_to_fit(); +} const SnortConfig* SnortConfig::get_conf() { return snort_conf; } @@ -373,8 +378,6 @@ TEST(ips_regex_option_relative, no_match) int main(int argc, char** argv) { - // FIXIT-L cpputest hangs or crashes in the leak detector - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/network_inspectors/appid/test/app_info_table_test.cc b/src/network_inspectors/appid/test/app_info_table_test.cc index 088ae2671..607db7cd0 100644 --- a/src/network_inspectors/appid/test/app_info_table_test.cc +++ b/src/network_inspectors/appid/test/app_info_table_test.cc @@ -68,15 +68,9 @@ AppInfoTableEntry* add_static_entry(AppId id, const char* name) TEST_GROUP(app_info_table) { - void setup() override - { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); - } - void teardown() override { app_info_mgr.cleanup_appid_info_table(); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc index 7057aa74f..2873fcd5a 100644 --- a/src/network_inspectors/appid/test/appid_api_test.cc +++ b/src/network_inspectors/appid/test/appid_api_test.cc @@ -200,7 +200,10 @@ TEST_GROUP(appid_api) char test_log[256]; void setup() override { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); + mock_init_appid_pegs(); + SfIp ip; + mock_session = new AppIdSession(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, + dummy_appid_inspector.get_ctxt().get_odp_ctxt()); flow = new Flow; flow->set_flow_data(mock_session); mock().setDataObject("test_log", "char", test_log); @@ -210,7 +213,9 @@ TEST_GROUP(appid_api) { delete flow; mock().clear(); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); + mock_cleanup_appid_pegs(); + delete &mock_session->get_api(); + delete mock_session; } }; @@ -382,13 +387,6 @@ TEST(appid_api, is_service_http_type) int main(int argc, char** argv) { - mock_init_appid_pegs(); - SfIp ip; - mock_session = new AppIdSession(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, - dummy_appid_inspector.get_ctxt().get_odp_ctxt()); int rc = CommandLineTestRunner::RunAllTests(argc, argv); - mock_cleanup_appid_pegs(); - delete &mock_session->get_api(); - delete mock_session; return rc; } diff --git a/src/network_inspectors/appid/test/appid_detector_test.cc b/src/network_inspectors/appid/test/appid_detector_test.cc index e38a03dd7..557dfb33a 100644 --- a/src/network_inspectors/appid/test/appid_detector_test.cc +++ b/src/network_inspectors/appid/test/appid_detector_test.cc @@ -66,7 +66,6 @@ TEST_GROUP(appid_detector_tests) void setup() override { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); SfIp ip; mock_session = new AppIdSession(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, dummy_appid_inspector.get_ctxt().get_odp_ctxt()); @@ -79,7 +78,6 @@ TEST_GROUP(appid_detector_tests) delete flow; delete &mock_session->get_api(); delete mock_session; - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; diff --git a/src/network_inspectors/appid/test/appid_http_event_test.cc b/src/network_inspectors/appid/test/appid_http_event_test.cc index 378a2dc1c..e4fc3766a 100644 --- a/src/network_inspectors/appid/test/appid_http_event_test.cc +++ b/src/network_inspectors/appid/test/appid_http_event_test.cc @@ -245,7 +245,6 @@ TEST_GROUP(appid_http_event) { void setup() override { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); flow = new Flow; SfIp ip; mock_session = new AppIdSession(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, stub_odp_ctxt); @@ -263,7 +262,6 @@ TEST_GROUP(appid_http_event) delete flow; mock().clear(); delete appidDebug; - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; diff --git a/src/network_inspectors/appid/test/appid_http_session_test.cc b/src/network_inspectors/appid/test/appid_http_session_test.cc index a21073293..b64de9c0f 100644 --- a/src/network_inspectors/appid/test/appid_http_session_test.cc +++ b/src/network_inspectors/appid/test/appid_http_session_test.cc @@ -45,6 +45,7 @@ #include #include + using namespace snort; namespace snort @@ -113,7 +114,9 @@ AppIdSession::AppIdSession(IpProtocol, const SfIp* ip, uint16_t, AppIdInspector& {} AppIdSession::~AppIdSession() -{} +{ + delete &api; +} void AppIdSession::set_client_appid_data(AppId, AppidChangeBits&, char*) { @@ -162,8 +165,6 @@ void Profiler::consolidate_stats() { } void Profiler::reset_stats() { } void Profiler::show_stats() { } -MemoryContext::MemoryContext(MemoryTracker&) { } -MemoryContext::~MemoryContext() { } void memory::MemoryCap::update_allocations(size_t) { } void memory::MemoryCap::update_deallocations(size_t) { } @@ -175,22 +176,24 @@ AppIdConfig::~AppIdConfig() { } unsigned AppIdSession::inspector_id = 0; THREAD_LOCAL AppIdDebug* appidDebug = nullptr; -SfIp sfip; -AppIdSession session(IpProtocol::IP, &sfip, 0, dummy_appid_inspector, stub_odp_ctxt); -AppIdHttpSession mock_hsession(session, 0); - TEST_GROUP(appid_http_session) { + AppIdHttpSession* mock_hsession; + AppIdSession* session; + void setup() override { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); + SfIp sfip; + session = new AppIdSession(IpProtocol::IP, &sfip, 0, dummy_appid_inspector, stub_odp_ctxt); + mock_hsession = new AppIdHttpSession(*session, 0); appidDebug = new AppIdDebug(); } void teardown() override { delete appidDebug; - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); + delete mock_hsession; + delete session; } }; @@ -201,49 +204,49 @@ TEST(appid_http_session, http_field_ids_enum_order) // in appid_http_session.h. AppidChangeBits change_bits; - mock_hsession.set_field( (HttpFieldIds)0, new std::string("agent"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)1, new std::string("host"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)2, new std::string("referer"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)3, new std::string("uri"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)4, new std::string("cookie"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)5, new std::string("req_body"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)6, new std::string("content_type"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)7, new std::string("location"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)8, new std::string("rsp_body"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)9, new std::string("via"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)10, new std::string("response_code"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)11, new std::string("server"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)12, new std::string("xww"), change_bits ); - mock_hsession.set_field( (HttpFieldIds)13, new std::string("url"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)0, new std::string("agent"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)1, new std::string("host"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)2, new std::string("referer"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)3, new std::string("uri"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)4, new std::string("cookie"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)5, new std::string("req_body"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)6, new std::string("content_type"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)7, new std::string("location"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)8, new std::string("rsp_body"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)9, new std::string("via"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)10, new std::string("response_code"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)11, new std::string("server"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)12, new std::string("xww"), change_bits ); + mock_hsession->set_field( (HttpFieldIds)13, new std::string("url"), change_bits ); const std::string* field; - field = mock_hsession.get_field(REQ_AGENT_FID); + field = mock_hsession->get_field(REQ_AGENT_FID); STRCMP_EQUAL(field->c_str(), "agent"); - field = mock_hsession.get_field(REQ_HOST_FID); + field = mock_hsession->get_field(REQ_HOST_FID); STRCMP_EQUAL(field->c_str(), "host"); - field = mock_hsession.get_field(REQ_REFERER_FID); + field = mock_hsession->get_field(REQ_REFERER_FID); STRCMP_EQUAL(field->c_str(), "referer"); - field = mock_hsession.get_field(REQ_URI_FID); + field = mock_hsession->get_field(REQ_URI_FID); STRCMP_EQUAL(field->c_str(), "uri"); - field = mock_hsession.get_field(REQ_COOKIE_FID); + field = mock_hsession->get_field(REQ_COOKIE_FID); STRCMP_EQUAL(field->c_str(), "cookie"); - field = mock_hsession.get_field(REQ_BODY_FID); + field = mock_hsession->get_field(REQ_BODY_FID); STRCMP_EQUAL(field->c_str(), "req_body"); - field = mock_hsession.get_field(RSP_CONTENT_TYPE_FID); + field = mock_hsession->get_field(RSP_CONTENT_TYPE_FID); STRCMP_EQUAL(field->c_str(), "content_type"); - field = mock_hsession.get_field(RSP_LOCATION_FID); + field = mock_hsession->get_field(RSP_LOCATION_FID); STRCMP_EQUAL(field->c_str(), "location"); - field = mock_hsession.get_field(RSP_BODY_FID); + field = mock_hsession->get_field(RSP_BODY_FID); STRCMP_EQUAL(field->c_str(), "rsp_body"); - field = mock_hsession.get_field(MISC_VIA_FID); + field = mock_hsession->get_field(MISC_VIA_FID); STRCMP_EQUAL(field->c_str(), "via"); - field = mock_hsession.get_field(MISC_RESP_CODE_FID); + field = mock_hsession->get_field(MISC_RESP_CODE_FID); STRCMP_EQUAL(field->c_str(), "response_code"); - field = mock_hsession.get_field(MISC_SERVER_FID); + field = mock_hsession->get_field(MISC_SERVER_FID); STRCMP_EQUAL(field->c_str(), "server"); - field = mock_hsession.get_field(MISC_XWW_FID); + field = mock_hsession->get_field(MISC_XWW_FID); STRCMP_EQUAL(field->c_str(), "xww"); - field = mock_hsession.get_field(MISC_URL_FID); + field = mock_hsession->get_field(MISC_URL_FID); STRCMP_EQUAL(field->c_str(), "url"); // Detect changes in host, url, user agent, response, and referer fields @@ -261,9 +264,9 @@ TEST(appid_http_session, set_tun_dest) SfIp ipv6; ipv6.set("2001:db8:85a3::8a2e:370:7334"); AppidChangeBits change_bits; - mock_hsession.set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:7334]:51413"), change_bits); - mock_hsession.set_tun_dest(); - tun_dest = mock_hsession.get_tun_dest(); + mock_hsession->set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:7334]:51413"), change_bits); + mock_hsession->set_tun_dest(); + tun_dest = mock_hsession->get_tun_dest(); CHECK(tun_dest != nullptr); CHECK_EQUAL(tun_dest->port, 51413); CHECK_EQUAL((ipv6 == tun_dest->ip), true); @@ -273,16 +276,16 @@ TEST(appid_http_session, set_tun_dest_bad_uri) { const TunnelDest* tun_dest = nullptr; AppidChangeBits change_bits; - mock_hsession.set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:1234]:51413"), change_bits); - mock_hsession.set_tun_dest(); - tun_dest = mock_hsession.get_tun_dest(); + mock_hsession->set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:1234]:51413"), change_bits); + mock_hsession->set_tun_dest(); + tun_dest = mock_hsession->get_tun_dest(); CHECK(tun_dest != nullptr); // Testing with bad URL - mock_hsession.free_tun_dest(); - mock_hsession.set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:1235]"), change_bits); - mock_hsession.set_tun_dest(); - tun_dest = mock_hsession.get_tun_dest(); + mock_hsession->free_tun_dest(); + mock_hsession->set_field(REQ_URI_FID, new std::string("[2001:db8:85a3::8a2e:370:1235]"), change_bits); + mock_hsession->set_tun_dest(); + tun_dest = mock_hsession->get_tun_dest(); CHECK(tun_dest == nullptr); } @@ -290,14 +293,13 @@ TEST(appid_http_session, change_bits_for_referred_appid) { // Testing set_referred_payload_app_id_data AppidChangeBits change_bits; - AppIdPegCounts::init_pegs(); AppIdConfig config; OdpContext odp_ctxt(config, nullptr); - session.set_service_id(APP_ID_HTTP, odp_ctxt); - session.scan_flags |= SCAN_HTTP_HOST_URL_FLAG; - mock_hsession.set_skip_simple_detect(false); - mock_hsession.set_field( (HttpFieldIds)2, new std::string("referer"), change_bits ); - mock_hsession.process_http_packet(APP_ID_FROM_INITIATOR, change_bits, odp_ctxt.get_http_matchers()); + session->set_service_id(APP_ID_HTTP, odp_ctxt); + session->scan_flags |= SCAN_HTTP_HOST_URL_FLAG; + mock_hsession->set_skip_simple_detect(false); + mock_hsession->set_field( (HttpFieldIds)2, new std::string("referer"), change_bits ); + mock_hsession->process_http_packet(APP_ID_FROM_INITIATOR, change_bits, odp_ctxt.get_http_matchers()); // Detect changes in referred appid CHECK_EQUAL(change_bits.test(APPID_REFERRED_BIT), true); diff --git a/src/network_inspectors/appid/test/appid_mock_definitions.h b/src/network_inspectors/appid/test/appid_mock_definitions.h index 2e343a153..05fd3426a 100644 --- a/src/network_inspectors/appid/test/appid_mock_definitions.h +++ b/src/network_inspectors/appid/test/appid_mock_definitions.h @@ -35,7 +35,9 @@ namespace snort { char* snort_strndup(const char* src, size_t dst_size) { - return strndup(src, dst_size); + char* dup = (char*)snort_calloc(dst_size + 1); + strncpy(dup, src, dst_size + 1); + return dup; } char* snort_strdup(const char* str) diff --git a/src/network_inspectors/appid/test/appid_mock_http_session.h b/src/network_inspectors/appid/test/appid_mock_http_session.h index 151020b16..aadf2ef3b 100644 --- a/src/network_inspectors/appid/test/appid_mock_http_session.h +++ b/src/network_inspectors/appid/test/appid_mock_http_session.h @@ -35,10 +35,9 @@ AppIdHttpSession::AppIdHttpSession(AppIdSession& session, uint32_t http2_stream_ AppIdHttpSession::~AppIdHttpSession() { for ( int i = 0; i < NUM_METADATA_FIELDS; i++) - { - if ( meta_data[i] ) - delete meta_data[i]; - } + delete meta_data[i]; + if (tun_dest) + delete tun_dest; } int AppIdHttpSession::process_http_packet(AppidSessionDirection, AppidChangeBits&, HttpPatternMatchers&) { return 0; } diff --git a/src/network_inspectors/appid/test/appid_session_api_test.cc b/src/network_inspectors/appid/test/appid_session_api_test.cc index 093d86a09..caafc38d7 100644 --- a/src/network_inspectors/appid/test/appid_session_api_test.cc +++ b/src/network_inspectors/appid/test/appid_session_api_test.cc @@ -66,7 +66,6 @@ TEST_GROUP(appid_session_api) { AppidChangeBits change_bits; - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); SfIp ip; mock_session = new AppIdSession(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, odpctxt); mock_session->set_ss_application_ids(APPID_UT_ID, APPID_UT_ID, APPID_UT_ID, @@ -77,7 +76,6 @@ TEST_GROUP(appid_session_api) { delete &mock_session->get_api(); delete mock_session; - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; diff --git a/src/network_inspectors/rna/rna_pnd.h b/src/network_inspectors/rna/rna_pnd.h index 6bd5c29cd..101af99ce 100644 --- a/src/network_inspectors/rna/rna_pnd.h +++ b/src/network_inspectors/rna/rna_pnd.h @@ -20,7 +20,7 @@ #ifndef RNA_PND_H #define RNA_PND_H -#include +#include #include "helpers/discovery_filter.h" #include "host_tracker/host_tracker.h" @@ -36,8 +36,6 @@ #include "rna_logger.h" #include "rna_mac_cache.h" -#define USHRT_MAX std::numeric_limits::max() - enum class TcpPacketType { SYN, SYN_ACK, MIDSTREAM diff --git a/src/network_inspectors/rna/test/rna_module_mock.h b/src/network_inspectors/rna/test/rna_module_mock.h index 74aa50520..02ae13a7b 100644 --- a/src/network_inspectors/rna/test/rna_module_mock.h +++ b/src/network_inspectors/rna/test/rna_module_mock.h @@ -26,15 +26,10 @@ bool Swapper::reload_in_progress = false; THREAD_LOCAL RnaStats rna_stats; THREAD_LOCAL ProfileStats rna_perf_stats; -static std::string message; static Request mock_request; const char* luaL_optlstring(lua_State*, int, const char*, size_t*) { return nullptr; } -void Request::respond(const char* msg, bool, bool) -{ - message = msg; -} Request& get_current_request() { return mock_request; } diff --git a/src/network_inspectors/rna/test/rna_module_test.cc b/src/network_inspectors/rna/test/rna_module_test.cc index e41a146e3..231ef880f 100644 --- a/src/network_inspectors/rna/test/rna_module_test.cc +++ b/src/network_inspectors/rna/test/rna_module_test.cc @@ -34,33 +34,35 @@ #include #include +#include + +void Request::respond(const char* msg, bool, bool) +{ + mock().actualCall("respond").onObject(this).withParameter("msg", msg); +} TEST_GROUP(rna_module_test) { - void setup() override - { - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); - } - void teardown() override - { - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); - } }; TEST(rna_module_test, reload_fingerprint) { // When another reload is pending + mock().expectOneCall("respond").onObject(&mock_request).withParameter("msg", "== reload pending; retry\n"); Swapper::set_reload_in_progress(true); reload_fingerprint(nullptr); Swapper::set_reload_in_progress(false); - CHECK_TRUE(message == "== reload pending; retry\n"); + mock().checkExpectations(); // When rna is not configured + mock().expectOneCall("respond").onObject(&mock_request).withParameter("msg", "== reload fingerprint failed - rna not enabled\n"); reload_fingerprint(nullptr); - CHECK_TRUE(message == "== reload fingerprint failed - rna not enabled\n"); + mock().checkExpectations(); // Reload in progress flag should remain unset at the end CHECK_FALSE(Swapper::get_reload_in_progress()); + + mock().clear(); } TEST(rna_module_test, push_tcp_fingerprints) diff --git a/src/search_engines/test/hyperscan_test.cc b/src/search_engines/test/hyperscan_test.cc index e509d2062..fd7a21add 100644 --- a/src/search_engines/test/hyperscan_test.cc +++ b/src/search_engines/test/hyperscan_test.cc @@ -116,7 +116,12 @@ int SnortConfig::request_scratch(ScratchAllocator* s) return 0; } -void SnortConfig::release_scratch(int) { } +void SnortConfig::release_scratch(int) +{ + scratcher = nullptr; + s_state.clear(); + s_state.shrink_to_fit(); +} const SnortConfig* SnortConfig::get_conf() { return snort_conf; } @@ -224,8 +229,6 @@ TEST_GROUP(mpse_hs_match) void setup() override { - // FIXIT-L cpputest hangs or crashes in the leak detector - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); CHECK(se_hyperscan); mod = mpse_api->base.mod_ctor(); hs = mpse_api->ctor(snort_conf, nullptr, &s_agent); @@ -239,7 +242,6 @@ TEST_GROUP(mpse_hs_match) if ( do_cleanup ) scratcher->cleanup(snort_conf); mpse_api->base.mod_dtor(mod); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; @@ -373,8 +375,6 @@ TEST_GROUP(mpse_hs_multi) void setup() override { - // FIXIT-L cpputest hangs or crashes in the leak detector - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); CHECK(se_hyperscan); mod = mpse_api->base.mod_ctor(); @@ -395,7 +395,6 @@ TEST_GROUP(mpse_hs_multi) if ( do_cleanup ) scratcher->cleanup(snort_conf); mpse_api->base.mod_dtor(mod); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; @@ -431,6 +430,9 @@ TEST(mpse_hs_multi, single) int main(int argc, char** argv) { + // FIXIT-L There is currently no external way to fully release the memory from the static + // s_scratch vector in hyperscan.cc + MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/side_channel/side_channel.cc b/src/side_channel/side_channel.cc index 331fc6fb1..0b83c1596 100644 --- a/src/side_channel/side_channel.cc +++ b/src/side_channel/side_channel.cc @@ -181,6 +181,7 @@ void SideChannelManager::term() delete scm; s_maps.clear(); + s_maps.shrink_to_fit(); } // receive at most max_messages. Zero indicates unlimited. diff --git a/src/side_channel/test/side_channel_test.cc b/src/side_channel/test/side_channel_test.cc index acefe8d74..780e73cba 100644 --- a/src/side_channel/test/side_channel_test.cc +++ b/src/side_channel/test/side_channel_test.cc @@ -146,10 +146,7 @@ TEST_GROUP(side_channel) { void setup() override { - // FIXIT-L workaround for issue with CppUTest memory leak detection - MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); SideChannelManager::pre_config_init(); - SCConnectors test_connectors; PortBitSet test_ports; @@ -200,7 +197,6 @@ TEST_GROUP(side_channel) { SideChannelManager::thread_term(); SideChannelManager::term(); - MemoryLeakWarningPlugin::turnOnNewDeleteOverloads(); } }; @@ -283,7 +279,6 @@ TEST(side_channel, test_connector_receive_process_dispatch_discard) { SideChannel* sc = SideChannelManager::get_side_channel(1); CHECK(sc != nullptr); - sc->register_receive_handler(receive_handler); bool success = sc->process(1); diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc index 8703e5fb1..b704283b3 100644 --- a/src/stream/ip/ip_defrag.cc +++ b/src/stream/ip/ip_defrag.cc @@ -1300,7 +1300,7 @@ int Defrag::insert(Packet* p, FragTracker* ft, FragEngine* fe) } debug_logf(stream_ip_trace, p, "left overlap, truncating new pkt (slide: %d)\n", - slide); + slide); break; @@ -1947,7 +1947,7 @@ int Defrag::add_frag_node( ft->frag_bytes += newfrag->size; debug_logf(stream_ip_trace, nullptr, "[#] accumulated bytes on FragTracker %u, count %d\n", - ft->frag_bytes, ft->fraglist_count); + ft->frag_bytes, ft->fraglist_count); *retFrag = newfrag; return FRAG_INSERT_OK; @@ -1981,7 +1981,7 @@ int Defrag::dup_frag_node( FragTracker* ft, Fragment* left, Fragment** retFrag) ft->frag_bytes += newfrag->size; debug_logf(stream_ip_trace, nullptr, "[#] accumulated bytes on FragTracker %u, count %d\n", - ft->frag_bytes, ft->fraglist_count); + ft->frag_bytes, ft->fraglist_count); *retFrag = newfrag; return FRAG_INSERT_OK; diff --git a/src/stream/tcp/segment_overlap_editor.cc b/src/stream/tcp/segment_overlap_editor.cc index dd1422138..318025e6e 100644 --- a/src/stream/tcp/segment_overlap_editor.cc +++ b/src/stream/tcp/segment_overlap_editor.cc @@ -396,7 +396,7 @@ void SegmentOverlapEditor::full_right_overlap_os1(TcpReassemblerState& trs) drop_old_segment(trs); } else - full_right_overlap_truncate_new(trs); + full_right_overlap_truncate_new(trs); } // REASSEMBLY_POLICY_LINUX: @@ -415,7 +415,7 @@ void SegmentOverlapEditor::full_right_overlap_os2(TcpReassemblerState& trs) drop_old_segment(trs); } else - full_right_overlap_truncate_new(trs); + full_right_overlap_truncate_new(trs); } // REASSEMBLY_POLICY_HPUX11: @@ -447,7 +447,7 @@ void SegmentOverlapEditor::full_right_overlap_os4(TcpReassemblerState& trs) void SegmentOverlapEditor::full_right_overlap_os5(TcpReassemblerState& trs) { - full_right_overlap_truncate_new(trs); + full_right_overlap_truncate_new(trs); } void SegmentOverlapEditor::print(TcpReassemblerState& trs) diff --git a/src/stream/tcp/tcp_segment_descriptor.cc b/src/stream/tcp/tcp_segment_descriptor.cc index b756bb7e7..125b32c8e 100644 --- a/src/stream/tcp/tcp_segment_descriptor.cc +++ b/src/stream/tcp/tcp_segment_descriptor.cc @@ -61,8 +61,8 @@ TcpSegmentDescriptor::TcpSegmentDescriptor(Flow* f, Packet* p, TcpEventLogger& t TcpSegmentDescriptor::TcpSegmentDescriptor (snort::Flow* f, snort::Packet* p, uint32_t meta_ack, uint16_t window) - : flow(f), pkt(ma_pseudo_packet), tcph(&ma_pseudo_tcph), - packet_number(p->context->packet_number) + : flow(f), pkt(ma_pseudo_packet), tcph(&ma_pseudo_tcph), + packet_number(p->context->packet_number) { // init tcp header fields for meta-ack packet ma_pseudo_tcph.th_dport = p->ptrs.tcph->raw_src_port(); @@ -76,7 +76,7 @@ TcpSegmentDescriptor::TcpSegmentDescriptor ma_pseudo_tcph.th_urp = 0; // init meta-ack Packet fields stream cares about for TCP ack processing - pkt->flow = p->flow; + pkt->flow = p->flow; pkt->context = p->context; pkt->dsize = 0; diff --git a/src/stream/tcp/test/tcp_normalizer_test.cc b/src/stream/tcp/test/tcp_normalizer_test.cc index 4c584e547..ac50a1b6a 100644 --- a/src/stream/tcp/test/tcp_normalizer_test.cc +++ b/src/stream/tcp/test/tcp_normalizer_test.cc @@ -229,7 +229,6 @@ TEST(tcp_normalizers, norm_options_enabled) int main(int argc, char** argv) { - //MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); return CommandLineTestRunner::RunAllTests(argc, argv); } diff --git a/src/utils/util.h b/src/utils/util.h index b96299674..ab439d91d 100644 --- a/src/utils/util.h +++ b/src/utils/util.h @@ -27,6 +27,7 @@ #if defined(__linux__) #include #endif +#include #include #include -- 2.47.3