From: Danny Browning Date: Mon, 18 Feb 2019 16:57:21 +0000 (-0700) Subject: source-pcap-file: Pcap File Init Failure Handling (#1694) X-Git-Tag: suricata-5.0.0-beta1~107 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F3747%2Fhead;p=thirdparty%2Fsuricata.git 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 40d77b3e4e..53bb4cefc7 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 d861f7ee1b..b91b3ef8d2 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 cd5cf0db7a..6760d0bd54 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); } 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); }