source-pcap-file: Pcap File Init Failure Handling (#1694)

Better handle case where pcap file receive thread fails to initialize. Allow
initialize to complete, but terminate the thread quickly. Delay exiting
unix socket runmode as late as possible.

(cherry picked from commit c0ab45aa6fbe1a299facf45e34ba2bcb3d76ce75)

diff --git a/src/runmode-unix-socket.c b/src/runmode-unix-socket.c
index 1fe2e026b3c73f74fd30bdbfd40c5a6d8e1345a8..7274d814ef7d89f91af15602dc9a5a6dd8ffc90b 100644 (file)
--- a/src/runmode-unix-socket.c
+++ b/src/runmode-unix-socket.c
@@ -569,6 +569,8 @@ static TmEcode UnixSocketPcapFilesCheck(void *data)
 
     this->current_file = cfile;
 
+    SCLogInfo("Starting run for '%s'", this->current_file->filename);
+
     PreRunInit(RUNMODE_PCAP_FILE);
     PreRunPostPrivsDropInit(RUNMODE_PCAP_FILE);
     RunModeDispatch(RUNMODE_PCAP_FILE, NULL);
@@ -578,8 +580,6 @@ static TmEcode UnixSocketPcapFilesCheck(void *data)
     PacketPoolPostRunmodes();
     TmThreadContinueThreads();
 
-    SCLogInfo("Starting run for '%s'", this->current_file->filename);
-
     return TM_ECODE_OK;
 }
 #endif
@@ -601,10 +601,12 @@ void RunModeUnixSocketRegister(void)
 TmEcode UnixSocketPcapFile(TmEcode tm, struct timespec *last_processed)
 {
 #ifdef BUILD_UNIX_SOCKET
-    SCCtrlMutexLock(&unix_manager_pcap_last_processed_mutex);
-    unix_manager_pcap_last_processed.tv_sec = last_processed->tv_sec;
-    unix_manager_pcap_last_processed.tv_nsec = last_processed->tv_nsec;
-    SCCtrlMutexUnlock(&unix_manager_pcap_last_processed_mutex);
+    if(last_processed) {
+        SCCtrlMutexLock(&unix_manager_pcap_last_processed_mutex);
+        unix_manager_pcap_last_processed.tv_sec = last_processed->tv_sec;
+        unix_manager_pcap_last_processed.tv_nsec = last_processed->tv_nsec;
+        SCCtrlMutexUnlock(&unix_manager_pcap_last_processed_mutex);
+    }
     switch (tm) {
         case TM_ECODE_DONE:
             SCLogInfo("Marking current task as done");
@@ -615,7 +617,7 @@ TmEcode UnixSocketPcapFile(TmEcode tm, struct timespec *last_processed)
             unix_manager_pcap_task_running = 0;
             unix_manager_pcap_task_failed = 1;
             //if we return failed, we can't stop the thread and suricata will fail to close
-            return TM_ECODE_DONE;
+            return TM_ECODE_FAILED;
         case TM_ECODE_OK:
             if (unix_manager_pcap_task_interrupted == 1) {
                 SCLogInfo("Interrupting current run mode");

diff --git a/src/source-pcap-file-helper.c b/src/source-pcap-file-helper.c
index 7339f80738bde0cf8a781481bd8cea5f8d88c8cb..4f625e29bc00c4024c883dd5edbe63030265f437 100644 (file)
--- a/src/source-pcap-file-helper.c
+++ b/src/source-pcap-file-helper.c
@@ -172,12 +172,7 @@ TmEcode InitPcapFile(PcapFileFileVars *pfv)
     pfv->pcap_handle = pcap_open_offline(pfv->filename, errbuf);
     if (pfv->pcap_handle == NULL) {
         SCLogError(SC_ERR_FOPEN, "%s", errbuf);
-        if (!RunModeUnixSocketIsActive()) {
-            SCReturnInt(TM_ECODE_FAILED);
-        } else {
-            UnixSocketPcapFile(TM_ECODE_FAILED, 0);
-            SCReturnInt(TM_ECODE_DONE);
-        }
+        SCReturnInt(TM_ECODE_FAILED);
     }
 
     if (pfv->shared != NULL && pfv->shared->bpf_string != NULL) {

diff --git a/src/source-pcap-file.c b/src/source-pcap-file.c
index 2bc22ac5ac5fed0885150947e82278e81af1af3d..12c42cc57745f4f109a6bcf58891bfb95ce31f40 100644 (file)
--- a/src/source-pcap-file.c
+++ b/src/source-pcap-file.c
@@ -67,7 +67,7 @@ static void CleanupPcapDirectoryFromThreadVars(PcapFileThreadVars *tv,
                                                PcapFileDirectoryVars *ptv);
 static void CleanupPcapFileFromThreadVars(PcapFileThreadVars *tv, PcapFileFileVars *pfv);
 static void CleanupPcapFileThreadVars(PcapFileThreadVars *tv);
-static TmEcode PcapFileExit(TmEcode status);
+static TmEcode PcapFileExit(TmEcode status, struct timespec *last_processed);
 
 void CleanupPcapFileFromThreadVars(PcapFileThreadVars *tv, PcapFileFileVars *pfv)
 {
@@ -142,10 +142,11 @@ void PcapFileGlobalInit()
     SC_ATOMIC_INIT(pcap_g.invalid_checksums);
 }
 
-TmEcode PcapFileExit(TmEcode status)
+TmEcode PcapFileExit(TmEcode status, struct timespec *last_processed)
 {
     if(RunModeUnixSocketIsActive()) {
-        SCReturnInt(TM_ECODE_DONE);
+        status = UnixSocketPcapFile(status, last_processed);
+        SCReturnInt(status);
     } else {
         EngineStop();
         SCReturnInt(status);
@@ -156,6 +157,14 @@ TmEcode ReceivePcapFileLoop(ThreadVars *tv, void *data, void *slot)
 {
     SCEnter();
 
+    if(unlikely(data == NULL)) {
+        SCLogError(SC_ERR_INVALID_ARGUMENT, "pcap file reader thread failed to initialize");
+
+        PcapFileExit(TM_ECODE_FAILED, NULL);
+
+        SCReturnInt(TM_ECODE_DONE);
+    }
+
     TmEcode status = TM_ECODE_OK;
     PcapFileThreadVars *ptv = (PcapFileThreadVars *) data;
     TmSlot *s = (TmSlot *)slot;
@@ -166,11 +175,6 @@ TmEcode ReceivePcapFileLoop(ThreadVars *tv, void *data, void *slot)
     if(ptv->is_directory == 0) {
         SCLogInfo("Starting file run for %s", ptv->behavior.file->filename);
         status = PcapFileDispatch(ptv->behavior.file);
-        if (!RunModeUnixSocketIsActive()) {
-            EngineStop();
-        } else {
-            UnixSocketPcapFile(status, &ptv->shared.last_processed);
-        }
         CleanupPcapFileFromThreadVars(ptv, ptv->behavior.file);
     } else {
         SCLogInfo("Starting directory run for %s", ptv->behavior.directory->filename);
@@ -180,8 +184,7 @@ TmEcode ReceivePcapFileLoop(ThreadVars *tv, void *data, void *slot)
 
     SCLogDebug("Pcap file loop complete with status %u", status);
 
-    status = PcapFileExit(status);
-
+    status = PcapFileExit(status, &ptv->shared.last_processed);
     SCReturnInt(status);
 }
 
@@ -196,14 +199,12 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
     if (initdata == NULL) {
         SCLogError(SC_ERR_INVALID_ARGUMENT, "error: initdata == NULL");
 
-        status = PcapFileExit(TM_ECODE_FAILED);
-        SCReturnInt(status);
+        SCReturnInt(TM_ECODE_OK);
     }
 
     PcapFileThreadVars *ptv = SCMalloc(sizeof(PcapFileThreadVars));
     if (unlikely(ptv == NULL)) {
-        status = PcapFileExit(TM_ECODE_FAILED);
-        SCReturnInt(status);
+        SCReturnInt(TM_ECODE_OK);
     }
     memset(ptv, 0, sizeof(PcapFileThreadVars));
     memset(&ptv->shared.last_processed, 0, sizeof(struct timespec));
@@ -224,10 +225,10 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
         ptv->shared.bpf_string = SCStrdup(tmp_bpf_string);
         if (unlikely(ptv->shared.bpf_string == NULL)) {
             SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate bpf_string");
+
             CleanupPcapFileThreadVars(ptv);
 
-            status = PcapFileExit(TM_ECODE_FAILED);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
     }
 
@@ -241,8 +242,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
     SCLogInfo("Checking file or directory %s", (char*)initdata);
     if(PcapDetermineDirectoryOrFile((char *)initdata, &directory) == TM_ECODE_FAILED) {
         CleanupPcapFileThreadVars(ptv);
-        status = PcapFileExit(TM_ECODE_FAILED);
-        SCReturnInt(status);
+        SCReturnInt(TM_ECODE_OK);
     }
 
     if(directory == NULL) {
@@ -251,8 +251,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
         if (unlikely(pv == NULL)) {
             SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate file vars");
             CleanupPcapFileThreadVars(ptv);
-            status = PcapFileExit(TM_ECODE_FAILED);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
         memset(pv, 0, sizeof(PcapFileFileVars));
 
@@ -261,8 +260,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
             SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate filename");
             CleanupPcapFileFileVars(pv);
             CleanupPcapFileThreadVars(ptv);
-            status = PcapFileExit(TM_ECODE_FAILED);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
 
         pv->shared = &ptv->shared;
@@ -272,12 +270,10 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
             ptv->behavior.file = pv;
         } else {
             SCLogWarning(SC_ERR_PCAP_DISPATCH,
-                         "Failed to init pcap file %s, skipping", (char *)initdata);
+                         "Failed to init pcap file %s, skipping", pv->filename);
             CleanupPcapFileFileVars(pv);
             CleanupPcapFileThreadVars(ptv);
-
-            status = PcapFileExit(status);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
     } else {
         SCLogInfo("Argument %s was a directory", (char *)initdata);
@@ -286,8 +282,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
             SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate directory vars");
             closedir(directory);
             CleanupPcapFileThreadVars(ptv);
-            status = PcapFileExit(TM_ECODE_FAILED);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
         memset(pv, 0, sizeof(PcapFileDirectoryVars));
 
@@ -296,8 +291,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
             SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate filename");
             CleanupPcapFileDirectoryVars(pv);
             CleanupPcapFileThreadVars(ptv);
-            status = PcapFileExit(TM_ECODE_FAILED);
-            SCReturnInt(status);
+            SCReturnInt(TM_ECODE_OK);
         }
 
         int should_loop = 0;
@@ -358,35 +352,39 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
 void ReceivePcapFileThreadExitStats(ThreadVars *tv, void *data)
 {
     SCEnter();
-    PcapFileThreadVars *ptv = (PcapFileThreadVars *)data;
-
-    if (pcap_g.conf_checksum_mode == CHECKSUM_VALIDATION_AUTO &&
-        pcap_g.cnt < CHECKSUM_SAMPLE_COUNT &&
-        SC_ATOMIC_GET(pcap_g.invalid_checksums)) {
-        uint64_t chrate = pcap_g.cnt / SC_ATOMIC_GET(pcap_g.invalid_checksums);
-        if (chrate < CHECKSUM_INVALID_RATIO)
-            SCLogWarning(SC_ERR_INVALID_CHECKSUM,
+    if(data != NULL) {
+        PcapFileThreadVars *ptv = (PcapFileThreadVars *)data;
+
+        if (pcap_g.conf_checksum_mode == CHECKSUM_VALIDATION_AUTO &&
+            pcap_g.cnt < CHECKSUM_SAMPLE_COUNT &&
+            SC_ATOMIC_GET(pcap_g.invalid_checksums)) {
+            uint64_t chrate = pcap_g.cnt / SC_ATOMIC_GET(pcap_g.invalid_checksums);
+            if (chrate < CHECKSUM_INVALID_RATIO)
+                SCLogWarning(SC_ERR_INVALID_CHECKSUM,
                          "1/%" PRIu64 "th of packets have an invalid checksum,"
                                  " consider setting pcap-file.checksum-checks variable to no"
                                  " or use '-k none' option on command line.",
                          chrate);
-        else
-            SCLogInfo("1/%" PRIu64 "th of packets have an invalid checksum",
+            else
+                SCLogInfo("1/%" PRIu64 "th of packets have an invalid checksum",
                       chrate);
-    }
-    SCLogNotice(
+        }
+        SCLogNotice(
             "Pcap-file module read %" PRIu64 " files, %" PRIu64 " packets, %" PRIu64 " bytes",
             ptv->shared.files,
             ptv->shared.pkts,
             ptv->shared.bytes
-    );
+        );
+    }
 }
 
 TmEcode ReceivePcapFileThreadDeinit(ThreadVars *tv, void *data)
 {
     SCEnter();
-    PcapFileThreadVars *ptv = (PcapFileThreadVars *)data;
-    CleanupPcapFileThreadVars(ptv);
+    if(data != NULL) {
+        PcapFileThreadVars *ptv = (PcapFileThreadVars *) data;
+        CleanupPcapFileThreadVars(ptv);
+    }
     SCReturnInt(TM_ECODE_OK);
 }