More lock fixes for the transaction update. Issues reported by Coverity.

diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c
index 404aa41b3798bb071acd1dfc8b7a0f94ab00f940..20f3d9c4324f1ffb96226fc285074a08b8d6aed2 100644 (file)
--- a/src/app-layer-parser.c
+++ b/src/app-layer-parser.c
@@ -1130,18 +1130,16 @@ uint64_t AppLayerTransactionGetInspectId(Flow *f, uint8_t flags)
 
 void AppLayerTransactionUpdateInspectId(Flow *f, uint8_t flags)
 {
-    DEBUG_ASSERT_FLOW_LOCKED(f);
-
     uint8_t direction = (flags & STREAM_TOSERVER) ? 0 : 1;
 
+    FLOWLOCK_WRLOCK(f);
     uint64_t total_txs = AppLayerGetTxCnt(f->alproto, f->alstate);
     uint64_t idx = AppLayerTransactionGetInspectId(f, flags);
     int state_done_progress = AppLayerGetAlstateProgressCompletionStatus(f->alproto, direction);
     void *tx;
-    int tx_updated_by = 0;
     int state_progress;
 
-    for (; idx < total_txs; idx++, tx_updated_by++) {
+    for (; idx < total_txs; idx++) {
         tx = AppLayerGetTx(f->alproto, f->alstate, idx);
         if (tx == NULL)
             continue;
@@ -1151,13 +1149,8 @@ void AppLayerTransactionUpdateInspectId(Flow *f, uint8_t flags)
         else
             break;
     }
-
     ((AppLayerParserStateStore *)f->alparser)->inspect_id[direction] = idx;
-    if (tx_updated_by > 0) {
-        SCMutexLock(&f->de_state_m);
-        DetectEngineStateReset(f->de_state, flags);
-        SCMutexUnlock(&f->de_state_m);
-    }
+    FLOWLOCK_UNLOCK(f);
 
     return;
 }

diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c
index 247bb8bee8b9b56485c499c025170e569bed08f2..e0df3ecdf90d0d2c08d06de0f610383b11c2539c 100644 (file)
--- a/src/detect-engine-state.c
+++ b/src/detect-engine-state.c
@@ -206,17 +206,19 @@ void DetectEngineStateFree(DetectEngineState *state)
     return;
 }
 
-int DeStateFlowHasInspectableState(Flow *f, uint16_t alversion, uint8_t flags)
+int DeStateFlowHasInspectableState(Flow *f, uint16_t alproto, uint16_t alversion, uint8_t flags)
 {
     int r = 0;
 
     SCMutexLock(&f->de_state_m);
     if (f->de_state == NULL || f->de_state->dir_state[flags & STREAM_TOSERVER ? 0 : 1].cnt == 0) {
-        if (AppLayerAlprotoSupportsTxs(f->alproto)) {
-            if (AppLayerTransactionGetInspectId(f, flags) >= AppLayerGetTxCnt(f->alproto, f->alstate))
+        if (AppLayerAlprotoSupportsTxs(alproto)) {
+            FLOWLOCK_RDLOCK(f);
+            if (AppLayerTransactionGetInspectId(f, flags) >= AppLayerGetTxCnt(alproto, f->alstate))
                 r = 2;
             else
                 r = 0;
+            FLOWLOCK_UNLOCK(f);
         }
     } else if (!(flags & STREAM_EOF) &&
                f->de_state->dir_state[flags & STREAM_TOSERVER ? 0 : 1].alversion == alversion) {
@@ -265,8 +267,6 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
 
         tx_id = AppLayerTransactionGetInspectId(f, flags);
         total_txs = AppLayerGetTxCnt(alproto, htp_state);
-        if (((total_txs - tx_id) > 1) && f->de_state != NULL)
-            DetectEngineStateReset(f->de_state, flags);
         for (; tx_id < total_txs; tx_id++) {
             tx = AppLayerGetTx(alproto, alstate, tx_id);
             if (tx == NULL)
@@ -420,6 +420,8 @@ void DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
     uint64_t inspect_tx_id = 0;
     uint64_t total_txs = 0;
     uint8_t alproto_supports_txs = 0;
+    uint8_t reset_de_state = 0;
+    uint8_t direction = (flags & STREAM_TOSERVER) ? 0 : 1;
 
     DeStateResetFileInspection(f, alproto, alstate, flags);
 
@@ -427,6 +429,16 @@ void DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
         FLOWLOCK_RDLOCK(f);
         inspect_tx_id = AppLayerTransactionGetInspectId(f, flags);
         total_txs = AppLayerGetTxCnt(alproto, alstate);
+        inspect_tx = AppLayerGetTx(alproto, alstate, inspect_tx_id);
+        if (inspect_tx == NULL) {
+            FLOWLOCK_UNLOCK(f);
+            SCMutexUnlock(&f->de_state_m);
+            return;
+        }
+        if (AppLayerGetAlstateProgress(alproto, inspect_tx, direction) >=
+            AppLayerGetAlstateProgressCompletionStatus(alproto, direction)) {
+            reset_de_state = 1;
+        }
         FLOWLOCK_UNLOCK(f);
         alproto_supports_txs = 1;
     }
@@ -504,13 +516,17 @@ void DeStateDetectContinueDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
                 htp_state = (HtpState *)alstate;
                 if (htp_state->connp == NULL || htp_state->connp->conn == NULL) {
                     FLOWLOCK_UNLOCK(f);
+                    RULE_PROFILING_END(det_ctx, s, match);
                     goto end;
                 }
 
                 engine = app_inspection_engine[alproto][(flags & STREAM_TOSERVER) ? 0 : 1];
                 inspect_tx = AppLayerGetTx(alproto, alstate, inspect_tx_id);
-                if (inspect_tx == NULL)
-                    continue;
+                if (inspect_tx == NULL) {
+                    FLOWLOCK_UNLOCK(f);
+                    RULE_PROFILING_END(det_ctx, s, match);
+                    goto end;
+                }
                 while (engine != NULL) {
                     if (!(item->flags & engine->inspect_flags) &&
                         s->sm_lists[engine->sm_list] != NULL)
@@ -610,17 +626,18 @@ end:
     if (f->de_state != NULL)
         dir_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_TC_NEW;
 
+    if (reset_de_state)
+        DetectEngineStateReset(f->de_state, flags);
+
     SCMutexUnlock(&f->de_state_m);
     return;
 }
 
 void DeStateUpdateInspectTransactionId(Flow *f, uint8_t direction)
 {
-    FLOWLOCK_WRLOCK(f);
     AppLayerTransactionUpdateInspectId(f, direction);
-    FLOWLOCK_UNLOCK(f);
 
-    SCReturn;
+    return;
 }
 
 

diff --git a/src/detect-engine-state.h b/src/detect-engine-state.h
index 24581d78510fcabe6a418bd9c47062fe7c056660..17ac46ebebeec8233e5b22fe0ddd71183141b5c3 100644 (file)
--- a/src/detect-engine-state.h
+++ b/src/detect-engine-state.h
@@ -148,7 +148,7 @@ void DetectEngineStateFree(DetectEngineState *state);
  * \retval 1 Has state.
  * \retval 0 Has no state.
  */
-int DeStateFlowHasInspectableState(Flow *f, uint16_t alversion, uint8_t direction);
+int DeStateFlowHasInspectableState(Flow *f, uint16_t alproto, uint16_t alversion, uint8_t flags);
 
 /**
  * \brief Match app layer sig list against app state and store relevant match

diff --git a/src/detect.c b/src/detect.c
index cdcaf9220e709faafd81ec7862455f652c174fa8..17460d9727ca4b8a928a1e82215aa142133e96cc 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1395,7 +1395,7 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
     if ((p->flags & PKT_HAS_FLOW) && alstate != NULL) {
         /* initialize to 0(DE_STATE_MATCH_HAS_NEW_STATE) */
         memset(det_ctx->de_state_sig_array, 0x00, det_ctx->de_state_sig_array_len);
-        int has_state = DeStateFlowHasInspectableState(p->flow, alversion, flags);
+        int has_state = DeStateFlowHasInspectableState(p->flow, alproto, alversion, flags);
         if (has_state == 1) {
             DeStateDetectContinueDetection(th_v, de_ctx, det_ctx, p, p->flow,
                                            flags, alstate, alproto, alversion);