diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 4de3971c94c3..da6ec5dc2319 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -60,6 +60,10 @@ Major changes - It is possible to see an increase of alerts, for the same rule-sets, if you use many stream/payload rules, due to Suricata triggering TCP stream reassembly earlier. +- Datasets of the type String had a bug fixed 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 index f0d75a9750c0..ae37a5bb7a69 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -127,10 +127,11 @@ 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 ((uint32_t)SCTIME_SECS(ts) >= cu->expire || cu->error) { if (SC_ATOMIC_GET(cu->hdata->use_cnt) == 0) { DEBUG_VALIDATE_BUG_ON(cu->files == NULL); return true; @@ -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 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 01ef5bb47c90..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, 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 index a17679605a22..256d9eeae815 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 @@ -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,12 @@ 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))); + SCLogNotice("memuse: %" PRIu64, SC_ATOMIC_GET(ctx->memuse)); + SCLogNotice("config->data_size: %u, thash_data_size: %" PRIu64, (ctx)->config.data_size, + (uint64_t)THASH_DATA_SIZE(ctx)); + SCLogNotice("sizeof(THashHashRow): %lu", sizeof(THashHashRow)); + SCLogNotice("memcap: %" PRIu64, ctx->config.memcap); /* pre allocate prealloc */ for (i = 0; i < ctx->config.prealloc; i++) { @@ -281,7 +293,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; @@ -294,7 +306,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 +317,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(); @@ -320,6 +335,7 @@ THashTableContext *THashInit(const char *cnf_prefix, size_t data_size, ctx->config.memcap = reset_memcap ? UINT64_MAX : THASH_DEFAULT_MEMCAP; } #endif + SCLogNotice("memcap: %" PRIu64, ctx->config.memcap); ctx->config.prealloc = THASH_DEFAULT_PREALLOC; SC_ATOMIC_INIT(ctx->counter); @@ -371,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, SC_ATOMIC_GET(ctx->memuse)); + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(ctx->memuse) != 0); SCFree(ctx); } @@ -407,6 +425,64 @@ 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); + 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); + 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. @@ -439,6 +515,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; } @@ -481,13 +562,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; } @@ -499,7 +584,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; } @@ -508,13 +593,32 @@ 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 */ + SCLogNotice("memuse: %" PRIu64, SC_ATOMIC_GET(ctx->memuse)); + SCLogNotice("config->data_size: %u, thash_data_size: %" PRIu64, (ctx)->config.data_size, + (uint64_t)THASH_DATA_SIZE(ctx)); + SCLogNotice("sizeof(THashHashRow): %lu", sizeof(THashHashRow)); + SCLogNotice("data_size: %u", data_size); + SCLogNotice("memuse + data_size: %" PRIu64, SC_ATOMIC_GET(ctx->memuse) + data_size); + SCLogNotice("memcap: %" PRIu64, ctx->config.memcap); + 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, THASH_DATA_SIZE(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)); + } + } } + SCLogNotice("memuse: %" PRIu64, 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; @@ -709,7 +813,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; @@ -755,11 +859,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; } diff --git a/src/util-thash.h b/src/util-thash.h index f45b6e843167..2e38d2f2697b 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 @@ -87,7 +87,7 @@ typedef struct THashData_ { SCMutex m; /** use cnt, reference counter */ - SC_ATOMIC_DECLARE(unsigned int, use_cnt); + SC_ATOMIC_DECLARE(uint32_t, use_cnt); void *data; @@ -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 */