From a7253f9ebef57e75bb96501637e889580194e489 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 2 Jun 2026 21:25:15 +0200 Subject: [PATCH 01/16] output/filestore: refactor file descriptor handling To assist code analyzers. Gcc -fanalyzer got confused about it. Also test data pointer and length before calling fwrite and check the result better. Use a single atomic for the max open files check. --- src/output-filestore.c | 50 ++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/output-filestore.c b/src/output-filestore.c index 84a68aa4e8fc..2453aeb2ab15 100644 --- a/src/output-filestore.c +++ b/src/output-filestore.c @@ -192,6 +192,9 @@ static void OutputFilestoreFinalizeFiles(ThreadVars *tv, const OutputFilestoreLo } } +/** + * \note `filestore_open_file_cnt` should only be used when FileGetMaxOpenFiles() is non-zero + */ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet *p, File *ff, void *tx, const uint64_t tx_id, const uint8_t *data, uint32_t data_len, uint8_t flags, uint8_t dir) @@ -201,6 +204,7 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet OutputFilestoreCtx *ctx = aft->ctx; char filename[PATH_MAX] = ""; int file_fd = -1; + bool close_file = false; SCLogDebug("ff %p, data %p, data_len %u", ff, data, data_len); @@ -218,18 +222,20 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet return -1; } - if (SC_ATOMIC_GET(filestore_open_file_cnt) < FileGetMaxOpenFiles()) { - SC_ATOMIC_ADD(filestore_open_file_cnt, 1); - ff->fd = file_fd; - } else { - if (FileGetMaxOpenFiles() > 0) { - StatsCounterIncr(&tv->stats, aft->counter_max_hits); - } + /* SC_ATOMIC_ADD returns value before the addition. */ + if (FileGetMaxOpenFiles() > 0 && + SC_ATOMIC_ADD(filestore_open_file_cnt, 1) >= FileGetMaxOpenFiles()) { + (void)SC_ATOMIC_SUB(filestore_open_file_cnt, 1); + StatsCounterIncr(&tv->stats, aft->counter_max_hits); ff->fd = -1; + close_file = true; /* not storing it, so need to close */ + } else { + ff->fd = file_fd; } /* we can get called with NULL data when we need to close */ } else if (data != NULL) { - if (ff->fd == -1) { + file_fd = ff->fd; + if (file_fd == -1) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -242,14 +248,15 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet strerror(errno)); return -1; } - } else { - file_fd = ff->fd; + close_file = true; /* close temporary open */ } + } else if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { + file_fd = ff->fd; } - if (file_fd != -1) { + if (file_fd != -1 && data != NULL && data_len > 0) { ssize_t r = write(file_fd, (const void *)data, (size_t)data_len); - if (r == -1) { + if (r == -1 || (ssize_t)data_len != r) { /* construct tmp file path */ char tmp_filename[PATH_MAX] = ""; snprintf(tmp_filename, sizeof(tmp_filename), "file.%u", ff->file_store_id); @@ -257,22 +264,23 @@ static int OutputFilestoreLogger(ThreadVars *tv, void *thread_data, const Packet StatsCounterIncr(&tv->stats, aft->fs_error_counter); WARN_ONCE(WOT_WRITE, "Filestore (v2) failed to write to %s: %s", filename, strerror(errno)); - if (ff->fd != -1) { + close_file = true; /* close in the error case */ + } + } + + /* close the open file if needed */ + if (file_fd != -1 && ((flags & OUTPUT_FILEDATA_FLAG_CLOSE) != 0 || close_file)) { + /* if it was stored in the ff, disconnect */ + if (ff->fd != -1) { + if (FileGetMaxOpenFiles()) { SC_ATOMIC_SUB(filestore_open_file_cnt, 1); } ff->fd = -1; } - if (ff->fd == -1) { - close(file_fd); - } + close(file_fd); } if (flags & OUTPUT_FILEDATA_FLAG_CLOSE) { - if (ff->fd != -1) { - close(ff->fd); - ff->fd = -1; - SC_ATOMIC_SUB(filestore_open_file_cnt, 1); - } OutputFilestoreFinalizeFiles(tv, aft, ctx, p, ff, tx, tx_id, dir); } From dfa3ba69dc68a5543069efb09082d94ae901726b Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 10:13:39 +0200 Subject: [PATCH 02/16] frames: avoid possible undefined behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code analyzer flagged FrameCopy as a possible source of UB due to both pointers passed to memcpy being the same. app-layer-frames.c: In function ‘FrameCopy’: app-layer-frames.c:236:5: warning: overlapping buffers passed as arguments to ‘memcpy’ [-Wanalyzer-overlapping-buffers] 236 | memcpy(dst, src, sizeof(*dst)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘FramePrune’: events 1-8 │ │ 750 | static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) │ | ^~~~~~~~~~ │ | | │ | (1) entry to ‘FramePrune’ │...... │ 766 | for (uint16_t i = 0; i < frames->cnt; i++) { │ | ~~~~~~~~~~~~~~~ │ | | │ | (2) following ‘true’ branch... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 767 |│ if (i < FRAMES_STATIC_CNT) { │ |│ ~ │ |│ | │ |└──────────>(3) ...to here │ | (4) following ‘true’ branch (when ‘i <= 2’)... ─>─┐ │ | │ │ | │ │ |┌─────────────────────────────────────────────────────────────┘ │ 768 |│ Frame *frame = &frames->sframes[i]; │ |│ ~~~~~~~~~~~~~~~~~~ │ |│ | │ |└──────────────────────────────────────────>(5) ...to here │ 769 | FrameDebug("prune(s)", frames, frame); │ 770 | if (eof || FrameIsDone(frame, acked)) { │ | ~ │ | | │ | (6) following ‘false’ branch... ─>─┐ │ | │ │...... │ | │ │ |┌──────────────────────────────────────────────────┘ │ 779 |│ const uint64_t fle = FrameLeftEdge(stream, frame); │ |│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ |│ | │ |└────────────────────────────────────>(7) ...to here │ | (8) calling ‘FrameLeftEdge’ from ‘FramePrune’ │ └──> ‘FrameLeftEdge’: event 9 │ │ 257 | static inline uint64_t FrameLeftEdge(const TcpStream *stream, const Frame *frame) │ | ^~~~~~~~~~~~~ │ | | │ | (9) entry to ‘FrameLeftEdge’ │ ‘FrameLeftEdge’: event 10 │ │suricata-common.h:323:27: │ 323 | #define BUG_ON(x) assert(!(x)) │ | ^~~~~~ │ | | │ | (10) following ‘false’ branch (when ‘frame_offset <= app_progress’)... ─>─┐ │ | │ util-validate.h:95:36: note: in expansion of macro ‘BUG_ON’ │ 95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp)) │ | ^~~~~~ app-layer-frames.c:266:5: note: in expansion of macro ‘DEBUG_VALIDATE_BUG_ON’ │ 266 | DEBUG_VALIDATE_BUG_ON(frame_offset > app_progress); │ | ^~~~~~~~~~~~~~~~~~~~~ │ ‘FrameLeftEdge’: event 11 │ │ | │ │ |┌────────────────────────────────────────────────────────────────────────────────────────────────────┘ │ 269 |│ if (frame->len < 0) { │ |│ ~~~~~^~~~~ │ |│ | │ |└────────────>(11) ...to here │ <──────┘ │ ‘FramePrune’: events 12-13 │ │ 779 | const uint64_t fle = FrameLeftEdge(stream, frame); │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (12) returning to ‘FramePrune’ from ‘FrameLeftEdge’ │...... │ 783 | FrameCopy(nframe, frame); │ | ~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (13) calling ‘FrameCopy’ from ‘FramePrune’ │ └──> ‘FrameCopy’: events 14-15 │ │ 234 | static void FrameCopy(Frame *dst, Frame *src) │ | ^~~~~~~~~ │ | | │ | (14) entry to ‘FrameCopy’ │ 235 | { │ 236 | memcpy(dst, src, sizeof(*dst)); │ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ | | │ | (15) ⚠️ overlapping buffers passed as arguments to ‘memcpy’ │ In file included from suricata-common.h:129, from app-layer-frames.c:25: /usr/include/string.h:47:14: note: the behavior of ‘memcpy’ is undefined for overlapping buffers 47 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src, | ^~~~~~ --- src/app-layer-frames.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/app-layer-frames.c b/src/app-layer-frames.c index 2d79fb463ab6..3962110414fa 100644 --- a/src/app-layer-frames.c +++ b/src/app-layer-frames.c @@ -233,6 +233,7 @@ static void FrameClean(Frame *frame) static void FrameCopy(Frame *dst, Frame *src) { + DEBUG_VALIDATE_BUG_ON(dst == src); memcpy(dst, src, sizeof(*dst)); } @@ -353,8 +354,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c #endif } else { Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -378,8 +379,8 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } le = MIN(le, FrameLeftEdge(stream, nframe)); @@ -780,8 +781,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) le = MIN(le, fle); SCLogDebug("le %" PRIu64 ", frame fle %" PRIu64, le, fle); Frame *nframe = &frames->sframes[x]; - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; @@ -808,8 +809,8 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof) } else { nframe = &frames->sframes[x]; } - FrameCopy(nframe, frame); if (frame != nframe) { + FrameCopy(nframe, frame); FrameClean(frame); } x++; From 4df08c3413bc386d54c05371ffb3e318ce499ab4 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:19:27 +0200 Subject: [PATCH 03/16] tm/queues: assist gcc -fanalyzer Work around TAILQ false positive. --- src/tm-queues.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tm-queues.c b/src/tm-queues.c index 81bbc3042b83..c36f97de4d4c 100644 --- a/src/tm-queues.c +++ b/src/tm-queues.c @@ -27,6 +27,7 @@ #include "threads.h" #include "tm-queues.h" #include "util-debug.h" +#include "util-validate.h" static TAILQ_HEAD(TmqList_, Tmq_) tmq_list = TAILQ_HEAD_INITIALIZER(tmq_list); @@ -83,6 +84,9 @@ void TmqResetQueues(void) while ((tmq = TAILQ_FIRST(&tmq_list))) { TAILQ_REMOVE(&tmq_list, tmq, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&tmq_list) == tmq); + if (tmq->name) { SCFree(tmq->name); } From 4560b7da7bc35bfa905de7f0c35db5300b79dd47 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:40:51 +0200 Subject: [PATCH 04/16] conf: assist gcc -fanalyzer Work around TAILQ false positive. --- src/conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf.c b/src/conf.c index 9c0c3e0ad4eb..49659266d80c 100644 --- a/src/conf.c +++ b/src/conf.c @@ -161,6 +161,8 @@ void SCConfNodeFree(SCConfNode *node) while ((tmp = TAILQ_FIRST(&node->head))) { TAILQ_REMOVE(&node->head, tmp, next); + /* help code checkers to understand what TAILQ_REMOVE does */ + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&node->head) == tmp); SCConfNodeFree(tmp); } From 1a510a942dd898e75274df65a8128dbcc800f0dc Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:48:04 +0200 Subject: [PATCH 05/16] decode/tcp: only set data ptr for valid option lengths --- src/decode-tcp.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/decode-tcp.c b/src/decode-tcp.c index a7d5ee4b1686..f2cad13d06bf 100644 --- a/src/decode-tcp.c +++ b/src/decode-tcp.c @@ -79,8 +79,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) } tcp_opts[tcp_opt_cnt].type = type; - tcp_opts[tcp_opt_cnt].len = olen; - tcp_opts[tcp_opt_cnt].data = (olen > 2) ? (pkt+2) : NULL; + tcp_opts[tcp_opt_cnt].len = olen; /* we are parsing the most commonly used opts to prevent * us from having to walk the opts list for these all the @@ -90,6 +89,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_WS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.wscale_set != 0) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -107,6 +107,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_MSS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.mss_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -119,6 +120,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_SACKOK_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (TCP_GET_SACKOK(p)) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -130,6 +132,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != TCP_OPT_TS_LEN) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.ts_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -149,6 +152,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !((olen - 2) % 8 == 0)) { ENGINE_SET_EVENT(p, TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.sack_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -166,6 +170,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) !(((olen - 2) & 0x1) == 0))) { ENGINE_SET_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); if (p->l4.vars.tcp.tfo_set) { ENGINE_SET_EVENT(p,TCP_OPT_DUPLICATE); } else { @@ -178,6 +183,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) case TCP_OPT_EXP2: SCLogDebug("TCP EXP option, len %u", olen); if (olen == 4 || olen == 12) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); uint16_t magic = DecodeTCPGetU16(tcp_opts[tcp_opt_cnt].data); if (magic == 0xf989) { if (p->l4.vars.tcp.tfo_set) { @@ -196,6 +202,7 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen != 18) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.md5_option_present = true; } @@ -206,10 +213,18 @@ static void DecodeTCPOptions(Packet *p, const uint8_t *pkt, uint16_t pktlen) if (olen < 4) { ENGINE_SET_INVALID_EVENT(p,TCP_OPT_INVALID_LEN); } else { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); /* we can't validate the option as the key is out of band */ p->l4.vars.tcp.ao_option_present = true; } break; + default: + if (olen > 2) { + tcp_opts[tcp_opt_cnt].data = (pkt + 2); + } else { + + tcp_opts[tcp_opt_cnt].data = NULL; + } } pkt += olen; From 290c5e296adbed23c32f66cbf8c3957e2602b997 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 12:57:18 +0200 Subject: [PATCH 06/16] detect/sigorder: handle allocation failure Addresses a gcc -fanalyzer warning. --- src/detect-engine-loader.c | 5 ++++- src/detect-engine-sigorder.c | 35 ++++++++++++++++++++++------------- src/detect-engine-sigorder.h | 2 +- src/detect-flowint.c | 6 +++--- src/util-unittest-helper.c | 4 +++- 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index ca9cda0ce0e7..038146a946b6 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -500,7 +500,10 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc } SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + ret = -1; + goto end; + } SCSigSignatureOrderingModuleCleanup(de_ctx); if (SCThresholdConfInitContext(de_ctx) < 0) { diff --git a/src/detect-engine-sigorder.c b/src/detect-engine-sigorder.c index b6a3ba85a55d..4168952e1f25 100644 --- a/src/detect-engine-sigorder.c +++ b/src/detect-engine-sigorder.c @@ -801,13 +801,14 @@ static inline SCSigSignatureWrapper *SCSigAllocSignatureWrapper(Signature *sig) * \param de_ctx Pointer to the Detection Engine Context that holds the * signatures to be ordered */ -void SCSigOrderSignatures(DetectEngineCtx *de_ctx) +int SCSigOrderSignatures(DetectEngineCtx *de_ctx) { if (de_ctx->sig_list == NULL) { SCLogDebug("no signatures to order"); - return; + return 0; } + int retval = 0; SCLogDebug("ordering signatures in memory"); SCSigSignatureWrapper *sigw = NULL; SCSigSignatureWrapper *td_sigw_list = NULL; /* unified td list */ @@ -818,6 +819,12 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) Signature *sig = de_ctx->sig_list; while (sig != NULL) { sigw = SCSigAllocSignatureWrapper(sig); + if (sigw == NULL) { + SCLogError("failed to alloc signature wrapper for rule ordering"); + retval = -1; + goto cleanup; + } + /* Push signature wrapper onto a list, order doesn't matter here. */ if (sig->init_data->firewall_rule) { if (sig->type == SIG_TYPE_PKT) { @@ -853,6 +860,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) /* Recreate the sig list in order */ de_ctx->sig_list = NULL; +cleanup: /* firewall list for hook packet_filter */ for (sigw = fw_pf_sigw_list; sigw != NULL;) { SCLogDebug("post-sort packet_filter: sid %u", sigw->sig->id); @@ -903,6 +911,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) sigw = sigw->next; SCFree(sigw_to_free); } + return retval; } /** @@ -1061,7 +1070,7 @@ static int SCSigOrderingTest02(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1200,7 +1209,7 @@ static int SCSigOrderingTest03(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1315,7 +1324,7 @@ static int SCSigOrderingTest04(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1413,7 +1422,7 @@ static int SCSigOrderingTest05(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1502,7 +1511,7 @@ static int SCSigOrderingTest06(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1589,7 +1598,7 @@ static int SCSigOrderingTest07(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1689,7 +1698,7 @@ static int SCSigOrderingTest08(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1795,7 +1804,7 @@ static int SCSigOrderingTest09(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1899,7 +1908,7 @@ static int SCSigOrderingTest10(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -1967,7 +1976,7 @@ static int SCSigOrderingTest11(void) SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPktvarCompare); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByPriorityCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); sig = de_ctx->sig_list; @@ -2058,7 +2067,7 @@ static int SCSigOrderingTest13(void) FAIL_IF_NULL(sig); SCSigRegisterSignatureOrderingFunc(de_ctx, SCSigOrderByFlowbitsCompare); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); #ifdef DEBUG sig = de_ctx->sig_list; diff --git a/src/detect-engine-sigorder.h b/src/detect-engine-sigorder.h index d859846c629e..358aa275c1a2 100644 --- a/src/detect-engine-sigorder.h +++ b/src/detect-engine-sigorder.h @@ -24,7 +24,7 @@ #ifndef SURICATA_DETECT_ENGINE_SIGORDER_H #define SURICATA_DETECT_ENGINE_SIGORDER_H -void SCSigOrderSignatures(DetectEngineCtx *); +int WARN_UNUSED SCSigOrderSignatures(DetectEngineCtx *); void SCSigRegisterSignatureOrderingFuncs(DetectEngineCtx *); void SCSigRegisterSignatureOrderingTests(void); void SCSigSignatureOrderingModuleCleanup(DetectEngineCtx *); diff --git a/src/detect-flowint.c b/src/detect-flowint.c index d83a9529f570..efc105468349 100644 --- a/src/detect-flowint.c +++ b/src/detect-flowint.c @@ -1131,7 +1131,7 @@ static int DetectFlowintTestPacket01Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1207,7 +1207,7 @@ static int DetectFlowintTestPacket02Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 5) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); @@ -1280,7 +1280,7 @@ static int DetectFlowintTestPacket03Real(void) FAIL_IF(UTHAppendSigs(de_ctx, sigs, 3) == 0); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + FAIL_IF(SCSigOrderSignatures(de_ctx) != 0); SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v,(void *) de_ctx,(void *) &det_ctx); diff --git a/src/util-unittest-helper.c b/src/util-unittest-helper.c index 8aa92b226f65..0d26588f19a9 100644 --- a/src/util-unittest-helper.c +++ b/src/util-unittest-helper.c @@ -738,7 +738,9 @@ int UTHMatchPackets(DetectEngineCtx *de_ctx, Packet **p, int num_packets) memset(&th_v, 0, sizeof(th_v)); StatsThreadInit(&th_v.stats); SCSigRegisterSignatureOrderingFuncs(de_ctx); - SCSigOrderSignatures(de_ctx); + if (SCSigOrderSignatures(de_ctx) != 0) { + result = 0; + } SCSigSignatureOrderingModuleCleanup(de_ctx); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); From 4afb8be6d6e9cb4e4ecf628bd88eed217ffba316 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 13:54:55 +0200 Subject: [PATCH 07/16] detect/flowvar: help gcc -fanalyzer Add debug validation statement to assert prev pointer is not NULL. --- src/detect-engine.c | 8 +++++--- src/detect-flowvar.c | 20 +++++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/detect-engine.c b/src/detect-engine.c index c526154027b2..6bdef074b46b 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -5206,9 +5206,11 @@ void DetectLowerSetupCallback( void SCDetectEngineRegisterRateFilterCallback(SCDetectRateFilterFunc fn, void *arg) { DetectEngineCtx *de_ctx = DetectEngineGetCurrent(); - de_ctx->RateFilterCallback = fn; - de_ctx->rate_filter_callback_arg = arg; - DetectEngineDeReference(&de_ctx); + if (de_ctx) { + de_ctx->RateFilterCallback = fn; + de_ctx->rate_filter_callback_arg = arg; + DetectEngineDeReference(&de_ctx); + } } int DetectEngineThreadCtxGetJsonContext(DetectEngineThreadCtx *det_ctx) diff --git a/src/detect-flowvar.c b/src/detect-flowvar.c index d5396742456f..e1e2397afa02 100644 --- a/src/detect-flowvar.c +++ b/src/detect-flowvar.c @@ -287,17 +287,16 @@ static int DetectFlowvarPostMatch( DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s, const SigMatchCtx *ctx) { - DetectVarList *fs, *prev; - const DetectFlowvarData *fd; - if (det_ctx->varlist == NULL) return 1; - fd = (const DetectFlowvarData *)ctx; + const DetectFlowvarData *fd = (const DetectFlowvarData *)ctx; + DetectVarList *prev = NULL; - prev = NULL; - fs = det_ctx->varlist; + DetectVarList *fs = det_ctx->varlist; while (fs != NULL) { + DetectVarList *next_fs = fs->next; + if (fd->idx == 0 || fd->idx == fs->idx) { SCLogDebug("adding to the flow %u:", fs->idx); //PrintRawDataFp(stdout, fs->buffer, fs->len); @@ -323,17 +322,16 @@ static int DetectFlowvarPostMatch( } if (fs == det_ctx->varlist) { - det_ctx->varlist = fs->next; SCFree(fs); - fs = det_ctx->varlist; + det_ctx->varlist = fs = next_fs; } else { - prev->next = fs->next; + DEBUG_VALIDATE_BUG_ON(prev == NULL); SCFree(fs); - fs = prev->next; + fs = prev->next = next_fs; } } else { prev = fs; - fs = fs->next; + fs = next_fs; } } return 1; From f84628251d327ccf58ebc868f808096a51d581b4 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 3 Jun 2026 14:51:54 +0200 Subject: [PATCH 08/16] detect/ip_proto: clean up parsing function Helps address a gcc -fanalyzer warning. --- src/detect-ipproto.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/detect-ipproto.c b/src/detect-ipproto.c index 7fc05b15e826..22b5624f346b 100644 --- a/src/detect-ipproto.c +++ b/src/detect-ipproto.c @@ -83,13 +83,6 @@ void DetectIPProtoRegister(void) */ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) { - DetectIPProtoData *data = NULL; - char *args[2] = { NULL, NULL }; - int res = 0; - size_t pcre2_len; - int i; - const char *str_ptr; - /* Execute the regex and populate args with captures. */ pcre2_match_data *match = NULL; int ret = DetectParsePcreExec(&parse_regex, &match, optstr, 0, 0); @@ -97,11 +90,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) SCLogError("pcre_exec parse error, ret" "%" PRId32 ", string %s", ret, optstr); - goto error; + if (match) { + pcre2_match_data_free(match); + } + return NULL; } - for (i = 0; i < (ret - 1); i++) { - res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); + char *args[2] = { NULL, NULL }; + DetectIPProtoData *data = NULL; + + for (int i = 0; i < 2; i++) { + const char *str_ptr = NULL; + size_t pcre2_len = 0; + int res = pcre2_substring_get_bynumber(match, i + 1, (PCRE2_UCHAR8 **)&str_ptr, &pcre2_len); if (res < 0) { SCLogError("pcre2_substring_get_bynumber failed"); goto error; @@ -110,7 +111,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) } /* Initialize the data */ - data = SCMalloc(sizeof(DetectIPProtoData)); + data = SCCalloc(1, sizeof(DetectIPProtoData)); if (unlikely(data == NULL)) goto error; data->op = DETECT_IPPROTO_OP_EQ; @@ -125,19 +126,19 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (!isdigit((unsigned char)*(args[1]))) { uint8_t proto; if (!SCGetProtoByName(args[1], &proto)) { - SCLogError("Unknown protocol name: \"%s\"", str_ptr); + SCLogError("Unknown protocol name: \"%s\"", args[1]); goto error; } data->proto = proto; } else { if (StringParseUint8(&data->proto, 10, 0, args[1]) <= 0) { - SCLogError("Malformed protocol number: %s", str_ptr); + SCLogError("Malformed protocol number: %s", args[1]); goto error; } } - for (i = 0; i < (ret - 1); i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } @@ -149,7 +150,7 @@ static DetectIPProtoData *DetectIPProtoParse(const char *optstr) if (match) { pcre2_match_data_free(match); } - for (i = 0; i < (ret - 1) && i < 2; i++){ + for (int i = 0; i < 2; i++) { if (args[i] != NULL) pcre2_substring_free((PCRE2_UCHAR8 *)args[i]); } From 7c01f8433c74fc628bb6f69dd3d18ddf97866697 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:05:46 +0200 Subject: [PATCH 09/16] log-pcap: address gcc analyzer warnings Reopen file descriptor for lz4 with the init function. This helps code analyzers understand the handle it's leaked. Improve flow of profiling dumps to avoid analyzer confusion around the file descriptor. Suppress TAILQ related warnings. --- src/log-pcap.c | 66 ++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/log-pcap.c b/src/log-pcap.c index 077fd9f6a481..7d6f3ccd018d 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -129,11 +129,11 @@ typedef struct PcapLogCompressionData_ { #ifdef HAVE_LIBLZ4 LZ4F_compressionContext_t lz4f_context; LZ4F_preferences_t lz4f_prefs; + FILE *pcap_buf_wrapper; #endif /* HAVE_LIBLZ4 */ FILE *file; uint8_t *pcap_buf; uint64_t pcap_buf_size; - FILE *pcap_buf_wrapper; uint64_t bytes_in_block; } PcapLogCompressionData; @@ -277,15 +277,7 @@ static int PcapLogCloseFile(ThreadVars *t, PcapLogData *pl) #ifdef HAVE_LIBLZ4 PcapLogCompressionData *comp = &pl->compression; if (comp->format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { - /* pcap_dump_close() has closed its output ``file'', - * so we need to call fmemopen again. */ - - comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, - comp->pcap_buf_size, "w"); - if (comp->pcap_buf_wrapper == NULL) { - SCLogError("SCFmemopen failed: %s", strerror(errno)); - return TM_ECODE_FAILED; - } + comp->pcap_buf_wrapper = NULL; } #endif /* HAVE_LIBLZ4 */ } @@ -367,6 +359,7 @@ static int PcapLogRotateFile(ThreadVars *t, PcapLogData *pl) } TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); pl->file_cnt--; } @@ -442,6 +435,12 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p) pl->fopen_err = 0; } + comp->pcap_buf_wrapper = SCFmemopen(comp->pcap_buf, comp->pcap_buf_size, "w"); + if (comp->pcap_buf_wrapper == NULL) { + fclose(comp->file); + comp->file = NULL; + return TM_ECODE_FAILED; + } if ((pl->pcap_dumper = pcap_dump_fopen(pl->pcap_dead_handle, comp->pcap_buf_wrapper)) == NULL) { if (!pl->pcap_open_err) { @@ -1015,6 +1014,7 @@ static TmEcode PcapLogInitRingBuffer(PcapLogData *pl) PcapFileName *pf = TAILQ_FIRST(&pl->pcap_file_list); while (pf != NULL && pl->file_cnt > pl->max_files) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); SCLogDebug("Removing PCAP file %s", pf->filename); if (remove(pf->filename) != 0) { @@ -1166,6 +1166,7 @@ static void PcapLogDataFree(PcapLogData *pl) PcapFileName *pf; while ((pf = TAILQ_FIRST(&pl->pcap_file_list)) != NULL) { TAILQ_REMOVE(&pl->pcap_file_list, pf, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&pl->pcap_file_list) == pf); PcapFileNameFree(pf); } if (pl == g_pcap_data) { @@ -1191,7 +1192,8 @@ static void PcapLogDataFree(PcapLogData *pl) #ifdef HAVE_LIBLZ4 if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { SCFree(pl->compression.buffer); - fclose(pl->compression.pcap_buf_wrapper); + if (pl->compression.pcap_buf_wrapper) + fclose(pl->compression.pcap_buf_wrapper); SCFree(pl->compression.pcap_buf); LZ4F_errorCode_t errcode = LZ4F_freeCompressionContext(pl->compression.lz4f_context); @@ -1217,7 +1219,7 @@ static TmEcode PcapLogDataDeinit(ThreadVars *t, void *thread_data) PcapLogData *pl = td->pcap_log; if (pl->pcap_dumper != NULL) { - if (PcapLogCloseFile(t,pl) < 0) { + if (PcapLogCloseFile(t, pl) != TM_ECODE_OK) { SCLogDebug("PcapLogCloseFile failed"); } } @@ -1482,7 +1484,9 @@ static OutputInitResult PcapLogInitCtx(SCConfNode *conf) comp->file = NULL; comp->pcap_buf = NULL; comp->pcap_buf_size = 0; +#ifdef HAVE_LIBLZ4 comp->pcap_buf_wrapper = NULL; +#endif } else if (strcmp(compression_str, "lz4") == 0) { #ifdef HAVE_LIBLZ4 pl->compression.format = PCAP_LOG_COMPRESSION_FORMAT_LZ4; @@ -1849,7 +1853,7 @@ static void FormatNumber(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fb", (float)num/1000000000UL); } -static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) +static void ProfileReportPair(FILE *fp, const char *name, const PcapLogProfileData *p) { char ticks_str[32] = "n/a"; char cnt_str[32] = "n/a"; @@ -1863,7 +1867,7 @@ static void ProfileReportPair(FILE *fp, const char *name, PcapLogProfileData *p) fprintf(fp, "%-28s %-10s %-10s %-10s\n", name, cnt_str, avg_str, ticks_str); } -static void ProfileReport(FILE *fp, PcapLogData *pl) +static void ProfileReport(FILE *fp, const PcapLogData *pl) { ProfileReportPair(fp, "open", &pl->profile_open); ProfileReportPair(fp, "close", &pl->profile_close); @@ -1886,23 +1890,8 @@ static void FormatBytes(uint64_t num, char *str, size_t size) snprintf(str, size, "%3.1fGiB", (float)num/1000000000UL); } -static void PcapLogProfilingDump(PcapLogData *pl) +static void DoDump(const PcapLogData *pl, FILE *fp) { - FILE *fp = NULL; - - if (profiling_pcaplog_enabled == 0) - return; - - if (profiling_pcaplog_output_to_file == 1) { - fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); - if (fp == NULL) { - SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); - return; - } - } else { - fp = stdout; - } - /* counters */ fprintf(fp, "\n\nOperation Cnt Avg ticks Total ticks\n"); fprintf(fp, "---------------------------- ---------- ---------- -----------\n"); @@ -1942,9 +1931,24 @@ static void PcapLogProfilingDump(PcapLogData *pl) if (ticks_per_gib > 0) FormatNumber(ticks_per_gib, ticks_per_gib_str, sizeof(ticks_per_gib_str)); fprintf(fp, " Ticks per GiB: %s\n", ticks_per_gib_str); +} - if (fp != stdout) +static void PcapLogProfilingDump(PcapLogData *pl) +{ + if (profiling_pcaplog_enabled == 0) + return; + + if (profiling_pcaplog_output_to_file == 1) { + FILE *fp = fopen(profiling_pcaplog_file_name, profiling_pcaplog_file_mode); + if (fp == NULL) { + SCLogError("failed to open %s: %s", profiling_pcaplog_file_name, strerror(errno)); + return; + } + DoDump(pl, fp); fclose(fp); + } else { + DoDump(pl, stdout); + } } void PcapLogProfileSetup(void) From 7b41afd7babe6dc6cbed7bd2e0ed288d5d67b604 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:06:03 +0200 Subject: [PATCH 10/16] output: suppress gcc analyzer warnings By teaching about TAILQ. --- src/output.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/output.c b/src/output.c index e9f76f4e79d9..cdecfe519fdb 100644 --- a/src/output.c +++ b/src/output.c @@ -656,6 +656,7 @@ void OutputDeregisterAll(void) while ((module = TAILQ_FIRST(&output_modules))) { TAILQ_REMOVE(&output_modules, module, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&output_modules) == module); SCFree(module); } SCFree(simple_json_applayer_loggers); @@ -898,6 +899,7 @@ void OutputClearActiveLoggers(void) RootLogger *logger; while ((logger = TAILQ_FIRST(&active_loggers)) != NULL) { TAILQ_REMOVE(&active_loggers, logger, entries); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&active_loggers) == logger); SCFree(logger); } } From ab65fd6c6d5d58b15b4f4b83cfa2a8a2011b0e23 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:06:23 +0200 Subject: [PATCH 11/16] affinity: gcc analyzer warnings The double strchr confused gcc -fanalyzer. --- src/util-affinity.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util-affinity.c b/src/util-affinity.c index 806310e3d941..08bc694ed919 100644 --- a/src/util-affinity.c +++ b/src/util-affinity.c @@ -229,6 +229,7 @@ int BuildCpusetWithCallback( { SCConfNode *lnode; TAILQ_FOREACH(lnode, &node->head, next) { + char *sep = NULL; uint32_t i; uint32_t a, b; uint32_t stop = 0; @@ -240,8 +241,7 @@ int BuildCpusetWithCallback( a = 0; b = max; stop = 1; - } else if (strchr(lnode->val, '-') != NULL) { - char *sep = strchr(lnode->val, '-'); + } else if ((sep = strchr(lnode->val, '-')) != NULL) { if (StringParseUint32(&a, 10, sep - lnode->val, lnode->val) <= 0) { SCLogError("%s: invalid cpu range (start invalid): \"%s\"", name, lnode->val); return -1; From b75a5c3ffd1f2717acc0039dbfbd8def80159ff7 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:33:26 +0200 Subject: [PATCH 12/16] spm/bm: match suff array to pattern size Avoids gcc -fanalyzer warning about out of bounds write to the array. --- src/util-spm-bm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util-spm-bm.c b/src/util-spm-bm.c index bfdb28600aa3..850662e42fa0 100644 --- a/src/util-spm-bm.c +++ b/src/util-spm-bm.c @@ -184,7 +184,7 @@ static void BoyerMooreSuffixes(const uint8_t *x, uint16_t m, uint16_t *suff) static int PreBmGs(const uint8_t *x, uint16_t m, uint16_t *bmGs) { int32_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixes(x, m, suff); @@ -260,7 +260,7 @@ static void BoyerMooreSuffixesNocase(const uint8_t *x, uint16_t m, static void PreBmGsNocase(const uint8_t *x, uint16_t m, uint16_t *bmGs) { uint16_t i, j; - uint16_t suff[m + 1]; + uint16_t suff[m]; BoyerMooreSuffixesNocase(x, m, suff); From b33a2e06c265a2e15eacb38df228a77d243eeef6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 10:34:51 +0200 Subject: [PATCH 13/16] util/var-name: help gcc analyzer Help it understand TAILQ. --- src/util-var-name.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util-var-name.c b/src/util-var-name.c index a24c9f4874de..f62675950f09 100644 --- a/src/util-var-name.c +++ b/src/util-var-name.c @@ -127,6 +127,7 @@ void VarNameStoreDestroy(void) while ((s = TAILQ_FIRST(&free_list))) { TAILQ_REMOVE(&free_list, s, next); + DEBUG_VALIDATE_BUG_ON(TAILQ_FIRST(&free_list) == s); HashListTableFree(s->names); HashListTableFree(s->ids); SCFree(s); From 43d5e6b59988ee2d818b4a684abcbd739231d1de Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 13:03:36 +0200 Subject: [PATCH 14/16] mpm/hs: remove useless pointer check Pointer can't be NULL, so don't check it. Helps gcc analyzer as well. --- src/util-mpm-hs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util-mpm-hs.c b/src/util-mpm-hs.c index 2d4daba19992..e0b9151df4df 100644 --- a/src/util-mpm-hs.c +++ b/src/util-mpm-hs.c @@ -775,7 +775,8 @@ int SCHSPreparePatterns(MpmConfig *mpm_conf, MpmCtx *mpm_ctx) } const char *cache_path = pd->no_cache || !mpm_conf ? NULL : mpm_conf->cache_dir_path; - if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0 && pd != NULL) { + if (PatternDatabaseGetCached(&pd, cd, cache_path) == 0) { + DEBUG_VALIDATE_BUG_ON(pd == NULL); cd = NULL; ctx->pattern_db = pd; if (PatternDatabaseGetSize(pd, &ctx->hs_db_size) != 0) { From e310df0b4b77fef3358dfaa5daa51917b3dde502 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 15:15:16 +0200 Subject: [PATCH 15/16] nfq: suppress gcc analyzer warnings --- src/source-nfq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/source-nfq.c b/src/source-nfq.c index 7f75cd171c1b..e664503b35f2 100644 --- a/src/source-nfq.c +++ b/src/source-nfq.c @@ -567,6 +567,7 @@ static int NFQCallBack(struct nfq_q_handle *qh, struct nfgenmsg *nfmsg, if (ret == -1) { #ifdef COUNTERS NFQQueueVars *q = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(q == NULL); q->errs++; q->pkts++; q->bytes += GET_PKT_LEN(p); @@ -976,6 +977,7 @@ static void NFQRecvPkt(NFQQueueVars *t, NFQThreadVars *tv) { int ret; int flag = NFQVerdictCacheLen(t) ? MSG_DONTWAIT : 0; + DEBUG_VALIDATE_BUG_ON(t == NULL); int rv = recv(t->fd, tv->data, tv->datalen, flag); if (rv < 0) { @@ -1049,6 +1051,7 @@ void ReceiveNFQThreadExitStats(ThreadVars *tv, void *data) { NFQThreadVars *ntv = (NFQThreadVars *)data; NFQQueueVars *nq = NFQGetQueue(ntv->nfq_index); + DEBUG_VALIDATE_BUG_ON(nq == NULL); #ifdef COUNTERS SCLogNotice("(%s) Treated: Pkts %" PRIu32 ", Bytes %" PRIu64 ", Errors %" PRIu32 "", tv->name, nq->pkts, nq->bytes, nq->errs); From f3fd3487cdc6cc5c2b2189b82782d2c9d1075f6e Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 4 Jun 2026 15:16:02 +0200 Subject: [PATCH 16/16] github-ci: add gcc analyzer build Make sure to not run against lua rust crate build, as it's not clean. --- .github/workflows/scan-build.yml | 60 ++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/.github/workflows/scan-build.yml b/.github/workflows/scan-build.yml index 49691d2d1934..990f0fa97a9c 100644 --- a/.github/workflows/scan-build.yml +++ b/.github/workflows/scan-build.yml @@ -165,3 +165,63 @@ jobs: name: scan-build-results path: scan-build-report/ retention-days: 5 + gcc-analyzer: + name: GCC analyzer + runs-on: ubuntu-latest + container: ubuntu:26.04 + steps: + - name: Cache scan-build + uses: actions/cache@9255dc7a253b0ccc959486e2bca901246202afeb + with: + path: ~/.cargo + key: scan-build + + - name: Install system packages + run: | + apt update + apt -y install \ + libpcre2-dev \ + build-essential \ + autoconf \ + automake \ + cargo \ + cbindgen \ + dpdk-dev \ + gcc-16 \ + git \ + libtool \ + libpcap-dev \ + libnet1-dev \ + libyaml-0-2 \ + libyaml-dev \ + libcap-ng-dev \ + libcap-ng0 \ + libmagic-dev \ + libnetfilter-log-dev \ + libnetfilter-queue-dev \ + libnetfilter-queue1 \ + libnfnetlink-dev \ + libnfnetlink0 \ + libnuma-dev \ + libhiredis-dev \ + libhyperscan-dev \ + libjansson-dev \ + libevent-dev \ + libevent-pthreads-2.1-7 \ + liblz4-dev \ + make \ + python3-yaml \ + rustc \ + software-properties-common \ + zlib1g \ + zlib1g-dev + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + - run: git config --global --add safe.directory /__w/suricata/suricata + - run: ./scripts/bundle.sh + - run: ./autogen.sh + - run: ./configure --enable-warnings --enable-dpdk --enable-nfqueue --enable-nflog --enable-debug-validation + env: + CC: gcc-16 + CFLAGS: "-fanalyzer -Werror" + SURICATA_LUA_SYS_CFLAGS: "" + - run: make