From: Eric Leblond <eric@regit.org>
Date: Sat, 4 Nov 2017 15:10:15 +0000 (-0400)
Subject: detect-metadata: add a string storage to de_ctx
X-Git-Tag: suricata-4.1.0-beta1~240
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9864552484bdf1c258248567033247f088a904d6;p=thirdparty%2Fsuricata.git

detect-metadata: add a string storage to de_ctx

To avoid to have a lot of string allocations, we use a hash table
stored in de_ctx to point to existing string instead of duplicating
them.
---

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 16ac8d832e..91822231a2 100644
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -1063,6 +1063,7 @@ static DetectEngineCtx *DetectEngineCtxInitReal(int minimal, const char *prefix)
     ThresholdHashInit(de_ctx);
     DetectParseDupSigHashInit(de_ctx);
     DetectAddressMapInit(de_ctx);
+    DetectMetadataHashInit(de_ctx);
 
     /* init iprep... ignore errors for now */
     (void)SRepInit(de_ctx);
@@ -1185,6 +1186,7 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx)
     DetectEngineCtxFreeFailedSigs(de_ctx);
 
     DetectAddressMapFree(de_ctx);
+    DetectMetadataHashFree(de_ctx);
 
     /* if we have a config prefix, remove the config from the tree */
     if (strlen(de_ctx->config_prefix) > 0) {
diff --git a/src/detect-metadata.c b/src/detect-metadata.c
index aa273dcc9c..0806889e10 100644
--- a/src/detect-metadata.c
+++ b/src/detect-metadata.c
@@ -66,18 +66,81 @@ void DetectMetadataFree(DetectMetadata *mdata)
 {
     SCEnter();
 
-    if (mdata->key != NULL) {
-        SCFree((void *)mdata->key);
-    }
-    if (mdata->value != NULL) {
-        SCFree((void *)mdata->value);
-    }
     SCFree(mdata);
 
     SCReturn;
 }
 
-static int DetectMetadataParse(Signature *s, const char *metadatastr)
+/* djb2 string hashing */
+static uint32_t StringHashFunc(HashTable *ht, void *data, uint16_t datalen)
+{
+    uint32_t hash = 5381;
+    int c;
+
+    while ((c = *(char *)data++))
+        hash = ((hash << 5) + hash) + c; /* hash * 33 + c */
+
+    hash = hash % ht->array_size;
+
+    return hash;
+}
+
+static char StringHashCompareFunc(void *data1, uint16_t datalen1,
+                               void *data2, uint16_t datalen2)
+{
+    int len1 = strlen((char *)data1);
+    int len2 = strlen((char *)data2);
+
+    if (len1 == len2 && memcmp(data1, data2, len1) == 0) {
+        return 1;
+    }
+
+    return 0;
+}
+
+static void StringHashFreeFunc(void *data)
+{
+    SCFree(data);
+}
+
+int DetectMetadataHashInit(DetectEngineCtx *de_ctx)
+{
+    if (! DetectEngineMustParseMetadata())
+        return 0;
+
+    de_ctx->metadata_table = HashTableInit(4096, StringHashFunc, StringHashCompareFunc, StringHashFreeFunc);
+    if (de_ctx->metadata_table == NULL)
+        return -1;
+    return 0;
+}
+
+void DetectMetadataHashFree(DetectEngineCtx *de_ctx)
+{
+    if (de_ctx->metadata_table)
+        HashTableFree(de_ctx->metadata_table);
+}
+
+static const char *DetectMedatataHashAdd(DetectEngineCtx *de_ctx, const char *string)
+{
+    const char * hstring = (char *)HashTableLookup(de_ctx->metadata_table, (void *)string, strlen(string));
+    if (hstring) {
+        return hstring;
+    }
+
+    const char *astring = SCStrdup(string);
+    if (astring == NULL) {
+        return NULL;
+    }
+
+    if (HashTableAdd(de_ctx->metadata_table, (void *)astring, strlen(astring)) == 0) {
+        return (char *)HashTableLookup(de_ctx->metadata_table, (void *)astring, strlen(astring));
+    } else {
+        SCFree((void *)astring);
+    }
+    return NULL;
+}
+
+static int DetectMetadataParse(DetectEngineCtx *de_ctx, Signature *s, const char *metadatastr)
 {
 #define MAX_SUBSTRINGS 30
     int ret = 0, res = 0;
@@ -93,16 +156,20 @@ static int DetectMetadataParse(Signature *s, const char *metadatastr)
         return 0;
     }
 
-    char *saveptr = NULL;
     size_t metadatalen = strlen(metadatastr)+1;
     char rawstr[metadatalen];
     strlcpy(rawstr, metadatastr, metadatalen);
-    char * kv = strtok_r(rawstr, ",", &saveptr);
     const char *key = NULL;
     const char *value = NULL;
     char pkey[256];
     char pvalue[256];
 
+    char *saveptr = NULL;
+    char *kv = strtok_r(rawstr, ",", &saveptr);
+    if (kv == NULL) {
+        goto error;
+    }
+
     /* now check key value */
     do {
         DetectMetadata *dkv;
@@ -119,7 +186,7 @@ static int DetectMetadataParse(Signature *s, const char *metadatastr)
             SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
-        key = SCStrdup(pkey);
+        key = DetectMedatataHashAdd(de_ctx, pkey);
         if (key == NULL) {
             SCLogError(SC_ERR_MEM_ALLOC, "can't create metadata key");
             goto error;
@@ -130,7 +197,7 @@ static int DetectMetadataParse(Signature *s, const char *metadatastr)
             SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
-        value = SCStrdup(pvalue);
+        value = DetectMedatataHashAdd(de_ctx, pvalue);
         if (value == NULL) {
             SCLogError(SC_ERR_MEM_ALLOC, "can't create metadata value");
             goto error;
@@ -139,23 +206,13 @@ static int DetectMetadataParse(Signature *s, const char *metadatastr)
         SCLogDebug("key: %s, value: %s", key, value);
 
         dkv = SCMalloc(sizeof(DetectMetadata));
-        if (dkv) {
-            dkv->key = key;
-            if (!dkv->key) {
-                SCFree(dkv);
-            } else {
-                dkv->value = value;
-                if (!dkv->value) {
-                    SCFree((void *)dkv->key);
-                    SCFree(dkv);
-                } else {
-                    dkv->next = s->metadata;
-                    s->metadata = dkv;
-                }
-            }
-        } else {
+        if (dkv == NULL) {
             goto error;
         }
+        dkv->key = key;
+        dkv->value = value;
+        dkv->next = s->metadata;
+        s->metadata = dkv;
 
         kv = strtok_r(NULL, ",", &saveptr);
     } while (kv);
@@ -163,17 +220,13 @@ static int DetectMetadataParse(Signature *s, const char *metadatastr)
     return 0;
 
 error:
-    if (key)
-        SCFree((void *)key);
-    if (value)
-        SCFree((void *)value);
     return -1;
 }
 
 static int DetectMetadataSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
     if (DetectEngineMustParseMetadata()) {
-        DetectMetadataParse(s, rawstr);
+        DetectMetadataParse(de_ctx, s, rawstr);
     }
 
     return 0;
@@ -183,20 +236,13 @@ static int DetectMetadataSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
 static int DetectMetadataParseTest01(void)
 {
+    DetectEngineUnsetParseMetadata();
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
-    int prev_state = 0;
 
-    if (DetectEngineMustParseMetadata()) {
-        prev_state = 1;
-        DetectEngineUnsetParseMetadata();
-    }
     Signature *sig = DetectEngineAppendSig(de_ctx,
                                            "alert tcp any any -> any any "
                                            "(metadata: toto 1; sid:1; rev:1;)");
-    if (prev_state == 1) {
-        DetectEngineSetParseMetadata();
-    }
     FAIL_IF_NULL(sig);
     FAIL_IF(sig->metadata); 
 
@@ -206,14 +252,9 @@ static int DetectMetadataParseTest01(void)
 
 static int DetectMetadataParseTest02(void)
 {
-    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    DetectEngineSetParseMetadata();
+    DetectEngineCtx *de_ctx = NULL;
     DetectMetadata *dm;
-    int prev_state = 1;
-
-    if (! DetectEngineMustParseMetadata()) {
-        prev_state = 0;
-        DetectEngineSetParseMetadata();
-    }
 
     de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
@@ -222,9 +263,6 @@ static int DetectMetadataParseTest02(void)
                                            "(metadata: toto 1; "
                                            "metadata: titi 2, jaivu gros_minet;"
                                            "sid:1; rev:1;)");
-    if (prev_state == 0) {
-        DetectEngineUnsetParseMetadata();
-    }
     FAIL_IF_NULL(sig);
     FAIL_IF_NULL(sig->metadata); 
     FAIL_IF_NULL(sig->metadata->key); 
diff --git a/src/detect-metadata.h b/src/detect-metadata.h
index c7ab33d8fa..99a5424980 100644
--- a/src/detect-metadata.h
+++ b/src/detect-metadata.h
@@ -28,9 +28,9 @@
  * \brief Signature metadata list.
  */
 typedef struct DetectMetadata_ {
-    /* pointer to key */
+    /* pointer to key stored in de_ctx hash table_metadata */
     const char *key;
-    /* value data */
+    /* value data stored in de_ctx hash table_metadata */
     const char *value;
     /* next reference in the signature */
     struct DetectMetadata_ *next;
diff --git a/src/detect.h b/src/detect.h
index 9d86f23633..b58be1910e 100644
--- a/src/detect.h
+++ b/src/detect.h
@@ -755,6 +755,9 @@ typedef struct DetectEngineCtx_ {
     /** table for storing the string representation with the parsers result */
     HashListTable *address_table;
 
+    /** table to store metadata keys and values */
+    HashTable *metadata_table;
+
     /** table with mpms and their registration function
      *  \todo we only need this at init, so perhaps this
      *        can move to a DetectEngineCtx 'init' struct */
@@ -1261,6 +1264,9 @@ void RuleMatchCandidateTxArrayFree(DetectEngineThreadCtx *det_ctx);
 
 void DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx);
 
+int DetectMetadataHashInit(DetectEngineCtx *de_ctx);
+void DetectMetadataHashFree(DetectEngineCtx *de_ctx);
+
 #include "detect-engine-build.h"
 #include "detect-engine-register.h"