Pull request #4370: file: fixing file context reuse

Merge in SNORT/snort3 from ~ASHIKTHO/snort3:CSCwj63921_tot_2 to master

Squashed commit of the following:

commit 3422d104dac341bf4c7036bd6f4b572c538c169b
Author: Ashik Thomas <ashiktho@cisco.com>
Date:   Fri Jun 28 03:12:47 2024 -0700

    file: fixing file context reuse

diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc
index e47859090c09a164c61dc561bceda39cd76021de..edc56a46092c2085b8a272798e830e5efe5447e3 100644 (file)
--- a/src/file_api/file_cache.cc
+++ b/src/file_api/file_cache.cc
@@ -38,6 +38,8 @@
 #include "file_service.h"
 #include "file_stats.h"
 
+#define DEFAULT_FILE_LOOKUP_TIMEOUT_CACHED_ITEM 3600    // 1 hour
+
 using namespace snort;
 
 class ExpectedFileCache : public XHash
@@ -246,7 +248,7 @@ FileContext* FileCache::find(const FileHashKey& hashKey, int64_t timeout)
 }
 
 FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create,
-    int64_t timeout)
+    int64_t timeout, bool using_cache_entry)
 {
     FileHashKey hashKey;
     hashKey.dip = flow->client_ip;
@@ -256,16 +258,22 @@ FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create,
     hashKey.file_id = file_id;
     hashKey.asid = flow->key->addressSpaceId;
     hashKey.padding[0] = hashKey.padding[1] = hashKey.padding[2] = 0;
-    FileContext* file = find(hashKey, timeout);
+    
+    FileContext* file = nullptr;
+    if (using_cache_entry)
+        file = find(hashKey, DEFAULT_FILE_LOOKUP_TIMEOUT_CACHED_ITEM);
+    else
+        file = find(hashKey, timeout);
+    
     if (to_create and !file)
         file = add(hashKey, timeout);
 
     return file;
 }
 
-FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create)
+FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create, bool using_cache_entry)
 {
-    return get_file(flow, file_id, to_create, lookup_timeout);
+    return get_file(flow, file_id, to_create, lookup_timeout, using_cache_entry);
 }
 
 FileVerdict FileCache::check_verdict(Packet* p, FileInfo* file,
@@ -309,7 +317,7 @@ int FileCache::store_verdict(Flow* flow, FileInfo* file, int64_t timeout)
         return 0;
     }
 
-    FileContext* file_got = get_file(flow, file_id, true, timeout);
+    FileContext* file_got = get_file(flow, file_id, true, timeout, false);
     if (file_got)
     {
         *((FileInfo*)(file_got)) = *file;
@@ -501,7 +509,7 @@ FileVerdict FileCache::cached_verdict_lookup(Packet* p, FileInfo* file,
         return verdict;
     }
 
-    FileContext* file_found = get_file(flow, file_id, false);
+    FileContext* file_found = get_file(flow, file_id, false, false);
 
     if (file_found)
     {

diff --git a/src/file_api/file_cache.h b/src/file_api/file_cache.h
index 52416cc7dcfddf6d65e1aa005908c323280bc6a9..fa21148c3963d822f1f2cbabafea27374f0c4c2f 100644 (file)
--- a/src/file_api/file_cache.h
+++ b/src/file_api/file_cache.h
@@ -60,7 +60,7 @@ PADDING_GUARD_END
     void set_lookup_timeout(int64_t);
     void set_max_files(int64_t);
 
-    snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create);
+    snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create, bool using_cache_entry);
     FileVerdict cached_verdict_lookup(snort::Packet*, snort::FileInfo*,
         snort::FilePolicyBase*);
     bool apply_verdict(snort::Packet*, snort::FileContext*, FileVerdict, bool resume,
@@ -71,7 +71,7 @@ private:
     snort::FileContext* find(const FileHashKey&, int64_t);
     snort::FileContext* find_add(const FileHashKey&, int64_t);
     snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create,
-        int64_t timeout);
+        int64_t timeout, bool using_cache_entry);
     FileVerdict check_verdict(snort::Packet*, snort::FileInfo*, snort::FilePolicyBase*);
     int store_verdict(snort::Flow*, snort::FileInfo*, int64_t timeout);
 

diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc
index 0f47c2b54447009590d6ab9023fe83ad99791015..ac867bf3354e62548943159f51b13a9c30ed6cb9 100644 (file)
--- a/src/file_api/file_flows.cc
+++ b/src/file_api/file_flows.cc
@@ -114,7 +114,7 @@ void FileFlows::handle_retransmit(Packet* p)
     if (!file->get_file_data())
     {
         if (file_cache)
-            file_got = file_cache->get_file(flow, pending_file_id, false);
+            file_got = file_cache->get_file(flow, pending_file_id, false, false);
         if (file_got and file_got->get_file_data() and file_got->verdict == FILE_VERDICT_PENDING)
         {
             file_got->user_file_data_mutex.lock();
@@ -175,7 +175,7 @@ void FileFlows::set_current_file_context(FileContext* ctx)
         current_context_delete_pending = false;
         FileCache* file_cache = FileService::get_file_cache();
         assert(file_cache);
-        FileContext* file_got = file_cache->get_file(flow, file_id, false);
+        FileContext* file_got = file_cache->get_file(flow, file_id, false, false);
         if (file_got and file_got->verdict == FILE_VERDICT_PENDING and current_context != file_got)
         {
             file_got->user_file_data_mutex.lock();
@@ -209,21 +209,6 @@ FileFlows::~FileFlows()
 {
     FileCache* file_cache = FileService::get_file_cache();
     assert(file_cache);
-    uint64_t file_id = 0;
-    if (current_context)
-        file_id = current_context->get_file_id();
-    else if (main_context)
-        file_id = main_context->get_file_id();
-
-    FileContext* file_got = file_cache->get_file(flow, file_id, false);
-
-    if (file_got and (file_got->verdict == FILE_VERDICT_PENDING))
-    {
-        file_got->user_file_data_mutex.lock();
-        delete (file_got->get_file_data());
-        file_got->set_file_data(nullptr);
-        file_got->user_file_data_mutex.unlock();
-    }
 
     delete(main_context);
     if (current_context_delete_pending)
@@ -291,7 +276,7 @@ FileContext* FileFlows::get_file_context(
     {
         FileCache* file_cache = FileService::get_file_cache();
         assert(file_cache);
-        context = file_cache->get_file(flow, file_id, false);
+        context = file_cache->get_file(flow, file_id, false, true);
         FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, GET_CURRENT_PACKET,
             "get_file_context:trying to get context from cache\n");
     }