From fd7b01ee6f951b00110400e32053f1b2b8e79ece Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Wed, 11 Dec 2024 08:26:01 -0500 Subject: [PATCH 1/2] output/log: Improve error handling This commit improves error handling for cases when file(s) cannot be opened. - Return NULL if file object can't be opened - checks whether the file object has been opened before dereferencing the per-file context. Issue: 7447 --- src/util-logopenfile.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/util-logopenfile.c b/src/util-logopenfile.c index 24063e10954b..a53ca3bbd414 100644 --- a/src/util-logopenfile.c +++ b/src/util-logopenfile.c @@ -346,14 +346,17 @@ static void ThreadLogFileHashFreeFunc(void *data) BUG_ON(data == NULL); ThreadLogFileHashEntry *thread_ent = (ThreadLogFileHashEntry *)data; - if (thread_ent) { + if (!thread_ent) + return; + + if (thread_ent->isopen) { LogFileCtx *lf_ctx = thread_ent->ctx; /* Free the leaf log file entries */ if (!lf_ctx->threaded) { LogFileFreeCtx(lf_ctx); } - SCFree(thread_ent); } + SCFree(thread_ent); } bool SCLogOpenThreadedFile(const char *log_path, const char *append, LogFileCtx *parent_ctx) @@ -712,6 +715,7 @@ LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *parent_ctx) if (!parent_ctx->threaded) return parent_ctx; + LogFileCtx *ret_ctx = NULL; SCMutexLock(&parent_ctx->threads->mutex); /* Find this thread's entry */ ThreadLogFileHashEntry *entry = LogFileThread2Slot(parent_ctx->threads, thread_id); @@ -721,12 +725,13 @@ LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *parent_ctx) bool new = entry->isopen; /* has it been opened yet? */ - if (!entry->isopen) { + if (!new) { SCLogDebug("%s: Opening new file for thread/id %d to file %s [ctx %p]", t_thread_name, thread_id, parent_ctx->filename, parent_ctx); if (LogFileNewThreadedCtx( parent_ctx, parent_ctx->filename, parent_ctx->threads->append, entry)) { entry->isopen = true; + ret_ctx = entry->ctx; } else { SCLogError( "Unable to open slot %d for file %s", entry->slot_number, parent_ctx->filename); @@ -742,7 +747,7 @@ LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *parent_ctx) } } - return entry->ctx; + return ret_ctx; } /** \brief LogFileThreadedName() Create file name for threaded EVE storage From bd1d010698c7315b6ca63aec8e904b2b5d8894af Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Thu, 12 Dec 2024 08:49:20 -0500 Subject: [PATCH 2/2] output/log: Remove extraneous error message Issue: 7447 When the output file can't be opened, 2 error messages are displayed for the same problem. The second message doesn't add value and lacks context (error reason, e.g., "Permission denied"). Retaining the second message as a debug message. Without this commit: Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428] Error: logopenfile: Unable to open slot 1 for file /home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.json [LogFileEnsureExists:util-logopenfile.c:737] Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692] With commit: Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428] Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692] --- src/util-logopenfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-logopenfile.c b/src/util-logopenfile.c index a53ca3bbd414..90f18133a4f5 100644 --- a/src/util-logopenfile.c +++ b/src/util-logopenfile.c @@ -733,7 +733,7 @@ LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *parent_ctx) entry->isopen = true; ret_ctx = entry->ctx; } else { - SCLogError( + SCLogDebug( "Unable to open slot %d for file %s", entry->slot_number, parent_ctx->filename); (void)HashTableRemove(parent_ctx->threads->ht, entry, 0); }