From: Ashik Thomas (ashiktho) Date: Mon, 8 Jul 2024 10:14:38 +0000 (+0000) Subject: Pull request #4370: file: fixing file context reuse X-Git-Tag: 3.3.1.0~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0efbfa7b83da3a09390da3a81c43d9ba9baaaafd;p=thirdparty%2Fsnort3.git 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 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 e47859090..edc56a460 100644 --- 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 52416cc7d..fa21148c3 100644 --- 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 0f47c2b54..ac867bf33 100644 --- 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"); }