Fix locking error in filestore handling. Add debug validate check for asserting a...

diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c
index c6dcfee57af47ac3da14291c1ed860e7ab051630..dc52cedf10e1647ff19adab1082050dd88ca8b8d 100644 (file)
--- a/src/app-layer-parser.c
+++ b/src/app-layer-parser.c
@@ -58,6 +58,7 @@
 #include "util-debug.h"
 #include "decode-events.h"
 #include "util-unittest-helper.h"
+#include "util-validate.h"
 
 static AppLayerProto al_proto_table[ALPROTO_MAX];   /**< Application layer protocol
                                                          table mapped to their
@@ -842,6 +843,8 @@ int AppLayerParse(void *local_data, Flow *f, uint8_t proto,
 {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     uint16_t parser_idx = 0;
     AppLayerProto *p = &al_proto_table[proto];
     TcpSession *ssn = NULL;
@@ -1034,6 +1037,8 @@ error:
 int AppLayerTransactionGetInspectId(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     AppLayerParserStateStore *parser_state_store =
         (AppLayerParserStateStore *)f->alparser;
 
@@ -1051,6 +1056,8 @@ error:
 uint16_t AppLayerTransactionGetAvailId(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     AppLayerParserStateStore *parser_state_store =
         (AppLayerParserStateStore *)f->alparser;
 
@@ -1066,6 +1073,8 @@ uint16_t AppLayerTransactionGetAvailId(Flow *f) {
 int AppLayerTransactionGetLoggableId(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     AppLayerParserStateStore *parser_state_store =
         (AppLayerParserStateStore *)f->alparser;
 
@@ -1093,6 +1102,8 @@ error:
 void AppLayerTransactionUpdateLoggedId(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     AppLayerParserStateStore *parser_state_store =
         (AppLayerParserStateStore *)f->alparser;
 
@@ -1111,6 +1122,8 @@ error:
 int AppLayerTransactionGetLoggedId(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     AppLayerParserStateStore *parser_state_store =
         (AppLayerParserStateStore *)f->alparser;
 
@@ -1133,6 +1146,9 @@ error:
  */
 uint16_t AppLayerGetStateVersion(Flow *f) {
     SCEnter();
+
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     uint16_t version = 0;
     AppLayerParserStateStore *parser_state_store = NULL;
 
@@ -1156,6 +1172,8 @@ int AppLayerTransactionUpdateInspectId(Flow *f, char direction)
 {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     int r = 0;
     AppLayerParserStateStore *parser_state_store = NULL;
 
@@ -1206,6 +1224,8 @@ int AppLayerTransactionUpdateInspectId(Flow *f, char direction)
 
 AppLayerDecoderEvents *AppLayerGetDecoderEventsForFlow(Flow *f)
 {
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     /* Get the parser state (if any) */
     AppLayerParserStateStore *parser_state_store = NULL;
 
@@ -1233,6 +1253,8 @@ AppLayerDecoderEvents *AppLayerGetDecoderEventsForFlow(Flow *f)
 void AppLayerTriggerRawStreamReassembly(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
 #ifdef DEBUG
     BUG_ON(f == NULL);
 #endif

diff --git a/src/app-layer.c b/src/app-layer.c
index 562df7f6ee2cd2d9462217bb4511fcf90890e985..609889b31ed4c4bb2bf5084a26642f1eadf09c5b 100644 (file)
--- a/src/app-layer.c
+++ b/src/app-layer.c
@@ -34,6 +34,7 @@
 #include "util-debug.h"
 #include "util-print.h"
 #include "util-profiling.h"
+#include "util-validate.h"
 
 //#define PRINT
 extern uint8_t engine_mode;
@@ -78,6 +79,8 @@ void *AppLayerGetProtoStateFromPacket(Packet *p) {
 void *AppLayerGetProtoStateFromFlow(Flow *f) {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     if (f == NULL) {
         SCReturnPtr(NULL, "void");
     }
@@ -110,6 +113,8 @@ int AppLayerHandleTCPData(AlpProtoDetectThreadCtx *dp_ctx, Flow *f,
 {
     SCEnter();
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     int r = 0;
 
 #if DEBUG

diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c
index bc0b94d382cf5ca64f0b697cb421d5a39a838541..275258b925a2fff103b88a8c4f7616e1bb8162d7 100644 (file)
--- a/src/detect-engine-state.c
+++ b/src/detect-engine-state.c
@@ -676,8 +676,12 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
 
         if (DeStateStoreFilestoreSigsCantMatch(det_ctx->sgh, f->de_state, flags) == 1) {
             SCLogDebug("disabling file storage for transaction %u", det_ctx->tx_id);
+
+            SCMutexLock(&f->m);
             FileDisableStoringForTransaction(f, flags & (STREAM_TOCLIENT|STREAM_TOSERVER),
                     det_ctx->tx_id);
+            SCMutexUnlock(&f->m);
+
             f->de_state->flags |= DE_STATE_FILE_STORE_DISABLED;
         }
     }
@@ -1140,8 +1144,12 @@ int DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx, Dete
     if (!(f->de_state->flags & DE_STATE_FILE_STORE_DISABLED)) {
         if (DeStateStoreFilestoreSigsCantMatch(det_ctx->sgh, f->de_state, flags) == 1) {
             SCLogDebug("disabling file storage for transaction");
+
+            SCMutexLock(&f->m);
             FileDisableStoringForTransaction(f, flags & (STREAM_TOCLIENT|STREAM_TOSERVER),
                     det_ctx->tx_id);
+            SCMutexUnlock(&f->m);
+
             f->de_state->flags |= DE_STATE_FILE_STORE_DISABLED;
         }
     }

diff --git a/src/util-file.c b/src/util-file.c
index 6cf52e78ef2d04a132e0e9477c02298b75e31ba9..faad596f8b25de64a89ba015032b186f82036516 100644 (file)
--- a/src/util-file.c
+++ b/src/util-file.c
@@ -32,6 +32,7 @@
 #include "util-memcmp.h"
 #include "util-print.h"
 #include "app-layer-parser.h"
+#include "util-validate.h"
 
 /** \brief switch to force magic checks on all files
  *         regardless of the rules.
@@ -647,13 +648,15 @@ void FileDisableStoringForFile(File *ff) {
 /**
  *  \brief disable file storing for files in a transaction
  *
- *  \param f flow
+ *  \param f *LOCKED* flow
  *  \param direction flow direction
  *  \param tx_id transaction id
  */
 void FileDisableStoringForTransaction(Flow *f, uint8_t direction, uint16_t tx_id) {
     File *ptr = NULL;
 
+    DEBUG_ASSERT_FLOW_LOCKED(f);
+
     SCEnter();
 
     FileContainer *ffc = AppLayerGetFilesFromFlow(f, direction);

diff --git a/src/util-validate.h b/src/util-validate.h
index 3dd6c6eb3bea467078f227d5d5d56c7421e92042..b26bab6175381f63e87b5f611a3e7b1a19b12462 100644 (file)
--- a/src/util-validate.h
+++ b/src/util-validate.h
@@ -33,6 +33,20 @@
 
 #ifdef DEBUG_VALIDATION
 
+/** \brief test if a flow is locked.
+ *
+ * If trylock returns 0 it got a lock. Which means
+ * the flow was previously unlocked.
+ */
+#define DEBUG_ASSERT_FLOW_LOCKED(f) do {            \
+    if ((f) != NULL) {                              \
+        int r = SCMutexTrylock(&(f)->m);            \
+        if (r == 0) {                               \
+            BUG_ON(1);                              \
+        }                                           \
+    }                                               \
+} while(0)
+
 /** \brief validate the integrity of the flow
  *
  *  BUG_ON's on problems
@@ -82,6 +96,7 @@
 
 #else /* DEBUG_VALIDATE */
 
+#define DEBUG_ASSERT_FLOW_LOCKED(f)
 #define DEBUG_VALIDATE_FLOW(f)
 #define DEBUG_VALIDATE_PACKET(p)