From: Stefan Bader <stefan.bader@canonical.com>
To: stable <stable@vger.kernel.org>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Eric Dumazet <edumazet@google.com>,
Sasha Levin <sashal@kernel.org>, Peter Oskolkov <posk@google.com>,
Ben Hutchings <ben.hutchings@codethink.co.uk>,
Andy Whitcroft <andy.whitcroft@canonical.com>,
Greg KH <gregkh@linuxfoundation.org>
Subject: [PATCH 2/4] ip: fail fast on IP defrag errors
Date: Wed, 29 May 2019 12:25:40 +0200 [thread overview]
Message-ID: <20190529102542.17742-3-stefan.bader@canonical.com> (raw)
In-Reply-To: <20190529102542.17742-1-stefan.bader@canonical.com>
From: Peter Oskolkov <posk@google.com>
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 <posk@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(backported from commit 0ff89efb524631ac9901b81446b453c29711c376)
[smb: context adjustments and ignoring those changes already done
in backport for "net: ipv4: do not handle duplicate fragments
as overlapping"]
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
net/ipv4/ip_fragment.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 5387e6ab78d7..a53652c8c0fd 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;
@@ -434,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
@@ -457,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.
@@ -494,15 +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;
+ if (err)
+ inet_frag_kill(&qp->q);
return err;
}
skb_dst_drop(skb);
return -EINPROGRESS;
+overlap:
+ IP_INC_STATS_BH(net, IPSTATS_MIB_REASM_OVERLAPS);
discard_qp:
inet_frag_kill(&qp->q);
- IP_INC_STATS_BH(net, IPSTATS_MIB_REASM_OVERLAPS);
err:
kfree_skb(skb);
return err;
--
2.17.1
next prev parent reply other threads:[~2019-05-29 10:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-29 10:25 [PATCH 0/4] ipv6: frags: fixups for linux-4.4.y Stefan Bader
2019-05-29 10:25 ` [PATCH 1/4] ipv4: ipv6: netfilter: Adjust the frag mem limit when truesize changes Stefan Bader
2019-05-29 10:37 ` Greg KH
2019-05-29 12:31 ` Stefan Bader
2019-06-04 13:32 ` Stefan Bader
2019-05-29 10:25 ` Stefan Bader [this message]
2019-05-29 10:25 ` [PATCH 3/4] net: IP defrag: encapsulate rbtree defrag code into callable functions Stefan Bader
2019-05-29 10:25 ` [PATCH 4/4] ipv6: frags: Use inet_frag_pull_head() in ip6_expire_frag_queue() Stefan Bader
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190529102542.17742-3-stefan.bader@canonical.com \
--to=stefan.bader@canonical.com \
--cc=andy.whitcroft@canonical.com \
--cc=ben.hutchings@codethink.co.uk \
--cc=edumazet@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=posk@google.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).