From 317f20c783e674b629ca6c35e7659dead23f81b9 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 | 52 +++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/output-filestore.c b/src/output-filestore.c index 84a68aa4e8fc..a5e2a3fa3052 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,22 @@ 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 if (FileGetMaxOpenFiles() == 0) { + close_file = true; /* max open files 0 means we immediately 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 +250,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 +266,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 942a8ee36ffd280bcce8e0a42266811ba03692d3 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 1765d7161a5d8c5004ec094972cacccbeb5895cf 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 d7762fc668bc042b958123f2cf8af98295b5be1d 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 e4ed67370080ebc78c44f2ce9de761c0fa669bf7 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 bd1b597cc88b20bf7d1d67789619928a6dc15ae1 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 9447b527393c..98012cf9d3f2 100644 --- a/src/detect-engine-sigorder.c +++ b/src/detect-engine-sigorder.c @@ -799,13 +799,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 */ @@ -816,6 +817,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) { @@ -851,6 +858,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); @@ -901,6 +909,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx) sigw = sigw->next; SCFree(sigw_to_free); } + return retval; } /** @@ -1059,7 +1068,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; @@ -1198,7 +1207,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; @@ -1313,7 +1322,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; @@ -1411,7 +1420,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; @@ -1500,7 +1509,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; @@ -1587,7 +1596,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; @@ -1687,7 +1696,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; @@ -1793,7 +1802,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; @@ -1897,7 +1906,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; @@ -1965,7 +1974,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; @@ -2056,7 +2065,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 9878c4fb09c9f9a95c68812c3713879db3c8cac3 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-flowvar.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) 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 a58e108abb53fb146d2cc0fbaeedd7b56de53deb 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 1f4abb0a4221d77a1cf8ba3d09bda82d783ff55f 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 | 68 +++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/log-pcap.c b/src/log-pcap.c index 077fd9f6a481..1468cd144780 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) { @@ -450,6 +449,8 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p) } fclose(comp->file); comp->file = NULL; + fclose(comp->pcap_buf_wrapper); + comp->pcap_buf_wrapper = NULL; return TM_ECODE_FAILED; } else { pl->pcap_open_err = false; @@ -1015,6 +1016,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 +1168,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 +1194,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 +1221,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 +1486,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 +1855,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 +1869,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 +1892,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 +1933,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 3af6e2e805dd18bd04e19cf2768e806dcbe5ef84 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 dd8ffac9277b94fd4990d88080d56cbb22d9b19e 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 cba5aec1d60a..9edd601c4f27 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 1324d3f62f5e75e04bf3cb1ca4f122b33fb7d5f4 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 2440905a9b159da1fa5540a6ea605ce9b1387f01 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 1c76fb1d4d0ef8f7b8b605f9582f552b500ef84b 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 3b77a4de44c5..a8b1e2d0ac22 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 4f02168e5bed24921635eb91bfc0db9b576dedfe 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 62481baf0838e680bf505b8faee6350c0975da76 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