detect-engine: memory handling of sm_lists

For lists that are registered multiple times, like http_header and
http_cookie, making the engines owner of the lists is complicated.
Multiple engines in a sig may be pointing to the same list. To
address this the 'free' code needs to be extra careful about not
double freeing, so it takes an approach to first fill an array
of the to-free pointers before freeing them.

diff --git a/src/detect-engine.c b/src/detect-engine.c
index be0a15e854bea25e965f833453757b2cb097ef27..dad076c42a8367d1ad07cf7cfa87e8c99c8541b0 100644 (file)
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -144,6 +144,8 @@ void DetectAppLayerInspectEngineRegister(AppProto alproto,
 
 int DetectEngineAppInspectionEngine2Signature(Signature *s)
 {
+    int lists_used[DETECT_SM_LIST_MAX] = { 0 };
+
     DetectEngineAppInspectionEngine *t = g_app_inspect_engines;
     while (t != NULL) {
         if (s->sm_lists[t->sm_list] == NULL)
@@ -189,9 +191,51 @@ next:
         t = t->next;
     }
 
+    int i;
+    for (i = 0; i < DETECT_SM_LIST_MAX; i++) {
+        if (lists_used[i]) {
+            s->sm_lists[i] = NULL;
+            s->sm_lists_tail[i] = NULL;
+        }
+    }
+
     return 0;
 }
 
+/** \brief free app inspect engines for a signature
+ *
+ *  For lists that are registered multiple times, like http_header and
+ *  http_cookie, making the engines owner of the lists is complicated.
+ *  Multiple engines in a sig may be pointing to the same list. To
+ *  address this the 'free' code needs to be extra careful about not
+ *  double freeing, so it takes an approach to first fill an array
+ *  of the to-free pointers before freeing them.
+ */
+void DetectEngineAppInspectionEngineSignatureFree(Signature *s)
+{
+    SigMatch *ptrs[DETECT_SM_LIST_MAX] = { NULL };
+
+    DetectEngineAppInspectionEngine *ie = s->app_inspect;
+    while (ie) {
+        DetectEngineAppInspectionEngine *next = ie->next;
+        BUG_ON(ptrs[ie->sm_list] != NULL && ptrs[ie->sm_list] != ie->sm);
+        ptrs[ie->sm_list] = ie->sm;
+        SCFree(ie);
+        ie = next;
+    }
+
+    int i;
+    for (i = 0; i < DETECT_SM_LIST_MAX; i++)
+    {
+        SigMatch *sm = ptrs[i];
+        while (sm != NULL) {
+            SigMatch *nsm = sm->next;
+            SigMatchFree(sm);
+            sm = nsm;
+        }
+    }
+}
+
 /* code to control the main thread to do a reload */
 
 enum DetectEngineSyncState {

diff --git a/src/detect-engine.h b/src/detect-engine.h
index 935a3d79e3969570fc35c841968128e372206423..378706187fbdf085d41103ef7555522c7535273f 100644 (file)
--- a/src/detect-engine.h
+++ b/src/detect-engine.h
@@ -87,5 +87,6 @@ void DetectAppLayerInspectEngineRegister(AppProto alproto,
         uint32_t dir, int32_t sm_list, InspectEngineFuncPtr Callback);
 
 int DetectEngineAppInspectionEngine2Signature(Signature *s);
+void DetectEngineAppInspectionEngineSignatureFree(Signature *s);
 
 #endif /* __DETECT_ENGINE_H__ */

diff --git a/src/detect-parse.c b/src/detect-parse.c
index a896e8bc2a2fcc51cc5614ce9db46cc5390cdaf9..76dc039706cc1cb5aa02e0e360bebbb3bcd69445 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -1094,18 +1094,7 @@ void SigFree(Signature *s)
 
     SigRefFree(s);
 
-    DetectEngineAppInspectionEngine *ie = s->app_inspect;
-    while (ie) {
-        DetectEngineAppInspectionEngine *next = ie->next;
-        SigMatch *sm = ie->sm;
-        while (sm != NULL) {
-            SigMatch *nsm = sm->next;
-            SigMatchFree(sm);
-            sm = nsm;
-        }
-        SCFree(ie);
-        ie = next;
-    }
+    DetectEngineAppInspectionEngineSignatureFree(s);
 
     SCFree(s);
 }