From: Philippe Antoine Date: Wed, 14 May 2025 18:59:47 +0000 (+0200) Subject: detect/tag: timeout handling precision improvement X-Git-Tag: suricata-8.0.0-rc1~274 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ee386ac6eb9c8033b486873fb41f54282cb69aa9;p=thirdparty%2Fsuricata.git detect/tag: timeout handling precision improvement As found by -Wshorten-64-to-32 warnings Ticket: #6186 Use SCTime_t instead of u32, which increases memory usage for the structures changed here, while making it more correct. --- diff --git a/src/detect-engine-tag.c b/src/detect-engine-tag.c index 2e8984c288..cef785d98b 100644 --- a/src/detect-engine-tag.c +++ b/src/detect-engine-tag.c @@ -150,7 +150,8 @@ int TagFlowAdd(Packet *p, DetectTagDataEntry *tde) if (new_tde != NULL) { new_tde->next = FlowGetStorageById(p->flow, flow_tag_id); FlowSetStorageById(p->flow, flow_tag_id, new_tde); - SCLogDebug("adding tag with first_ts %u", new_tde->first_ts); + SCLogDebug( + "adding tag with first_ts %" PRIu64, (uint64_t)SCTIME_SECS(new_tde->first_ts)); (void) SC_ATOMIC_ADD(num_tags, 1); } } else if (tag_cnt == DETECT_TAG_MAX_TAGS) { @@ -254,7 +255,7 @@ static void TagHandlePacketFlow(Flow *f, Packet *p) while (iter != NULL) { /* update counters */ - iter->last_ts = SCTIME_SECS(p->ts); + iter->last_ts = p->ts; switch (iter->metric) { case DETECT_TAG_METRIC_PACKET: iter->packets++; @@ -329,10 +330,14 @@ static void TagHandlePacketFlow(Flow *f, Packet *p) case DETECT_TAG_METRIC_SECONDS: /* last_ts handles this metric, but also a generic time based * expiration to prevent dead sessions/hosts */ - if (iter->last_ts - iter->first_ts > iter->count) { - SCLogDebug("flow tag expired: %u - %u = %u > %u", - iter->last_ts, iter->first_ts, - (iter->last_ts - iter->first_ts), iter->count); + if (SCTIME_SECS(iter->last_ts) - SCTIME_SECS(iter->first_ts) > iter->count) { + // cast needed as gcc and clang behave differently + SCLogDebug("flow tag expired: %" PRIu64 " - %" PRIu64 " = %" PRIu64 " > %u", + (uint64_t)SCTIME_SECS(iter->last_ts), + (uint64_t)SCTIME_SECS(iter->first_ts), + (uint64_t)(SCTIME_SECS(iter->last_ts) - + SCTIME_SECS(iter->first_ts)), + iter->count); /* tag expired */ if (prev != NULL) { tde = iter; @@ -376,7 +381,7 @@ static void TagHandlePacketHost(Host *host, Packet *p) prev = NULL; while (iter != NULL) { /* update counters */ - iter->last_ts = SCTIME_SECS(p->ts); + iter->last_ts = p->ts; switch (iter->metric) { case DETECT_TAG_METRIC_PACKET: iter->packets++; @@ -448,10 +453,13 @@ static void TagHandlePacketHost(Host *host, Packet *p) case DETECT_TAG_METRIC_SECONDS: /* last_ts handles this metric, but also a generic time based * expiration to prevent dead sessions/hosts */ - if (iter->last_ts - iter->first_ts > iter->count) { - SCLogDebug("host tag expired: %u - %u = %u > %u", - iter->last_ts, iter->first_ts, - (iter->last_ts - iter->first_ts), iter->count); + if (SCTIME_SECS(iter->last_ts) - SCTIME_SECS(iter->first_ts) > iter->count) { + SCLogDebug("host tag expired: %" PRIu64 " - %" PRIu64 " = %" PRIu64 " > %u", + (uint64_t)SCTIME_SECS(iter->last_ts), + (uint64_t)SCTIME_SECS(iter->first_ts), + (uint64_t)(SCTIME_SECS(iter->last_ts) - + SCTIME_SECS(iter->first_ts)), + iter->count); /* tag expired */ if (prev != NULL) { tde = iter; @@ -568,7 +576,7 @@ int TagTimeoutCheck(Host *host, SCTime_t ts) prev = NULL; while (tmp != NULL) { - SCTime_t timeout_at = SCTIME_FROM_SECS(tmp->last_ts + TAG_MAX_LAST_TIME_SEEN); + SCTime_t timeout_at = SCTIME_ADD_SECS(tmp->last_ts, TAG_MAX_LAST_TIME_SEEN); if (SCTIME_CMP_GTE(timeout_at, ts)) { prev = tmp; tmp = tmp->next; diff --git a/src/detect-tag.c b/src/detect-tag.c index bab756b3b6..1828b1b701 100644 --- a/src/detect-tag.c +++ b/src/detect-tag.c @@ -106,7 +106,7 @@ static int DetectTagMatch(DetectEngineThreadCtx *det_ctx, Packet *p, tde.sid = s->id; tde.gid = s->gid; - tde.last_ts = tde.first_ts = SCTIME_SECS(p->ts); + tde.last_ts = tde.first_ts = p->ts; tde.metric = td->metric; tde.count = td->count; if (td->direction == DETECT_TAG_DIR_SRC) @@ -123,12 +123,12 @@ static int DetectTagMatch(DetectEngineThreadCtx *det_ctx, Packet *p, /* If it already exists it will be updated */ tde.sid = s->id; tde.gid = s->gid; - tde.last_ts = tde.first_ts = SCTIME_SECS(p->ts); + tde.last_ts = tde.first_ts = p->ts; tde.metric = td->metric; tde.count = td->count; - SCLogDebug("Adding to or updating flow; first_ts %u count %u", - tde.first_ts, tde.count); + SCLogDebug("Adding to or updating flow; first_ts %" PRIu64 " count %u", + (uint64_t)SCTIME_SECS(tde.first_ts), tde.count); TagFlowAdd(p, &tde); } else { SCLogDebug("No flow to append the session tag"); diff --git a/src/detect-tag.h b/src/detect-tag.h index fa0c3aa4aa..6a6a8bc48b 100644 --- a/src/detect-tag.h +++ b/src/detect-tag.h @@ -79,11 +79,8 @@ typedef struct DetectTagDataEntry_ { uint32_t packets; /**< number of packets (metric packets) */ uint32_t bytes; /**< number of bytes (metric bytes) */ }; - uint32_t first_ts; /**< First time seen (for metric = seconds) */ - uint32_t last_ts; /**< Last time seen (to prune old sessions) */ -#if __WORDSIZE == 64 - uint32_t pad1; -#endif + SCTime_t first_ts; /**< First time seen (for metric = seconds) */ + SCTime_t last_ts; /**< Last time seen (to prune old sessions) */ struct DetectTagDataEntry_ *next; /**< Pointer to the next tag of this * session/src_host/dst_host (if any from other rule) */ } DetectTagDataEntry;