From f1b44ca8c7ed35a7c6add1c427327fef2b843046 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Thu, 23 May 2024 15:43:51 +0530 Subject: [PATCH] datasets: fix memuse to include string len 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 | 71 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/src/util-thash.c b/src/util-thash.c index 427979561836..649d8b98b845 100644 --- a/src/util-thash.c +++ b/src/util-thash.c @@ -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; }