ip: discard IPv4 datagrams with overlapping segments.

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>

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index e5ebc83827abbcaaf82e1f46011540fc273c65f2..f80135e5feaa886000009db6dff75b2bc2d637b2 100644 (file)
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -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
 };
 

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index d14d741fb05e571d6d0b03248da77fd993debdee..960bf5eab59fc2412aa38bb9bdcadffe76a094e0 100644 (file)
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -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;

diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index b46e4cf9a55a1aa58e1fa344443e184053e05ffd..70289682a6701438aed99a00a9705c39fa4394d3 100644 (file)
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -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
 };