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.

diff --git a/src/runmode-unix-socket.c b/src/runmode-unix-socket.c
index 40d77b3e4e25b7ba37eac811f0193c4f210dffe6..53bb4cefc775f7b10bcbbbfb2f5c25d067fe74f0 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 d861f7ee1b77098969728243fabd3ac517d7f8ec..b91b3ef8d238bb4effb37810c317c9b4045ad27d 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 cd5cf0db7a2495c4ccdaf92a52c1a05eb8328604..6760d0bd54ebf231664204aec1d1a852a0e436dd 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);
         }
 
         status = InitPcapFile(pv);
@@ -273,12 +271,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);
@@ -287,8 +283,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));
 
@@ -297,8 +292,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;
@@ -359,35 +353,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);
 }