netdev - [PATCH net] net: ipv4: do not handle duplicate fragments as overlapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20181213022800.9D298E1116@unicorn.suse.cz>
Date:   Thu, 13 Dec 2018 03:28:00 +0100 (CET)
From:   Michal Kubecek <mkubecek@...e.cz>
To:     "David S. Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Peter Oskolkov <posk@...gle.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Gustavo Figueira <gfigueira@...e.com>
Subject: [PATCH net] net: ipv4: do not handle duplicate fragments as
 overlapping

Since commit 7969e5c40dfd ("ip: discard IPv4 datagrams with overlapping
segments.") IPv4 reassembly code drops the whole queue whenever an
overlapping fragment is received. However, the test is written in a way
which detects duplicate fragments as overlapping so that in environments
with many duplicate packets, fragmented packets may be undeliverable.

Add an extra test and for (potentially) duplicate fragment, only drop the
new fragment rather than the whole queue. Only starting offset and length
are checked, not the contents of the fragments as that would be too
expensive.  Check for duplicity with last (tail) fragment first as in real
life scenarios this should be the most frequent case and we would have to
iterate through the whole "run" otherwise.

Fixes: 7969e5c40dfd ("ip: discard IPv4 datagrams with overlapping segments.")
Signed-off-by: Michal Kubecek <mkubecek@...e.cz>
---
 net/ipv4/ip_fragment.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index aa0b22697998..f09e3683b209 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -436,6 +436,10 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 			ip4_frag_append_to_last_run(&qp->q, skb);
 		else
 			ip4_frag_create_run(&qp->q, skb);
+	} else if (offset == prev_tail->ip_defrag_offset &&
+		   skb->len == prev_tail->len) {
+		/* potential duplicate of last fragment */
+		goto err;
 	} else {
 		/* Binary search. Note that skb can become the first fragment,
 		 * but not the last (covered above).
@@ -449,8 +453,16 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 			else if (offset >= skb1->ip_defrag_offset +
 						FRAG_CB(skb1)->frag_run_len)
 				rbn = &parent->rb_right;
-			else /* Found an overlap with skb1. */
+			else {
+				/* check for potential duplicate */
+				while (skb1 && skb1->ip_defrag_offset < offset)
+					skb1 = FRAG_CB(skb1)->next_frag;
+				if (skb1 && offset == skb1->ip_defrag_offset &&
+				    skb->len == skb1->len)
+					goto err;
+				/* Found an overlap */
 				goto overlap;
+			}
 		} while (*rbn);
 		/* Here we have parent properly set, and rbn pointing to
 		 * one of its NULL left/right children. Insert skb.
-- 
2.19.2