]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
datasets: fix memuse to include string len
authorShivani Bhardwaj <shivani@oisf.net>
Thu, 23 May 2024 10:13:51 +0000 (15:43 +0530)
committerVictor Julien <victor@inliniac.net>
Thu, 4 Jul 2024 04:37:22 +0000 (06:37 +0200)
So far, when the data size was passed to the THash API, it was sent as
a sizeof(Struct) which works fine for the other data types as they have
a fixed length but not for the StringType.
However, because of the sizeof construct, the length of a string type
dataset was always taken to be 16 Bytes which is only the size of the struct
itself. It did not accomodate the actual size of the string that the
StringType holds. Fix this so that the memuse that is used to determine
whether memcap was reached also takes into consideration the size of the
actual string.

Bug 3910

src/util-thash.c

index 42797956183693dd98e06022a1365489ada81c3e..649d8b98b845ed936a1cfb0d8a5b0eabf8f34118 100644 (file)
@@ -35,7 +35,7 @@
 #include "util-hash-lookup3.h"
 #include "util-validate.h"
 
-static THashData *THashGetUsed(THashTableContext *ctx);
+static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size);
 static void THashDataEnqueue (THashDataQueue *q, THashData *h);
 
 void THashDataMoveToSpare(THashTableContext *ctx, THashData *h)
@@ -157,17 +157,19 @@ static uint32_t THashDataQueueLen(THashDataQueue *q)
 }
 #endif
 
-static THashData *THashDataAlloc(THashTableContext *ctx)
+static THashData *THashDataAlloc(THashTableContext *ctx, uint32_t data_size)
 {
-    const size_t data_size = THASH_DATA_SIZE(ctx);
+    const size_t thash_data_size = THASH_DATA_SIZE(ctx);
 
-    if (!(THASH_CHECK_MEMCAP(ctx, data_size))) {
+    if (!(THASH_CHECK_MEMCAP(ctx, thash_data_size + data_size))) {
         return NULL;
     }
 
-    (void) SC_ATOMIC_ADD(ctx->memuse, data_size);
+    size_t total_data_size = thash_data_size + data_size;
 
-    THashData *h = SCCalloc(1, data_size);
+    (void)SC_ATOMIC_ADD(ctx->memuse, total_data_size);
+
+    THashData *h = SCCalloc(1, thash_data_size);
     if (unlikely(h == NULL))
         goto error;
 
@@ -181,6 +183,7 @@ static THashData *THashDataAlloc(THashTableContext *ctx)
     return h;
 
 error:
+    (void)SC_ATOMIC_SUB(ctx->memuse, total_data_size);
     return NULL;
 }
 
@@ -189,12 +192,16 @@ static void THashDataFree(THashTableContext *ctx, THashData *h)
     if (h != NULL) {
         DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) != 0);
 
+        uint32_t data_size = 0;
         if (h->data != NULL) {
+            if (ctx->config.DataSize) {
+                data_size = 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)data_size);
     }
 }
 
@@ -268,7 +275,7 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix)
     for (i = 0; i < ctx->config.hash_size; i++) {
         HRLOCK_INIT(&ctx->array[i]);
     }
-    (void) SC_ATOMIC_ADD(ctx->memuse, (ctx->config.hash_size * sizeof(THashHashRow)));
+    (void)SC_ATOMIC_ADD(ctx->memuse, (ctx->config.hash_size * sizeof(THashHashRow)));
 
     /* pre allocate prealloc */
     for (i = 0; i < ctx->config.prealloc; i++) {
@@ -281,7 +288,7 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix)
             return -1;
         }
 
-        THashData *h = THashDataAlloc(ctx);
+        THashData *h = THashDataAlloc(ctx, 0 /* as we don't have string data here */);
         if (h == NULL) {
             SCLogError("preallocating data failed: %s", strerror(errno));
             return -1;
@@ -374,6 +381,7 @@ void THashShutdown(THashTableContext *ctx)
     }
     (void) SC_ATOMIC_SUB(ctx->memuse, ctx->config.hash_size * sizeof(THashHashRow));
     THashDataQueueDestroy(&ctx->spare_q);
+    DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(ctx->memuse) != 0);
     SCFree(ctx);
 }
 
@@ -447,6 +455,11 @@ uint32_t THashExpire(THashTableContext *ctx, const SCTime_t ts)
                 h->next = NULL;
                 h->prev = NULL;
                 SCLogDebug("timeout: removing data %p", h);
+                if (ctx->config.DataSize) {
+                    uint32_t data_size = ctx->config.DataSize(h->data);
+                    if (data_size > 0)
+                        (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)data_size);
+                }
                 ctx->config.DataFree(h->data);
                 THashDataUnlock(h);
                 THashDataMoveToSpare(ctx, h);
@@ -495,6 +508,11 @@ void THashCleanup(THashTableContext *ctx)
                     hb->tail = h->prev;
                 h->next = NULL;
                 h->prev = NULL;
+                if (ctx->config.DataSize) {
+                    uint32_t data_size = ctx->config.DataSize(h->data);
+                    if (data_size > 0)
+                        (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)data_size);
+                }
                 THashDataMoveToSpare(ctx, h);
                 h = n;
             }
@@ -537,13 +555,17 @@ static inline int THashCompare(const THashConfig *cnf, void *a, void *b)
 static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
 {
     THashData *h = NULL;
+    uint32_t data_size = 0;
+    if (ctx->config.DataSize) {
+        data_size = ctx->config.DataSize(data);
+    }
 
     /* get data from the spare queue */
     h = THashDataDequeue(&ctx->spare_q);
     if (h == NULL) {
         /* If we reached the max memcap, we get used data */
-        if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx)))) {
-            h = THashGetUsed(ctx);
+        if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx) + data_size))) {
+            h = THashGetUsed(ctx, data_size);
             if (h == NULL) {
                 return NULL;
             }
@@ -555,7 +577,7 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
             /* freed data, but it's unlocked */
         } else {
             /* now see if we can alloc a new data */
-            h = THashDataAlloc(ctx);
+            h = THashDataAlloc(ctx, data_size);
             if (h == NULL) {
                 return NULL;
             }
@@ -564,13 +586,24 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
         }
     } else {
         /* data has been recycled before it went into the spare queue */
-
         /* data is initialized (recycled) but *unlocked* */
+        /* the recycled data was THashData and again does not include
+         * the size of current data to be added */
+        if (data_size > 0) {
+            /* Since it is prealloc'd data, it already has THashData in its memuse */
+            (void)SC_ATOMIC_ADD(ctx->memuse, data_size);
+            if (!(THASH_CHECK_MEMCAP(ctx, data_size))) {
+                if (!SC_ATOMIC_GET(ctx->memcap_reached)) {
+                    SC_ATOMIC_SET(ctx->memcap_reached, true);
+                }
+                SCLogError("Adding data will exceed memcap: %" PRIu64 ", current memuse: %" PRIu64,
+                        (ctx)->config.memcap, SC_ATOMIC_GET(ctx->memuse));
+            }
+        }
     }
 
     // setup the data
     BUG_ON(ctx->config.DataSet(h->data, data) != 0);
-
     (void) SC_ATOMIC_ADD(ctx->counter, 1);
     SCMutexLock(&h->m);
     return h;
@@ -765,7 +798,7 @@ THashData *THashLookupFromHash (THashTableContext *ctx, void *data)
  *
  *  \retval h data or NULL
  */
-static THashData *THashGetUsed(THashTableContext *ctx)
+static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size)
 {
     uint32_t idx = SC_ATOMIC_GET(ctx->prune_idx) % ctx->config.hash_size;
     uint32_t cnt = ctx->config.hash_size;
@@ -811,11 +844,19 @@ static THashData *THashGetUsed(THashTableContext *ctx)
         HRLOCK_UNLOCK(hb);
 
         if (h->data != NULL) {
+            if (ctx->config.DataSize) {
+                uint32_t h_data_size = ctx->config.DataSize(h->data);
+                if (h_data_size > 0) {
+                    (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)h_data_size);
+                }
+            }
             ctx->config.DataFree(h->data);
         }
         SCMutexUnlock(&h->m);
 
         (void) SC_ATOMIC_ADD(ctx->prune_idx, (ctx->config.hash_size - cnt));
+        if (data_size > 0)
+            (void)SC_ATOMIC_ADD(ctx->memuse, data_size);
         return h;
     }