From: Victor Julien Date: Wed, 22 Sep 2021 17:26:02 +0000 (+0200) Subject: detect: use hashes for all buffer to id X-Git-Tag: suricata-7.0.0-beta1~1138 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5bcaae0a018ad3062f1023968b6340819166a039;p=thirdparty%2Fsuricata.git detect: use hashes for all buffer to id Instead of a map that is constantly realloc'd, use 2 hash tables for DetectBufferType entries: one by name (+transforms), the other by id. Use these everywhere. --- diff --git a/src/detect-engine-analyzer.c b/src/detect-engine-analyzer.c index 7ca4bf3573..562f4687b2 100644 --- a/src/detect-engine-analyzer.c +++ b/src/detect-engine-analyzer.c @@ -534,9 +534,9 @@ static void EngineAnalysisRulesPrintFP(const DetectEngineCtx *de_ctx, const Sign } fprintf(rule_engine_analysis_FD, "\" "); - if (de_ctx->buffer_type_map[list_type] && de_ctx->buffer_type_map[list_type]->transforms.cnt) { - fprintf(rule_engine_analysis_FD, "(with %d transform(s)) ", - de_ctx->buffer_type_map[list_type]->transforms.cnt); + const DetectBufferType *bt = DetectBufferTypeGetById(de_ctx, list_type); + if (bt && bt->transforms.cnt) { + fprintf(rule_engine_analysis_FD, "(with %d transform(s)) ", bt->transforms.cnt); } fprintf(rule_engine_analysis_FD, "buffer.\n"); @@ -1058,12 +1058,12 @@ error: void DumpPatterns(DetectEngineCtx *de_ctx) { - if (de_ctx->buffer_type_map_elements == 0 || de_ctx->pattern_hash_table == NULL) + if (de_ctx->pattern_hash_table == NULL) return; JsonBuilder *root_jb = jb_new_object(); - JsonBuilder *arrays[de_ctx->buffer_type_map_elements]; - memset(&arrays, 0, sizeof(JsonBuilder *) * de_ctx->buffer_type_map_elements); + JsonBuilder *arrays[de_ctx->buffer_type_id]; + memset(&arrays, 0, sizeof(JsonBuilder *) * de_ctx->buffer_type_id); jb_open_array(root_jb, "buffers"); @@ -1102,7 +1102,7 @@ void DumpPatterns(DetectEngineCtx *de_ctx) jb_close(jb); } - for (uint32_t i = 0; i < de_ctx->buffer_type_map_elements; i++) { + for (uint32_t i = 0; i < de_ctx->buffer_type_id; i++) { JsonBuilder *jb = arrays[i]; if (jb == NULL) continue; diff --git a/src/detect-engine-mpm.c b/src/detect-engine-mpm.c index 03aac1134a..5376031d19 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -1112,10 +1112,10 @@ void MpmStoreReportStats(const DetectEngineCtx *de_ctx) HashListTableBucket *htb = NULL; uint32_t stats[MPMB_MAX] = {0}; - int app_mpms_cnt = de_ctx->buffer_type_map_elements; + int app_mpms_cnt = de_ctx->buffer_type_id; uint32_t appstats[app_mpms_cnt + 1]; // +1 to silence scan-build memset(&appstats, 0x00, sizeof(appstats)); - int pkt_mpms_cnt = de_ctx->buffer_type_map_elements; + int pkt_mpms_cnt = de_ctx->buffer_type_id; uint32_t pktstats[pkt_mpms_cnt + 1]; // +1 to silence scan-build memset(&pktstats, 0x00, sizeof(pktstats)); diff --git a/src/detect-engine.c b/src/detect-engine.c index a61d141695..3828d10de5 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -690,20 +690,24 @@ int DetectBufferTypeMaxId(void) return g_buffer_type_id; } -static uint32_t DetectBufferTypeHashFunc(HashListTable *ht, void *data, uint16_t datalen) +static uint32_t DetectBufferTypeHashNameFunc(HashListTable *ht, void *data, uint16_t datalen) { const DetectBufferType *map = (DetectBufferType *)data; - uint32_t hash = 0; - - hash = hashlittle_safe(map->string, strlen(map->string), 0); + uint32_t hash = hashlittle_safe(map->string, strlen(map->string), 0); hash += hashlittle_safe((uint8_t *)&map->transforms, sizeof(map->transforms), 0); hash %= ht->array_size; + return hash; +} +static uint32_t DetectBufferTypeHashIdFunc(HashListTable *ht, void *data, uint16_t datalen) +{ + const DetectBufferType *map = (DetectBufferType *)data; + uint32_t hash = map->id; + hash %= ht->array_size; return hash; } -static char DetectBufferTypeCompareFunc(void *data1, uint16_t len1, void *data2, - uint16_t len2) +static char DetectBufferTypeCompareNameFunc(void *data1, uint16_t len1, void *data2, uint16_t len2) { DetectBufferType *map1 = (DetectBufferType *)data1; DetectBufferType *map2 = (DetectBufferType *)data2; @@ -713,6 +717,13 @@ static char DetectBufferTypeCompareFunc(void *data1, uint16_t len1, void *data2, return r; } +static char DetectBufferTypeCompareIdFunc(void *data1, uint16_t len1, void *data2, uint16_t len2) +{ + DetectBufferType *map1 = (DetectBufferType *)data1; + DetectBufferType *map2 = (DetectBufferType *)data2; + return map1->id == map2->id; +} + static void DetectBufferTypeFreeFunc(void *data) { DetectBufferType *map = (DetectBufferType *)data; @@ -740,10 +751,8 @@ static void DetectBufferTypeFreeFunc(void *data) static int DetectBufferTypeInit(void) { BUG_ON(g_buffer_type_hash); - g_buffer_type_hash = HashListTableInit(256, - DetectBufferTypeHashFunc, - DetectBufferTypeCompareFunc, - DetectBufferTypeFreeFunc); + g_buffer_type_hash = HashListTableInit(256, DetectBufferTypeHashNameFunc, + DetectBufferTypeCompareNameFunc, DetectBufferTypeFreeFunc); if (g_buffer_type_hash == NULL) return -1; @@ -835,23 +844,20 @@ int DetectBufferTypeGetByName(const char *name) return exists->id; } -const char *DetectBufferTypeGetNameById(const DetectEngineCtx *de_ctx, const int id) +const DetectBufferType *DetectBufferTypeGetById(const DetectEngineCtx *de_ctx, const int id) { - BUG_ON(id < 0 || (uint32_t)id >= de_ctx->buffer_type_map_elements); - BUG_ON(de_ctx->buffer_type_map == NULL); - - if (de_ctx->buffer_type_map[id] == NULL) - return NULL; - - return de_ctx->buffer_type_map[id]->string; + DetectBufferType lookup; + memset(&lookup, 0, sizeof(lookup)); + lookup.id = id; + const DetectBufferType *res = + HashListTableLookup(de_ctx->buffer_type_hash_id, (void *)&lookup, 0); + return res; } -static const DetectBufferType *DetectBufferTypeGetById(const DetectEngineCtx *de_ctx, const int id) +const char *DetectBufferTypeGetNameById(const DetectEngineCtx *de_ctx, const int id) { - BUG_ON(id < 0 || (uint32_t)id >= de_ctx->buffer_type_map_elements); - BUG_ON(de_ctx->buffer_type_map == NULL); - - return de_ctx->buffer_type_map[id]; + const DetectBufferType *res = DetectBufferTypeGetById(de_ctx, id); + return res ? res->string : NULL; } void DetectBufferTypeSetDescriptionByName(const char *name, const char *desc) @@ -1210,32 +1216,35 @@ static void DetectBufferTypeSetupDetectEngine(DetectEngineCtx *de_ctx) const int size = g_buffer_type_id; BUG_ON(!(size > 0)); - de_ctx->buffer_type_map = SCCalloc(size, sizeof(DetectBufferType *)); - BUG_ON(!de_ctx->buffer_type_map); - de_ctx->buffer_type_map_elements = size; - SCLogDebug("de_ctx->buffer_type_map %p with %u members", de_ctx->buffer_type_map, size); + de_ctx->buffer_type_hash_name = HashListTableInit(256, DetectBufferTypeHashNameFunc, + DetectBufferTypeCompareNameFunc, DetectBufferTypeFreeFunc); + BUG_ON(de_ctx->buffer_type_hash_name == NULL); + de_ctx->buffer_type_hash_id = + HashListTableInit(256, DetectBufferTypeHashIdFunc, DetectBufferTypeCompareIdFunc, + NULL); // entries owned by buffer_type_hash_name + BUG_ON(de_ctx->buffer_type_hash_id == NULL); + de_ctx->buffer_type_id = g_buffer_type_id; SCLogDebug("DETECT_SM_LIST_DYNAMIC_START %u", DETECT_SM_LIST_DYNAMIC_START); HashListTableBucket *b = HashListTableGetListHead(g_buffer_type_hash); while (b) { DetectBufferType *map = HashListTableGetListData(b); - de_ctx->buffer_type_map[map->id] = map; + + DetectBufferType *copy = SCCalloc(1, sizeof(*copy)); + BUG_ON(!copy); + memcpy(copy, map, sizeof(*copy)); + int r = HashListTableAdd(de_ctx->buffer_type_hash_name, (void *)copy, 0); + BUG_ON(r != 0); + r = HashListTableAdd(de_ctx->buffer_type_hash_id, (void *)copy, 0); + BUG_ON(r != 0); + SCLogDebug("name %s id %d mpm %s packet %s -- %s. " - "Callbacks: Setup %p Validate %p", map->string, map->id, - map->mpm ? "true" : "false", map->packet ? "true" : "false", + "Callbacks: Setup %p Validate %p", + map->string, map->id, map->mpm ? "true" : "false", map->packet ? "true" : "false", map->description, map->SetupCallback, map->ValidateCallback); b = HashListTableGetListNext(b); } - de_ctx->buffer_type_hash = HashListTableInit(256, - DetectBufferTypeHashFunc, - DetectBufferTypeCompareFunc, - DetectBufferTypeFreeFunc); - if (de_ctx->buffer_type_hash == NULL) { - BUG_ON(1); - } - de_ctx->buffer_type_id = g_buffer_type_id; - PrefilterInit(de_ctx); DetectMpmInitializeAppMpms(de_ctx); DetectAppLayerInspectEngineCopyListToDetectCtx(de_ctx); @@ -1246,10 +1255,10 @@ static void DetectBufferTypeSetupDetectEngine(DetectEngineCtx *de_ctx) static void DetectBufferTypeFreeDetectEngine(DetectEngineCtx *de_ctx) { if (de_ctx) { - if (de_ctx->buffer_type_map) - SCFree(de_ctx->buffer_type_map); - if (de_ctx->buffer_type_hash) - HashListTableFree(de_ctx->buffer_type_hash); + if (de_ctx->buffer_type_hash_name) + HashListTableFree(de_ctx->buffer_type_hash_name); + if (de_ctx->buffer_type_hash_id) + HashListTableFree(de_ctx->buffer_type_hash_id); DetectEngineAppInspectionEngine *ilist = de_ctx->app_inspect_engines; while (ilist) { @@ -1309,7 +1318,7 @@ int DetectBufferTypeGetByIdTransforms(DetectEngineCtx *de_ctx, const int id, t.cnt = transform_cnt; DetectBufferType lookup_map = { (char *)base_map->string, NULL, 0, 0, 0, 0, false, NULL, NULL, t }; - DetectBufferType *res = HashListTableLookup(de_ctx->buffer_type_hash, &lookup_map, 0); + DetectBufferType *res = HashListTableLookup(de_ctx->buffer_type_hash_name, &lookup_map, 0); SCLogDebug("res %p", res); if (res != NULL) { @@ -1336,24 +1345,15 @@ int DetectBufferTypeGetByIdTransforms(DetectEngineCtx *de_ctx, const int id, map->id, map->parent_id, &map->transforms); } - BUG_ON(HashListTableAdd(de_ctx->buffer_type_hash, (void *)map, 0) != 0); + BUG_ON(HashListTableAdd(de_ctx->buffer_type_hash_name, (void *)map, 0) != 0); + BUG_ON(HashListTableAdd(de_ctx->buffer_type_hash_id, (void *)map, 0) != 0); SCLogDebug("buffer %s registered with id %d, parent %d", map->string, map->id, map->parent_id); + de_ctx->buffer_type_id++; - if (map->id >= 0 && (uint32_t)map->id >= de_ctx->buffer_type_map_elements) { - void *ptr = SCRealloc(de_ctx->buffer_type_map, (map->id + 1) * sizeof(DetectBufferType *)); - BUG_ON(ptr == NULL); - SCLogDebug("de_ctx->buffer_type_map resized to %u (was %u)", (map->id + 1), de_ctx->buffer_type_map_elements); - de_ctx->buffer_type_map = ptr; - de_ctx->buffer_type_map[map->id] = map; - de_ctx->buffer_type_map_elements = map->id + 1; - - if (map->packet) { - DetectPktInspectEngineCopy(de_ctx, map->parent_id, map->id, - &map->transforms); - } else { - DetectAppLayerInspectEngineCopy(de_ctx, map->parent_id, map->id, - &map->transforms); - } + if (map->packet) { + DetectPktInspectEngineCopy(de_ctx, map->parent_id, map->id, &map->transforms); + } else { + DetectAppLayerInspectEngineCopy(de_ctx, map->parent_id, map->id, &map->transforms); } return map->id; } diff --git a/src/detect-engine.h b/src/detect-engine.h index f952669ba8..f1227579ca 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -58,6 +58,7 @@ void DetectBufferTypeRegisterSetupCallback(const char *name, void (*Callback)(const DetectEngineCtx *, Signature *)); void DetectBufferTypeRegisterValidateCallback(const char *name, bool (*ValidateCallback)(const Signature *, const char **sigerror)); +const DetectBufferType *DetectBufferTypeGetById(const DetectEngineCtx *de_ctx, const int id); int DetectBufferTypeGetByIdTransforms(DetectEngineCtx *de_ctx, const int id, TransformData *transforms, int transform_cnt); diff --git a/src/detect.h b/src/detect.h index 0a0ac44d1c..c39ab27fa2 100644 --- a/src/detect.h +++ b/src/detect.h @@ -913,13 +913,11 @@ typedef struct DetectEngineCtx_ { /** table to store metadata keys and values */ HashTable *metadata_table; - DetectBufferType **buffer_type_map; - uint32_t buffer_type_map_elements; - - /* hash table with rule-time buffer registration. Start time registration + /* hash tables with rule-time buffer registration. Start time registration * is in detect-engine.c::g_buffer_type_hash */ - HashListTable *buffer_type_hash; - int buffer_type_id; + HashListTable *buffer_type_hash_name; + HashListTable *buffer_type_hash_id; + uint32_t buffer_type_id; /* list with app inspect engines. Both the start-time registered ones and * the rule-time registered ones. */