Application layer detection improvements

- improve locking of application layer handling, making sure that the flow cannot be freed/cleared when the detection engine is still working with it.
- add a check to the app layer detection to make sure that a match function will only inspect an app layer state if it's of the right type.

src/app-layer-detect-proto.c

@@ -356,7 +356,8 @@ int AppLayerHandleMsg(StreamMsg *smsg, char need_lock)
             }
         } else {
             SCLogDebug("stream data (len %" PRIu32 " (%" PRIu32 ")), alproto "
-                      "%"PRIu16"", smsg->data.data_len, MSG_DATA_SIZE, alproto);
+                      "%"PRIu16" (flow %p)", smsg->data.data_len, MSG_DATA_SIZE,
+                      alproto, smsg->flow);
 
             //printf("=> Stream Data -- start\n");
             //PrintRawDataFp(stdout, smsg->data.data, smsg->data.data_len);

src/app-layer-htp.c

@@ -41,14 +41,22 @@ static uint64_t htp_state_memcnt = 0;
  */
 static void *HTPStateAlloc(void)
 {
-    void *s = malloc(sizeof(HtpState));
-    if (s == NULL)
-        return NULL;
+    SCEnter();
+
+    HtpState *s = malloc(sizeof(HtpState));
+    if (s == NULL) {
+        goto error;
+    }
 
-    memset(s, 0, sizeof(HtpState));
+    memset(s, 0x00, sizeof(HtpState));
 
     /* create the connection parser structure to be used by HTP library */
-    ((HtpState *)(s))->connp = htp_connp_create(cfg);
+    s->connp = htp_connp_create(cfg);
+    if (s->connp == NULL) {
+        goto error;
+    }
+
+    SCLogDebug("s->connp %p", s->connp);
 
 #ifdef DEBUG
     SCMutexLock(&htp_state_mem_lock);
@@ -56,26 +64,42 @@ static void *HTPStateAlloc(void)
     htp_state_memuse+=sizeof(HtpState);
     SCMutexUnlock(&htp_state_mem_lock);
 #endif
-    return s;
+
+    SCReturnPtr((void *)s, "void");
+
+error:
+    if (s != NULL)
+        free(s);
+
+    SCReturnPtr(NULL, "void");
 }
 
 /** \brief Function to frees the HTTP state memory and also frees the HTTP
  *         connection parser memory which was used by the HTP library
  */
-static void HTPStateFree(void *s)
+static void HTPStateFree(void *state)
 {
+    SCEnter();
+
+    HtpState *s = (HtpState *)state;
+
     /* free the connection parser memory used by HTP library */
-    if (s != NULL)
-        if (((HtpState *)(s))->connp != NULL)
-            htp_connp_destroy_all(((HtpState *)(s))->connp);
+    if (s != NULL) {
+        if (s->connp != NULL) {
+            htp_connp_destroy_all(s->connp);
+        }
+    }
 
     free(s);
+
 #ifdef DEBUG
     SCMutexLock(&htp_state_mem_lock);
     htp_state_memcnt--;
     htp_state_memuse-=sizeof(HtpState);
     SCMutexUnlock(&htp_state_mem_lock);
 #endif
+
+    SCReturn;
 }
 
 /**
@@ -118,12 +142,12 @@ static int HTPHandleRequestData(Flow *f, void *htp_state,
 
     /* Open the HTTP connection on receiving the first request */
     if (!(hstate->flags & HTP_FLAG_STATE_OPEN)) {
-        SCLogDebug("opening htp handle");
+        SCLogDebug("opening htp handle at %p", hstate->connp);
 
         htp_connp_open(hstate->connp, NULL, f->sp, NULL, f->dp, 0);
         hstate->flags |= HTP_FLAG_STATE_OPEN;
     } else {
-        SCLogDebug("using existing htp handle");
+        SCLogDebug("using existing htp handle at %p", hstate->connp);
     }
 
     r = htp_connp_req_data(hstate->connp, 0, input, input_len);
@@ -157,6 +181,7 @@ static int HTPHandleRequestData(Flow *f, void *htp_state,
         SCLogDebug("stream eof encountered, closing htp handle");
     }
 
+    SCLogDebug("hstate->connp %p", hstate->connp);
     SCReturnInt(ret);
 }
 
@@ -212,6 +237,7 @@ static int HTPHandleResponseData(Flow *f, void *htp_state,
         hstate->flags |= HTP_FLAG_STATE_CLOSED;
     }
 
+    SCLogDebug("hstate->connp %p", hstate->connp);
     SCReturnInt(ret);
 }
 

src/app-layer-htp.h

@@ -5,8 +5,8 @@
  * Created on November 14, 2009, 12:48 AM
  */
 
-#ifndef _APP_LAYER_HTP_H
-#define	_APP_LAYER_HTP_H
+#ifndef __APP_LAYER_HTP_H__
+#define __APP_LAYER_HTP_H__
 
 #include <htp/htp.h>
 
@@ -33,5 +33,6 @@ void RegisterHTPParsers(void);
 void HTPParserRegisterTests(void);
 void HTPAtExitPrintStats(void);
 void HTPFreeConfig(void);
-#endif	/* _APP_LAYER_HTP_H */
+
+#endif	/* __APP_LAYER_HTP_H__ */
 

src/app-layer-parser.c

@@ -540,7 +540,7 @@ int AppLayerRegisterProto(char *name, uint8_t proto, uint8_t flags,
     }
 
     SCLogDebug("registered %p at proto %" PRIu32 " flags %02X, al_proto_table "
-                "idx %" PRIu32 ", storage_id %" PRIu32 " %s \n", AppLayerParser, proto,
+                "idx %" PRIu32 ", storage_id %" PRIu32 " %s", AppLayerParser, proto,
                 flags, al_max_parsers, al_proto_table[proto].storage_id, name);
     return 0;
 }
@@ -698,11 +698,11 @@ int AppLayerParse(Flow *f, uint8_t proto, uint8_t flags, uint8_t *input,
         parser_state_store = (AppLayerParserStateStore *)
                                                     ssn->aldata[app_layer_sid];
         if (parser_state_store == NULL) {
+            if (need_lock == TRUE) SCMutexLock(&f->m);
             parser_state_store = AppLayerParserStateStoreAlloc();
             if (parser_state_store == NULL)
                 goto error;
 
-            if (need_lock == TRUE) SCMutexLock(&f->m);
             ssn->aldata[app_layer_sid] = (void *)parser_state_store;
             if (need_lock == TRUE) SCMutexUnlock(&f->m);
         }
@@ -717,7 +717,7 @@ int AppLayerParse(Flow *f, uint8_t proto, uint8_t flags, uint8_t *input,
 
     AppLayerParserState *parser_state = NULL;
     if (flags & STREAM_TOSERVER) {
-        SCLogDebug("to_server msg");
+        SCLogDebug("to_server msg (flow %p)", f);
 
         parser_state = &parser_state_store->to_server;
         if (!(parser_state->flags & APP_LAYER_PARSER_USE)) {
@@ -730,7 +730,7 @@ int AppLayerParse(Flow *f, uint8_t proto, uint8_t flags, uint8_t *input,
             parser_idx = parser_state->cur_parser;
         }
     } else {
-        SCLogDebug("to_client msg");
+        SCLogDebug("to_client msg (flow %p)", f);
 
         parser_state = &parser_state_store->to_client;
         if (!(parser_state->flags & APP_LAYER_PARSER_USE)) {
@@ -756,17 +756,22 @@ int AppLayerParse(Flow *f, uint8_t proto, uint8_t flags, uint8_t *input,
     void *app_layer_state = NULL;
     if (need_lock == TRUE) SCMutexLock(&f->m);
     app_layer_state = ssn->aldata[p->storage_id];
-    if (need_lock == TRUE) SCMutexUnlock(&f->m);
 
     if (app_layer_state == NULL) {
+        /* lock the allocation of state as we may
+         * alloc more than one otherwise */
         app_layer_state = p->StateAlloc();
-        if (app_layer_state == NULL)
+        if (app_layer_state == NULL) {
+            if (need_lock == TRUE) SCMutexUnlock(&f->m);
             goto error;
+        }
 
-        if (need_lock == TRUE) SCMutexLock(&f->m);
         ssn->aldata[p->storage_id] = app_layer_state;
-        if (need_lock == TRUE) SCMutexUnlock(&f->m);
+        SCLogDebug("alloced new app layer state %p (p->storage_id %u, name %s)", app_layer_state, p->storage_id, al_proto_table[ssn->alproto].name);
+    } else {
+        SCLogDebug("using existing app layer state %p (p->storage_id %u, name %s))", app_layer_state, p->storage_id, al_proto_table[ssn->alproto].name);
     }
+    if (need_lock == TRUE) SCMutexUnlock(&f->m);
 
     /* invoke the recursive parser */
     int r = AppLayerDoParse(f, app_layer_state, parser_state, input, input_len,
@@ -795,10 +800,11 @@ error:
     if (ssn != NULL) {
         /* Clear the app layer protocol state memory and the given function also
          * cleans the parser state memory */
-        AppLayerParserCleanupState(ssn);
+        if (need_lock == TRUE) SCMutexLock(&f->m);
+        if (f->use_cnt == 0)
+            AppLayerParserCleanupState(ssn);
 
         /* Set the no reassembly flag for both the stream in this TcpSession */
-        if (need_lock == TRUE) SCMutexLock(&f->m);
         StreamTcpSetSessionNoReassemblyFlag(ssn, flags & STREAM_TOCLIENT ? 1 : 0);
         StreamTcpSetSessionNoReassemblyFlag(ssn, flags & STREAM_TOSERVER ? 1 : 0);
 
@@ -961,7 +967,7 @@ void AppLayerParsersInitPostProcess(void)
                 continue;
 
            SCLogDebug("al_proto_table[%" PRIu32 "].map[%" PRIu32 "]->parser_id:"
-                      " %" PRIu32 "\n", u16, x, al_proto_table[u16].map[x]->parser_id);
+                      " %" PRIu32 "", u16, x, al_proto_table[u16].map[x]->parser_id);
         }
     }
 }

src/app-layer.c

@@ -3,6 +3,27 @@
 #include "stream-tcp-private.h"
 #include "util-debug.h"
 
+/** \brief Get the active app layer proto from the packet
+ *  \param p packet pointer
+ *  \retval alstate void pointer to the state
+ *  \retval proto (ALPROTO_UNKNOWN if no proto yet) */
+uint16_t AppLayerGetProtoFromPacket(Packet *p) {
+    SCEnter();
+
+    if (p == NULL || p->flow == NULL) {
+        SCReturnUInt(ALPROTO_UNKNOWN);
+    }
+
+    TcpSession *ssn = (TcpSession *)p->flow->protoctx;
+    if (ssn == NULL) {
+        SCReturnUInt(ALPROTO_UNKNOWN);
+    }
+
+    SCLogDebug("ssn->alproto %"PRIu16"", ssn->alproto);
+
+    SCReturnUInt(ssn->alproto);
+}
+
 /** \brief Get the active app layer state from the packet
  *  \param p packet pointer
  *  \retval alstate void pointer to the state
@@ -19,9 +40,11 @@ void *AppLayerGetProtoStateFromPacket(Packet *p) {
         SCReturnPtr(NULL, "void");
     }
 
-    SCLogDebug("ssn->alproto %u", ssn->alproto);
+    SCLogDebug("ssn->alproto %"PRIu16"", ssn->alproto);
 
     void *alstate = ssn->aldata[AlpGetStateIdx(ssn->alproto)];
+
+    SCLogDebug("p->flow %p", p->flow);
     SCReturnPtr(alstate, "void");
 }
 
@@ -39,7 +62,7 @@ void *AppLayerGetProtoStateFromFlow(Flow *f) {
     if (ssn == NULL || ssn->aldata == NULL)
         SCReturnPtr(NULL, "void");
 
-    SCLogDebug("ssn->alproto %u", ssn->alproto);
+    SCLogDebug("ssn->alproto %"PRIu16"", ssn->alproto);
 
     void *alstate = ssn->aldata[AlpGetStateIdx(ssn->alproto)];
     SCReturnPtr(alstate, "void");

src/app-layer.h

@@ -8,6 +8,7 @@
 #include "app-layer-parser.h"
 #include "stream.h"
 
+uint16_t AppLayerGetProtoFromPacket(Packet *);
 void *AppLayerGetProtoStateFromPacket(Packet *);
 void *AppLayerGetProtoStateFromFlow(Flow *);
 

src/decode.c

@@ -26,7 +26,7 @@ void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
  *
  * \param p Packet to set the flag in
  */
-void DecodeSetNoPayloadInspectionFlag(Packet *p) {
+inline void DecodeSetNoPayloadInspectionFlag(Packet *p) {
     p->flags |= PKT_NOPAYLOAD_INSPECTION;
 }
 
@@ -34,7 +34,7 @@ void DecodeSetNoPayloadInspectionFlag(Packet *p) {
  *
  * \param p Packet to set the flag in
  */
-void DecodeSetNoPacketInspectionFlag(Packet *p) {
+inline void DecodeSetNoPacketInspectionFlag(Packet *p) {
     p->flags |= PKT_NOPACKET_INSPECTION;
 }
 

src/decode.h

@@ -432,8 +432,8 @@ void DecodeGRE(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t,
 Packet *SetupPkt (void);
 Packet *TunnelPktSetup(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t, uint8_t);
 
-void DecodeSetNoPayloadInspectionFlag(Packet *);
-void DecodeSetNoPacketInspectionFlag(Packet *);
+inline void DecodeSetNoPayloadInspectionFlag(Packet *);
+inline void DecodeSetNoPacketInspectionFlag(Packet *);
 
 #define DECODER_SET_EVENT(p, e)   ((p)->events[(e/8)] |= (1<<(e%8)))
 #define DECODER_ISSET_EVENT(p, e) ((p)->events[(e/8)] & (1<<(e%8)))

src/detect-fast-pattern.c

@@ -63,9 +63,9 @@ int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, SigMatch *m,
         m = SigMatchGetLastSM(s, DETECT_CONTENT);
         if (m == NULL) {
             SCLogWarning(SC_ERR_INVALID_SIGNATURE, "fast_pattern found inside "
-                         "the rule, without a content context.  Please use a "
-                         "content keyword before using fast pattern");
-        return -1;
+                    "the rule, without a content context. Please use a "
+                    "content keyword before using fast pattern");
+            return -1;
         }
     }
 

src/detect-ftpbounce.c

@@ -46,6 +46,7 @@ void DetectFtpbounceRegister(void)
     //sigmatch_table[DETECT_FTPBOUNCE].Match = DetectFtpbounceMatch;
     sigmatch_table[DETECT_FTPBOUNCE].Match = NULL;
     sigmatch_table[DETECT_FTPBOUNCE].AppLayerMatch = DetectFtpbounceALMatch;
+    sigmatch_table[DETECT_FTPBOUNCE].alproto = ALPROTO_FTP;
     sigmatch_table[DETECT_FTPBOUNCE].Free  = NULL;
     sigmatch_table[DETECT_FTPBOUNCE].RegisterTests = DetectFtpbounceRegisterTests;
     return;

src/detect-http-cookie.c

@@ -21,6 +21,7 @@
 #include "util-debug.h"
 #include "util-unittest.h"
 #include "util-binsearch.h"
+#include "util-print.h"
 
 #include "app-layer.h"
 
@@ -42,6 +43,7 @@ void DetectHttpCookieRegister (void) {
     sigmatch_table[DETECT_AL_HTTP_COOKIE].name = "http_cookie";
     sigmatch_table[DETECT_AL_HTTP_COOKIE].Match = NULL;
     sigmatch_table[DETECT_AL_HTTP_COOKIE].AppLayerMatch = DetectHttpCookieMatch;
+    sigmatch_table[DETECT_AL_HTTP_COOKIE].alproto = ALPROTO_HTTP;
     sigmatch_table[DETECT_AL_HTTP_COOKIE].Setup = DetectHttpCookieSetup;
     sigmatch_table[DETECT_AL_HTTP_COOKIE].Free  = NULL;
     sigmatch_table[DETECT_AL_HTTP_COOKIE].RegisterTests = DetectHttpCookieRegisterTests;
@@ -68,37 +70,50 @@ int DetectHttpCookieMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
                            Flow *f, uint8_t flags, void *state, Signature *s,
                            SigMatch *m)
 {
+    SCEnter();
+
+    int ret = 0;
+
+    SCMutexLock(&f->m);
+    SCLogDebug("got lock %p", &f->m);
+
     DetectHttpCookieData *co = (DetectHttpCookieData *)m->ctx;
+
     HtpState *htp_state = (HtpState *)state;
     if (htp_state == NULL) {
         SCLogDebug("no HTTP layer state has been received, so no match");
-        return 0;
+        goto end;
     }
 
-    int ret = 0;
-    SCMutexLock(&f->m);
+    if (!(htp_state->flags & HTP_FLAG_STATE_OPEN)) {
+        SCLogDebug("HTP state not yet properly setup, so no match");
+        goto end;
+    }
+
+    SCLogDebug("htp_state %p, flow %p", htp_state, f);
+    SCLogDebug("htp_state->connp %p", htp_state->connp);
+    SCLogDebug("htp_state->connp->conn %p", htp_state->connp->conn);
+
     if (htp_state->connp == NULL || htp_state->connp->conn == NULL) {
         SCLogDebug("HTTP connection structure is NULL");
-        ret = 0;
         goto end;
     }
 
     htp_tx_t *tx = list_get(htp_state->connp->conn->transactions, 0);
-
     if (tx == NULL) {
         SCLogDebug("No HTTP transaction has received on the connection");
-        ret = 0;
         goto end;
     }
 
     htp_header_t *h = NULL;
     h = (htp_header_t *)table_getc(tx->request_headers, "Cookie");
     if (h == NULL) {
-        SCLogDebug("no HTTP Cookie hearder in the received request");
-        ret = 0;
+        SCLogDebug("no HTTP Cookie header in the received request");
         goto end;
     }
 
+    SCLogDebug("we have a cookie header");
+
     if (BinSearch((const uint8_t *)bstr_ptr(h->value), bstr_size(h->value), co->data,
             co->data_len) != NULL)
     {
@@ -109,7 +124,8 @@ int DetectHttpCookieMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx,
 
 end:
     SCMutexUnlock(&f->m);
-    return ret;
+    SCLogDebug("released lock %p", &f->m);
+    SCReturnInt(ret);
 }
 
 /**

src/detect-tls-version.c

@@ -47,6 +47,7 @@ void DetectTlsVersionRegister (void) {
     sigmatch_table[DETECT_AL_TLS_VERSION].name = "tls.version";
     sigmatch_table[DETECT_AL_TLS_VERSION].Match = NULL;
     sigmatch_table[DETECT_AL_TLS_VERSION].AppLayerMatch = DetectTlsVersionMatch;
+    sigmatch_table[DETECT_AL_TLS_VERSION].alproto = ALPROTO_TLS;
     sigmatch_table[DETECT_AL_TLS_VERSION].Setup = DetectTlsVersionSetup;
     sigmatch_table[DETECT_AL_TLS_VERSION].Free  = DetectTlsVersionFree;
     sigmatch_table[DETECT_AL_TLS_VERSION].RegisterTests = DetectTlsVersionRegisterTests;

src/detect.c

@@ -444,6 +444,8 @@ static int SigMatchSignaturesAppLayer(ThreadVars *th_v, DetectEngineCtx *de_ctx,
     SigMatch *sm = NULL;
     uint32_t idx, sig;
     uint8_t flags = 0;
+    uint16_t alproto = ALPROTO_UNKNOWN;
+    void *alstate = NULL;
 
     /* if we didn't get a sig group head, we
      * have nothing to do.... */
@@ -453,7 +455,20 @@ static int SigMatchSignaturesAppLayer(ThreadVars *th_v, DetectEngineCtx *de_ctx,
     }
 
     /* grab the protocol state we will detect on */
-    void *alstate = AppLayerGetProtoStateFromPacket(p);
+    if (p->flow == NULL) {
+        SCReturnInt(0);
+    }
+
+    SCMutexLock(&p->flow->m);
+    p->flow->use_cnt++;
+    alstate = AppLayerGetProtoStateFromPacket(p);
+    alproto = AppLayerGetProtoFromPacket(p);
+    SCMutexUnlock(&p->flow->m);
+
+    if (alproto == ALPROTO_UNKNOWN) {
+        SCLogDebug("application layer state proto still unknown");
+        SCReturnInt(0);
+    }
     if (alstate == NULL) {
         SCLogDebug("no application layer state to detect");
         SCReturnInt(0);
@@ -534,6 +549,12 @@ static int SigMatchSignaturesAppLayer(ThreadVars *th_v, DetectEngineCtx *de_ctx,
                     /* if no match function we assume this sm is a match */
                     match = 1;
                     SCLogDebug("no app layer match function, sigmatch is (pkt)Match only");
+                } else if (sigmatch_table[sm->type].alproto != alproto) {
+                    match = 0;
+                    SCLogDebug("app layer match function %s is for proto "
+                            "%"PRIu16", got proto %"PRIu16"",
+                            sigmatch_table[sm->type].name,
+                            sigmatch_table[sm->type].alproto, alproto);
                 } else {
                     match = sigmatch_table[sm->type].AppLayerMatch(th_v, det_ctx, p->flow, flags, alstate, s, sm);
                     SCLogDebug("sigmatch AppLayerMatch function returned match %"PRIu32"", match);
@@ -578,6 +599,9 @@ static int SigMatchSignaturesAppLayer(ThreadVars *th_v, DetectEngineCtx *de_ctx,
         }
     }
 
+    SCMutexLock(&p->flow->m);
+    p->flow->use_cnt--;
+    SCMutexUnlock(&p->flow->m);
     SCReturnInt(fmatch);
 }
 

src/detect.h

@@ -358,6 +358,8 @@ typedef struct SigTableElmt_ {
     int (*Match)(ThreadVars *, DetectEngineThreadCtx *, Packet *, Signature *, SigMatch *);
     /** AppLayer match function */
     int (*AppLayerMatch)(ThreadVars *, DetectEngineThreadCtx *, Flow *, uint8_t flags, void *alstate, Signature *, SigMatch *);
+    uint16_t alproto; /**< app layer proto from app-layer-protos.h this match
+                           applies to */
 
     int (*Setup)(DetectEngineCtx *, Signature *, SigMatch *, char *);
     void (*Free)(void *);

src/flow-util.c

@@ -9,6 +9,7 @@
 #include "flow-var.h"
 
 #include "util-var.h"
+#include "util-debug.h"
 
 /* Allocate a flow */
 Flow *FlowAlloc(void)
@@ -73,6 +74,9 @@ int FlowGetProtoMapping(uint8_t proto) {
  * we see from it. */
 void FlowInit(Flow *f, Packet *p)
 {
+    SCEnter();
+    SCLogDebug("flow %p", f);
+
     CLEAR_FLOW(f);
 
     f->proto = p->proto;
@@ -103,5 +107,7 @@ void FlowInit(Flow *f, Packet *p)
     COPY_TIMESTAMP(&p->ts, &f->startts);
 
     f->protomap = FlowGetProtoMapping(f->proto);
+
+    SCReturn;
 }
 

src/flow.c

@@ -293,6 +293,18 @@ void FlowSetIPOnlyFlag(Flow *f, char direction) {
     SCMutexUnlock(&f->m);
 }
 
+/** \brief increase the use cnt of a flow
+ *  \param tv thread vars (\todo unused?)
+ *  \param p packet with flow to decrease use cnt for
+ */
+void FlowIncrUsecnt(ThreadVars *tv, Packet *p) {
+    if (p == NULL || p->flow == NULL)
+        return;
+
+    SCMutexLock(&p->flow->m);
+    p->flow->use_cnt++;
+    SCMutexUnlock(&p->flow->m);
+}
 /** \brief decrease the use cnt of a flow
  *  \param tv thread vars (\todo unused?)
  *  \param p packet with flow to decrease use cnt for
@@ -680,6 +692,8 @@ void FlowInitFlowProto(void) {
  */
 
 static int FlowClearMemory(Flow* f, uint8_t proto_map) {
+    SCEnter();
+
     /* call the protocol specific free function if we have one */
     if (flow_proto[proto_map].Freefunc != NULL) {
         flow_proto[proto_map].Freefunc(f->protoctx);
@@ -687,7 +701,7 @@ static int FlowClearMemory(Flow* f, uint8_t proto_map) {
     f->protoctx = NULL;
 
     CLEAR_FLOW(f);
-    return 1;
+    SCReturnInt(1);
 }
 
 /**

src/stream-tcp.c

@@ -110,13 +110,19 @@ void StreamTcpReturnStreamSegments (TcpStream *stream)
 /** \brief Function to return the stream back to the pool. It returns the
  *         segments in the stream to the segment pool.
  *
+ *  This function is called when the flow is destroyed, so it should free
+ *  *everything* related to the tcp session. So including the app layer
+ *  data. We are guaranteed to only get here when the flow's use_cnt is 0.
+ *
  *  \param ssn Void ptr to the ssn.
  */
 void StreamTcpSessionClear(void *ssnptr)
 {
+    SCEnter();
+
     TcpSession *ssn = (TcpSession *)ssnptr;
     if (ssn == NULL)
-        return;
+        SCReturn;
 
     StreamTcpReturnStreamSegments(&ssn->client);
     StreamTcpReturnStreamSegments(&ssn->server);
@@ -133,36 +139,45 @@ void StreamTcpSessionClear(void *ssnptr)
     ssn_pool_cnt--;
     SCMutexUnlock(&ssn_pool_cnt_mutex);
 #endif
+
+    SCReturn;
 }
 
 /** \brief Function to return the stream back to the pool. It returns the
  *         segments in the stream to the segment pool.
  *
+ *  We don't clear out the app layer storage here as that is under protection
+ *  of the "use_cnt" reference counter in the flow. This function is called
+ *  when the use_cnt is always at least 1 (this pkt has incremented the flow
+ *  use_cnt itself), so we don't bother.
+ *
  *  \param p Packet used to identify the stream.
  */
 static void StreamTcpSessionPktFree (Packet *p)
 {
+    SCEnter();
+
     TcpSession *ssn = (TcpSession *)p->flow->protoctx;
     if (ssn == NULL)
-        return;
+        SCReturn;
 
     StreamTcpReturnStreamSegments(&ssn->client);
     StreamTcpReturnStreamSegments(&ssn->server);
-
-    AppLayerParserCleanupState(ssn);
-
+/*
     memset(ssn, 0, sizeof(TcpSession));
     SCMutexLock(&ssn_pool_mutex);
     PoolReturn(ssn_pool, p->flow->protoctx);
     SCMutexUnlock(&ssn_pool_mutex);
 
     p->flow->protoctx = NULL;
-
+*/
 #ifdef DEBUG
     SCMutexLock(&ssn_pool_cnt_mutex);
     ssn_pool_cnt--;
     SCMutexUnlock(&ssn_pool_cnt_mutex);
 #endif
+
+    SCReturn;
 }
 
 /** \brief Stream alloc function for the Pool
@@ -2230,6 +2245,9 @@ static int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt)
     if (ssn == NULL || ssn->state == TCP_NONE) {
         if (StreamTcpPacketStateNone(tv, p, stt, ssn) == -1)
             SCReturnInt(-1);
+
+        if (ssn != NULL)
+            SCLogDebug("ssn->alproto %"PRIu16"", ssn->alproto);
     } else {
         /* check if the packet is in right direction, when we missed the
            SYN packet and picked up midstream session. */
@@ -2713,7 +2731,7 @@ static int StreamTcpTest01 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -2800,9 +2818,7 @@ static int StreamTcpTest02 (void) {
     if (StreamTcpPacket(&tv, &p, &stt) == -1)
         goto end;
 
-    StreamTcpSessionPktFree(&p);
-    if (p.flow->protoctx != NULL)
-        goto end;
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -2870,7 +2886,7 @@ static int StreamTcpTest03 (void) {
             ((TcpSession *)(p.flow->protoctx))->server.next_seq != 11)
         goto end;
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -2931,7 +2947,7 @@ static int StreamTcpTest04 (void) {
             ((TcpSession *)(p.flow->protoctx))->server.next_seq != 20)
         goto end;
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3030,7 +3046,7 @@ static int StreamTcpTest05 (void) {
             ((TcpSession *)(p.flow->protoctx))->server.next_seq != 23)
         goto end;
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3161,7 +3177,7 @@ static int StreamTcpTest07 (void) {
             goto end;
         }
 
-        StreamTcpSessionPktFree(&p);
+        StreamTcpSessionClear(p.flow->protoctx);
         ret = 1;
     }
 end:
@@ -3247,7 +3263,7 @@ static int StreamTcpTest08 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3322,7 +3338,7 @@ static int StreamTcpTest09 (void) {
     if (((TcpSession *) (p.flow->protoctx))->client.seg_list->next == NULL)
         ret = 1;
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 end:
     StreamTcpFreeConfig(TRUE);
     return ret;
@@ -3413,7 +3429,7 @@ static int StreamTcpTest10 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3507,7 +3523,7 @@ static int StreamTcpTest11 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3593,7 +3609,7 @@ static int StreamTcpTest12 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3692,7 +3708,7 @@ static int StreamTcpTest13 (void) {
         goto end;
     }
 
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
 
     ret = 1;
 end:
@@ -3768,7 +3784,7 @@ static int StreamTcp4WHSTest01 (void) {
 
     ret = 1;
 end:
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
     StreamTcpFreeConfig(TRUE);
     return ret;
 }
@@ -3831,7 +3847,7 @@ static int StreamTcp4WHSTest02 (void) {
 
     ret = 1;
 end:
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
     StreamTcpFreeConfig(TRUE);
     return ret;
 }
@@ -3905,7 +3921,7 @@ static int StreamTcp4WHSTest03 (void) {
 
     ret = 1;
 end:
-    StreamTcpSessionPktFree(&p);
+    StreamTcpSessionClear(p.flow->protoctx);
     StreamTcpFreeConfig(TRUE);
     return ret;
 }