Pull request #3874: file_api: Avoid file cache lookup after creating new file cache...

Merge in SNORT/snort3 from ~STECHEW/snort3:file_cache_optimization to master

Squashed commit of the following:

commit 6c08c968d9d0b2de85ffc928916c6c033e7654df
Author: Steve Chew <stechew@cisco.com>
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 ff6eb172f60cdfc3abb8ad59b12c5e6d83551120..1d1d41feeb8175021f525096a5ca3d2d0b0dd806 100644 (file)
--- 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 b6bf1b9a7da2b66429333c1fc133d6edfdde1907..04ccfb20081cc7ce2450ae8721849d3a106c9122 100644 (file)
--- 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 ffddaf8c003a42f049dca5f3bf751c6cfbe39807..cd50843d3398f0907b2b698bac1fe28b41840a50 100644 (file)
--- 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 4fc3c3255dab394e8cde7733d7c69f88a50f18f5..5b0156779c06edb9a6f78caae54f56c57b3fe617 100644 (file)
--- 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<std::mutex> 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,