Pull request #3726: file_api: Handling filedata in multithreading context

Merge in SNORT/snort3 from ~PRERAMA2/snort3:file_data_handling to master

Squashed commit of the following:

commit 7727011a1e1005b8b94365be3d7a6960adf672e8
Author: Preethi Ramachandra <prerama2@cisco.com>
Date:   Mon Dec 19 10:30:56 2022 +0530

    file_api: Handling filedata in multithreading context

diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc
index 1960a3f3379fde3cf48c732dcb0ce565cb17b260..ff776aaac29ec185fe6878e7ab914976add0c2a3 100644 (file)
--- a/src/file_api/file_cache.cc
+++ b/src/file_api/file_cache.cc
@@ -122,6 +122,43 @@ void FileCache::set_max_files(int64_t max)
     fileHash->set_max_nodes(max_files);
 }
 
+FileContext* FileCache::find_add(const FileHashKey& hashKey, int64_t timeout)
+{
+
+    if ( !fileHash->get_num_nodes() )
+        return nullptr;
+
+    HashNode* hash_node = fileHash->find_node(&hashKey);
+    if ( !hash_node )
+        return nullptr;
+
+    FileNode* node = (FileNode*)hash_node->data;
+    if ( !node )
+    {
+        fileHash->release_node(hash_node);
+        return nullptr;
+    }
+
+    struct timeval now;
+    packet_gettimeofday(&now);
+
+    if ( timercmp(&node->cache_expire_time, &now, <) )
+    {
+        fileHash->release_node(hash_node);
+        return nullptr;
+    }
+
+    struct timeval next_expire_time;
+    struct timeval time_to_add = { static_cast<time_t>(timeout), 0 };
+    timeradd(&now, &time_to_add, &next_expire_time);
+
+    //  Refresh the timer on the cache.
+    if (timercmp(&node->cache_expire_time, &next_expire_time, <))
+        node->cache_expire_time = next_expire_time;
+
+    return node->file;
+}
+
 FileContext* FileCache::add(const FileHashKey& hashKey, int64_t timeout)
 {
     FileNode new_node;
@@ -141,21 +178,32 @@ FileContext* FileCache::add(const FileHashKey& hashKey, int64_t timeout)
 
     std::lock_guard<std::mutex> lock(cache_mutex);
 
-    if (fileHash->insert((void*)&hashKey, &new_node) != HASH_OK)
+    FileContext* file = find_add(hashKey, timeout);
+
+    if (!file) {
+        if (fileHash->insert((void*)&hashKey, &new_node) != HASH_OK)
+        {
+            /* Uh, shouldn't get here...
+             * There is already a node or couldn't alloc space
+             * for key.  This means bigger problems, but fail
+             * gracefully.
+             */
+            FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_CRITICAL_LEVEL, GET_CURRENT_PACKET,
+                "add:Insert failed in file cache, returning\n");
+            file_counts.cache_add_fails++;
+            delete new_node.file;
+            return nullptr;
+        }
+        return new_node.file;
+    }
+    else
     {
-        /* Uh, shouldn't get here...
-         * There is already a node or couldn't alloc space
-         * for key.  This means bigger problems, but fail
-         * gracefully.
-         */
         FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_CRITICAL_LEVEL, GET_CURRENT_PACKET,
-            "add:Insert failed in file cache, returning\n"); 
+            "add:file already found in file cache, returning\n");
         file_counts.cache_add_fails++;
         delete new_node.file;
-        return nullptr;
+        return file;
     }
-
-    return new_node.file;
 }
 
 FileContext* FileCache::find(const FileHashKey& hashKey, int64_t timeout)
@@ -269,8 +317,10 @@ int FileCache::store_verdict(Flow* flow, FileInfo* file, int64_t timeout)
         {
             if (file->get_file_data() and !file_got->get_file_data())
             {
+                file_got->user_file_data_mutex.lock();
                 file_got->set_file_data(file->get_file_data());
                 file->set_file_data(nullptr);
+                file_got->user_file_data_mutex.unlock();
             }
         }
         else

diff --git a/src/file_api/file_cache.h b/src/file_api/file_cache.h
index 69de8c2e25c46807f14009543dbd780649458f8c..6bed058b9ed6c386164d0ba0acc8a313b2f6e7c3 100644 (file)
--- a/src/file_api/file_cache.h
+++ b/src/file_api/file_cache.h
@@ -69,7 +69,9 @@ PADDING_GUARD_END
 private:
     snort::FileContext* add(const FileHashKey&, int64_t timeout);
     snort::FileContext* find(const FileHashKey&, int64_t);
-    snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create, int64_t timeout);
+    snort::FileContext* find_add(const FileHashKey&, int64_t);
+    snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create,
+        int64_t timeout);
     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 42fce79b765a4d3f2cc58e7bfceedd39d90abd6c..73bdb0d0c9a896287beff66b84f6ffded9043605 100644 (file)
--- a/src/file_api/file_flows.cc
+++ b/src/file_api/file_flows.cc
@@ -113,7 +113,7 @@ void FileFlows::handle_retransmit(Packet* p)
     {
         if (file_cache)
             file_got = file_cache->get_file(flow, pending_file_id, false);
-        if (file_got and file_got->get_file_data())
+        if (file_got and file_got->get_file_data() and file_got->verdict == FILE_VERDICT_PENDING)
         {
             file_got->user_file_data_mutex.lock();
             file->set_file_data(file_got->get_file_data());
@@ -121,8 +121,10 @@ void FileFlows::handle_retransmit(Packet* p)
             file_got->user_file_data_mutex.unlock();
         }
     }
-
+    file->user_file_data_mutex.lock();
     FileVerdict verdict = file_policy->signature_lookup(p, file);
+    file->user_file_data_mutex.unlock();
+
     if (file_cache)
     {
         FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p,
@@ -174,8 +176,10 @@ void FileFlows::set_current_file_context(FileContext* ctx)
         FileContext* file_got = file_cache->get_file(flow, file_id, false);
         if (file_got and file_got->verdict == FILE_VERDICT_PENDING and current_context != file_got)
         {
+            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();
         }
     }
     current_context = ctx;