From: Steve Chew (stechew) Date: Wed, 21 Jun 2023 16:00:48 +0000 (+0000) Subject: Pull request #3874: file_api: Avoid file cache lookup after creating new file cache... X-Git-Tag: 3.1.65.0~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8879704e05658d3e90e6f75b7d94c9b3d4475ae4;p=thirdparty%2Fsnort3.git Pull request #3874: file_api: Avoid file cache lookup after creating new file cache entry. Merge in SNORT/snort3 from ~STECHEW/snort3:file_cache_optimization to master Squashed commit of the following: commit 6c08c968d9d0b2de85ffc928916c6c033e7654df Author: Steve Chew Date: Fri Jun 9 14:40:27 2023 -0400 file_api: Avoid file cache lookup after creating new file cache entry. --- diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc index ff6eb172f..1d1d41fee 100644 --- a/src/file_api/file_flows.cc +++ b/src/file_api/file_flows.cc @@ -98,7 +98,8 @@ void FileFlows::handle_retransmit(Packet* p) if (file_policy == nullptr) return; - FileContext* file = get_file_context(pending_file_id, false); + bool is_new_context = false; + FileContext* file = get_file_context(pending_file_id, false, is_new_context); if ((file == nullptr) or (file->verdict != FILE_VERDICT_PENDING)) { FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_ERROR_LEVEL, p, @@ -191,7 +192,8 @@ FileContext* FileFlows::get_current_file_context() { if (current_file_id) { - return get_file_context(current_file_id, false); + bool is_new_context = false; + return get_file_context(current_file_id, false, is_new_context); } return current_context; } @@ -273,8 +275,11 @@ FileContext* FileFlows::get_partially_processed_context(uint64_t file_id) } FileContext* FileFlows::get_file_context( - uint64_t file_id, bool to_create, uint64_t multi_file_processing_id) + uint64_t file_id, bool to_create, bool& is_new_context, + uint64_t multi_file_processing_id) { + is_new_context = false; + // First check if this file is currently being processed if (!multi_file_processing_id) multi_file_processing_id = file_id; @@ -306,6 +311,7 @@ FileContext* FileFlows::get_file_context( else { context = new FileContext; + is_new_context = true; partially_processed_contexts[multi_file_processing_id] = context; FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, GET_CURRENT_PACKET, "get_file_context:creating new context\n"); @@ -331,10 +337,12 @@ void FileFlows::remove_processed_file_context(uint64_t file_id) // Remove file context explicitly void FileFlows::remove_processed_file_context(uint64_t file_id, uint64_t multi_file_processing_id) { + bool is_new_context = false; + if (!multi_file_processing_id) multi_file_processing_id = file_id; - FileContext* context = get_file_context(file_id, false, multi_file_processing_id); + FileContext* context = get_file_context(file_id, false, is_new_context, multi_file_processing_id); if (context) { set_current_file_context(context); @@ -356,6 +364,7 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da int64_t file_depth = FileService::get_max_file_depth(); bool continue_processing; bool cacheable = file_id or offset; + bool is_new_context = false; if (!multi_file_processing_id) multi_file_processing_id = file_id; @@ -367,7 +376,7 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da return false; } - FileContext* context = get_file_context(file_id, true, multi_file_processing_id); + FileContext* context = get_file_context(file_id, true, is_new_context, multi_file_processing_id); if (!context) { @@ -393,7 +402,7 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da context->set_file_id(file_id); } - if ( context->is_cacheable()) + if (context->is_cacheable() and not is_new_context) { FileVerdict verdict = FileService::get_file_cache()->cached_verdict_lookup(p, context, file_policy); @@ -487,9 +496,10 @@ bool FileFlows::file_process(Packet* p, const uint8_t* file_data, int data_size, bool FileFlows::set_file_name(const uint8_t* fname, uint32_t name_size, uint64_t file_id, uint64_t multi_file_processing_id, const uint8_t* url, uint32_t url_size) { + bool is_new_context = false; FileContext* context; if (file_id) - context = get_file_context(file_id, false, multi_file_processing_id); + context = get_file_context(file_id, false, is_new_context, multi_file_processing_id); else context = get_current_file_context(); if ( !context ) diff --git a/src/file_api/file_flows.h b/src/file_api/file_flows.h index b6bf1b9a7..04ccfb200 100644 --- a/src/file_api/file_flows.h +++ b/src/file_api/file_flows.h @@ -72,7 +72,7 @@ public: // Get file context based on file id, create it if does not exist FileContext* get_file_context(uint64_t file_id, bool to_create, - uint64_t multi_file_processing_id=0); + bool& is_new_context, uint64_t multi_file_processing_id=0); // Get a partially processed file context from the flow object FileContext* get_partially_processed_context(uint64_t file_id); // Remove a file from the flow object when processing is complete diff --git a/src/service_inspectors/dce_rpc/dce_smb2_file.cc b/src/service_inspectors/dce_rpc/dce_smb2_file.cc index ffddaf8c0..cd50843d3 100644 --- a/src/service_inspectors/dce_rpc/dce_smb2_file.cc +++ b/src/service_inspectors/dce_rpc/dce_smb2_file.cc @@ -337,7 +337,8 @@ bool Dce2Smb2FileTracker::process_data(const uint32_t current_flow_key, const ui if (updated_flow.first) { // update the new file context in case of flow switch - FileContext* file = file_flows->get_file_context(file_name_hash, true, file_id); + bool is_new_context = false; + FileContext* file = file_flows->get_file_context(file_name_hash, true, is_new_context, file_id); file->set_file_name(file_name, file_name_len); file->set_file_size(file_size.load() ? file_size.load() : UNKNOWN_FILE_SIZE); } diff --git a/src/service_inspectors/dce_rpc/dce_smb_common.cc b/src/service_inspectors/dce_rpc/dce_smb_common.cc index 4fc3c3255..5b0156779 100644 --- a/src/service_inspectors/dce_rpc/dce_smb_common.cc +++ b/src/service_inspectors/dce_rpc/dce_smb_common.cc @@ -185,8 +185,9 @@ FileContext* get_smb_file_context(Flow* flow, uint64_t file_id, return nullptr; } + bool is_new_context = false; std::lock_guard guard(file_flows->file_flow_context_mutex); - return file_flows->get_file_context(file_id, to_create, multi_file_processing_id); + return file_flows->get_file_context(file_id, to_create, is_new_context, multi_file_processing_id); } char* get_smb_file_name(const uint8_t* data, uint32_t data_len, bool unicode,