Skip to content

Gcc analyzer/v1 #15559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
victorjulien wants to merge 16 commits into from
Closed

Gcc analyzer/v1 #15559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a7253f9
output/filestore: refactor file descriptor handling
victorjulien Jun 2, 2026
dfa3ba6
frames: avoid possible undefined behavior
victorjulien Jun 3, 2026
4df08c3
tm/queues: assist gcc -fanalyzer
victorjulien Jun 3, 2026
4560b7d
conf: assist gcc -fanalyzer
victorjulien Jun 3, 2026
1a510a9
decode/tcp: only set data ptr for valid option lengths
victorjulien Jun 3, 2026
290c5e2
detect/sigorder: handle allocation failure
victorjulien Jun 3, 2026
4afb8be
detect/flowvar: help gcc -fanalyzer
victorjulien Jun 3, 2026
f846282
detect/ip_proto: clean up parsing function
victorjulien Jun 3, 2026
7c01f84
log-pcap: address gcc analyzer warnings
victorjulien Jun 4, 2026
7b41afd
output: suppress gcc analyzer warnings
victorjulien Jun 4, 2026
ab65fd6
affinity: gcc analyzer warnings
victorjulien Jun 4, 2026
b75a5c3
spm/bm: match suff array to pattern size
victorjulien Jun 4, 2026
b33a2e0
util/var-name: help gcc analyzer
victorjulien Jun 4, 2026
43d5e6b
mpm/hs: remove useless pointer check
victorjulien Jun 4, 2026
e310df0
nfq: suppress gcc analyzer warnings
victorjulien Jun 4, 2026
f3fd348
github-ci: add gcc analyzer build
victorjulien Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .github/workflows/scan-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 5 additions & 4 deletions src/app-layer-frames.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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++;
Expand All @@ -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++;
Expand Down
2 changes: 2 additions & 0 deletions src/conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
19 changes: 17 additions & 2 deletions src/decode-tcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion src/detect-engine-loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 22 additions & 13 deletions src/detect-engine-sigorder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -903,6 +911,7 @@ void SCSigOrderSignatures(DetectEngineCtx *de_ctx)
sigw = sigw->next;
SCFree(sigw_to_free);
}
return retval;
}

/**
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/detect-engine-sigorder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
8 changes: 5 additions & 3 deletions src/detect-engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

@victorjulien victorjulien Jun 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will drop this after #15558 gets merged

de_ctx->RateFilterCallback = fn;
de_ctx->rate_filter_callback_arg = arg;
DetectEngineDeReference(&de_ctx);
}
}

int DetectEngineThreadCtxGetJsonContext(DetectEngineThreadCtx *det_ctx)
Expand Down
Loading
Loading