From 716189d6c9dc6b3fef454ad0d39f853e9bd30607 Mon Sep 17 00:00:00 2001 From: Danny Browning Date: Mon, 18 Feb 2019 09:57:21 -0700 Subject: [PATCH] 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) --- src/runmode-unix-socket.c | 16 ++++--- src/source-pcap-file-helper.c | 7 +-- src/source-pcap-file.c | 88 +++++++++++++++++------------------ 3 files changed, 53 insertions(+), 58 deletions(-) diff --git a/src/runmode-unix-socket.c b/src/runmode-unix-socket.c index 1fe2e026b3..7274d814ef 100644 --- 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 7339f80738..4f625e29bc 100644 --- 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 2bc22ac5ac..12c42cc577 100644 --- 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); } -- 2.47.2