ip: discard IPv4 datagrams with overlapping segments.
authorPeter Oskolkov <posk@google.com>
Thu, 2 Aug 2018 23:34:37 +0000 (23:34 +0000)
committerDavid S. Miller <davem@davemloft.net>
Mon, 6 Aug 2018 00:16:46 +0000 (17:16 -0700)
This behavior is required in IPv6, and there is little need
to tolerate overlapping fragments in IPv4. This change
simplifies the code and eliminates potential DDoS attack vectors.

Tested: ran ip_defrag selftest (not yet available uptream).

Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Peter Oskolkov <posk@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/uapi/linux/snmp.h
net/ipv4/ip_fragment.c
net/ipv4/proc.c

index e5ebc83827abbcaaf82e1f46011540fc273c65f2..f80135e5feaa886000009db6dff75b2bc2d637b2 100644 (file)
@@ -56,6 +56,7 @@ enum
        IPSTATS_MIB_ECT1PKTS,                   /* InECT1Pkts */
        IPSTATS_MIB_ECT0PKTS,                   /* InECT0Pkts */
        IPSTATS_MIB_CEPKTS,                     /* InCEPkts */
+       IPSTATS_MIB_REASM_OVERLAPS,             /* ReasmOverlaps */
        __IPSTATS_MIB_MAX
 };
 
index d14d741fb05e571d6d0b03248da77fd993debdee..960bf5eab59fc2412aa38bb9bdcadffe76a094e0 100644 (file)
@@ -277,6 +277,7 @@ static int ip_frag_reinit(struct ipq *qp)
 /* Add new segment to existing queue. */
 static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 {
+       struct net *net = container_of(qp->q.net, struct net, ipv4.frags);
        struct sk_buff *prev, *next;
        struct net_device *dev;
        unsigned int fragsize;
@@ -357,65 +358,23 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
        }
 
 found:
-       /* We found where to put this one.  Check for overlap with
-        * preceding fragment, and, if needed, align things so that
-        * any overlaps are eliminated.
+       /* RFC5722, Section 4, amended by Errata ID : 3089
+        *                          When reassembling an IPv6 datagram, if
+        *   one or more its constituent fragments is determined to be an
+        *   overlapping fragment, the entire datagram (and any constituent
+        *   fragments) MUST be silently discarded.
+        *
+        * We do the same here for IPv4.
         */
-       if (prev) {
-               int i = (prev->ip_defrag_offset + prev->len) - offset;
 
-               if (i > 0) {
-                       offset += i;
-                       err = -EINVAL;
-                       if (end <= offset)
-                               goto err;
-                       err = -ENOMEM;
-                       if (!pskb_pull(skb, i))
-                               goto err;
-                       if (skb->ip_summed != CHECKSUM_UNNECESSARY)
-                               skb->ip_summed = CHECKSUM_NONE;
-               }
-       }
+       /* Is there an overlap with the previous fragment? */
+       if (prev &&
+           (prev->ip_defrag_offset + prev->len) > offset)
+               goto discard_qp;
 
-       err = -ENOMEM;
-
-       while (next && next->ip_defrag_offset < end) {
-               int i = end - next->ip_defrag_offset; /* overlap is 'i' bytes */
-
-               if (i < next->len) {
-                       int delta = -next->truesize;
-
-                       /* Eat head of the next overlapped fragment
-                        * and leave the loop. The next ones cannot overlap.
-                        */
-                       if (!pskb_pull(next, i))
-                               goto err;
-                       delta += next->truesize;
-                       if (delta)
-                               add_frag_mem_limit(qp->q.net, delta);
-                       next->ip_defrag_offset += i;
-                       qp->q.meat -= i;
-                       if (next->ip_summed != CHECKSUM_UNNECESSARY)
-                               next->ip_summed = CHECKSUM_NONE;
-                       break;
-               } else {
-                       struct sk_buff *free_it = next;
-
-                       /* Old fragment is completely overridden with
-                        * new one drop it.
-                        */
-                       next = next->next;
-
-                       if (prev)
-                               prev->next = next;
-                       else
-                               qp->q.fragments = next;
-
-                       qp->q.meat -= free_it->len;
-                       sub_frag_mem_limit(qp->q.net, free_it->truesize);
-                       kfree_skb(free_it);
-               }
-       }
+       /* Is there an overlap with the next fragment? */
+       if (next && next->ip_defrag_offset < end)
+               goto discard_qp;
 
        /* Note : skb->ip_defrag_offset and skb->dev share the same location */
        dev = skb->dev;
@@ -463,6 +422,10 @@ found:
        skb_dst_drop(skb);
        return -EINPROGRESS;
 
+discard_qp:
+       inet_frag_kill(&qp->q);
+       err = -EINVAL;
+       __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 err:
        kfree_skb(skb);
        return err;
index b46e4cf9a55a1aa58e1fa344443e184053e05ffd..70289682a6701438aed99a00a9705c39fa4394d3 100644 (file)
@@ -119,6 +119,7 @@ static const struct snmp_mib snmp4_ipextstats_list[] = {
        SNMP_MIB_ITEM("InECT1Pkts", IPSTATS_MIB_ECT1PKTS),
        SNMP_MIB_ITEM("InECT0Pkts", IPSTATS_MIB_ECT0PKTS),
        SNMP_MIB_ITEM("InCEPkts", IPSTATS_MIB_CEPKTS),
+       SNMP_MIB_ITEM("ReasmOverlaps", IPSTATS_MIB_REASM_OVERLAPS),
        SNMP_MIB_SENTINEL
 };