]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/tag: timeout handling precision improvement
authorPhilippe Antoine <pantoine@oisf.net>
Wed, 14 May 2025 18:59:47 +0000 (20:59 +0200)
committerVictor Julien <victor@inliniac.net>
Fri, 16 May 2025 19:33:52 +0000 (21:33 +0200)
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.

src/detect-engine-tag.c
src/detect-tag.c
src/detect-tag.h

index 2e8984c288a52dae239f75ad366356ff60b6dc59..cef785d98b1b81c77721bbe0c97a0183ef48b48e 100644 (file)
@@ -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;
index bab756b3b601b283b7d8e2aa17b1c126c7906887..1828b1b7018076de519cebb6b9fb12eae31b0bc0 100644 (file)
@@ -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");
index fa0c3aa4aa52db78661ad6b752a2b03b52942bb5..6a6a8bc48b231f739f6c21d1f78a1ae76ac8b5e8 100644 (file)
@@ -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;