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

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

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

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;
         }
     }

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);

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)