From: Peter Oskolkov
commit 0ff89efb524631ac9901b81446b453c29711c376 upstream
The current behavior of IP defragmentation is inconsistent:
- some overlapping/wrong length fragments are dropped without
affecting the queue;
- most overlapping fragments cause the whole frag queue to be dropped.
This patch brings consistency: if a bad fragment is detected,
the whole frag queue is dropped. Two major benefits:
- fail fast: corrupted frag queues are cleared immediately, instead of
by timeout;
- testing of overlapping fragments is now much easier: any kind of
random fragment length mutation now leads to the frag queue being
discarded (IP packet dropped); before this patch, some overlaps were
"corrected", with tests not seeing expected packet drops.
Note that in one case (see "if (end&7)" conditional) the current
behavior is preserved as there are concerns that this could be
legitimate padding.
Signed-off-by: Peter Oskolkov
Reviewed-by: Eric Dumazet
Reviewed-by: Willem de Bruijn
Signed-off-by: David S. Miller
Signed-off-by: Zubin Mithra
---
Backport Note:
- Syzkaller reported a UAF, as 0ff89efb5246 ("ip: fail fast on IP defrag
errors") was not applied prior to applying d5f9565c8d5a ("net: ipv4: do
not handle duplicate fragments as overlapping").
Conflicts occur when 0ff89efb5246 is now applied onto 4.14.y/4.19.y,
which this patch addresses.
- An alternative to this patch would be to do the following :-
- revert "net: ipv4: do not handle duplicate fragments as overlapping"
(d5f9565c8d5ad on 4.19.y, 95b4b711444a on 4.14.y)
- apply "ip: fail fast on IP defrag errors" (0ff89efb5246)
- apply "net: ipv4: do not handle duplicate fragments as overlapping"
(ade446403bfb)
net/ipv4/ip_fragment.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index f8bbd693c19c2..03576ff7557e0 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -382,7 +382,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
*/
if (end < qp->q.len ||
((qp->q.flags & INET_FRAG_LAST_IN) && end != qp->q.len))
- goto err;
+ goto discard_qp;
qp->q.flags |= INET_FRAG_LAST_IN;
qp->q.len = end;
} else {
@@ -394,20 +394,20 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
if (end > qp->q.len) {
/* Some bits beyond end -> corruption. */
if (qp->q.flags & INET_FRAG_LAST_IN)
- goto err;
+ goto discard_qp;
qp->q.len = end;
}
}
if (end == offset)
- goto err;
+ goto discard_qp;
err = -ENOMEM;
if (!pskb_pull(skb, skb_network_offset(skb) + ihl))
- goto err;
+ goto discard_qp;
err = pskb_trim_rcsum(skb, end - offset);
if (err)
- goto err;
+ goto discard_qp;
/* Note : skb->rbnode and skb->dev share the same location. */
dev = skb->dev;
@@ -425,6 +425,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
* fragment.
*/
+ err = -EINVAL;
/* Find out where to put this fragment. */
prev_tail = qp->q.fragments_tail;
if (!prev_tail)
@@ -433,7 +434,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
/* This is the common case: skb goes to the end. */
/* Detect and discard overlaps. */
if (offset < prev_tail->ip_defrag_offset + prev_tail->len)
- goto discard_qp;
+ goto overlap;
if (offset == prev_tail->ip_defrag_offset + prev_tail->len)
ip4_frag_append_to_last_run(&qp->q, skb);
else
@@ -456,7 +457,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
end <= skb1_run_end)
goto err; /* No new data, potential duplicate */
else
- goto discard_qp; /* Found an overlap */
+ goto overlap; /* Found an overlap */
} while (*rbn);
/* Here we have parent properly set, and rbn pointing to
* one of its NULL left/right children. Insert skb.
@@ -493,16 +494,18 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff
*skb)
skb->_skb_refdst = 0UL;
err = ip_frag_reasm(qp, skb, prev_tail, dev);
skb->_skb_refdst = orefdst;
+
