From: Alexander Gozman <a.gozman@securitycode.ru>
Date: Sun, 3 Mar 2019 10:25:46 +0000 (+0300)
Subject: Bug 2857: NFQ ASAN 'heap-use-after-free' error.
X-Git-Tag: suricata-5.0.0-beta1~111
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=928fe1b859afb448f6fe344f80d8952a3b69c470;p=thirdparty%2Fsuricata.git

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 9e8f7b8d31..8c4273b6a4 100644
--- 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 25474812c2..5c4ac5e16f 100644
--- 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 a9c5620585..e00df93615 100644
--- 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