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

From: Michal Kubecek
Date: Wed Dec 12 2018 - 21:28:05 EST


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@xxxxxxx>
---
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