OISF · inashivb · Jan 10, 2024 · May 13, 2024 · May 23, 2024 · May 9, 2024
diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst
@@ -57,6 +57,10 @@ Major changes
   Instead, both the SDP parser and logger depend on being invoked by another parser (or logger).
 - ARP decoder and logger have been introduced.
   Since ARP can be quite verbose and produce many events, the logger is disabled by default.
+- Datasets of the type String had a bug fix which now takes into account the length of the string
+  as well when checking for memcaps. This may lead to memcaps being hit for older setups that didn't
+  take that into account.
+  For more details, check https://redmine.openinfosecfoundation.org/issues/3910
 
 Upgrading 6.0 to 7.0
 --------------------

diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c
@@ -127,8 +127,9 @@ static void ContainerUrlRangeFree(void *s)
     }
 }
 
-static inline bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, const SCTime_t ts)
+static inline bool ContainerValueRangeTimeout(void *data, const SCTime_t ts)
 {
+    HttpRangeContainerFile *cu = data;
     // we only timeout if we have no flow referencing us
     if ((uint32_t)SCTIME_SECS(ts) > cu->expire || cu->error) {
         if (SC_ATOMIC_GET(cu->hdata->use_cnt) == 0) {
@@ -171,10 +172,10 @@ void HttpRangeContainersInit(void)
         }
     }
 
-    ContainerUrlRangeList.ht =
-            THashInit("app-layer.protocols.http.byterange", sizeof(HttpRangeContainerFile),
-                    ContainerUrlRangeSet, ContainerUrlRangeFree, ContainerUrlRangeHash,
-                    ContainerUrlRangeCompare, false, memcap, CONTAINER_URLRANGE_HASH_SIZE);
+    ContainerUrlRangeList.ht = THashInit("app-layer.protocols.http.byterange",
+            sizeof(HttpRangeContainerFile), ContainerUrlRangeSet, ContainerUrlRangeFree,
+            ContainerUrlRangeHash, ContainerUrlRangeCompare, ContainerValueRangeTimeout, NULL,
+            false, memcap, CONTAINER_URLRANGE_HASH_SIZE);
     ContainerUrlRangeList.timeout = timeout;
 
     SCLogDebug("containers started");

diff --git a/src/datasets-string.c b/src/datasets-string.c
@@ -98,6 +98,12 @@ uint32_t StringHash(void *s)
     return hash;
 }
 
+uint32_t StringGetLength(void *s)
+{
+    StringType *str = s;
+    return str->len;
+}
+
 // base data stays in hash
 void StringFree(void *s)
 {

diff --git a/src/datasets-string.h b/src/datasets-string.h
@@ -35,6 +35,7 @@ typedef struct StringType {
 int StringSet(void *dst, void *src);
 bool StringCompare(void *a, void *b);
 uint32_t StringHash(void *s);
+uint32_t StringGetLength(void *s);
 void StringFree(void *s);
 int StringAsBase64(const void *s, char *out, size_t out_size);
 

diff --git a/src/datasets.c b/src/datasets.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2017-2020 Open Information Security Foundation
+/* Copyright (C) 2017-2024 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -700,7 +700,8 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save,
     switch (type) {
         case DATASET_TYPE_MD5:
             set->hash = THashInit(cnf_name, sizeof(Md5Type), Md5StrSet, Md5StrFree, Md5StrHash,
-                    Md5StrCompare, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
+                    Md5StrCompare, NULL, NULL, load != NULL ? 1 : 0,
+                    memcap > 0 ? memcap : default_memcap,
                     hashsize > 0 ? hashsize : default_hashsize);
             if (set->hash == NULL)
                 goto out_err;
@@ -709,7 +710,8 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save,
             break;
         case DATASET_TYPE_STRING:
             set->hash = THashInit(cnf_name, sizeof(StringType), StringSet, StringFree, StringHash,
-                    StringCompare, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
+                    StringCompare, NULL, StringGetLength, load != NULL ? 1 : 0,
+                    memcap > 0 ? memcap : default_memcap,
                     hashsize > 0 ? hashsize : default_hashsize);
             if (set->hash == NULL)
                 goto out_err;
@@ -718,7 +720,7 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save,
             break;
         case DATASET_TYPE_SHA256:
             set->hash = THashInit(cnf_name, sizeof(Sha256Type), Sha256StrSet, Sha256StrFree,
-                    Sha256StrHash, Sha256StrCompare, load != NULL ? 1 : 0,
+                    Sha256StrHash, Sha256StrCompare, NULL, NULL, load != NULL ? 1 : 0,
                     memcap > 0 ? memcap : default_memcap,
                     hashsize > 0 ? hashsize : default_hashsize);
             if (set->hash == NULL)
@@ -727,18 +729,20 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save,
                 goto out_err;
             break;
         case DATASET_TYPE_IPV4:
-            set->hash = THashInit(cnf_name, sizeof(IPv4Type), IPv4Set, IPv4Free, IPv4Hash,
-                    IPv4Compare, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
-                    hashsize > 0 ? hashsize : default_hashsize);
+            set->hash =
+                    THashInit(cnf_name, sizeof(IPv4Type), IPv4Set, IPv4Free, IPv4Hash, IPv4Compare,
+                            NULL, NULL, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
+                            hashsize > 0 ? hashsize : default_hashsize);
             if (set->hash == NULL)
                 goto out_err;
             if (DatasetLoadIPv4(set) < 0)
                 goto out_err;
             break;
         case DATASET_TYPE_IPV6:
-            set->hash = THashInit(cnf_name, sizeof(IPv6Type), IPv6Set, IPv6Free, IPv6Hash,
-                    IPv6Compare, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
-                    hashsize > 0 ? hashsize : default_hashsize);
+            set->hash =
+                    THashInit(cnf_name, sizeof(IPv6Type), IPv6Set, IPv6Free, IPv6Hash, IPv6Compare,
+                            NULL, NULL, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap,
+                            hashsize > 0 ? hashsize : default_hashsize);
             if (set->hash == NULL)
                 goto out_err;
             if (DatasetLoadIPv6(set) < 0)

diff --git a/src/util-thash.c b/src/util-thash.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2016 Open Information Security Foundation
+/* Copyright (C) 2007-2024 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -189,12 +189,16 @@ static void THashDataFree(THashTableContext *ctx, THashData *h)
     if (h != NULL) {
         DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) != 0);
 
+        uint32_t len = 0;
         if (h->data != NULL) {
+            if (ctx->config.DataSize) {
+                len = ctx->config.DataSize(h->data);
+            }
             ctx->config.DataFree(h->data);
         }
         SCMutexDestroy(&h->m);
         SCFree(h);
-        (void) SC_ATOMIC_SUB(ctx->memuse, THASH_DATA_SIZE(ctx));
+        (void)SC_ATOMIC_SUB(ctx->memuse, THASH_DATA_SIZE(ctx) + (uint64_t)len);
     }
 }
 
@@ -294,7 +298,8 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix)
 
 THashTableContext *THashInit(const char *cnf_prefix, size_t data_size,
         int (*DataSet)(void *, void *), void (*DataFree)(void *), uint32_t (*DataHash)(void *),
-        bool (*DataCompare)(void *, void *), bool reset_memcap, uint64_t memcap, uint32_t hashsize)
+        bool (*DataCompare)(void *, void *), bool (*DataExpired)(void *, SCTime_t),
+        uint32_t (*DataSize)(void *), bool reset_memcap, uint64_t memcap, uint32_t hashsize)
 {
     THashTableContext *ctx = SCCalloc(1, sizeof(*ctx));
     BUG_ON(!ctx);
@@ -304,6 +309,8 @@ THashTableContext *THashInit(const char *cnf_prefix, size_t data_size,
     ctx->config.DataFree = DataFree;
     ctx->config.DataHash = DataHash;
     ctx->config.DataCompare = DataCompare;
+    ctx->config.DataExpired = DataExpired;
+    ctx->config.DataSize = DataSize;
 
     /* set defaults */
     ctx->config.hash_rand = (uint32_t)RandomGet();
@@ -371,6 +378,8 @@ void THashShutdown(THashTableContext *ctx)
     }
     (void) SC_ATOMIC_SUB(ctx->memuse, ctx->config.hash_size * sizeof(THashHashRow));
     THashDataQueueDestroy(&ctx->spare_q);
+    SCLogDebug("memuse: %ld", (uint64_t)SC_ATOMIC_GET(ctx->memuse));
+    DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(ctx->memuse) != 0);
     SCFree(ctx);
     return;
 }
@@ -408,6 +417,62 @@ int THashWalk(THashTableContext *ctx, THashFormatFunc FormatterFunc, THashOutput
     return 0;
 }
 
+/** \brief expire data from the hash
+ *  Walk the hash table and remove data that is exprired according to the
+ *  DataExpired callback.
+ *  \retval cnt number of items successfully expired/removed
+ */
+uint32_t THashExpire(THashTableContext *ctx, const SCTime_t ts)
+{
+    if (ctx->config.DataExpired == NULL)
+        return 0;
+
+    SCLogDebug("timeout: starting");
+    uint32_t cnt = 0;
+
+    for (uint32_t i = 0; i < ctx->config.hash_size; i++) {
+        THashHashRow *hb = &ctx->array[i];
+        if (HRLOCK_TRYLOCK(hb) != 0)
+            continue;
+        /* hash bucket is now locked */
+        THashData *h = hb->head;
+        while (h) {
+            THashData *next = h->next;
+            THashDataLock(h);
+            DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) > (uint32_t)INT_MAX);
+            /* only consider items with no references to it */
+            if (SC_ATOMIC_GET(h->use_cnt) == 0 && ctx->config.DataExpired(h->data, ts)) {
+                /* remove from the hash */
+                if (h->prev != NULL)
+                    h->prev->next = h->next;
+                if (h->next != NULL)
+                    h->next->prev = h->prev;
+                if (hb->head == h)
+                    hb->head = h->next;
+                if (hb->tail == h)
+                    hb->tail = h->prev;
+                h->next = NULL;
+                h->prev = NULL;
+                SCLogDebug("timeout: removing data %p", h);
+                ctx->config.DataFree(h->data);
+                if (ctx->config.DataSize) {
+                    uint32_t len = ctx->config.DataSize(h->data);
+                    (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)len);
+                }
+                THashDataUnlock(h);
+                THashDataMoveToSpare(ctx, h);
+            } else {
+                THashDataUnlock(h);
+            }
+            h = next;
+        }
+        HRLOCK_UNLOCK(hb);
+    }
+
+    SCLogDebug("timeout: ending: %u entries expired", cnt);
+    return cnt;
+}
+
 /** \brief Cleanup the thash engine
  *
  * Cleanup the thash engine from tag and threshold.
@@ -516,7 +581,10 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
 
     // setup the data
     BUG_ON(ctx->config.DataSet(h->data, data) != 0);
-
+    if (ctx->config.DataSize) {
+        uint32_t len = ctx->config.DataSize(data);
+        (void)SC_ATOMIC_ADD(ctx->memuse, (uint64_t)len);
+    }
     (void) SC_ATOMIC_ADD(ctx->counter, 1);
     SCMutexLock(&h->m);
     return h;
@@ -758,6 +826,10 @@ static THashData *THashGetUsed(THashTableContext *ctx)
 
         if (h->data != NULL) {
             ctx->config.DataFree(h->data);
+            if (ctx->config.DataSize) {
+                uint32_t len = ctx->config.DataSize(h->data);
+                (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)len);
+            }
         }
         SCMutexUnlock(&h->m);
 

diff --git a/src/util-thash.h b/src/util-thash.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2016 Open Information Security Foundation
+/* Copyright (C) 2007-2024 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -132,6 +132,8 @@ typedef struct THashDataConfig_ {
     void (*DataFree)(void *);
     uint32_t (*DataHash)(void *);
     bool (*DataCompare)(void *, void *);
+    bool (*DataExpired)(void *, SCTime_t ts);
+    uint32_t (*DataSize)(void *);
 } THashConfig;
 
 #define THASH_DATA_SIZE(ctx) (sizeof(THashData) + (ctx)->config.data_size)
@@ -169,7 +171,8 @@ typedef struct THashTableContext_ {
 
 THashTableContext *THashInit(const char *cnf_prefix, size_t data_size,
         int (*DataSet)(void *dst, void *src), void (*DataFree)(void *),
-        uint32_t (*DataHash)(void *), bool (*DataCompare)(void *, void *), bool reset_memcap,
+        uint32_t (*DataHash)(void *), bool (*DataCompare)(void *, void *),
+        bool (*DataExpired)(void *, SCTime_t), uint32_t (*DataSize)(void *), bool reset_memcap,
         uint64_t memcap, uint32_t hashsize);
 
 void THashShutdown(THashTableContext *ctx);
@@ -197,5 +200,6 @@ int THashWalk(THashTableContext *, THashFormatFunc, THashOutputFunc, void *);
 int THashRemoveFromHash (THashTableContext *ctx, void *data);
 void THashConsolidateMemcap(THashTableContext *ctx);
 void THashDataMoveToSpare(THashTableContext *ctx, THashData *h);
+uint32_t THashExpire(THashTableContext *ctx, const SCTime_t ts);
 
 #endif /* SURICATA_THASH_H */