diff --git a/src/detect-engine-build.c b/src/detect-engine-build.c index b5d10ba39d..aa12ab56a8 100644 --- a/src/detect-engine-build.c +++ b/src/detect-engine-build.c @@ -2035,7 +2035,7 @@ int SigGroupBuild(DetectEngineCtx *de_ctx) ThresholdHashAllocate(de_ctx); if (!DetectEngineMultiTenantEnabled()) { - VarNameStoreActivateStaging(); + VarNameStoreActivate(); } return 0; } diff --git a/src/detect-engine.c b/src/detect-engine.c index 4ec6d9a284..06e21ec1bc 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -2530,7 +2530,6 @@ static DetectEngineCtx *DetectEngineCtxInitReal(enum DetectEngineType type, cons } de_ctx->version = DetectEngineGetVersion(); - VarNameStoreSetupStaging(de_ctx->version); SCLogDebug("dectx with version %u", de_ctx->version); return de_ctx; error: @@ -2658,8 +2657,6 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) DetectPortCleanupList(de_ctx, de_ctx->udp_whitelist); DetectBufferTypeFreeDetectEngine(de_ctx); - /* freed our var name hash */ - VarNameStoreFree(de_ctx->version); SCClassConfDeinit(de_ctx); SCReferenceConfDeinit(de_ctx); @@ -4277,7 +4274,7 @@ int DetectEngineMultiTenantSetup(const bool unix_socket) goto error; } - VarNameStoreActivateStaging(); + VarNameStoreActivate(); } else { SCLogDebug("multi-detect not enabled (multi tenancy)"); diff --git a/src/detect-flowbits.c b/src/detect-flowbits.c index 0450d16e60..144eb89f88 100644 --- a/src/detect-flowbits.c +++ b/src/detect-flowbits.c @@ -121,7 +121,7 @@ static int FlowbitOrAddData(DetectEngineCtx *de_ctx, DetectFlowbitsData *cd, cha if (unlikely(cd->or_list == NULL)) return -1; for (uint8_t j = 0; j < cd->or_list_size ; j++) { - cd->or_list[j] = VarNameStoreSetupAdd(strarr[j], VAR_TYPE_FLOW_BIT); + cd->or_list[j] = VarNameStoreRegister(strarr[j], VAR_TYPE_FLOW_BIT); de_ctx->max_fb_id = MAX(cd->or_list[j], de_ctx->max_fb_id); } @@ -329,7 +329,7 @@ int DetectFlowbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawst } cd->cmd = fb_cmd; } else { - cd->idx = VarNameStoreSetupAdd(fb_name, VAR_TYPE_FLOW_BIT); + cd->idx = VarNameStoreRegister(fb_name, VAR_TYPE_FLOW_BIT); de_ctx->max_fb_id = MAX(cd->idx, de_ctx->max_fb_id); cd->cmd = fb_cmd; cd->or_list_size = 0; @@ -383,8 +383,13 @@ void DetectFlowbitFree (DetectEngineCtx *de_ctx, void *ptr) DetectFlowbitsData *fd = (DetectFlowbitsData *)ptr; if (fd == NULL) return; - if (fd->or_list != NULL) + VarNameStoreUnregister(fd->idx, VAR_TYPE_FLOW_BIT); + if (fd->or_list != NULL) { + for (uint8_t i = 0; i < fd->or_list_size; i++) { + VarNameStoreUnregister(fd->or_list[i], VAR_TYPE_FLOW_BIT); + } SCFree(fd->or_list); + } SCFree(fd); } @@ -581,7 +586,7 @@ int DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx) /* walk array to see if all bits make sense */ for (uint32_t i = 0; i < array_size; i++) { - char *varname = VarNameStoreSetupLookup(i, VAR_TYPE_FLOW_BIT); + const char *varname = VarNameStoreSetupLookup(i, VAR_TYPE_FLOW_BIT); if (varname == NULL) continue; @@ -634,7 +639,6 @@ int DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx) "stateful rules that set flowbit %s", s->id, varname); } } - SCFree(varname); } if (rule_engine_analysis_set) { @@ -664,7 +668,7 @@ static void DetectFlowbitsAnalyzeDump(const DetectEngineCtx *de_ctx, jb_open_array(js, "flowbits"); for (uint32_t x = 0; x < elements; x++) { - char *varname = VarNameStoreSetupLookup(x, VAR_TYPE_FLOW_BIT); + const char *varname = VarNameStoreSetupLookup(x, VAR_TYPE_FLOW_BIT); if (varname == NULL) continue; @@ -724,7 +728,6 @@ static void DetectFlowbitsAnalyzeDump(const DetectEngineCtx *de_ctx, } jb_close(js); } - SCFree(varname); jb_close(js); } jb_close(js); // array @@ -903,8 +906,8 @@ static int FlowBitsTestSig04(void) s = de_ctx->sig_list = SigInit(de_ctx,"alert ip any any -> any any (msg:\"isset option\"; flowbits:isset,fbt; content:\"GET \"; sid:1;)"); FAIL_IF_NULL(s); - idx = VarNameStoreSetupAdd("fbt", VAR_TYPE_FLOW_BIT); - FAIL_IF(idx != 1); + idx = VarNameStoreRegister("fbt", VAR_TYPE_FLOW_BIT); + FAIL_IF(idx == 0); SigGroupBuild(de_ctx); DetectEngineCtxFree(de_ctx); @@ -986,7 +989,7 @@ static int FlowBitsTestSig06(void) s = de_ctx->sig_list = SigInit(de_ctx,"alert ip any any -> any any (msg:\"Flowbit set\"; flowbits:set,myflow; sid:10;)"); FAIL_IF_NULL(s); - idx = VarNameStoreSetupAdd("myflow", VAR_TYPE_FLOW_BIT); + idx = VarNameStoreRegister("myflow", VAR_TYPE_FLOW_BIT); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -1060,7 +1063,7 @@ static int FlowBitsTestSig07(void) s = s->next = SigInit(de_ctx,"alert ip any any -> any any (msg:\"Flowbit unset\"; flowbits:unset,myflow2; sid:11;)"); FAIL_IF_NULL(s); - idx = VarNameStoreSetupAdd("myflow", VAR_TYPE_FLOW_BIT); + idx = VarNameStoreRegister("myflow", VAR_TYPE_FLOW_BIT); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); @@ -1136,7 +1139,7 @@ static int FlowBitsTestSig08(void) s = s->next = SigInit(de_ctx,"alert ip any any -> any any (msg:\"Flowbit unset\"; flowbits:toggle,myflow2; sid:11;)"); FAIL_IF_NULL(s); - idx = VarNameStoreSetupAdd("myflow", VAR_TYPE_FLOW_BIT); + idx = VarNameStoreRegister("myflow", VAR_TYPE_FLOW_BIT); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); diff --git a/src/detect-flowint.c b/src/detect-flowint.c index facf1c8ad7..6a28e980ce 100644 --- a/src/detect-flowint.c +++ b/src/detect-flowint.c @@ -331,7 +331,7 @@ static DetectFlowintData *DetectFlowintParse(DetectEngineCtx *de_ctx, const char SCLogError("malloc from strdup failed"); goto error; } - sfd->idx = VarNameStoreSetupAdd(varname, VAR_TYPE_FLOW_INT); + sfd->idx = VarNameStoreRegister(varname, VAR_TYPE_FLOW_INT); SCLogDebug("sfd->name %s id %u", sfd->name, sfd->idx); sfd->modifier = modifier; @@ -422,6 +422,7 @@ void DetectFlowintFree(DetectEngineCtx *de_ctx, void *tmp) { DetectFlowintData *sfd =(DetectFlowintData*) tmp; if (sfd != NULL) { + VarNameStoreUnregister(sfd->idx, VAR_TYPE_FLOW_INT); if (sfd->name != NULL) SCFree(sfd->name); if (sfd->targettype == FLOWINT_TARGET_VAR) diff --git a/src/detect-flowvar.c b/src/detect-flowvar.c index 6df7f38c77..4386a38caa 100644 --- a/src/detect-flowvar.c +++ b/src/detect-flowvar.c @@ -78,6 +78,9 @@ static void DetectFlowvarDataFree(DetectEngineCtx *de_ctx, void *ptr) SCReturn; DetectFlowvarData *fd = (DetectFlowvarData *)ptr; + /* leave unregistration to pcre keyword */ + if (!fd->post_match) + VarNameStoreUnregister(fd->idx, VAR_TYPE_FLOW_VAR); if (fd->name) SCFree(fd->name); @@ -177,7 +180,7 @@ static int DetectFlowvarSetup (DetectEngineCtx *de_ctx, Signature *s, const char fd->name = SCStrdup(varname); if (unlikely(fd->name == NULL)) goto error; - fd->idx = VarNameStoreSetupAdd(varname, VAR_TYPE_FLOW_VAR); + fd->idx = VarNameStoreRegister(varname, VAR_TYPE_FLOW_VAR); /* Okay so far so good, lets get this into a SigMatch * and put it in the Signature. */ @@ -272,6 +275,7 @@ int DetectFlowvarPostMatchSetup(DetectEngineCtx *de_ctx, Signature *s, uint32_t /* we only need the idx */ fv->idx = idx; + fv->post_match = true; sm = SigMatchAlloc(); if (unlikely(sm == NULL)) diff --git a/src/detect-flowvar.h b/src/detect-flowvar.h index ef26057daf..b2988a6351 100644 --- a/src/detect-flowvar.h +++ b/src/detect-flowvar.h @@ -29,6 +29,8 @@ typedef struct DetectFlowvarData_ { uint32_t idx; uint8_t *content; uint16_t content_len; + /** set to true if used in a post-match */ + bool post_match; uint32_t flags; } DetectFlowvarData; diff --git a/src/detect-hostbits.c b/src/detect-hostbits.c index 8297b2f8ae..764bf62805 100644 --- a/src/detect-hostbits.c +++ b/src/detect-hostbits.c @@ -395,7 +395,7 @@ int DetectHostbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawst if (unlikely(cd == NULL)) goto error; - cd->idx = VarNameStoreSetupAdd(fb_name, VAR_TYPE_HOST_BIT); + cd->idx = VarNameStoreRegister(fb_name, VAR_TYPE_HOST_BIT); cd->cmd = fb_cmd; cd->tracker = hb_dir; cd->type = VAR_TYPE_HOST_BIT; @@ -451,6 +451,7 @@ void DetectHostbitFree (DetectEngineCtx *de_ctx, void *ptr) if (fd == NULL) return; + VarNameStoreUnregister(fd->idx, VAR_TYPE_HOST_BIT); SCFree(fd); } @@ -688,8 +689,8 @@ static int HostBitsTestSig03(void) s = de_ctx->sig_list = SigInit(de_ctx,"alert ip any any -> any any (msg:\"isset option\"; hostbits:isset,fbt; content:\"GET \"; sid:1;)"); FAIL_IF_NULL(s); - idx = VarNameStoreSetupAdd("fbt", VAR_TYPE_HOST_BIT); - FAIL_IF(idx != 1); + idx = VarNameStoreRegister("fbt", VAR_TYPE_HOST_BIT); + FAIL_IF(idx == 0); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); diff --git a/src/detect-lua.c b/src/detect-lua.c index 18302cf7bd..dfb26dcbe6 100644 --- a/src/detect-lua.c +++ b/src/detect-lua.c @@ -794,7 +794,7 @@ static int DetectLuaSetupPrime(DetectEngineCtx *de_ctx, DetectLuaData *ld, const goto error; } - uint32_t idx = VarNameStoreSetupAdd((char *)value, VAR_TYPE_FLOW_VAR); + uint32_t idx = VarNameStoreRegister(value, VAR_TYPE_FLOW_VAR); ld->flowvar[ld->flowvars++] = idx; SCLogDebug("script uses flowvar %u with script id %u", idx, ld->flowvars - 1); } @@ -816,7 +816,7 @@ static int DetectLuaSetupPrime(DetectEngineCtx *de_ctx, DetectLuaData *ld, const goto error; } - uint32_t idx = VarNameStoreSetupAdd((char *)value, VAR_TYPE_FLOW_INT); + uint32_t idx = VarNameStoreRegister(value, VAR_TYPE_FLOW_INT); ld->flowint[ld->flowints++] = idx; SCLogDebug("script uses flowint %u with script id %u", idx, ld->flowints - 1); } @@ -1166,6 +1166,13 @@ static void DetectLuaFree(DetectEngineCtx *de_ctx, void *ptr) if (lua->filename) SCFree(lua->filename); + for (uint16_t i = 0; i < lua->flowints; i++) { + VarNameStoreUnregister(lua->flowint[i], VAR_TYPE_FLOW_INT); + } + for (uint16_t i = 0; i < lua->flowvars; i++) { + VarNameStoreUnregister(lua->flowvar[i], VAR_TYPE_FLOW_VAR); + } + DetectUnregisterThreadCtxFuncs(de_ctx, lua, "lua"); SCFree(lua); @@ -1281,7 +1288,10 @@ static int LuaMatchTest01(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1398,7 +1408,10 @@ static int LuaMatchTest01a(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1503,7 +1516,10 @@ static int LuaMatchTest02(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1606,7 +1622,10 @@ static int LuaMatchTest02a(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1709,7 +1728,10 @@ static int LuaMatchTest03(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1811,7 +1833,10 @@ static int LuaMatchTest03a(void) SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_VAR); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_str.value_len != 1); @@ -1925,7 +1950,10 @@ static int LuaMatchTest04(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 2); @@ -2040,7 +2068,10 @@ static int LuaMatchTest04a(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 2); @@ -2148,7 +2179,10 @@ static int LuaMatchTest05(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 2); @@ -2256,7 +2290,10 @@ static int LuaMatchTest05a(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 2); @@ -2369,7 +2406,10 @@ static int LuaMatchTest06(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 0); @@ -2482,7 +2522,10 @@ static int LuaMatchTest06a(void) FAIL_IF_NOT(PacketAlertCheck(p2, 1)); - FlowVar *fv = FlowVarGet(&f, 1); + uint32_t id = VarNameStoreLookupByName("cnt", VAR_TYPE_FLOW_INT); + FAIL_IF(id == 0); + + FlowVar *fv = FlowVarGet(&f, id); FAIL_IF_NULL(fv); FAIL_IF(fv->data.fv_int.value != 0); diff --git a/src/detect-pcre.c b/src/detect-pcre.c index b1f7b101d9..ce5155f7e2 100644 --- a/src/detect-pcre.c +++ b/src/detect-pcre.c @@ -750,12 +750,14 @@ static int DetectPcreParseCapture(const char *regexstr, DetectEngineCtx *de_ctx, return -1; } else if (strncmp(name_array[name_idx], "flow:", 5) == 0) { - pd->capids[pd->idx] = VarNameStoreSetupAdd(name_array[name_idx]+5, VAR_TYPE_FLOW_VAR); + pd->capids[pd->idx] = + VarNameStoreRegister(name_array[name_idx] + 5, VAR_TYPE_FLOW_VAR); pd->captypes[pd->idx] = VAR_TYPE_FLOW_VAR; pd->idx++; } else if (strncmp(name_array[name_idx], "pkt:", 4) == 0) { - pd->capids[pd->idx] = VarNameStoreSetupAdd(name_array[name_idx]+4, VAR_TYPE_PKT_VAR); + pd->capids[pd->idx] = + VarNameStoreRegister(name_array[name_idx] + 4, VAR_TYPE_PKT_VAR); pd->captypes[pd->idx] = VAR_TYPE_PKT_VAR; SCLogDebug("id %u type %u", pd->capids[pd->idx], pd->captypes[pd->idx]); pd->idx++; @@ -817,12 +819,12 @@ static int DetectPcreParseCapture(const char *regexstr, DetectEngineCtx *de_ctx, } if (strcmp(type_str, "pkt") == 0) { - pd->capids[pd->idx] = VarNameStoreSetupAdd((char *)capture_str, VAR_TYPE_PKT_VAR); + pd->capids[pd->idx] = VarNameStoreRegister((char *)capture_str, VAR_TYPE_PKT_VAR); pd->captypes[pd->idx] = VAR_TYPE_PKT_VAR; SCLogDebug("id %u type %u", pd->capids[pd->idx], pd->captypes[pd->idx]); pd->idx++; } else if (strcmp(type_str, "flow") == 0) { - pd->capids[pd->idx] = VarNameStoreSetupAdd((char *)capture_str, VAR_TYPE_FLOW_VAR); + pd->capids[pd->idx] = VarNameStoreRegister((char *)capture_str, VAR_TYPE_FLOW_VAR); pd->captypes[pd->idx] = VAR_TYPE_FLOW_VAR; pd->idx++; } @@ -968,6 +970,9 @@ static void DetectPcreFree(DetectEngineCtx *de_ctx, void *ptr) DetectParseFreeRegex(&pd->parse_regex); DetectUnregisterThreadCtxFuncs(de_ctx, pd, "pcre"); + for (uint8_t i = 0; i < pd->idx; i++) { + VarNameStoreUnregister(pd->capids[i], pd->captypes[i]); + } SCFree(pd); return; @@ -2006,10 +2011,11 @@ static int DetectPcreParseCaptureTest(void) SigGroupBuild(de_ctx); - uint32_t capid = VarNameStoreLookupByName("somecapture", VAR_TYPE_FLOW_VAR); - FAIL_IF (capid != 1); - capid = VarNameStoreLookupByName("anothercap", VAR_TYPE_PKT_VAR); - FAIL_IF (capid != 2); + uint32_t capid1 = VarNameStoreLookupByName("somecapture", VAR_TYPE_FLOW_VAR); + FAIL_IF(capid1 == 0); + uint32_t capid2 = VarNameStoreLookupByName("anothercap", VAR_TYPE_PKT_VAR); + FAIL_IF(capid2 == 0); + FAIL_IF(capid1 == capid2); DetectEngineCtxFree(de_ctx); PASS; diff --git a/src/detect-pktvar.c b/src/detect-pktvar.c index ec34463259..a9e24168a6 100644 --- a/src/detect-pktvar.c +++ b/src/detect-pktvar.c @@ -81,6 +81,7 @@ static void DetectPktvarFree(DetectEngineCtx *de_ctx, void *ptr) { DetectPktvarData *data = ptr; if (data != NULL) { + VarNameStoreUnregister(data->id, VAR_TYPE_PKT_VAR); SCFree(data->content); SCFree(data); } @@ -146,7 +147,7 @@ static int DetectPktvarSetup (DetectEngineCtx *de_ctx, Signature *s, const char cd->content = content; cd->content_len = len; - cd->id = VarNameStoreSetupAdd(varname, VAR_TYPE_PKT_VAR); + cd->id = VarNameStoreRegister(varname, VAR_TYPE_PKT_VAR); pcre2_substring_free((PCRE2_UCHAR8 *)varname); /* Okay so far so good, lets get this into a SigMatch diff --git a/src/detect-xbits.c b/src/detect-xbits.c index 9c5871f7af..4fae441481 100644 --- a/src/detect-xbits.c +++ b/src/detect-xbits.c @@ -320,7 +320,7 @@ static int DetectXbitParse(DetectEngineCtx *de_ctx, if (unlikely(cd == NULL)) return -1; - cd->idx = VarNameStoreSetupAdd(fb_name, var_type); + cd->idx = VarNameStoreRegister(fb_name, var_type); cd->cmd = fb_cmd; cd->tracker = hb_dir; cd->type = var_type; @@ -387,6 +387,7 @@ static void DetectXbitFree (DetectEngineCtx *de_ctx, void *ptr) if (fd == NULL) return; + VarNameStoreUnregister(fd->idx, fd->type); SCFree(fd); } diff --git a/src/suricata.c b/src/suricata.c index 48aadfa770..ebb1f8c80b 100644 --- a/src/suricata.c +++ b/src/suricata.c @@ -142,6 +142,7 @@ #include "util-signal.h" #include "util-time.h" #include "util-validate.h" +#include "util-var-name.h" #ifdef WINDIVERT #include "decode-sll.h" @@ -414,6 +415,8 @@ static void GlobalsDestroy(SCInstance *suri) SCPidfileRemove(suri->pid_filename); SCFree(suri->pid_filename); suri->pid_filename = NULL; + + VarNameStoreDestroy(); } /** @@ -2885,6 +2888,7 @@ int InitGlobal(void) /* Initialize the configuration module. */ ConfInit(); + VarNameStoreInit(); return 0; } diff --git a/src/util-var-name.c b/src/util-var-name.c index 426ea146d2..b0a88bface 100644 --- a/src/util-var-name.c +++ b/src/util-var-name.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2016 Open Information Security Foundation +/* Copyright (C) 2007-2023 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 @@ -25,365 +25,351 @@ #include "suricata-common.h" #include "detect.h" +#include "util-hash-string.h" #include "util-hashlist.h" #include "util-var-name.h" +#include "util-validate.h" -/* the way this can be used w/o locking lookups: - * - Lookups use only g_varnamestore_current which is read only - * - Detection setups a new ctx in staging, which will include the 'current' - * entries keeping ID's stable - * - Detection hot swaps staging into current after a new detect engine was - * created. Current remains available through 'old'. - * - When detect reload is complete (threads are all moved over), 'old' can - * be freed. +/* Overall Design: + * + * Base Store: "base" + * + * Used during keyword registration. Operates under lock. Base is shared + * between all detect engines, detect engine versions and tenants. + * Each variable name is ref counted. + * + * During the freeing of a detect engine / tenant, unregistration decreases + * the ref cnt. + * + * Base has both a string to id and a id to string hash table. String to + * id is used during parsing/registration. id to string during unregistration. + * + * + * Active Store Pointer (atomic) + * + * The "active" store atomic pointer points to the active store. The call + * to `VarNameStoreActivate` will build a new lookup store and hot swap + * the pointer. + * + * Ensuring memory safety. During the hot swap, the pointer is replaced, so + * any new call to the lookup functions will automatically use the new store. + * This leaves the case of any lookup happening concurrently with the pointer + * swap. For this case we add the old store to a free list. It gets a timestamp + * before which it cannot be freed. + * + * + * Free List + * + * The free list contains old stores that are waiting to get removed. They + * contain a timestamp that is checked before they are freed. + * */ - typedef struct VarNameStore_ { HashListTable *names; HashListTable *ids; uint32_t max_id; - uint32_t de_ctx_version; /**< de_ctx version 'owning' this */ + struct timeval free_after; + TAILQ_ENTRY(VarNameStore_) next; } VarNameStore; - -static int initialized = 0; -/* currently VarNameStore that is READ ONLY. This way lookups can - * be done w/o locking or synchronization */ -SC_ATOMIC_DECLARE(VarNameStore *, g_varnamestore_current); - -/* old VarNameStore on the way out */ -static VarNameStore *g_varnamestore_old = NULL; - -/* new VarNameStore that is being prepared. Multiple DetectLoader threads - * may be updating it so a lock is used for synchronization. */ -static VarNameStore *g_varnamestore_staging = NULL; -static SCMutex g_varnamestore_staging_m = SCMUTEX_INITIALIZER; +typedef VarNameStore *VarNameStorePtr; /** \brief Name2idx mapping structure for flowbits, flowvars and pktvars. */ typedef struct VariableName_ { char *name; enum VarTypes type; /* flowbit, pktvar, etc */ - uint32_t idx; + uint32_t id; + uint32_t ref_cnt; } VariableName; #define VARNAME_HASHSIZE 0x1000 #define VARID_HASHSIZE 0x1000 -static uint32_t VariableNameHash(HashListTable *ht, void *buf, uint16_t buflen) -{ - VariableName *fn = (VariableName *)buf; - uint32_t hash = strlen(fn->name) + fn->type; - uint16_t u; - - for (u = 0; u < buflen; u++) { - hash += fn->name[u]; - } +static SCMutex base_lock = SCMUTEX_INITIALIZER; +static VarNameStore base = { .names = NULL, .ids = NULL, .max_id = 0 }; +static TAILQ_HEAD(, VarNameStore_) free_list = TAILQ_HEAD_INITIALIZER(free_list); +static SC_ATOMIC_DECLARE(VarNameStorePtr, active); - return (hash % VARNAME_HASHSIZE); -} +static uint32_t VariableNameHash(HashListTable *ht, void *buf, uint16_t buflen); +static char VariableNameCompare(void *buf1, uint16_t len1, void *buf2, uint16_t len2); +static uint32_t VariableIdHash(HashListTable *ht, void *ptr, uint16_t _unused); +static char VariableIdCompare(void *ptr1, uint16_t _unused1, void *ptr2, uint16_t _unused2); +static void VariableNameFree(void *data); -static char VariableNameCompare(void *buf1, uint16_t len1, void *buf2, uint16_t len2) +void VarNameStoreInit(void) { - VariableName *fn1 = (VariableName *)buf1; - VariableName *fn2 = (VariableName *)buf2; - - if (fn1->type != fn2->type) - return 0; - - if (strcmp(fn1->name,fn2->name) == 0) - return 1; - - return 0; -} - -static uint32_t VariableIdxHash(HashListTable *ht, void *buf, uint16_t buflen) -{ - VariableName *fn = (VariableName *)buf; - uint32_t hash = fn->idx + fn->type; - return (hash % VARID_HASHSIZE); -} - -static char VariableIdxCompare(void *buf1, uint16_t len1, void *buf2, uint16_t len2) -{ - VariableName *fn1 = (VariableName *)buf1; - VariableName *fn2 = (VariableName *)buf2; - - if (fn1->type != fn2->type) - return 0; - - if (fn1->idx == fn2->idx) - return 1; - - return 0; -} - -static void VariableNameFree(void *data) -{ - VariableName *fn = (VariableName *)data; - - if (fn == NULL) - return; - - if (fn->name != NULL) { - SCFree(fn->name); - fn->name = NULL; + SCMutexLock(&base_lock); + base.names = HashListTableInit( + VARNAME_HASHSIZE, VariableNameHash, VariableNameCompare, VariableNameFree); + if (base.names == NULL) { + FatalError("failed to initialize variable name hash (names)"); } - SCFree(fn); + /* base.names owns the allocation, so use a NULL Free pointer here */ + base.ids = HashListTableInit(VARID_HASHSIZE, VariableIdHash, VariableIdCompare, NULL); + if (base.ids == NULL) { + FatalError("failed to initialize variable name hash (names)"); + } + SC_ATOMIC_INITPTR(active); + SCMutexUnlock(&base_lock); } -/** \brief Initialize the Name idx hash. - */ -static VarNameStore *VarNameStoreInit(void) +void VarNameStoreDestroy(void) { - VarNameStore *v = SCCalloc(1, sizeof(*v)); - if (v == NULL) - return NULL; - - v->names = HashListTableInit(VARNAME_HASHSIZE, VariableNameHash, VariableNameCompare, VariableNameFree); - if (v->names == NULL) { - SCFree(v); - return NULL; + SCMutexLock(&base_lock); + VarNameStore *s = SC_ATOMIC_GET(active); + if (s) { + HashListTableFree(s->names); + HashListTableFree(s->ids); + SCFree(s); + s = NULL; } + SC_ATOMIC_SET(active, NULL); - v->ids = HashListTableInit(VARID_HASHSIZE, VariableIdxHash, VariableIdxCompare, NULL); - if (v->ids == NULL) { - HashListTableFree(v->names); - SCFree(v); - return NULL; + while ((s = TAILQ_FIRST(&free_list))) { + TAILQ_REMOVE(&free_list, s, next); + HashListTableFree(s->names); + HashListTableFree(s->ids); + SCFree(s); } - v->max_id = 0; - return v; -} - -static void VarNameStoreDoFree(VarNameStore *v) -{ - if (v) { - HashListTableFree(v->names); - HashListTableFree(v->ids); - SCFree(v); + for (HashListTableBucket *b = HashListTableGetListHead(base.names); b != NULL; + b = HashListTableGetListNext(b)) { + VariableName *vn = HashListTableGetListData(b); + DEBUG_VALIDATE_BUG_ON(vn->ref_cnt > 0); + if (vn->ref_cnt > 0) { + SCLogWarning("%s (type %u, id %u) still has ref_cnt %u", vn->name, vn->type, vn->id, + vn->ref_cnt); + } } + HashListTableFree(base.ids); + base.ids = NULL; + HashListTableFree(base.names); + base.names = NULL; + base.max_id = 0; + SCMutexUnlock(&base_lock); } - -/** \brief Get a name idx for a name. If the name is already used reuse the idx. - * \param name nul terminated string with the name - * \param type variable type - * \retval 0 in case of error - * \retval idx the idx or 0 +/** + * \retval id or 0 on error */ -static uint32_t VariableNameGetIdx(VarNameStore *v, const char *name, enum VarTypes type) +uint32_t VarNameStoreRegister(const char *name, const enum VarTypes type) { - uint32_t idx = 0; - - VariableName *fn = SCMalloc(sizeof(VariableName)); - if (unlikely(fn == NULL)) - goto error; + SCMutexLock(&base_lock); + uint32_t id = 0; - memset(fn, 0, sizeof(VariableName)); - - fn->type = type; - fn->name = SCStrdup(name); - if (fn->name == NULL) - goto error; - - VariableName *lookup_fn = (VariableName *)HashListTableLookup(v->names, (void *)fn, 0); - if (lookup_fn == NULL) { - v->max_id++; - - idx = fn->idx = v->max_id; - HashListTableAdd(v->names, (void *)fn, 0); - HashListTableAdd(v->ids, (void *)fn, 0); - SCLogDebug("new registration %s id %u type %u", fn->name, fn->idx, fn->type); + SCLogDebug("registering: name %s type %u", name, type); + VariableName lookup = { .type = type, .name = (char *)name }; + VariableName *found = (VariableName *)HashListTableLookup(base.names, (void *)&lookup, 0); + if (found == NULL) { + VariableName *vn = SCCalloc(1, sizeof(VariableName)); + if (likely(vn != NULL)) { + vn->type = type; + vn->name = SCStrdup(name); + if (vn->name != NULL) { + vn->ref_cnt = 1; + id = vn->id = ++base.max_id; + HashListTableAdd(base.names, (void *)vn, 0); + HashListTableAdd(base.ids, (void *)vn, 0); + SCLogDebug( + "new registration %s id %u type %u -> %u", vn->name, vn->id, vn->type, id); + } else { + SCFree(vn); + } + } } else { - idx = lookup_fn->idx; - VariableNameFree(fn); + id = found->id; + found->ref_cnt++; + SCLogDebug("existing registration %s ref_cnt %u -> %u", name, found->ref_cnt, id); } - - return idx; -error: - VariableNameFree(fn); - return 0; + SCMutexUnlock(&base_lock); + return id; } -/** \brief Get a name from the idx. - * \param idx index of the variable whose name is to be fetched - * \param type variable type - * \retval NULL in case of error - * \retval name of the variable if successful. - * \todo no alloc on lookup - */ -static char *VariableIdxGetName(VarNameStore *v, uint32_t idx, enum VarTypes type) +const char *VarNameStoreSetupLookup(const uint32_t id, const enum VarTypes type) { - VariableName *fn = SCMalloc(sizeof(VariableName)); - if (unlikely(fn == NULL)) - goto error; - - char *name = NULL; - memset(fn, 0, sizeof(VariableName)); - - fn->type = type; - fn->idx = idx; - - VariableName *lookup_fn = (VariableName *)HashListTableLookup(v->ids, (void *)fn, 0); - if (lookup_fn != NULL) { - name = SCStrdup(lookup_fn->name); - if (unlikely(name == NULL)) - goto error; - - VariableNameFree(fn); - } else { - goto error; + const char *name = NULL; + SCMutexLock(&base_lock); + VariableName lookup = { .type = type, .id = id }; + VariableName *found = (VariableName *)HashListTableLookup(base.ids, (void *)&lookup, 0); + if (found) { + name = found->name; } - + SCMutexUnlock(&base_lock); return name; -error: - VariableNameFree(fn); - return NULL; } -/** \brief setup staging store. Include current store if there is one. - */ -int VarNameStoreSetupStaging(uint32_t de_ctx_version) +void VarNameStoreUnregister(const uint32_t id, const enum VarTypes type) { - SCMutexLock(&g_varnamestore_staging_m); - - if (!initialized) { - SC_ATOMIC_INITPTR(g_varnamestore_current); - initialized = 1; + SCMutexLock(&base_lock); + VariableName lookup = { .type = type, .id = id }; + VariableName *found = (VariableName *)HashListTableLookup(base.ids, (void *)&lookup, 0); + if (found) { + SCLogDebug("found %s ref_cnt %u", found->name, found->ref_cnt); + DEBUG_VALIDATE_BUG_ON(found->ref_cnt == 0); + found->ref_cnt--; } + SCMutexUnlock(&base_lock); +} - if (g_varnamestore_staging != NULL && - g_varnamestore_staging->de_ctx_version == de_ctx_version) { - SCMutexUnlock(&g_varnamestore_staging_m); - return 0; - } +int VarNameStoreActivate(void) +{ + int result = 0; + SCMutexLock(&base_lock); + SCLogDebug("activating new lookup store"); + + VarNameStore *new_active = NULL; + + // create lookup hash for id to string, strings should point to base + for (HashListTableBucket *b = HashListTableGetListHead(base.names); b != NULL; + b = HashListTableGetListNext(b)) { + VariableName *vn = HashListTableGetListData(b); + BUG_ON(vn == NULL); + SCLogDebug("base: %s/%u/%u", vn->name, vn->id, vn->ref_cnt); + if (vn->ref_cnt == 0) + continue; + + if (new_active == NULL) { + new_active = SCCalloc(1, sizeof(*new_active)); + if (new_active == NULL) { + result = -1; + goto out; + } + + new_active->names = HashListTableInit( + VARNAME_HASHSIZE, VariableNameHash, VariableNameCompare, NULL); + if (new_active->names == NULL) { + SCFree(new_active); + result = -1; + goto out; + } + new_active->ids = + HashListTableInit(VARID_HASHSIZE, VariableIdHash, VariableIdCompare, NULL); + if (new_active->ids == NULL) { + HashListTableFree(new_active->names); + SCFree(new_active); + result = -1; + goto out; + } + } - VarNameStore *nv = VarNameStoreInit(); - if (nv == NULL) { - SCMutexUnlock(&g_varnamestore_staging_m); - return -1; + /* memory is still owned by "base" */ + HashListTableAdd(new_active->names, (void *)vn, 0); + HashListTableAdd(new_active->ids, (void *)vn, 0); } - g_varnamestore_staging = nv; - nv->de_ctx_version = de_ctx_version; - VarNameStore *current = SC_ATOMIC_GET(g_varnamestore_current); - if (current) { - /* add all entries from the current hash into this new one. */ - HashListTableBucket *b = HashListTableGetListHead(current->names); - while (b) { - VariableName *var = HashListTableGetListData(b); - - VariableName *newvar = SCCalloc(1, sizeof(*newvar)); - BUG_ON(newvar == NULL); - memcpy(newvar, var, sizeof(*newvar)); - newvar->name = SCStrdup(var->name); - BUG_ON(newvar->name == NULL); - - HashListTableAdd(nv->names, (void *)newvar, 0); - HashListTableAdd(nv->ids, (void *)newvar, 0); - nv->max_id = MAX(nv->max_id, newvar->idx); - SCLogDebug("xfer %s id %u type %u", newvar->name, newvar->idx, newvar->type); - - b = HashListTableGetListNext(b); + if (new_active) { + VarNameStore *old_active = SC_ATOMIC_GET(active); + if (old_active) { + struct timeval ts, add; + memset(&ts, 0, sizeof(ts)); + memset(&add, 0, sizeof(add)); + gettimeofday(&ts, NULL); + add.tv_sec = 60; + timeradd(&ts, &add, &ts); + old_active->free_after = ts; + + TAILQ_INSERT_TAIL(&free_list, old_active, next); + SCLogDebug("old active is stored in free list"); } - } - SCLogDebug("set up staging with detect engine ver %u", nv->de_ctx_version); - SCMutexUnlock(&g_varnamestore_staging_m); - return 0; + SC_ATOMIC_SET(active, new_active); + SCLogDebug("new store active"); + + struct timeval now; + memset(&now, 0, sizeof(now)); + gettimeofday(&now, NULL); + + VarNameStore *s = NULL; + while ((s = TAILQ_FIRST(&free_list))) { + char timebuf[64]; + CreateIsoTimeString(SCTIME_FROM_TIMEVAL(&s->free_after), timebuf, sizeof(timebuf)); + + if (!timercmp(&now, &s->free_after, >)) { + SCLogDebug("not yet freeing store %p before %s", s, timebuf); + break; + } + SCLogDebug("freeing store %p with time %s", s, timebuf); + TAILQ_REMOVE(&free_list, s, next); + HashListTableFree(s->names); + HashListTableFree(s->ids); + SCFree(s); + } + } +out: + SCLogDebug("activating new lookup store: complete %d", result); + SCMutexUnlock(&base_lock); + return result; } +/** \brief find name for id+type at packet time. */ const char *VarNameStoreLookupById(const uint32_t id, const enum VarTypes type) { - VarNameStore *current = SC_ATOMIC_GET(g_varnamestore_current); - BUG_ON(current == NULL); - VariableName lookup = { NULL, type, id }; - VariableName *found = (VariableName *)HashListTableLookup(current->ids, (void *)&lookup, 0); - if (found == NULL) { - return NULL; + const char *name = NULL; + + const VarNameStore *current = SC_ATOMIC_GET(active); + if (current) { + VariableName lookup = { .type = type, .id = id }; + const VariableName *found = HashListTableLookup(current->ids, (void *)&lookup, 0); + if (found) { + return found->name; + } } - return found->name; + + return name; } +/** \brief find name for id+type at packet time. */ uint32_t VarNameStoreLookupByName(const char *name, const enum VarTypes type) { - VarNameStore *current = SC_ATOMIC_GET(g_varnamestore_current); - BUG_ON(current == NULL); - VariableName lookup = { (char *)name, type, 0 }; - VariableName *found = (VariableName *)HashListTableLookup(current->names, (void *)&lookup, 0); - if (found == NULL) { - return 0; + const VarNameStore *current = SC_ATOMIC_GET(active); + if (current) { + VariableName lookup = { .name = (char *)name, .type = type }; + const VariableName *found = HashListTableLookup(current->names, (void *)&lookup, 0); + if (found) { + return found->id; + } } - SCLogDebug("found %u for %s type %u", found->idx, name, type); - return found->idx; -} -/** \brief add to staging or return existing id if already in there */ -uint32_t VarNameStoreSetupAdd(const char *name, const enum VarTypes type) -{ - uint32_t id; - SCMutexLock(&g_varnamestore_staging_m); - id = VariableNameGetIdx(g_varnamestore_staging, name, type); - SCMutexUnlock(&g_varnamestore_staging_m); - return id; + return 0; } -char *VarNameStoreSetupLookup(uint32_t idx, const enum VarTypes type) +static uint32_t VariableNameHash(HashListTable *ht, void *buf, uint16_t buflen) { - SCMutexLock(&g_varnamestore_staging_m); - char *name = VariableIdxGetName(g_varnamestore_staging, idx, type); - SCMutexUnlock(&g_varnamestore_staging_m); - return name; + VariableName *vn = (VariableName *)buf; + uint32_t hash = StringHashDjb2((const uint8_t *)vn->name, strlen(vn->name)) + vn->type; + return (hash % VARNAME_HASHSIZE); } -void VarNameStoreActivateStaging(void) +static char VariableNameCompare(void *buf1, uint16_t len1, void *buf2, uint16_t len2) { - SCMutexLock(&g_varnamestore_staging_m); - if (g_varnamestore_old) { - VarNameStoreDoFree(g_varnamestore_old); - g_varnamestore_old = NULL; - } - g_varnamestore_old = SC_ATOMIC_GET(g_varnamestore_current); - SC_ATOMIC_SET(g_varnamestore_current, g_varnamestore_staging); - g_varnamestore_staging = NULL; - SCMutexUnlock(&g_varnamestore_staging_m); + VariableName *vn1 = (VariableName *)buf1; + VariableName *vn2 = (VariableName *)buf2; + return (vn1->type == vn2->type && strcmp(vn1->name, vn2->name) == 0); } -void VarNameStoreFreeOld(void) +static uint32_t VariableIdHash(HashListTable *ht, void *ptr, uint16_t _unused) { - SCMutexLock(&g_varnamestore_staging_m); - SCLogDebug("freeing g_varnamestore_old %p", g_varnamestore_old); - if (g_varnamestore_old) { - VarNameStoreDoFree(g_varnamestore_old); - g_varnamestore_old = NULL; - } - SCMutexUnlock(&g_varnamestore_staging_m); + VariableName *vn = (VariableName *)ptr; + uint32_t hash = vn->id << vn->type; + return (hash % VARID_HASHSIZE); } -void VarNameStoreFree(uint32_t de_ctx_version) +static char VariableIdCompare(void *ptr1, uint16_t _unused1, void *ptr2, uint16_t _unused2) { - SCLogDebug("freeing detect engine version %u", de_ctx_version); - SCMutexLock(&g_varnamestore_staging_m); - if (g_varnamestore_old && g_varnamestore_old->de_ctx_version == de_ctx_version) { - VarNameStoreDoFree(g_varnamestore_old); - g_varnamestore_old = NULL; - SCLogDebug("freeing detect engine version %u: old done", de_ctx_version); - } + VariableName *vn1 = (VariableName *)ptr1; + VariableName *vn2 = (VariableName *)ptr2; - /* if at this point we have a staging area which matches our version - * we didn't complete the setup and are cleaning up the mess. */ - if (g_varnamestore_staging && g_varnamestore_staging->de_ctx_version == de_ctx_version) { - VarNameStoreDoFree(g_varnamestore_staging); - g_varnamestore_staging = NULL; - SCLogDebug("freeing detect engine version %u: staging done", de_ctx_version); - } + return (vn1->id == vn2->id && vn1->type == vn2->type); +} - VarNameStore *current = SC_ATOMIC_GET(g_varnamestore_current); - if (current && current->de_ctx_version == de_ctx_version) { - VarNameStoreDoFree(current); - SC_ATOMIC_SET(g_varnamestore_current, NULL); - SCLogDebug("freeing detect engine version %u: current done", de_ctx_version); +static void VariableNameFree(void *data) +{ + VariableName *vn = (VariableName *)data; + if (vn == NULL) + return; + if (vn->name != NULL) { + SCFree(vn->name); + vn->name = NULL; } - SCMutexUnlock(&g_varnamestore_staging_m); + SCFree(vn); } diff --git a/src/util-var-name.h b/src/util-var-name.h index 5b0b18d716..5f21ea3fa4 100644 --- a/src/util-var-name.h +++ b/src/util-var-name.h @@ -24,14 +24,16 @@ #ifndef __UTIL_VAR_NAME_H__ #define __UTIL_VAR_NAME_H__ -int VarNameStoreSetupStaging(uint32_t de_ctx_version); +void VarNameStoreInit(void); +void VarNameStoreDestroy(void); + +uint32_t VarNameStoreRegister(const char *name, const enum VarTypes type); +const char *VarNameStoreSetupLookup(const uint32_t id, const enum VarTypes type); +void VarNameStoreUnregister(const uint32_t id, const enum VarTypes type); +int VarNameStoreActivate(void); + const char *VarNameStoreLookupById(const uint32_t id, const enum VarTypes type); -uint32_t VarNameStoreLookupByName(const char *name, const enum VarTypes type); -uint32_t VarNameStoreSetupAdd(const char *name, const enum VarTypes type); -char *VarNameStoreSetupLookup(uint32_t idx, const enum VarTypes type); -void VarNameStoreActivateStaging(void); -void VarNameStoreFreeOld(void); -void VarNameStoreFree(uint32_t de_ctx_version); +uint32_t VarNameStoreLookupByName(const char *, const enum VarTypes type); #endif