Bug 2857: NFQ ASAN 'heap-use-after-free' error.

Global NFQ contexts were not freed properly causing
'use-after-free' error. Moving contexts cleanup to a
separate NFQContextsCleanup() and calling it from
GlobalsDestroy(), like it's done for AFPacket, solves
the problem.

diff --git a/src/source-nfq.c b/src/source-nfq.c
index 9e8f7b8d3125145baf99e714300c41c198f06dab..8c4273b6a45727847b953430b29032c64029a594 100644 (file)
--- a/src/source-nfq.c
+++ b/src/source-nfq.c
@@ -801,14 +801,6 @@ TmEcode ReceiveNFQThreadDeinit(ThreadVars *t, void *data)
 
     NFQDestroyQueue(nq);
 
-    SCMutexLock(&nfq_init_lock);
-    if (--receive_queue_num == 0) {
-        // No more active queues, we may now free global contexts
-        SCFree(g_nfq_t);
-        SCFree(g_nfq_q);
-    }
-    SCMutexUnlock(&nfq_init_lock);
-
     return TM_ECODE_OK;
 }
 
@@ -959,7 +951,7 @@ int NFQParseAndRegisterQueues(const char *queues)
  */
 void *NFQGetQueue(int number)
 {
-    if (number >= receive_queue_num)
+    if (unlikely(number < 0 || number >= receive_queue_num || g_nfq_q == NULL))
         return NULL;
 
     return (void *)&g_nfq_q[number];
@@ -977,7 +969,7 @@ void *NFQGetQueue(int number)
  */
 void *NFQGetThread(int number)
 {
-    if (number >= receive_queue_num)
+    if (unlikely(number < 0 || number >= receive_queue_num || g_nfq_t == NULL))
         return NULL;
 
     return (void *)&g_nfq_t[number];
@@ -1311,4 +1303,20 @@ TmEcode DecodeNFQThreadDeinit(ThreadVars *tv, void *data)
     SCReturnInt(TM_ECODE_OK);
 }
 
+/**
+ * \brief Clean global contexts. Must be called on exit.
+ */
+void NFQContextsClean()
+{
+    if (g_nfq_q != NULL) {
+        SCFree(g_nfq_q);
+        g_nfq_q = NULL;
+    }
+
+    if (g_nfq_t != NULL) {
+        SCFree(g_nfq_t);
+        g_nfq_t = NULL;
+    }
+}
+
 #endif /* NFQ */

diff --git a/src/source-nfq.h b/src/source-nfq.h
index 25474812c22d2bd94d503bac739de9789424eecd..5c4ac5e16f549f8a490a39a77665172632c282e0 100644 (file)
--- a/src/source-nfq.h
+++ b/src/source-nfq.h
@@ -96,6 +96,7 @@ int NFQGetQueueCount(void);
 void *NFQGetQueue(int number);
 int NFQGetQueueNum(int number);
 void *NFQGetThread(int number);
+void NFQContextsClean(void);
 #endif /* NFQ */
 #endif /* __SOURCE_NFQ_H__ */
 

diff --git a/src/suricata.c b/src/suricata.c
index a9c562058511037f20227764599a3212cfb3a800..e00df93615fd9dd943fe2d96f1df306bb81310d8 100644 (file)
--- a/src/suricata.c
+++ b/src/suricata.c
@@ -392,6 +392,10 @@ static void GlobalsDestroy(SCInstance *suri)
     AFPPeersListClean();
 #endif
 
+#ifdef NFQ
+    NFQContextsClean();
+#endif
+
     SC_ATOMIC_DESTROY(engine_stage);
 
 #ifdef BUILD_HYPERSCAN