From: Bhargava Jandhyala (bjandhya) Date: Thu, 19 Jan 2023 06:15:38 +0000 (+0000) Subject: Pull request #3726: file_api: Handling filedata in multithreading context X-Git-Tag: 3.1.53.0~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=077ae5d2af9758a3183f878dc9d156db1a593f59;p=thirdparty%2Fsnort3.git 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 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 1960a3f33..ff776aaac 100644 --- 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(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 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 69de8c2e2..6bed058b9 100644 --- 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 42fce79b7..73bdb0d0c 100644 --- 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;