From 65a08e1ffbc7b01e6e8b85136a71b657aa113def Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 10 Jan 2024 09:49:10 +0100 Subject: [PATCH 1/5] thash: add expiration logic Add a callback and helper function to handle data expiration. Update datasets to explicitly not use expiration. --- src/app-layer-htp-range.c | 11 ++++---- src/datasets.c | 10 +++---- src/util-thash.c | 59 +++++++++++++++++++++++++++++++++++++-- src/util-thash.h | 7 +++-- 4 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index f0d75a9750c0..009196e61c00 100644 --- 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, false, + memcap, CONTAINER_URLRANGE_HASH_SIZE); ContainerUrlRangeList.timeout = timeout; SCLogDebug("containers started"); diff --git a/src/datasets.c b/src/datasets.c index 01ef5bb47c90..8b067b796b34 100644 --- a/src/datasets.c +++ b/src/datasets.c @@ -700,7 +700,7 @@ 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, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap, hashsize > 0 ? hashsize : default_hashsize); if (set->hash == NULL) goto out_err; @@ -709,7 +709,7 @@ 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, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap, hashsize > 0 ? hashsize : default_hashsize); if (set->hash == NULL) goto out_err; @@ -718,7 +718,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, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap, hashsize > 0 ? hashsize : default_hashsize); if (set->hash == NULL) @@ -728,7 +728,7 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save, 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, + IPv4Compare, NULL, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap, hashsize > 0 ? hashsize : default_hashsize); if (set->hash == NULL) goto out_err; @@ -737,7 +737,7 @@ Dataset *DatasetGet(const char *name, enum DatasetTypes type, const char *save, 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, + IPv6Compare, NULL, load != NULL ? 1 : 0, memcap > 0 ? memcap : default_memcap, hashsize > 0 ? hashsize : default_hashsize); if (set->hash == NULL) goto out_err; diff --git a/src/util-thash.c b/src/util-thash.c index 6443990bc219..f4b8ef179514 100644 --- 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 @@ -294,7 +294,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), + bool reset_memcap, uint64_t memcap, uint32_t hashsize) { THashTableContext *ctx = SCCalloc(1, sizeof(*ctx)); BUG_ON(!ctx); @@ -304,6 +305,7 @@ 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; /* set defaults */ ctx->config.hash_rand = (uint32_t)RandomGet(); @@ -408,6 +410,59 @@ 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); + THashDataUnlock(h); + THashDataMoveToSpare(ctx, h); + cnt++; + } 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. diff --git a/src/util-thash.h b/src/util-thash.h index f45b6e843167..569f1ff9c795 100644 --- a/src/util-thash.h +++ b/src/util-thash.h @@ -132,6 +132,7 @@ typedef struct THashDataConfig_ { void (*DataFree)(void *); uint32_t (*DataHash)(void *); bool (*DataCompare)(void *, void *); + bool (*DataExpired)(void *, SCTime_t ts); } THashConfig; #define THASH_DATA_SIZE(ctx) (sizeof(THashData) + (ctx)->config.data_size) @@ -169,8 +170,9 @@ 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, - uint64_t memcap, uint32_t hashsize); + uint32_t (*DataHash)(void *), bool (*DataCompare)(void *, void *), + bool (*DataExpired)(void *, SCTime_t), bool reset_memcap, uint64_t memcap, + uint32_t hashsize); void THashShutdown(THashTableContext *ctx); @@ -197,5 +199,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 */ From 661a545800449c0e2c7d6be23315fb8fbf52c893 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Mon, 13 May 2024 16:05:19 +0530 Subject: [PATCH 2/5] util/thash: add a length getter fn In order to have access to the length of datatypes with variable lengths to correctly update memuse to calculate memcaps. Bug 3910 --- src/app-layer-htp-range.c | 4 ++-- src/datasets-string.c | 6 ++++++ src/datasets-string.h | 1 + src/datasets.c | 24 ++++++++++++++---------- src/util-thash.c | 3 ++- src/util-thash.h | 7 ++++--- 6 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index 009196e61c00..b1401c69fc26 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -174,8 +174,8 @@ void HttpRangeContainersInit(void) ContainerUrlRangeList.ht = THashInit("app-layer.protocols.http.byterange", sizeof(HttpRangeContainerFile), ContainerUrlRangeSet, ContainerUrlRangeFree, - ContainerUrlRangeHash, ContainerUrlRangeCompare, ContainerValueRangeTimeout, false, - memcap, CONTAINER_URLRANGE_HASH_SIZE); + 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 index 4a572898ceb3..91e44bfb2a9b 100644 --- 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 index b9c3c3002454..1d5463cd9c0a 100644 --- 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 index 8b067b796b34..a1534f90c136 100644 --- 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, NULL, 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, NULL, 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, NULL, 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, NULL, 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, NULL, 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 index f4b8ef179514..676e570f9c2c 100644 --- a/src/util-thash.c +++ b/src/util-thash.c @@ -295,7 +295,7 @@ 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 (*DataExpired)(void *, SCTime_t), - bool reset_memcap, uint64_t memcap, uint32_t hashsize) + uint32_t (*DataSize)(void *), bool reset_memcap, uint64_t memcap, uint32_t hashsize) { THashTableContext *ctx = SCCalloc(1, sizeof(*ctx)); BUG_ON(!ctx); @@ -306,6 +306,7 @@ THashTableContext *THashInit(const char *cnf_prefix, size_t data_size, 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(); diff --git a/src/util-thash.h b/src/util-thash.h index 569f1ff9c795..346c528a292a 100644 --- 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 @@ -133,6 +133,7 @@ typedef struct THashDataConfig_ { 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) @@ -171,8 +172,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 (*DataExpired)(void *, SCTime_t), bool reset_memcap, uint64_t memcap, - uint32_t hashsize); + bool (*DataExpired)(void *, SCTime_t), uint32_t (*DataSize)(void *), bool reset_memcap, + uint64_t memcap, uint32_t hashsize); void THashShutdown(THashTableContext *ctx); From d4a4a09dc49771ba16063055af68246441d9b11d Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Thu, 23 May 2024 15:43:51 +0530 Subject: [PATCH 3/5] 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 | 49 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/src/util-thash.c b/src/util-thash.c index 676e570f9c2c..14c1664dcb7c 100644 --- a/src/util-thash.c +++ b/src/util-thash.c @@ -157,15 +157,18 @@ static uint32_t THashDataQueueLen(THashDataQueue *q) } #endif -static THashData *THashDataAlloc(THashTableContext *ctx) +static THashData *THashDataAlloc(THashTableContext *ctx, uint32_t len) { const size_t data_size = THASH_DATA_SIZE(ctx); - if (!(THASH_CHECK_MEMCAP(ctx, data_size))) { + if (!(THASH_CHECK_MEMCAP(ctx, data_size + len))) { return NULL; } - (void) SC_ATOMIC_ADD(ctx->memuse, data_size); + size_t data_len = data_size + len; + + (void)SC_ATOMIC_ADD(ctx->memuse, data_len); + SCLogNotice("ADD memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); THashData *h = SCCalloc(1, data_size); if (unlikely(h == NULL)) @@ -173,14 +176,18 @@ static THashData *THashDataAlloc(THashTableContext *ctx) /* points to data right after THashData block */ h->data = (uint8_t *)h + sizeof(THashData); + // NOTE that using above h->data does not work as it has not been + // populated yet -// memset(h, 0x00, data_size); + // memset(h, 0x00, data_size); SCMutexInit(&h->m, NULL); SC_ATOMIC_INIT(h->use_cnt); return h; error: + (void)SC_ATOMIC_SUB(ctx->memuse, data_len); + SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); return NULL; } @@ -189,12 +196,17 @@ 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); + SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); } } @@ -269,6 +281,7 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix) HRLOCK_INIT(&ctx->array[i]); } (void) SC_ATOMIC_ADD(ctx->memuse, (ctx->config.hash_size * sizeof(THashHashRow))); + SCLogNotice("ADD memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); /* pre allocate prealloc */ for (i = 0; i < ctx->config.prealloc; i++) { @@ -281,7 +294,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 +387,8 @@ void THashShutdown(THashTableContext *ctx) } (void) SC_ATOMIC_SUB(ctx->memuse, ctx->config.hash_size * sizeof(THashHashRow)); THashDataQueueDestroy(&ctx->spare_q); + SCLogNotice("FINAL memuse: %" PRIu64, (uint64_t)SC_ATOMIC_GET(ctx->memuse)); + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(ctx->memuse) != 0); SCFree(ctx); return; } @@ -449,6 +464,12 @@ uint32_t THashExpire(THashTableContext *ctx, const SCTime_t ts) 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); + if (len > 0) + (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)len); + SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); + } THashDataUnlock(h); THashDataMoveToSpare(ctx, h); cnt++; @@ -539,12 +560,16 @@ static inline int THashCompare(const THashConfig *cnf, void *a, void *b) static THashData *THashDataGetNew(THashTableContext *ctx, void *data) { THashData *h = NULL; + uint32_t len = 0; + if (ctx->config.DataSize) { + len = 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)))) { + if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx) + len))) { h = THashGetUsed(ctx); if (h == NULL) { return NULL; @@ -557,7 +582,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, len); if (h == NULL) { return NULL; } @@ -572,7 +597,7 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data) // setup the data BUG_ON(ctx->config.DataSet(h->data, data) != 0); - + // NOTE that h->data is malloc'd in the above call (void) SC_ATOMIC_ADD(ctx->counter, 1); SCMutexLock(&h->m); return h; @@ -814,6 +839,12 @@ 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); + if (len > 0) + (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)len); + SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); + } } SCMutexUnlock(&h->m); From 996a102a6276d50105e8dc26d62ee1b3c6d2fcb9 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Thu, 9 May 2024 21:21:25 +0530 Subject: [PATCH 4/5] doc: add note about datasets string memcaps Bug 3910 --- doc/userguide/upgrade.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 4af0ad1c727f..4554ac000a00 100644 --- 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 -------------------- From e2b2b95a3d3715104413c420a74dca94e6e59938 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Thu, 30 May 2024 16:14:38 +0530 Subject: [PATCH 5/5] fix memuse update for reused hash obj --- src/util-thash.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/util-thash.c b/src/util-thash.c index 14c1664dcb7c..71d20146651b 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, size_t data_size); static void THashDataEnqueue (THashDataQueue *q, THashData *h); void THashDataMoveToSpare(THashTableContext *ctx, THashData *h) @@ -570,7 +570,7 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data) if (h == NULL) { /* If we reached the max memcap, we get used data */ if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx) + len))) { - h = THashGetUsed(ctx); + h = THashGetUsed(ctx, THASH_DATA_SIZE(ctx) + len); if (h == NULL) { return NULL; } @@ -792,7 +792,7 @@ THashData *THashLookupFromHash (THashTableContext *ctx, void *data) * * \retval h data or NULL */ -static THashData *THashGetUsed(THashTableContext *ctx) +static THashData *THashGetUsed(THashTableContext *ctx, size_t data_size) { uint32_t idx = SC_ATOMIC_GET(ctx->prune_idx) % ctx->config.hash_size; uint32_t cnt = ctx->config.hash_size; @@ -838,17 +838,18 @@ static THashData *THashGetUsed(THashTableContext *ctx) HRLOCK_UNLOCK(hb); if (h->data != NULL) { + size_t data_len = THASH_DATA_SIZE(ctx); ctx->config.DataFree(h->data); if (ctx->config.DataSize) { - uint32_t len = ctx->config.DataSize(h->data); - if (len > 0) - (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)len); - SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); + data_len += ctx->config.DataSize(h->data); } + (void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)data_len); + SCLogNotice("SUB memuse: %ld", SC_ATOMIC_GET(ctx->memuse)); } SCMutexUnlock(&h->m); (void) SC_ATOMIC_ADD(ctx->prune_idx, (ctx->config.hash_size - cnt)); + (void)SC_ATOMIC_ADD(ctx->memuse, data_size); return h; }