defrag: don't modify packet if defrag fails

If defrag fails dur to an invalid decoding, we are not modifying
the origin packet anymore.

diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c
index 268286472ab77aa0e12dce91a60a07df2badc340..e1442fa9d7e3f94ab626f5eaa62adc8ebb4d7caa 100644 (file)
--- a/src/decode-ipv4.c
+++ b/src/decode-ipv4.c
@@ -46,8 +46,6 @@
 #include "util-print.h"
 #include "util-profiling.h"
 
-#include "tmqh-packetpool.h"
-
 /* Generic validation
  *
  * [--type--][--len---]
@@ -517,8 +515,6 @@ static int DecodeIPV4Packet(Packet *p, uint8_t *pkt, uint16_t len)
 
 int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, uint16_t len, PacketQueue *pq)
 {
-    int ret;
-
     SCPerfCounterIncr(dtv->counter_ipv4, tv->sc_perf_pca);
 
     SCLogDebug("pkt %p len %"PRIu16"", pkt, len);
@@ -533,15 +529,9 @@ int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
 
     /* If a fragment, pass off for re-assembly. */
     if (unlikely(IPV4_GET_IPOFFSET(p) > 0 || IPV4_GET_MF(p) == 1)) {
-        Packet *rp = Defrag(tv, dtv, p);
+        Packet *rp = Defrag(tv, dtv, p, pq);
         if (rp != NULL) {
-            /* Got re-assembled packet, re-run through decoder. */
-            ret = DecodeIPV4(tv, dtv, rp, (void *)rp->ip4h, IPV4_GET_IPLEN(rp), pq);
-            if (unlikely(ret != TM_ECODE_OK)) {
-                TmqhOutputPacketpool(tv, rp);
-            } else {
-                PacketEnqueue(pq, rp);
-            }
+            PacketEnqueue(pq, rp);
         }
         p->flags |= PKT_IS_FRAGMENT;
         return TM_ECODE_OK;

diff --git a/src/decode-ipv6.c b/src/decode-ipv6.c
index 013926b6aa7ae03610a9d36f5da6c59268b3d9c5..95fe2cd781e824687de743c707c5568b52204882 100644 (file)
--- a/src/decode-ipv6.c
+++ b/src/decode-ipv6.c
@@ -44,8 +44,6 @@
 #include "util-profiling.h"
 #include "host.h"
 
-#include "tmqh-packetpool.h"
-
 #define IPV6_EXTHDRS     ip6eh.ip6_exthdrs
 #define IPV6_EH_CNT      ip6eh.ip6_exthdrs_cnt
 
@@ -596,18 +594,9 @@ int DecodeIPV6(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
 
     /* Pass to defragger if a fragment. */
     if (IPV6_EXTHDR_ISSET_FH(p)) {
-        Packet *rp = Defrag(tv, dtv, p);
+        Packet *rp = Defrag(tv, dtv, p, pq);
         if (rp != NULL) {
-            ret = DecodeIPV6(tv, dtv, rp, (uint8_t *)rp->ip6h, IPV6_GET_PLEN(rp) + IPV6_HEADER_LEN, pq);
-            if (unlikely(ret != TM_ECODE_OK)) {
-                TmqhOutputPacketpool(tv, rp);
-            } else {
-                /* add the tp to the packet queue. */
-                PacketEnqueue(pq,rp);
-                /* Not really a tunnel packet, but we're piggybacking that
-                 * functionality for now. */
-                SET_TUNNEL_PKT(p);
-            }
+            PacketEnqueue(pq,rp);
         }
     }
 

diff --git a/src/decode.c b/src/decode.c
index 3928ed944e929a98f9be9bb81b0239a0ebee7e68..8989f4f9176c7630bc2ab4be658fe3ee2838c763 100644 (file)
--- a/src/decode.c
+++ b/src/decode.c
@@ -302,11 +302,14 @@ Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t
     p->ts.tv_sec = parent->ts.tv_sec;
     p->ts.tv_usec = parent->ts.tv_usec;
     p->datalink = DLT_RAW;
-
-    /* set tunnel flags */
-
     /* tell new packet it's part of a tunnel */
     SET_TUNNEL_PKT(p);
+
+    SCReturnPtr(p, "Packet");
+}
+
+void PacketDefragPktFinishSetup(Packet *p, Packet *parent)
+{
     /* tell parent packet it's part of a tunnel */
     SET_TUNNEL_PKT(parent);
 
@@ -317,7 +320,6 @@ Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t
      * is the packet we will now run through the system separately. We do
      * check it against the ip/port/other header checks though */
     DecodeSetNoPayloadInspectionFlag(parent);
-    SCReturnPtr(p, "Packet");
 }
 
 void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv)

diff --git a/src/decode.h b/src/decode.h
index 444b579bc8ab46723445a86e4fad7bb15901dffb..fe98764f4e7b677c4f11f7df0b9ef3f666b26b64 100644 (file)
--- a/src/decode.h
+++ b/src/decode.h
@@ -815,6 +815,7 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *, ThreadVars *);
 Packet *PacketTunnelPktSetup(ThreadVars *tv, DecodeThreadVars *dtv, Packet *parent,
                              uint8_t *pkt, uint16_t len, uint8_t proto, PacketQueue *pq);
 Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto);
+void PacketDefragPktFinishSetup(Packet *p, Packet *parent);
 Packet *PacketGetFromQueueOrAlloc(void);
 Packet *PacketGetFromAlloc(void);
 void PacketFree(Packet *p);

diff --git a/src/defrag.c b/src/defrag.c
index 9a9603d144f96e7511f85005f2b87b3863dfd0c0..f0dccc5fa7dfde802525577a70f3a95044cb8769 100644 (file)
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -55,6 +55,9 @@
 #include "defrag-queue.h"
 #include "defrag-config.h"
 
+#include "tmqh-packetpool.h"
+#include "decode.h"
+
 #ifdef UNITTESTS
 #include "util-unittest.h"
 #endif
@@ -466,7 +469,7 @@ done:
  * \todo Allocate packet buffers from a pool.
  */
 static Packet *
-DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker, Packet *p)
+DefragInsertFrag(ThreadVars *tv, DecodeThreadVars *dtv, DefragTracker *tracker, Packet *p, PacketQueue *pq)
 {
     Packet *r = NULL;
     int ltrim = 0;
@@ -714,17 +717,31 @@ insert:
             if (r != NULL && tv != NULL && dtv != NULL) {
                 SCPerfCounterIncr(dtv->counter_defrag_ipv4_reassembled,
                     tv->sc_perf_pca);
+                if (pq && DecodeIPV4(tv, dtv, r, (void *)r->ip4h,
+                               IPV4_GET_IPLEN(r), pq) != TM_ECODE_OK) {
+                    TmqhOutputPacketpool(tv, r);
+                } else {
+                    PacketDefragPktFinishSetup(r, p);
+                }
             }
         }
         else if (tracker->af == AF_INET6) {
             r = Defrag6Reassemble(tv, tracker, p);
             if (r != NULL && tv != NULL && dtv != NULL) {
                 SCPerfCounterIncr(dtv->counter_defrag_ipv6_reassembled,
-                    tv->sc_perf_pca);
+                        tv->sc_perf_pca);
+                if (pq && DecodeIPV6(tv, dtv, r, (uint8_t *)r->ip6h,
+                               IPV6_GET_PLEN(r) + IPV6_HEADER_LEN,
+                               pq) != TM_ECODE_OK) {
+                    TmqhOutputPacketpool(tv, r);
+                } else {
+                    PacketDefragPktFinishSetup(r, p);
+                }
             }
         }
     }
 
+
 done:
     if (overlap) {
         if (af == AF_INET) {
@@ -825,7 +842,7 @@ DefragGetTracker(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p)
  *     NULL is returned.
  */
 Packet *
-Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p)
+Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, PacketQueue *pq)
 {
     uint16_t frag_offset;
     uint8_t more_frags;
@@ -866,7 +883,7 @@ Defrag(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p)
     if (tracker == NULL)
         return NULL;
 
-    Packet *rp = DefragInsertFrag(tv, dtv, tracker, p);
+    Packet *rp = DefragInsertFrag(tv, dtv, tracker, p, pq);
     DefragTrackerRelease(tracker);
 
     return rp;
@@ -1076,12 +1093,12 @@ DefragInOrderSimpleTest(void)
     if (p3 == NULL)
         goto end;
 
-    if (Defrag(NULL, NULL, p1) != NULL)
+    if (Defrag(NULL, NULL, p1, NULL) != NULL)
         goto end;
-    if (Defrag(NULL, NULL, p2) != NULL)
+    if (Defrag(NULL, NULL, p2, NULL) != NULL)
         goto end;
 
-    reassembled = Defrag(NULL, NULL, p3);
+    reassembled = Defrag(NULL, NULL, p3, NULL);
     if (reassembled == NULL) {
         goto end;
     }
@@ -1153,12 +1170,12 @@ DefragReverseSimpleTest(void)
     if (p3 == NULL)
         goto end;
 
-    if (Defrag(NULL, NULL, p3) != NULL)
+    if (Defrag(NULL, NULL, p3, NULL) != NULL)
         goto end;
-    if (Defrag(NULL, NULL, p2) != NULL)
+    if (Defrag(NULL, NULL, p2, NULL) != NULL)
         goto end;
 
-    reassembled = Defrag(NULL, NULL, p1);
+    reassembled = Defrag(NULL, NULL, p1, NULL);
     if (reassembled == NULL)
         goto end;
 
@@ -1225,11 +1242,11 @@ IPV6DefragInOrderSimpleTest(void)
     if (p3 == NULL)
         goto end;
 
-    if (Defrag(NULL, NULL, p1) != NULL)
+    if (Defrag(NULL, NULL, p1, NULL) != NULL)
         goto end;
-    if (Defrag(NULL, NULL, p2) != NULL)
+    if (Defrag(NULL, NULL, p2, NULL) != NULL)
         goto end;
-    reassembled = Defrag(NULL, NULL, p3);
+    reassembled = Defrag(NULL, NULL, p3, NULL);
     if (reassembled == NULL)
         goto end;
 
@@ -1295,11 +1312,11 @@ IPV6DefragReverseSimpleTest(void)
     if (p3 == NULL)
         goto end;
 
-    if (Defrag(NULL, NULL, p3) != NULL)
+    if (Defrag(NULL, NULL, p3, NULL) != NULL)
         goto end;
-    if (Defrag(NULL, NULL, p2) != NULL)
+    if (Defrag(NULL, NULL, p2, NULL) != NULL)
         goto end;
-    reassembled = Defrag(NULL, NULL, p1);
+    reassembled = Defrag(NULL, NULL, p1, NULL);
     if (reassembled == NULL)
         goto end;
 
@@ -1417,7 +1434,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
 
     /* Send all but the last. */
     for (i = 0; i < 9; i++) {
-        Packet *tp = Defrag(NULL, NULL, packets[i]);
+        Packet *tp = Defrag(NULL, NULL, packets[i], NULL);
         if (tp != NULL) {
             SCFree(tp);
             goto end;
@@ -1428,7 +1445,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
     }
     int overlap = 0;
     for (; i < 16; i++) {
-        Packet *tp = Defrag(NULL, NULL, packets[i]);
+        Packet *tp = Defrag(NULL, NULL, packets[i], NULL);
         if (tp != NULL) {
             SCFree(tp);
             goto end;
@@ -1442,7 +1459,7 @@ DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
     }
 
     /* And now the last one. */
-    Packet *reassembled = Defrag(NULL, NULL, packets[16]);
+    Packet *reassembled = Defrag(NULL, NULL, packets[16], NULL);
     if (reassembled == NULL) {
         goto end;
     }
@@ -1552,7 +1569,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
 
     /* Send all but the last. */
     for (i = 0; i < 9; i++) {
-        Packet *tp = Defrag(NULL, NULL, packets[i]);
+        Packet *tp = Defrag(NULL, NULL, packets[i], NULL);
         if (tp != NULL) {
             SCFree(tp);
             goto end;
@@ -1563,7 +1580,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
     }
     int overlap = 0;
     for (; i < 16; i++) {
-        Packet *tp = Defrag(NULL, NULL, packets[i]);
+        Packet *tp = Defrag(NULL, NULL, packets[i], NULL);
         if (tp != NULL) {
             SCFree(tp);
             goto end;
@@ -1576,7 +1593,7 @@ IPV6DefragDoSturgesNovakTest(int policy, u_char *expected, size_t expected_len)
         goto end;
 
     /* And now the last one. */
-    Packet *reassembled = Defrag(NULL, NULL, packets[16]);
+    Packet *reassembled = Defrag(NULL, NULL, packets[16], NULL);
     if (reassembled == NULL)
         goto end;
     if (memcmp(GET_PKT_DATA(reassembled) + 40, expected, expected_len) != 0)
@@ -2036,7 +2053,7 @@ DefragTimeoutTest(void)
         if (p == NULL)
             goto end;
 
-        Packet *tp = Defrag(NULL, NULL, p);
+        Packet *tp = Defrag(NULL, NULL, p, NULL);
 
         SCFree(p);
 
@@ -2053,7 +2070,7 @@ DefragTimeoutTest(void)
         goto end;
 
     p->ts.tv_sec += (defrag_context->timeout + 1);
-    Packet *tp = Defrag(NULL, NULL, p);
+    Packet *tp = Defrag(NULL, NULL, p, NULL);
 
     if (tp != NULL) {
         SCFree(tp);
@@ -2101,7 +2118,7 @@ DefragIPv4NoDataTest(void)
         goto end;
 
     /* We do not expect a packet returned. */
-    if (Defrag(NULL, NULL, p) != NULL)
+    if (Defrag(NULL, NULL, p, NULL) != NULL)
         goto end;
 
     /* The fragment should have been ignored so no fragments should
@@ -2140,7 +2157,7 @@ DefragIPv4TooLargeTest(void)
         goto end;
 
     /* We do not expect a packet returned. */
-    if (Defrag(NULL, NULL, p) != NULL)
+    if (Defrag(NULL, NULL, p, NULL) != NULL)
         goto end;
     if (!ENGINE_ISSET_EVENT(p, IPV4_FRAG_PKT_TOO_LARGE))
         goto end;
@@ -2181,18 +2198,18 @@ DefragVlanTest(void) {
         goto end;
 
     /* With no VLAN IDs set, packets should re-assemble. */
-    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+    if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL)
         goto end;
-    if ((r = Defrag(NULL, NULL, p2)) == NULL)
+    if ((r = Defrag(NULL, NULL, p2, NULL)) == NULL)
         goto end;
     SCFree(r);
 
     /* With mismatched VLANs, packets should not re-assemble. */
     p1->vlan_id[0] = 1;
     p2->vlan_id[0] = 2;
-    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+    if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL)
         goto end;
-    if ((r = Defrag(NULL, NULL, p2)) != NULL)
+    if ((r = Defrag(NULL, NULL, p2, NULL)) != NULL)
         goto end;
 
     /* Pass. */
@@ -2226,9 +2243,9 @@ DefragVlanQinQTest(void) {
         goto end;
 
     /* With no VLAN IDs set, packets should re-assemble. */
-    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+    if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL)
         goto end;
-    if ((r = Defrag(NULL, NULL, p2)) == NULL)
+    if ((r = Defrag(NULL, NULL, p2, NULL)) == NULL)
         goto end;
     SCFree(r);
 
@@ -2237,9 +2254,9 @@ DefragVlanQinQTest(void) {
     p2->vlan_id[0] = 1;
     p1->vlan_id[1] = 1;
     p2->vlan_id[1] = 2;
-    if ((r = Defrag(NULL, NULL, p1)) != NULL)
+    if ((r = Defrag(NULL, NULL, p1, NULL)) != NULL)
         goto end;
-    if ((r = Defrag(NULL, NULL, p2)) != NULL)
+    if ((r = Defrag(NULL, NULL, p2, NULL)) != NULL)
         goto end;
 
     /* Pass. */

diff --git a/src/defrag.h b/src/defrag.h
index cf5abe87241bac7cfa16358aeb0d9d9c31758c2d..dd4aac09bd98ebb1eb6df84ec46254763babfb72 100644 (file)
--- a/src/defrag.h
+++ b/src/defrag.h
@@ -133,7 +133,7 @@ void DefragReload(void); /**< use only in unittests */
 
 uint8_t DefragGetOsPolicy(Packet *);
 void DefragTrackerFreeFrags(DefragTracker *);
-Packet *Defrag(ThreadVars *, DecodeThreadVars *, Packet *);
+Packet *Defrag(ThreadVars *, DecodeThreadVars *, Packet *, PacketQueue *);
 void DefragRegisterTests(void);
 
 #endif /* __DEFRAG_H__ */