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.

Suggested-by: David S. Miller <da...@davemloft.net>
Signed-off-by: Peter Oskolkov <p...@google.com>
Signed-off-by: Eric Dumazet <eduma...@google.com>
Cc: Florian Westphal <f...@strlen.de>
---
 include/uapi/linux/snmp.h |  1 +
 net/ipv4/ip_fragment.c    | 75 ++++++++++-----------------------------
 net/ipv4/proc.c           |  1 +
 3 files changed, 21 insertions(+), 56 deletions(-)

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index e5ebc83827ab..da1a144f1a51 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -40,6 +40,7 @@ enum
        IPSTATS_MIB_REASMREQDS,                 /* ReasmReqds */
        IPSTATS_MIB_REASMOKS,                   /* ReasmOKs */
        IPSTATS_MIB_REASMFAILS,                 /* ReasmFails */
+       IPSTATS_MIB_REASM_OVERLAPS,             /* ReasmOverlaps */
        IPSTATS_MIB_FRAGOKS,                    /* FragOKs */
        IPSTATS_MIB_FRAGFAILS,                  /* FragFails */
        IPSTATS_MIB_FRAGCREATES,                /* FragCreates */
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index d14d741fb05e..960bf5eab59f 100644
--- 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 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff 
*skb)
        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 b46e4cf9a55a..70289682a670 100644
--- 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
 };
 
-- 
2.18.0.597.ga71716f1ad-goog

Reply via email to