Make sure all smsgs are handled every time, even in case or error. The fuzzer found an issue where unhandled messages remained in the queue leading to threading issues.

src/app-layer-parser.c

@@ -812,11 +812,6 @@ int AppLayerParse(Flow *f, uint8_t proto, uint8_t flags, uint8_t *input,
     SCReturnInt(0);
 error:
     if (ssn != NULL) {
-        /* Clear the app layer protocol state memory and the given function also
-         * cleans the parser state memory */
-        if (f->use_cnt == 0)
-            AppLayerParserCleanupState(ssn);
-
         /* Set the no reassembly flag for both the stream in this TcpSession */
         StreamTcpSetSessionNoReassemblyFlag(ssn, flags & STREAM_TOCLIENT ? 1 : 0);
         StreamTcpSetSessionNoReassemblyFlag(ssn, flags & STREAM_TOSERVER ? 1 : 0);
@@ -834,7 +829,6 @@ error:
                 "address %s, destination IP address %s, src port %"PRIu16" and "
                 "dst port %"PRIu16"", al_proto_table[ssn->alproto].name,
                 f->proto, src, dst, f->sp, f->dp);
-
         } else {
             char dst6[46];
             char src6[46];
@@ -890,6 +884,7 @@ void AppLayerParserCleanupState(TcpSession *ssn)
         }
     }
 
+    /* free the app layer parser api state */
     if (ssn->aldata != NULL) {
         if (ssn->aldata[app_layer_sid] != NULL) {
             SCLogDebug("calling AppLayerParserStateStoreFree");

src/stream-tcp-reassemble.c

@@ -1,6 +1,7 @@
-/** Copyright (c) 2008 Victor Julien <victor@inliniac.net>
- *  Copyright (c) 2009 Open Information Security Foundation
- *
+/* Copyright (c) 2008 Victor Julien <victor@inliniac.net>
+ * Copyright (c) 2009 Open Information Security Foundation */
+
+/**
  * \file
  * \author Gurvinder Singh <gurvindersinghdahiya@gmail.com>
  * \author Victor Julien <victor@inliniac.net>
@@ -1612,25 +1613,32 @@ int StreamTcpReassembleHandleSegmentUpdateACK (TcpReassemblyThreadCtx *ra_ctx,
 
 /** \brief Handle the queue'd smsgs containing reassembled app layer data when
  *         we're running the app layer handling as part of the stream threads.
- *  \param ra_ctx Reassembly thread ctx, contains the queue
+ *
+ *  \param ra_ctx Reassembly thread ctx, contains the queue with stream msgs
+ *
+ *  \todo Currently we process all msgs even if we encounter an error in one
+ *        of them. We do this to make sure the thread ctx's queue is emptied.
+ *        Maybe we should just clear & return the msgs in case of error.
+ *
  *  \retval 0 ok
+ *  \retval -1 error
  */
 int StreamTcpReassembleProcessAppLayer(TcpReassemblyThreadCtx *ra_ctx)
 {
     SCEnter();
+
     int r = 0;
     if (ra_ctx != NULL && ra_ctx->stream_q && ra_ctx->stream_q->len > 0) {
         StreamMsg *smsg = NULL;
         do {
             smsg = StreamMsgGetFromQueue(ra_ctx->stream_q);
-            if (smsg == NULL)
-                break;
-
-            /** Handle the stream msg. No need to use locking, flow is already
-             *  locked at this point. */
-            r = AppLayerHandleMsg(&ra_ctx->dp_ctx, smsg);
-            if (r < 0)
-                break;
+            if (smsg != NULL) {
+                /* Handle the stream msg. No need to use locking, flow is
+                 * already locked at this point. Don't break out of the
+                 * loop if we encounter an error. */
+                if (AppLayerHandleMsg(&ra_ctx->dp_ctx, smsg) != 0)
+                    r = -1;
+            }
         } while (ra_ctx->stream_q->len > 0);
     }