Netdev List
 help / color / mirror / Atom feed
* [PATCH stable 4.4 03/11] net: modify skb_rbtree_purge to return the truesize of all purged skbs.
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Peter Oskolkov <posk@google.com>

[ Upstream commit 385114dec8a49b5e5945e77ba7de6356106713f4 ]

Tested: see the next patch is the series.

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 include/linux/skbuff.h | 2 +-
 net/core/skbuff.c      | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a490dd7..8bfefdd 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2273,7 +2273,7 @@ static inline void __skb_queue_purge(struct sk_buff_head *list)
 		kfree_skb(skb);
 }
 
-void skb_rbtree_purge(struct rb_root *root);
+unsigned int skb_rbtree_purge(struct rb_root *root);
 
 void *netdev_alloc_frag(unsigned int fragsz);
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8a57bba..49f73fb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2380,23 +2380,27 @@ EXPORT_SYMBOL(skb_queue_purge);
 /**
  *	skb_rbtree_purge - empty a skb rbtree
  *	@root: root of the rbtree to empty
+ *	Return value: the sum of truesizes of all purged skbs.
  *
  *	Delete all buffers on an &sk_buff rbtree. Each buffer is removed from
  *	the list and one reference dropped. This function does not take
  *	any lock. Synchronization should be handled by the caller (e.g., TCP
  *	out-of-order queue is protected by the socket lock).
  */
-void skb_rbtree_purge(struct rb_root *root)
+unsigned int skb_rbtree_purge(struct rb_root *root)
 {
 	struct rb_node *p = rb_first(root);
+	unsigned int sum = 0;
 
 	while (p) {
 		struct sk_buff *skb = rb_entry(p, struct sk_buff, rbnode);
 
 		p = rb_next(p);
 		rb_erase(&skb->rbnode, root);
+		sum += skb->truesize;
 		kfree_skb(skb);
 	}
+	return sum;
 }
 
 /**
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 00/11] fix FragmentSmack in stable branch (CVE-2018-5391)
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet

There is one CVE: CVE-2018-5391 kernel: IP fragments with random offsets allow a 
remote denial of service (FragmentSmack), 
A fix is a merge commit in the Linux kernel tree:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c30f1fc041b74ecdb072dd44f858750414b8b19f

consisting of the following commits:
7969e5c40dfd04799d4341f1b7cd266b6e47f227 ip: discard IPv4 datagrams with overlapping segments.
385114dec8a49b5e5945e77ba7de6356106713f4 net: modify skb_rbtree_purge to return the truesize of all purged skbs.
fa0f527358bd900ef92f925878ed6bfbd51305cc ip: use rb trees for IP frag queue.

All above patches are with rb tree to fix this CVE, which is very similar the CVE-2018-5390, that I have backport
to stable 4.4 branch in last year.

In these patchset, I will backport some patches to fix CVE-2018-5391 with rb tree.  

Dan Carpenter (1):
  ipv4: frags: precedence bug in ip_expire()

Eric Dumazet (2):
  net: speed up skb_rbtree_purge()
  inet: frags: get rif of inet_frag_evicting()

Florian Westphal (1):
  ipv6: defrag: drop non-last frags smaller than min mtu

Michal Kubecek (1):
  net: ipv4: do not handle duplicate fragments as overlapping

Peter Oskolkov (5):
  ip: discard IPv4 datagrams with overlapping segments.
  net: modify skb_rbtree_purge to return the truesize of all purged
    skbs.
  ip: use rb trees for IP frag queue.
  ip: add helpers to process in-order fragments faster.
  ip: process in-order fragments efficiently

Taehee Yoo (1):
  ip: frags: fix crash in ip_do_fragment()

 include/linux/skbuff.h                  |   4 +-
 include/net/inet_frag.h                 |  12 +-
 include/uapi/linux/snmp.h               |   1 +
 net/core/skbuff.c                       |  17 +-
 net/ipv4/inet_fragment.c                |  16 +-
 net/ipv4/ip_fragment.c                  | 410 +++++++++++++++++++-------------
 net/ipv4/proc.c                         |   1 +
 net/ipv6/netfilter/nf_conntrack_reasm.c |   6 +
 net/ipv6/reassembly.c                   |   9 +-
 9 files changed, 292 insertions(+), 184 deletions(-)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH stable 4.4 10/11] ip: frags: fix crash in ip_do_fragment()
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Taehee Yoo <ap420073@gmail.com>

[ Upstream commit 5d407b071dc369c26a38398326ee2be53651cfe4 ]

A kernel crash occurrs when defragmented packet is fragmented
in ip_do_fragment().
In defragment routine, skb_orphan() is called and
skb->ip_defrag_offset is set. but skb->sk and
skb->ip_defrag_offset are same union member. so that
frag->sk is not NULL.
Hence crash occurrs in skb->sk check routine in ip_do_fragment() when
defragmented packet is fragmented.

test commands:
   %iptables -t nat -I POSTROUTING -j MASQUERADE
   %hping3 192.168.4.2 -s 1000 -p 2000 -d 60000

splat looks like:
[  261.069429] kernel BUG at net/ipv4/ip_output.c:636!
[  261.075753] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  261.083854] CPU: 1 PID: 1349 Comm: hping3 Not tainted 4.19.0-rc2+ #3
[  261.100977] RIP: 0010:ip_do_fragment+0x1613/0x2600
[  261.106945] Code: e8 e2 38 e3 fe 4c 8b 44 24 18 48 8b 74 24 08 e9 92 f6 ff ff 80 3c 02 00 0f 85 da 07 00 00 48 8b b5 d0 00 00 00 e9 25 f6 ff ff <0f> 0b 0f 0b 44 8b 54 24 58 4c 8b 4c 24 18 4c 8b 5c 24 60 4c 8b 6c
[  261.127015] RSP: 0018:ffff8801031cf2c0 EFLAGS: 00010202
[  261.134156] RAX: 1ffff1002297537b RBX: ffffed0020639e6e RCX: 0000000000000004
[  261.142156] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880114ba9bd8
[  261.150157] RBP: ffff880114ba8a40 R08: ffffed0022975395 R09: ffffed0022975395
[  261.158157] R10: 0000000000000001 R11: ffffed0022975394 R12: ffff880114ba9ca4
[  261.166159] R13: 0000000000000010 R14: ffff880114ba9bc0 R15: dffffc0000000000
[  261.174169] FS:  00007fbae2199700(0000) GS:ffff88011b400000(0000) knlGS:0000000000000000
[  261.183012] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  261.189013] CR2: 00005579244fe000 CR3: 0000000119bf4000 CR4: 00000000001006e0
[  261.198158] Call Trace:
[  261.199018]  ? dst_output+0x180/0x180
[  261.205011]  ? save_trace+0x300/0x300
[  261.209018]  ? ip_copy_metadata+0xb00/0xb00
[  261.213034]  ? sched_clock_local+0xd4/0x140
[  261.218158]  ? kill_l4proto+0x120/0x120 [nf_conntrack]
[  261.223014]  ? rt_cpu_seq_stop+0x10/0x10
[  261.227014]  ? find_held_lock+0x39/0x1c0
[  261.233008]  ip_finish_output+0x51d/0xb50
[  261.237006]  ? ip_fragment.constprop.56+0x220/0x220
[  261.243011]  ? nf_ct_l4proto_register_one+0x5b0/0x5b0 [nf_conntrack]
[  261.250152]  ? rcu_is_watching+0x77/0x120
[  261.255010]  ? nf_nat_ipv4_out+0x1e/0x2b0 [nf_nat_ipv4]
[  261.261033]  ? nf_hook_slow+0xb1/0x160
[  261.265007]  ip_output+0x1c7/0x710
[  261.269005]  ? ip_mc_output+0x13f0/0x13f0
[  261.273002]  ? __local_bh_enable_ip+0xe9/0x1b0
[  261.278152]  ? ip_fragment.constprop.56+0x220/0x220
[  261.282996]  ? nf_hook_slow+0xb1/0x160
[  261.287007]  raw_sendmsg+0x21f9/0x4420
[  261.291008]  ? dst_output+0x180/0x180
[  261.297003]  ? sched_clock_cpu+0x126/0x170
[  261.301003]  ? find_held_lock+0x39/0x1c0
[  261.306155]  ? stop_critical_timings+0x420/0x420
[  261.311004]  ? check_flags.part.36+0x450/0x450
[  261.315005]  ? _raw_spin_unlock_irq+0x29/0x40
[  261.320995]  ? _raw_spin_unlock_irq+0x29/0x40
[  261.326142]  ? cyc2ns_read_end+0x10/0x10
[  261.330139]  ? raw_bind+0x280/0x280
[  261.334138]  ? sched_clock_cpu+0x126/0x170
[  261.338995]  ? check_flags.part.36+0x450/0x450
[  261.342991]  ? __lock_acquire+0x4500/0x4500
[  261.348994]  ? inet_sendmsg+0x11c/0x500
[  261.352989]  ? dst_output+0x180/0x180
[  261.357012]  inet_sendmsg+0x11c/0x500
[ ... ]

v2:
 - clear skb->sk at reassembly routine.(Eric Dumarzet)

Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv4/ip_fragment.c                  | 1 +
 net/ipv6/netfilter/nf_conntrack_reasm.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 500dfdd..0e75881 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -661,6 +661,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 			nextp = &fp->next;
 			fp->prev = NULL;
 			memset(&fp->rbnode, 0, sizeof(fp->rbnode));
+			fp->sk = NULL;
 			head->data_len += fp->len;
 			head->len += fp->len;
 			if (head->ip_summed != fp->ip_summed)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index c5033a2..17f80dd 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -454,6 +454,7 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct net_device *dev)
 		else if (head->ip_summed == CHECKSUM_COMPLETE)
 			head->csum = csum_add(head->csum, fp->csum);
 		head->truesize += fp->truesize;
+		fp->sk = NULL;
 	}
 	sub_frag_mem_limit(fq->q.net, head->truesize);
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 09/11] net: ipv4: do not handle duplicate fragments as overlapping
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Michal Kubecek <mkubecek@suse.cz>

[ Upstream commit ade446403bfb79d3528d56071a84b15351a139ad ]

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. For similar reason, linear list ("run") of a rbtree node is not
iterated, we only check if the new fragment is a subset of the interval
covered by existing consecutive fragments.

v2: instead of an exact check iterating through linear list of an rbtree
node, only check if the new fragment is subset of the "run" (suggested
by Eric Dumazet)

Fixes: 7969e5c40dfd ("ip: discard IPv4 datagrams with overlapping segments.")
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv4/ip_fragment.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 61f4216..500dfdd 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -400,10 +400,10 @@ 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 rb_node **rbn, *parent;
 	struct sk_buff *skb1, *prev_tail;
+	int ihl, end, skb1_run_end;
 	struct net_device *dev;
 	unsigned int fragsize;
 	int flags, offset;
-	int ihl, end;
 	int err = -ENOENT;
 	u8 ecn;
 
@@ -473,7 +473,9 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 	 *   overlapping fragment, the entire datagram (and any constituent
 	 *   fragments) MUST be silently discarded.
 	 *
-	 * We do the same here for IPv4 (and increment an snmp counter).
+	 * We do the same here for IPv4 (and increment an snmp counter) but
+	 * we do not want to drop the whole queue in response to a duplicate
+	 * fragment.
 	 */
 
 	/* Find out where to put this fragment.  */
@@ -497,13 +499,17 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 		do {
 			parent = *rbn;
 			skb1 = rb_to_skb(parent);
+			skb1_run_end = FRAG_CB(skb1)->offset +
+				       FRAG_CB(skb1)->frag_run_len;
 			if (end <= FRAG_CB(skb1)->offset)
 				rbn = &parent->rb_left;
-			else if (offset >= FRAG_CB(skb1)->offset +
-						FRAG_CB(skb1)->frag_run_len)
+			else if (offset >= skb1_run_end)
 				rbn = &parent->rb_right;
-			else /* Found an overlap with skb1. */
-				goto discard_qp;
+			else if (offset >= FRAG_CB(skb1)->offset &&
+				 end <= skb1_run_end)
+				goto err; /* No new data, potential duplicate */
+			else
+				goto discard_qp; /* Found an overlap */
 		} while (*rbn);
 		/* Here we have parent properly set, and rbn pointing to
 		 * one of its NULL left/right children. Insert skb.
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 11/11] ipv4: frags: precedence bug in ip_expire()
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Dan Carpenter <dan.carpenter@oracle.com>

[ Upstream commit 70837ffe3085c9a91488b52ca13ac84424da1042 ]

We accidentally removed the parentheses here, but they are required
because '!' has higher precedence than '&'.

Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv4/ip_fragment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 0e75881..eb8955c 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -257,7 +257,7 @@ static void ip_expire(unsigned long arg)
 	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMFAILS);
 	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMTIMEOUT);
 
-	if (!qp->q.flags & INET_FRAG_FIRST_IN)
+	if (!(qp->q.flags & INET_FRAG_FIRST_IN))
 		goto out;
 
 	/* sk_buff::dev and sk_buff::rbnode are unionized. So we
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 05/11] ip: use rb trees for IP frag queue.
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Peter Oskolkov <posk@google.com>

[ Upstream commit fa0f527358bd900ef92f925878ed6bfbd51305cc ]

Similar to TCP OOO RX queue, it makes sense to use rb trees to store
IP fragments, so that OOO fragments are inserted faster.

Tested:

- a follow-up patch contains a rather comprehensive ip defrag
  self-test (functional)
- ran neper `udp_stream -c -H <host> -F 100 -l 300 -T 20`:
    netstat --statistics
    Ip:
        282078937 total packets received
        0 forwarded
        0 incoming packets discarded
        946760 incoming packets delivered
        18743456 requests sent out
        101 fragments dropped after timeout
        282077129 reassemblies required
        944952 packets reassembled ok
        262734239 packet reassembles failed
   (The numbers/stats above are somewhat better re:
    reassemblies vs a kernel without this patchset. More
    comprehensive performance testing TBD).

Reported-by: Jann Horn <jannh@google.com>
Reported-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 include/linux/skbuff.h                  |   2 +-
 include/net/inet_frag.h                 |   3 +-
 net/ipv4/inet_fragment.c                |  16 ++-
 net/ipv4/ip_fragment.c                  | 190 ++++++++++++++++++--------------
 net/ipv6/netfilter/nf_conntrack_reasm.c |   1 +
 net/ipv6/reassembly.c                   |   1 +
 6 files changed, 121 insertions(+), 92 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8bfefdd..5c73c79 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -556,7 +556,7 @@ struct sk_buff {
 				struct skb_mstamp skb_mstamp;
 			};
 		};
-		struct rb_node	rbnode; /* used in netem & tcp stack */
+		struct rb_node	rbnode; /* used in netem, ip4 defrag, and tcp stack */
 	};
 	struct sock		*sk;
 	struct net_device	*dev;
diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index 09472b8..861d24c 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -45,7 +45,8 @@ struct inet_frag_queue {
 	struct timer_list	timer;
 	struct hlist_node	list;
 	atomic_t		refcnt;
-	struct sk_buff		*fragments;
+	struct sk_buff		*fragments;  /* Used in IPv6. */
+	struct rb_root		rb_fragments; /* Used in IPv4. */
 	struct sk_buff		*fragments_tail;
 	ktime_t			stamp;
 	int			len;
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index b2001b2..2b3a926 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -306,12 +306,16 @@ void inet_frag_destroy(struct inet_frag_queue *q, struct inet_frags *f)
 	/* Release all fragment data. */
 	fp = q->fragments;
 	nf = q->net;
-	while (fp) {
-		struct sk_buff *xp = fp->next;
-
-		sum_truesize += fp->truesize;
-		frag_kfree_skb(nf, f, fp);
-		fp = xp;
+	if (fp) {
+		do {
+			struct sk_buff *xp = fp->next;
+
+			sum_truesize += fp->truesize;
+			kfree_skb(fp);
+			fp = xp;
+		} while (fp);
+	} else {
+		sum_truesize = skb_rbtree_purge(&q->rb_fragments);
 	}
 	sum = sum_truesize + f->qsize;
 
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 264f382..e820eb9 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -194,7 +194,7 @@ static bool frag_expire_skip_icmp(u32 user)
  */
 static void ip_expire(unsigned long arg)
 {
-	struct sk_buff *clone, *head;
+	struct sk_buff *head = NULL;
 	const struct iphdr *iph;
 	struct net *net;
 	struct ipq *qp;
@@ -211,14 +211,31 @@ static void ip_expire(unsigned long arg)
 
 	ipq_kill(qp);
 	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMFAILS);
-
-	head = qp->q.fragments;
-
 	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMTIMEOUT);
 
-	if (!(qp->q.flags & INET_FRAG_FIRST_IN) || !head)
+	if (!qp->q.flags & INET_FRAG_FIRST_IN)
 		goto out;
 
+	/* sk_buff::dev and sk_buff::rbnode are unionized. So we
+	 * pull the head out of the tree in order to be able to
+	 * deal with head->dev.
+	 */
+	if (qp->q.fragments) {
+		head = qp->q.fragments;
+		qp->q.fragments = head->next;
+	} else {
+		head = skb_rb_first(&qp->q.rb_fragments);
+		if (!head)
+			goto out;
+		rb_erase(&head->rbnode, &qp->q.rb_fragments);
+		memset(&head->rbnode, 0, sizeof(head->rbnode));
+		barrier();
+	}
+	if (head == qp->q.fragments_tail)
+		qp->q.fragments_tail = NULL;
+
+	sub_frag_mem_limit(qp->q.net, head->truesize);
+
 	head->dev = dev_get_by_index_rcu(net, qp->iif);
 	if (!head->dev)
 		goto out;
@@ -237,20 +254,17 @@ static void ip_expire(unsigned long arg)
 	    (skb_rtable(head)->rt_type != RTN_LOCAL))
 		goto out;
 
-	clone = skb_clone(head, GFP_ATOMIC);
-
 	/* Send an ICMP "Fragment Reassembly Timeout" message. */
-	if (clone) {
-		spin_unlock(&qp->q.lock);
-		icmp_send(clone, ICMP_TIME_EXCEEDED,
-			  ICMP_EXC_FRAGTIME, 0);
-		consume_skb(clone);
-		goto out_rcu_unlock;
-	}
+	spin_unlock(&qp->q.lock);
+	icmp_send(head, ICMP_TIME_EXCEEDED, ICMP_EXC_FRAGTIME, 0);
+	goto out_rcu_unlock;
+
 out:
 	spin_unlock(&qp->q.lock);
 out_rcu_unlock:
 	rcu_read_unlock();
+	if (head)
+		kfree_skb(head);
 	ipq_put(qp);
 }
 
@@ -294,7 +308,7 @@ static int ip_frag_too_far(struct ipq *qp)
 	end = atomic_inc_return(&peer->rid);
 	qp->rid = end;
 
-	rc = qp->q.fragments && (end - start) > max;
+	rc = qp->q.fragments_tail && (end - start) > max;
 
 	if (rc) {
 		struct net *net;
@@ -308,7 +322,6 @@ static int ip_frag_too_far(struct ipq *qp)
 
 static int ip_frag_reinit(struct ipq *qp)
 {
-	struct sk_buff *fp;
 	unsigned int sum_truesize = 0;
 
 	if (!mod_timer(&qp->q.timer, jiffies + qp->q.net->timeout)) {
@@ -316,20 +329,14 @@ static int ip_frag_reinit(struct ipq *qp)
 		return -ETIMEDOUT;
 	}
 
-	fp = qp->q.fragments;
-	do {
-		struct sk_buff *xp = fp->next;
-
-		sum_truesize += fp->truesize;
-		kfree_skb(fp);
-		fp = xp;
-	} while (fp);
+	sum_truesize = skb_rbtree_purge(&qp->q.rb_fragments);
 	sub_frag_mem_limit(qp->q.net, sum_truesize);
 
 	qp->q.flags = 0;
 	qp->q.len = 0;
 	qp->q.meat = 0;
 	qp->q.fragments = NULL;
+	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
 	qp->iif = 0;
 	qp->ecn = 0;
@@ -341,7 +348,8 @@ static int ip_frag_reinit(struct ipq *qp)
 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 rb_node **rbn, *parent;
+	struct sk_buff *skb1;
 	struct net_device *dev;
 	unsigned int fragsize;
 	int flags, offset;
@@ -404,56 +412,60 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 	if (err)
 		goto err;
 
-	/* Find out which fragments are in front and at the back of us
-	 * in the chain of fragments so far.  We must know where to put
-	 * this fragment, right?
-	 */
-	prev = qp->q.fragments_tail;
-	if (!prev || FRAG_CB(prev)->offset < offset) {
-		next = NULL;
-		goto found;
-	}
-	prev = NULL;
-	for (next = qp->q.fragments; next != NULL; next = next->next) {
-		if (FRAG_CB(next)->offset >= offset)
-			break;	/* bingo! */
-		prev = next;
-	}
+	/* Note : skb->rbnode and skb->dev share the same location. */
+	dev = skb->dev;
+	/* Makes sure compiler wont do silly aliasing games */
+	barrier();
 
-found:
 	/* 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.
+	 * We do the same here for IPv4 (and increment an snmp counter).
 	 */
-	/* Is there an overlap with the previous fragment? */
-	if (prev &&
-	    (FRAG_CB(prev)->offset + prev->len) > offset)
-		goto discard_qp;
-
-	/* Is there an overlap with the next fragment? */
-	if (next && FRAG_CB(next)->offset < end)
-		goto discard_qp;
-
-	FRAG_CB(skb)->offset = offset;
 
-	/* Insert this fragment in the chain of fragments. */
-	skb->next = next;
-	if (!next)
+	/* Find out where to put this fragment.  */
+	skb1 = qp->q.fragments_tail;
+	if (!skb1) {
+		/* This is the first fragment we've received. */
+		rb_link_node(&skb->rbnode, NULL, &qp->q.rb_fragments.rb_node);
 		qp->q.fragments_tail = skb;
-	if (prev)
-		prev->next = skb;
-	else
-		qp->q.fragments = skb;
+	} else if ((FRAG_CB(skb1)->offset + skb1->len) < end) {
+		/* This is the common/special case: skb goes to the end. */
+		/* Detect and discard overlaps. */
+		if (offset < (FRAG_CB(skb1)->offset + skb1->len))
+			goto discard_qp;
+		/* Insert after skb1. */
+		rb_link_node(&skb->rbnode, &skb1->rbnode, &skb1->rbnode.rb_right);
+		qp->q.fragments_tail = skb;
+	} else {
+		/* Binary search. Note that skb can become the first fragment, but
+		 * not the last (covered above). */
+		rbn = &qp->q.rb_fragments.rb_node;
+		do {
+			parent = *rbn;
+			skb1 = rb_to_skb(parent);
+			if (end <= FRAG_CB(skb1)->offset)
+				rbn = &parent->rb_left;
+			else if (offset >= FRAG_CB(skb1)->offset + skb1->len)
+				rbn = &parent->rb_right;
+			else /* Found an overlap with skb1. */
+				goto discard_qp;
+		} while (*rbn);
+		/* Here we have parent properly set, and rbn pointing to
+		 * one of its NULL left/right children. Insert skb. */
+		rb_link_node(&skb->rbnode, parent, rbn);
+	}
+	rb_insert_color(&skb->rbnode, &qp->q.rb_fragments);
 
-	dev = skb->dev;
 	if (dev) {
 		qp->iif = dev->ifindex;
 		skb->dev = NULL;
 	}
+	FRAG_CB(skb)->offset = offset;
+
 	qp->q.stamp = skb->tstamp;
 	qp->q.meat += skb->len;
 	qp->ecn |= ecn;
@@ -475,7 +487,7 @@ found:
 		unsigned long orefdst = skb->_skb_refdst;
 
 		skb->_skb_refdst = 0UL;
-		err = ip_frag_reasm(qp, prev, dev);
+		err = ip_frag_reasm(qp, skb, dev);
 		skb->_skb_refdst = orefdst;
 		return err;
 	}
@@ -492,15 +504,15 @@ err:
 	return err;
 }
 
-
 /* Build a new IP datagram from all its fragments. */
-
-static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
+static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 			 struct net_device *dev)
 {
 	struct net *net = container_of(qp->q.net, struct net, ipv4.frags);
 	struct iphdr *iph;
-	struct sk_buff *fp, *head = qp->q.fragments;
+	struct sk_buff *fp, *head = skb_rb_first(&qp->q.rb_fragments);
+	struct sk_buff **nextp; /* To build frag_list. */
+	struct rb_node *rbn;
 	int len;
 	int ihlen;
 	int err;
@@ -514,25 +526,21 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
 		goto out_fail;
 	}
 	/* Make the one we just received the head. */
-	if (prev) {
-		head = prev->next;
-		fp = skb_clone(head, GFP_ATOMIC);
+	if (head != skb) {
+		fp = skb_clone(skb, GFP_ATOMIC);
 		if (!fp)
 			goto out_nomem;
 
-		fp->next = head->next;
-		if (!fp->next)
+		rb_replace_node(&skb->rbnode, &fp->rbnode, &qp->q.rb_fragments);
+		if (qp->q.fragments_tail == skb)
 			qp->q.fragments_tail = fp;
-		prev->next = fp;
-
-		skb_morph(head, qp->q.fragments);
-		head->next = qp->q.fragments->next;
-
-		consume_skb(qp->q.fragments);
-		qp->q.fragments = head;
+		skb_morph(skb, head);
+		rb_replace_node(&head->rbnode, &skb->rbnode,
+				&qp->q.rb_fragments);
+		consume_skb(head);
+		head = skb;
 	}
 
-	WARN_ON(!head);
 	WARN_ON(FRAG_CB(head)->offset != 0);
 
 	/* Allocate a new buffer for the datagram. */
@@ -557,24 +565,35 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
 		clone = alloc_skb(0, GFP_ATOMIC);
 		if (!clone)
 			goto out_nomem;
-		clone->next = head->next;
-		head->next = clone;
 		skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
 		skb_frag_list_init(head);
 		for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
 			plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
 		clone->len = clone->data_len = head->data_len - plen;
-		head->data_len -= clone->len;
-		head->len -= clone->len;
+		skb->truesize += clone->truesize;
 		clone->csum = 0;
 		clone->ip_summed = head->ip_summed;
 		add_frag_mem_limit(qp->q.net, clone->truesize);
+		skb_shinfo(head)->frag_list = clone;
+		nextp = &clone->next;
+	} else {
+		nextp = &skb_shinfo(head)->frag_list;
 	}
 
-	skb_shinfo(head)->frag_list = head->next;
 	skb_push(head, head->data - skb_network_header(head));
 
-	for (fp=head->next; fp; fp = fp->next) {
+	/* Traverse the tree in order, to build frag_list. */
+	rbn = rb_next(&head->rbnode);
+	rb_erase(&head->rbnode, &qp->q.rb_fragments);
+	while (rbn) {
+		struct rb_node *rbnext = rb_next(rbn);
+		fp = rb_to_skb(rbn);
+		rb_erase(rbn, &qp->q.rb_fragments);
+		rbn = rbnext;
+		*nextp = fp;
+		nextp = &fp->next;
+		fp->prev = NULL;
+		memset(&fp->rbnode, 0, sizeof(fp->rbnode));
 		head->data_len += fp->len;
 		head->len += fp->len;
 		if (head->ip_summed != fp->ip_summed)
@@ -585,7 +604,9 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
 	}
 	sub_frag_mem_limit(qp->q.net, head->truesize);
 
+	*nextp = NULL;
 	head->next = NULL;
+	head->prev = NULL;
 	head->dev = dev;
 	head->tstamp = qp->q.stamp;
 	IPCB(head)->frag_max_size = max(qp->max_df_size, qp->q.max_size);
@@ -613,6 +634,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
 
 	IP_INC_STATS_BH(net, IPSTATS_MIB_REASMOKS);
 	qp->q.fragments = NULL;
+	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
 	return 0;
 
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 5a9ae56..9cd8863 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -472,6 +472,7 @@ nf_ct_frag6_reasm(struct frag_queue *fq, struct net_device *dev)
 					  head->csum);
 
 	fq->q.fragments = NULL;
+	fq->q.rb_fragments = RB_ROOT;
 	fq->q.fragments_tail = NULL;
 
 	/* all original skbs are linked into the NFCT_FRAG6_CB(head).orig */
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index ee4789b..adc7512 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -499,6 +499,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
 	IP6_INC_STATS_BH(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
 	rcu_read_unlock();
 	fq->q.fragments = NULL;
+	fq->q.rb_fragments = RB_ROOT;
 	fq->q.fragments_tail = NULL;
 	return 1;
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 02/11] ip: discard IPv4 datagrams with overlapping segments.
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Peter Oskolkov <posk@google.com>

[ Upstream commit 7969e5c40dfd04799d4341f1b7cd266b6e47f227 ]

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.

Tested: ran ip_defrag selftest (not yet available uptream).

Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Peter Oskolkov <posk@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 include/uapi/linux/snmp.h |  1 +
 net/ipv4/ip_fragment.c    | 71 +++++++++++++----------------------------------
 net/ipv4/proc.c           |  1 +
 3 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index 25a9ad8..9de808e 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -55,6 +55,7 @@ enum
 	IPSTATS_MIB_ECT1PKTS,			/* InECT1Pkts */
 	IPSTATS_MIB_ECT0PKTS,			/* InECT0Pkts */
 	IPSTATS_MIB_CEPKTS,			/* InCEPkts */
+	IPSTATS_MIB_REASM_OVERLAPS,		/* ReasmOverlaps */
 	__IPSTATS_MIB_MAX
 };
 
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 7291565..4e64879 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -342,6 +342,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;
@@ -422,60 +423,22 @@ 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 = (FRAG_CB(prev)->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;
-		}
-	}
-
-	err = -ENOMEM;
+	/* Is there an overlap with the previous fragment? */
+	if (prev &&
+	    (FRAG_CB(prev)->offset + prev->len) > offset)
+		goto discard_qp;
 
-	while (next && FRAG_CB(next)->offset < end) {
-		int i = end - FRAG_CB(next)->offset; /* overlap is 'i' bytes */
-
-		if (i < next->len) {
-			/* Eat head of the next overlapped fragment
-			 * and leave the loop. The next ones cannot overlap.
-			 */
-			if (!pskb_pull(next, i))
-				goto err;
-			FRAG_CB(next)->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 && FRAG_CB(next)->offset < end)
+		goto discard_qp;
 
 	FRAG_CB(skb)->offset = offset;
 
@@ -522,6 +485,10 @@ found:
 	skb_dst_drop(skb);
 	return -EINPROGRESS;
 
+discard_qp:
+	ipq_kill(qp);
+	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 3abd9d7..55545d0 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -132,6 +132,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
 };
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 06/11] ipv6: defrag: drop non-last frags smaller than min mtu
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Florian Westphal <fw@strlen.de>

[ Upstream commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6 ]

don't bother with pathological cases, they only waste cycles.
IPv6 requires a minimum MTU of 1280 so we should never see fragments
smaller than this (except last frag).

v3: don't use awkward "-offset + len"
v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
    There were concerns that there could be even smaller frags
    generated by intermediate nodes, e.g. on radio networks.

Cc: Peter Oskolkov <posk@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
 net/ipv6/reassembly.c                   | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 9cd8863..c5033a2 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -602,6 +602,10 @@ struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
 	hdr = ipv6_hdr(clone);
 	fhdr = (struct frag_hdr *)skb_transport_header(clone);
 
+	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
+	    fhdr->frag_off & htons(IP6_MF))
+		return -EINVAL;
+
 	skb_orphan(skb);
 	fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
 		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index adc7512..44c7f4c 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -549,6 +549,10 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
 		return 1;
 	}
 
+	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
+	    fhdr->frag_off & htons(IP6_MF))
+		goto fail_hdr;
+
 	fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
 		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
 	if (fq) {
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH stable 4.4 08/11] ip: process in-order fragments efficiently
From: Mao Wenan @ 2019-01-23  2:19 UTC (permalink / raw)
  To: netdev, gregkh, eric.dumazet, davem, stable, edumazet
In-Reply-To: <1548209986-83527-1-git-send-email-maowenan@huawei.com>

From: Peter Oskolkov <posk@google.com>

[ Upstream commit a4fd284a1f8fd4b6c59aa59db2185b1e17c5c11c ]

This patch changes the runtime behavior of IP defrag queue:
incoming in-order fragments are added to the end of the current
list/"run" of in-order fragments at the tail.

On some workloads, UDP stream performance is substantially improved:

RX: ./udp_stream -F 10 -T 2 -l 60
TX: ./udp_stream -c -H <host> -F 10 -T 5 -l 60

with this patchset applied on a 10Gbps receiver:

  throughput=9524.18
  throughput_units=Mbit/s

upstream (net-next):

  throughput=4608.93
  throughput_units=Mbit/s

Reported-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv4/inet_fragment.c |   2 +-
 net/ipv4/ip_fragment.c   | 110 +++++++++++++++++++++++++++++------------------
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index 2b3a926..046c6c3 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -315,7 +315,7 @@ void inet_frag_destroy(struct inet_frag_queue *q, struct inet_frags *f)
 			fp = xp;
 		} while (fp);
 	} else {
-		sum_truesize = skb_rbtree_purge(&q->rb_fragments);
+		sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
 	}
 	sum = sum_truesize + f->qsize;
 
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 73ec3a9..61f4216 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -139,8 +139,8 @@ int ip_frag_mem(struct net *net)
 	return sum_frag_mem_limit(&net->ipv4.frags);
 }
 
-static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
-			 struct net_device *dev);
+static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
+			struct sk_buff *prev_tail, struct net_device *dev);
 
 struct ip4_create_arg {
 	struct iphdr *iph;
@@ -271,7 +271,12 @@ static void ip_expire(unsigned long arg)
 		head = skb_rb_first(&qp->q.rb_fragments);
 		if (!head)
 			goto out;
-		rb_erase(&head->rbnode, &qp->q.rb_fragments);
+		if (FRAG_CB(head)->next_frag)
+			rb_replace_node(&head->rbnode,
+					&FRAG_CB(head)->next_frag->rbnode,
+					&qp->q.rb_fragments);
+		else
+			rb_erase(&head->rbnode, &qp->q.rb_fragments);
 		memset(&head->rbnode, 0, sizeof(head->rbnode));
 		barrier();
 	}
@@ -373,7 +378,7 @@ static int ip_frag_reinit(struct ipq *qp)
 		return -ETIMEDOUT;
 	}
 
-	sum_truesize = skb_rbtree_purge(&qp->q.rb_fragments);
+	sum_truesize = inet_frag_rbtree_purge(&qp->q.rb_fragments);
 	sub_frag_mem_limit(qp->q.net, sum_truesize);
 
 	qp->q.flags = 0;
@@ -382,6 +387,7 @@ static int ip_frag_reinit(struct ipq *qp)
 	qp->q.fragments = NULL;
 	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
+	qp->q.last_run_head = NULL;
 	qp->iif = 0;
 	qp->ecn = 0;
 
@@ -393,7 +399,7 @@ 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 rb_node **rbn, *parent;
-	struct sk_buff *skb1;
+	struct sk_buff *skb1, *prev_tail;
 	struct net_device *dev;
 	unsigned int fragsize;
 	int flags, offset;
@@ -471,38 +477,41 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 	 */
 
 	/* Find out where to put this fragment.  */
-	skb1 = qp->q.fragments_tail;
-	if (!skb1) {
-		/* This is the first fragment we've received. */
-		rb_link_node(&skb->rbnode, NULL, &qp->q.rb_fragments.rb_node);
-		qp->q.fragments_tail = skb;
-	} else if ((FRAG_CB(skb1)->offset + skb1->len) < end) {
-		/* This is the common/special case: skb goes to the end. */
+	prev_tail = qp->q.fragments_tail;
+	if (!prev_tail)
+		ip4_frag_create_run(&qp->q, skb);  /* First fragment. */
+	else if (FRAG_CB(prev_tail)->offset + prev_tail->len < end) {
+		/* This is the common case: skb goes to the end. */
 		/* Detect and discard overlaps. */
-		if (offset < (FRAG_CB(skb1)->offset + skb1->len))
+		if (offset < FRAG_CB(prev_tail)->offset + prev_tail->len)
 			goto discard_qp;
-		/* Insert after skb1. */
-		rb_link_node(&skb->rbnode, &skb1->rbnode, &skb1->rbnode.rb_right);
-		qp->q.fragments_tail = skb;
+		if (offset == FRAG_CB(prev_tail)->offset + prev_tail->len)
+			ip4_frag_append_to_last_run(&qp->q, skb);
+		else
+			ip4_frag_create_run(&qp->q, skb);
 	} else {
-		/* Binary search. Note that skb can become the first fragment, but
-		 * not the last (covered above). */
+		/* Binary search. Note that skb can become the first fragment,
+		 * but not the last (covered above).
+		 */
 		rbn = &qp->q.rb_fragments.rb_node;
 		do {
 			parent = *rbn;
 			skb1 = rb_to_skb(parent);
 			if (end <= FRAG_CB(skb1)->offset)
 				rbn = &parent->rb_left;
-			else if (offset >= FRAG_CB(skb1)->offset + skb1->len)
+			else if (offset >= FRAG_CB(skb1)->offset +
+						FRAG_CB(skb1)->frag_run_len)
 				rbn = &parent->rb_right;
 			else /* Found an overlap with skb1. */
 				goto discard_qp;
 		} while (*rbn);
 		/* Here we have parent properly set, and rbn pointing to
-		 * one of its NULL left/right children. Insert skb. */
+		 * one of its NULL left/right children. Insert skb.
+		 */
+		ip4_frag_init_run(skb);
 		rb_link_node(&skb->rbnode, parent, rbn);
+		rb_insert_color(&skb->rbnode, &qp->q.rb_fragments);
 	}
-	rb_insert_color(&skb->rbnode, &qp->q.rb_fragments);
 
 	if (dev) {
 		qp->iif = dev->ifindex;
@@ -531,7 +540,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 		unsigned long orefdst = skb->_skb_refdst;
 
 		skb->_skb_refdst = 0UL;
-		err = ip_frag_reasm(qp, skb, dev);
+		err = ip_frag_reasm(qp, skb, prev_tail, dev);
 		skb->_skb_refdst = orefdst;
 		return err;
 	}
@@ -550,7 +559,7 @@ err:
 
 /* Build a new IP datagram from all its fragments. */
 static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
-			 struct net_device *dev)
+			 struct sk_buff *prev_tail, struct net_device *dev)
 {
 	struct net *net = container_of(qp->q.net, struct net, ipv4.frags);
 	struct iphdr *iph;
@@ -575,10 +584,16 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 		if (!fp)
 			goto out_nomem;
 
-		rb_replace_node(&skb->rbnode, &fp->rbnode, &qp->q.rb_fragments);
+		FRAG_CB(fp)->next_frag = FRAG_CB(skb)->next_frag;
+		if (RB_EMPTY_NODE(&skb->rbnode))
+			FRAG_CB(prev_tail)->next_frag = fp;
+		else
+			rb_replace_node(&skb->rbnode, &fp->rbnode,
+					&qp->q.rb_fragments);
 		if (qp->q.fragments_tail == skb)
 			qp->q.fragments_tail = fp;
 		skb_morph(skb, head);
+		FRAG_CB(skb)->next_frag = FRAG_CB(head)->next_frag;
 		rb_replace_node(&head->rbnode, &skb->rbnode,
 				&qp->q.rb_fragments);
 		consume_skb(head);
@@ -614,7 +629,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 		for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
 			plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
 		clone->len = clone->data_len = head->data_len - plen;
-		skb->truesize += clone->truesize;
+		head->truesize += clone->truesize;
 		clone->csum = 0;
 		clone->ip_summed = head->ip_summed;
 		add_frag_mem_limit(qp->q.net, clone->truesize);
@@ -627,24 +642,36 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 	skb_push(head, head->data - skb_network_header(head));
 
 	/* Traverse the tree in order, to build frag_list. */
+	fp = FRAG_CB(head)->next_frag;
 	rbn = rb_next(&head->rbnode);
 	rb_erase(&head->rbnode, &qp->q.rb_fragments);
-	while (rbn) {
-		struct rb_node *rbnext = rb_next(rbn);
-		fp = rb_to_skb(rbn);
-		rb_erase(rbn, &qp->q.rb_fragments);
-		rbn = rbnext;
-		*nextp = fp;
-		nextp = &fp->next;
-		fp->prev = NULL;
-		memset(&fp->rbnode, 0, sizeof(fp->rbnode));
-		head->data_len += fp->len;
-		head->len += fp->len;
-		if (head->ip_summed != fp->ip_summed)
-			head->ip_summed = CHECKSUM_NONE;
-		else if (head->ip_summed == CHECKSUM_COMPLETE)
-			head->csum = csum_add(head->csum, fp->csum);
-		head->truesize += fp->truesize;
+	while (rbn || fp) {
+		/* fp points to the next sk_buff in the current run;
+		 * rbn points to the next run.
+		 */
+		/* Go through the current run. */
+		while (fp) {
+			*nextp = fp;
+			nextp = &fp->next;
+			fp->prev = NULL;
+			memset(&fp->rbnode, 0, sizeof(fp->rbnode));
+			head->data_len += fp->len;
+			head->len += fp->len;
+			if (head->ip_summed != fp->ip_summed)
+				head->ip_summed = CHECKSUM_NONE;
+			else if (head->ip_summed == CHECKSUM_COMPLETE)
+				head->csum = csum_add(head->csum, fp->csum);
+			head->truesize += fp->truesize;
+			fp = FRAG_CB(fp)->next_frag;
+		}
+		/* Move to the next run. */
+		if (rbn) {
+			struct rb_node *rbnext = rb_next(rbn);
+
+			fp = rb_to_skb(rbn);
+			rb_erase(rbn, &qp->q.rb_fragments);
+			rbn = rbnext;
+		}
 	}
 	sub_frag_mem_limit(qp->q.net, head->truesize);
 
@@ -680,6 +707,7 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 	qp->q.fragments = NULL;
 	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
+	qp->q.last_run_head = NULL;
 	return 0;
 
 out_nomem:
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag
From: Tom Herbert @ 2019-01-23  2:13 UTC (permalink / raw)
  To: Peter Oskolkov
  Cc: David Miller, Linux Kernel Network Developers, Peter Oskolkov,
	Eric Dumazet, Florian Westphal
In-Reply-To: <CAPNVh5cN7VJnatnyKJWyWFMpxGwXwESoH+PGZrwLoPc3JfFbTA@mail.gmail.com>

On Tue, Jan 22, 2019 at 5:13 PM Peter Oskolkov <posk@google.com> wrote:
>
> On Tue, Jan 22, 2019 at 5:04 PM Tom Herbert <tom@herbertland.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 4:53 PM Peter Oskolkov <posk@google.com> wrote:
> > >
> > > On Tue, Jan 22, 2019 at 4:37 PM Tom Herbert <tom@herbertland.com> wrote:
> > > >
> > > > On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@google.com> wrote:
> > > > >
> > > > > Currently, IPv6 defragmentation code drops non-last fragments that
> > > > > are smaller than 1280 bytes: see
> > > > > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
> > > > >
> > > > Peter,
> > > >
> > > > I'm a bit confused on why so much code is required to undo this patch
> > > > which was a whole three lines of code to begin with.
> > >
> > > I agree that if the goal is to just lower the 1280 IPv6 fragment
> > > limit, but keep it reasonably high, then my patchset is not needed.
> > > However, if the decision (ultimately by David Miller, I guess) is to
> > > accept fragments of any size, as the standard seems to imply, then
> > > something similar to what I suggested here is needed, I think. This
> > > was done in IPv4...
> >
> > Peter,
> >
> > Sorry, I'm still not following. Before "ipv6: defrag: drop non-last
> > frags smaller than min mtu" wouldn't we have accepted fragments of any
> > size? Are you saying that things were previously broken otherwise?
>
> Yes, before the patch you reference above, fragments of any size had
> been accepted. And this had been identified as a DDOS threat and
> mitigated in IPv6 by setting the 1280 fragment limit and in IPv4 by
> using rbtrees. The IPv4/IPv6 difference was due to the belief, at the
> time, that IPv6 standard actually required non-last fragments less
> than 1280 to be dropped, while IPv4 standard required accepting
> fragments of any size.
>
> So things were broken in the sense of having this DDOS vulnerability,
> and the vulnerability was fixed in IPv4 and IPv6 using different
> approaches...

I see, thanks for the explanation! Even if there is a limit on
fragment size, it should be configurable so your patches would still
be relevant. As for eliminating the limit completely, I think that
question is what is the remaining DOSability with a tiny fragment
attack. Have you done any analysis that might shed light on that?

Tom

>
> Thanks,
> Peter
>
> >
> > Tom
> >
> > >
> > > Thanks,
> > > Peter
> > >
> > > >
> > > > > This behavior is not specified in IPv6 RFCs and appears to break
> > > > > compatibility with some IPv6 implemenations, as reported here:
> > > > > https://www.spinics.net/lists/netdev/msg543846.html
> > > > >
> > > > I am not yet convinced that the patch should be reverted. While it is
> > > > not conformant, it is a potential mitigation against DOS attack. For
> > > > instance, a single fragmented packet could be composed of over 8000
> > > > fragments with the minimum fragment size of eight bytes. Receiving
> > > > packets like that attacks both memory and CPU, and I don't see any
> > > > purpose to sending tiny fragments other than DOS attack. For practical
> > > > implemenation, I believe a limit is justified. The 1280 MTU limit in
> > > > the patch was too austere, I think it should be smaller than that.
> > > > Also, any such limit should apply to fragments payload length and not
> > > > length of the packet since a clever attacker could stuff the packet
> > > > with various other extension headers and still end up sending tiny
> > > > fragments. I have a patch along these lines with limit configurable by
> > > > sysctl. I will post shortly.
> > > >
> > > > Tom
> > > >
> > > > > This patch re-uses common IP defragmentation queueing and reassembly
> > > > > code in IPv6, removing the 1280 byte restriction.
> > > > >
> > > > > Signed-off-by: Peter Oskolkov <posk@google.com>
> > > > > Reported-by: Tom Herbert <tom@herbertland.com>
> > > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > > Cc: Florian Westphal <fw@strlen.de>
> > > > > ---
> > > > >  include/net/ipv6_frag.h |  11 +-
> > > > >  net/ipv6/reassembly.c   | 233 +++++++++++-----------------------------
> > > > >  2 files changed, 71 insertions(+), 173 deletions(-)
> > > > >
> > > > > diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
> > > > > index 6ced1e6899b6..28aa9b30aece 100644
> > > > > --- a/include/net/ipv6_frag.h
> > > > > +++ b/include/net/ipv6_frag.h
> > > > > @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
> > > > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
> > > > >
> > > > >         /* Don't send error if the first segment did not arrive. */
> > > > > -       head = fq->q.fragments;
> > > > > -       if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
> > > > > +       if (!(fq->q.flags & INET_FRAG_FIRST_IN))
> > > > > +               goto out;
> > > > > +
> > > > > +       /* sk_buff::dev and sk_buff::rbnode are unionized. So we
> > > > > +        * pull the head out of the tree in order to be able to
> > > > > +        * deal with head->dev.
> > > > > +        */
> > > > > +       head = inet_frag_pull_head(&fq->q);
> > > > > +       if (!head)
> > > > >                 goto out;
> > > > >
> > > > >         head->dev = dev;
> > > > > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> > > > > index 36a3d8dc61f5..24264d0a4b85 100644
> > > > > --- a/net/ipv6/reassembly.c
> > > > > +++ b/net/ipv6/reassembly.c
> > > > > @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
> > > > >
> > > > >  static struct inet_frags ip6_frags;
> > > > >
> > > > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > > -                         struct net_device *dev);
> > > > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > > > +                         struct sk_buff *prev_tail, struct net_device *dev);
> > > > >
> > > > >  static void ip6_frag_expire(struct timer_list *t)
> > > > >  {
> > > > > @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > > >                           struct frag_hdr *fhdr, int nhoff,
> > > > >                           u32 *prob_offset)
> > > > >  {
> > > > > -       struct sk_buff *prev, *next;
> > > > > -       struct net_device *dev;
> > > > > -       int offset, end, fragsize;
> > > > >         struct net *net = dev_net(skb_dst(skb)->dev);
> > > > > +       int offset, end, fragsize;
> > > > > +       struct sk_buff *prev_tail;
> > > > > +       struct net_device *dev;
> > > > > +       int err = -ENOENT;
> > > > >         u8 ecn;
> > > > >
> > > > >         if (fq->q.flags & INET_FRAG_COMPLETE)
> > > > >                 goto err;
> > > > >
> > > > > +       err = -EINVAL;
> > > > >         offset = ntohs(fhdr->frag_off) & ~0x7;
> > > > >         end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
> > > > >                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
> > > > >
> > > > >         if ((unsigned int)end > IPV6_MAXPLEN) {
> > > > >                 *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
> > > > > +               /* note that if prob_offset is set, the skb is freed elsewhere,
> > > > > +                * we do not free it here.
> > > > > +                */
> > > > >                 return -1;
> > > > >         }
> > > > >
> > > > > @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > > >         if (end == offset)
> > > > >                 goto discard_fq;
> > > > >
> > > > > +       err = -ENOMEM;
> > > > >         /* Point into the IP datagram 'data' part. */
> > > > >         if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data))
> > > > >                 goto discard_fq;
> > > > >
> > > > > -       if (pskb_trim_rcsum(skb, end - offset))
> > > > > +       err = pskb_trim_rcsum(skb, end - offset);
> > > > > +       if (err)
> > > > >                 goto discard_fq;
> > > > >
> > > > > -       /* Find out which fragments are in front and at the back of us
> > > > > -        * in the chain of fragments so far.  We must know where to put
> > > > > -        * this fragment, right?
> > > > > -        */
> > > > > -       prev = fq->q.fragments_tail;
> > > > > -       if (!prev || prev->ip_defrag_offset < offset) {
> > > > > -               next = NULL;
> > > > > -               goto found;
> > > > > -       }
> > > > > -       prev = NULL;
> > > > > -       for (next = fq->q.fragments; next != NULL; next = next->next) {
> > > > > -               if (next->ip_defrag_offset >= offset)
> > > > > -                       break;  /* bingo! */
> > > > > -               prev = next;
> > > > > -       }
> > > > > -
> > > > > -found:
> > > > > -       /* 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.
> > > > > -        */
> > > > > -
> > > > > -       /* Check for overlap with preceding fragment. */
> > > > > -       if (prev &&
> > > > > -           (prev->ip_defrag_offset + prev->len) > offset)
> > > > > -               goto discard_fq;
> > > > > -
> > > > > -       /* Look for overlap with succeeding segment. */
> > > > > -       if (next && next->ip_defrag_offset < end)
> > > > > -               goto discard_fq;
> > > > > -
> > > > > -       /* Note : skb->ip_defrag_offset and skb->sk share the same location */
> > > > > +       /* Note : skb->rbnode and skb->dev share the same location. */
> > > > >         dev = skb->dev;
> > > > > -       if (dev)
> > > > > -               fq->iif = dev->ifindex;
> > > > >         /* Makes sure compiler wont do silly aliasing games */
> > > > >         barrier();
> > > > > -       skb->ip_defrag_offset = offset;
> > > > >
> > > > > -       /* Insert this fragment in the chain of fragments. */
> > > > > -       skb->next = next;
> > > > > -       if (!next)
> > > > > -               fq->q.fragments_tail = skb;
> > > > > -       if (prev)
> > > > > -               prev->next = skb;
> > > > > -       else
> > > > > -               fq->q.fragments = skb;
> > > > > +       prev_tail = fq->q.fragments_tail;
> > > > > +       err = inet_frag_queue_insert(&fq->q, skb, offset, end);
> > > > > +       if (err)
> > > > > +               goto insert_error;
> > > > > +
> > > > > +       if (dev)
> > > > > +               fq->iif = dev->ifindex;
> > > > >
> > > > >         fq->q.stamp = skb->tstamp;
> > > > >         fq->q.meat += skb->len;
> > > > > @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > > >
> > > > >         if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > > > >             fq->q.meat == fq->q.len) {
> > > > > -               int res;
> > > > >                 unsigned long orefdst = skb->_skb_refdst;
> > > > >
> > > > >                 skb->_skb_refdst = 0UL;
> > > > > -               res = ip6_frag_reasm(fq, prev, dev);
> > > > > +               err = ip6_frag_reasm(fq, skb, prev_tail, dev);
> > > > >                 skb->_skb_refdst = orefdst;
> > > > > -               return res;
> > > > > +               return err;
> > > > >         }
> > > > >
> > > > >         skb_dst_drop(skb);
> > > > > -       return -1;
> > > > > +       return -EINPROGRESS;
> > > > >
> > > > > +insert_error:
> > > > > +       if (err == IPFRAG_DUP) {
> > > > > +               kfree_skb(skb);
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +       err = -EINVAL;
> > > > > +       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > > > +                       IPSTATS_MIB_REASM_OVERLAPS);
> > > > >  discard_fq:
> > > > >         inet_frag_kill(&fq->q);
> > > > > -err:
> > > > >         __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > > >                         IPSTATS_MIB_REASMFAILS);
> > > > > +err:
> > > > >         kfree_skb(skb);
> > > > > -       return -1;
> > > > > +       return err;
> > > > >  }
> > > > >
> > > > >  /*
> > > > >   *     Check if this packet is complete.
> > > > > - *     Returns NULL on failure by any reason, and pointer
> > > > > - *     to current nexthdr field in reassembled frame.
> > > > >   *
> > > > >   *     It is called with locked fq, and caller must check that
> > > > >   *     queue is eligible for reassembly i.e. it is not COMPLETE,
> > > > >   *     the last and the first frames arrived and all the bits are here.
> > > > >   */
> > > > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > > -                         struct net_device *dev)
> > > > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > > > +                         struct sk_buff *prev_tail, struct net_device *dev)
> > > > >  {
> > > > >         struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> > > > > -       struct sk_buff *fp, *head = fq->q.fragments;
> > > > > -       int    payload_len, delta;
> > > > >         unsigned int nhoff;
> > > > > -       int sum_truesize;
> > > > > +       void *reasm_data;
> > > > > +       int payload_len;
> > > > >         u8 ecn;
> > > > >
> > > > >         inet_frag_kill(&fq->q);
> > > > > @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > >         if (unlikely(ecn == 0xff))
> > > > >                 goto out_fail;
> > > > >
> > > > > -       /* Make the one we just received the head. */
> > > > > -       if (prev) {
> > > > > -               head = prev->next;
> > > > > -               fp = skb_clone(head, GFP_ATOMIC);
> > > > > -
> > > > > -               if (!fp)
> > > > > -                       goto out_oom;
> > > > > -
> > > > > -               fp->next = head->next;
> > > > > -               if (!fp->next)
> > > > > -                       fq->q.fragments_tail = fp;
> > > > > -               prev->next = fp;
> > > > > -
> > > > > -               skb_morph(head, fq->q.fragments);
> > > > > -               head->next = fq->q.fragments->next;
> > > > > -
> > > > > -               consume_skb(fq->q.fragments);
> > > > > -               fq->q.fragments = head;
> > > > > -       }
> > > > > -
> > > > > -       WARN_ON(head == NULL);
> > > > > -       WARN_ON(head->ip_defrag_offset != 0);
> > > > > +       reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail);
> > > > > +       if (!reasm_data)
> > > > > +               goto out_oom;
> > > > >
> > > > > -       /* Unfragmented part is taken from the first segment. */
> > > > > -       payload_len = ((head->data - skb_network_header(head)) -
> > > > > +       payload_len = ((skb->data - skb_network_header(skb)) -
> > > > >                        sizeof(struct ipv6hdr) + fq->q.len -
> > > > >                        sizeof(struct frag_hdr));
> > > > >         if (payload_len > IPV6_MAXPLEN)
> > > > >                 goto out_oversize;
> > > > >
> > > > > -       delta = - head->truesize;
> > > > > -
> > > > > -       /* Head of list must not be cloned. */
> > > > > -       if (skb_unclone(head, GFP_ATOMIC))
> > > > > -               goto out_oom;
> > > > > -
> > > > > -       delta += head->truesize;
> > > > > -       if (delta)
> > > > > -               add_frag_mem_limit(fq->q.net, delta);
> > > > > -
> > > > > -       /* If the first fragment is fragmented itself, we split
> > > > > -        * it to two chunks: the first with data and paged part
> > > > > -        * and the second, holding only fragments. */
> > > > > -       if (skb_has_frag_list(head)) {
> > > > > -               struct sk_buff *clone;
> > > > > -               int i, plen = 0;
> > > > > -
> > > > > -               clone = alloc_skb(0, GFP_ATOMIC);
> > > > > -               if (!clone)
> > > > > -                       goto out_oom;
> > > > > -               clone->next = head->next;
> > > > > -               head->next = clone;
> > > > > -               skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
> > > > > -               skb_frag_list_init(head);
> > > > > -               for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
> > > > > -                       plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
> > > > > -               clone->len = clone->data_len = head->data_len - plen;
> > > > > -               head->data_len -= clone->len;
> > > > > -               head->len -= clone->len;
> > > > > -               clone->csum = 0;
> > > > > -               clone->ip_summed = head->ip_summed;
> > > > > -               add_frag_mem_limit(fq->q.net, clone->truesize);
> > > > > -       }
> > > > > -
> > > > >         /* We have to remove fragment header from datagram and to relocate
> > > > >          * header in order to calculate ICV correctly. */
> > > > >         nhoff = fq->nhoffset;
> > > > > -       skb_network_header(head)[nhoff] = skb_transport_header(head)[0];
> > > > > -       memmove(head->head + sizeof(struct frag_hdr), head->head,
> > > > > -               (head->data - head->head) - sizeof(struct frag_hdr));
> > > > > -       if (skb_mac_header_was_set(head))
> > > > > -               head->mac_header += sizeof(struct frag_hdr);
> > > > > -       head->network_header += sizeof(struct frag_hdr);
> > > > > -
> > > > > -       skb_reset_transport_header(head);
> > > > > -       skb_push(head, head->data - skb_network_header(head));
> > > > > -
> > > > > -       sum_truesize = head->truesize;
> > > > > -       for (fp = head->next; fp;) {
> > > > > -               bool headstolen;
> > > > > -               int delta;
> > > > > -               struct sk_buff *next = fp->next;
> > > > > -
> > > > > -               sum_truesize += fp->truesize;
> > > > > -               if (head->ip_summed != fp->ip_summed)
> > > > > -                       head->ip_summed = CHECKSUM_NONE;
> > > > > -               else if (head->ip_summed == CHECKSUM_COMPLETE)
> > > > > -                       head->csum = csum_add(head->csum, fp->csum);
> > > > > -
> > > > > -               if (skb_try_coalesce(head, fp, &headstolen, &delta)) {
> > > > > -                       kfree_skb_partial(fp, headstolen);
> > > > > -               } else {
> > > > > -                       fp->sk = NULL;
> > > > > -                       if (!skb_shinfo(head)->frag_list)
> > > > > -                               skb_shinfo(head)->frag_list = fp;
> > > > > -                       head->data_len += fp->len;
> > > > > -                       head->len += fp->len;
> > > > > -                       head->truesize += fp->truesize;
> > > > > -               }
> > > > > -               fp = next;
> > > > > -       }
> > > > > -       sub_frag_mem_limit(fq->q.net, sum_truesize);
> > > > > +       skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0];
> > > > > +       memmove(skb->head + sizeof(struct frag_hdr), skb->head,
> > > > > +               (skb->data - skb->head) - sizeof(struct frag_hdr));
> > > > > +       if (skb_mac_header_was_set(skb))
> > > > > +               skb->mac_header += sizeof(struct frag_hdr);
> > > > > +       skb->network_header += sizeof(struct frag_hdr);
> > > > > +
> > > > > +       skb_reset_transport_header(skb);
> > > > > +
> > > > > +       inet_frag_reasm_finish(&fq->q, skb, reasm_data);
> > > > >
> > > > > -       skb_mark_not_on_list(head);
> > > > > -       head->dev = dev;
> > > > > -       head->tstamp = fq->q.stamp;
> > > > > -       ipv6_hdr(head)->payload_len = htons(payload_len);
> > > > > -       ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
> > > > > -       IP6CB(head)->nhoff = nhoff;
> > > > > -       IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
> > > > > -       IP6CB(head)->frag_max_size = fq->q.max_size;
> > > > > +       skb->dev = dev;
> > > > > +       ipv6_hdr(skb)->payload_len = htons(payload_len);
> > > > > +       ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn);
> > > > > +       IP6CB(skb)->nhoff = nhoff;
> > > > > +       IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
> > > > > +       IP6CB(skb)->frag_max_size = fq->q.max_size;
> > > > >
> > > > >         /* Yes, and fold redundant checksum back. 8) */
> > > > > -       skb_postpush_rcsum(head, skb_network_header(head),
> > > > > -                          skb_network_header_len(head));
> > > > > +       skb_postpush_rcsum(skb, skb_network_header(skb),
> > > > > +                          skb_network_header_len(skb));
> > > > >
> > > > >         rcu_read_lock();
> > > > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> > > > > @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > >         fq->q.fragments = NULL;
> > > > >         fq->q.rb_fragments = RB_ROOT;
> > > > >         fq->q.fragments_tail = NULL;
> > > > > +       fq->q.last_run_head = NULL;
> > > > >         return 1;
> > > > >
> > > > >  out_oversize:
> > > > > @@ -464,10 +358,6 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > > > >                 return 1;
> > > > >         }
> > > > >
> > > > > -       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> > > > > -           fhdr->frag_off & htons(IP6_MF))
> > > > > -               goto fail_hdr;
> > > > > -
> > > > >         iif = skb->dev ? skb->dev->ifindex : 0;
> > > > >         fq = fq_find(net, fhdr->identification, hdr, iif);
> > > > >         if (fq) {
> > > > > @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > > > >                 if (prob_offset) {
> > > > >                         __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
> > > > >                                         IPSTATS_MIB_INHDRERRORS);
> > > > > +                       /* icmpv6_param_prob() calls kfree_skb(skb) */
> > > > >                         icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
> > > > >                 }
> > > > >                 return ret;
> > > > > --
> > > > > 2.20.1.321.g9e740568ce-goog
> > > > >

^ permalink raw reply

* Re: Regression: mv88e6xxx packet loss after 4.18's PHYLINK merge
From: Andrew Lunn @ 2019-01-23  2:10 UTC (permalink / raw)
  To: Samu Nuutamo; +Cc: netdev, Vivien Didelot, Florian Fainelli, David S. Miller
In-Reply-To: <20190117113722.GA22476@samu-ThinkPad-T480s>

> Hi Andrew,
> 
> I tested the patch and it has an issue that prevents the carrier from turning
> on. I think it's caused by the change from using link_state to phy->phy_state,
> as later in the same function the link_state attributes are read when
> deciding if the carrier needs to be turned on.
 
Hi Samu

This version has had better testing. So i'm a bit more confident about
it.

	Andrew

From d093492f9aba75cb4844d15148e0efd014f193bb Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 16 Jan 2019 20:22:04 -0600
Subject: [PATCH] net: phy: phylink: Only change mac when fixed link changes
 state

phylink polls the fixed-link once per second to see if the GPIO has
changed state, or if the callback indicates if there has been a change
in state. It then calls the MAC to reconfigure itself to the current
state.

For some MACs, reconfiguration can result in packets being dropped.
Hence only reconfigure the MAC if the link state differs from the
netdev carrier state.

Reported-by: Samu Nuutamo <samu.nuutamo@vincit.fi>
Fixes: 9cd00a8aa42e ("net: phy: phylink: Poll link GPIOs")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phylink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index e7becc7379d7..394b1eb5b55a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -409,7 +409,8 @@ static void phylink_resolve(struct work_struct *w)
 
 		case MLO_AN_FIXED:
 			phylink_get_fixed_state(pl, &link_state);
-			phylink_mac_config(pl, &link_state);
+			if (link_state.link != netif_carrier_ok(ndev))
+				phylink_mac_config(pl, &link_state);
 			break;
 
 		case MLO_AN_INBAND:
-- 
2.20.1


^ permalink raw reply related

* Re: stable backport for the BPF speculation series?
From: Daniel Borkmann @ 2019-01-23  1:55 UTC (permalink / raw)
  To: David Miller, jannh
  Cc: alexei.starovoitov, ast, jakub.kicinski, netdev, gregkh
In-Reply-To: <20190122.084448.904846922511108864.davem@davemloft.net>

On 01/22/2019 05:44 PM, David Miller wrote:
> From: Jann Horn <jannh@google.com>
> Date: Tue, 22 Jan 2019 15:36:54 +0100
> 
>> On Thu, Jan 3, 2019 at 1:08 AM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> On Thu, Jan 03, 2019 at 12:58:26AM +0100, Daniel Borkmann wrote:
>>>> This set fixes an out of bounds case under speculative execution
>>>> by implementing masking of pointer alu into the verifier. For
>>>> details please see the individual patches.
>>>>
>>>> Thanks!
>>>>
>>>> v2 -> v3:
>>>>   - 8/9: change states_equal condition into old->speculative &&
>>>>     !cur->speculative, thanks Jakub!
>>>>   - 8/9: remove incorrect speculative state test in
>>>>     propagate_liveness(), thanks Jakub!
>>>> v1 -> v2:
>>>>   - Typo fixes in commit msg and a comment, thanks David!
>>>
>>> Applied, Thanks
>>
>> This series and the followup fix ("bpf: fix sanitation of alu op with
>> pointer / scalar type from different paths") have been in Linus' tree
>> for six days, but from what I can tell, they aren't queued up for
>> stable yet.
>>
>> @davem: Are you going to send this through stable, or is this only
>> going to be in 5.0?
> 
> The BPF developers handle their -stable submissions.

Will get this to stable towards end of week. We wanted to let this sit
for a while in Linus' tree given the complexity of the fix to get some
more coverage. We also need 9d5564ddcf2a ("bpf: fix inner map masking
to prevent oob under speculation") in addition.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH] net: altera_tse: fix connect_local_phy error path
From: David Miller @ 2019-01-23  1:46 UTC (permalink / raw)
  To: atsushi.nemoto; +Cc: thor.thayer, netdev
In-Reply-To: <20190121.172641.1897494275608518036.atsushi.nemoto@sord.co.jp>

From: Atsushi Nemoto <atsushi.nemoto@sord.co.jp>
Date: Mon, 21 Jan 2019 17:26:41 +0900

> The connect_local_phy should return NULL (not negative errno) on
> error, since its caller expects it.
> 
> Signed-off-by: Atsushi Nemoto <atsushi.nemoto@sord.co.jp>

Applied.

^ permalink raw reply

* Re: [v2] net: dpaa2: improve PTP Kconfig option
From: David Miller @ 2019-01-23  1:38 UTC (permalink / raw)
  To: yangbo.lu; +Cc: netdev, richardcochran, ruxandra.radulescu
In-Reply-To: <20190121062637.48449-1-yangbo.lu@nxp.com>

From: Yangbo Lu <yangbo.lu@nxp.com>
Date: Mon, 21 Jan 2019 14:26:37 +0800

> Converted to use "imply" instead of "select" for PTP_1588_CLOCK
> driver selecting. This could break the hard dependency between
> the PTP clock subsystem and ethernet drivers.
> This patch also set "default y" for dpaa2 ptp driver building to
> provide user an available ptp clock in default.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- Converted to use imply for PTP_1588_CLOCK.
> 	- Modified commit message.

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/1] net: stmmac: implement the SIOCGHWTSTAMP ioctl
From: David Miller @ 2019-01-23  1:37 UTC (permalink / raw)
  To: panfilov.artyom
  Cc: peppe.cavallaro, alexandre.torgue, joabreu, mcoquelin.stm32,
	netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <20190120160515.26473-1-panfilov.artyom@gmail.com>

From: Artem Panfilov <panfilov.artyom@gmail.com>
Date: Sun, 20 Jan 2019 19:05:15 +0300

> This patch adds support for the SIOCGHWTSTAMP ioctl which enables user
> processes to read the current hwtstamp_config settings
> non-destructively.
> 
> Signed-off-by: Artem Panfilov <panfilov.artyom@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/3] qed*: Error recovery process
From: David Miller @ 2019-01-23  1:37 UTC (permalink / raw)
  To: michal.kalderon; +Cc: ariel.elior, netdev
In-Reply-To: <20190120093639.11781-1-michal.kalderon@cavium.com>

From: Michal Kalderon <michal.kalderon@cavium.com>
Date: Sun, 20 Jan 2019 11:36:36 +0200

> Parity errors might happen in the device's memories due to momentary bit
> flips which are caused by radiation.
> Errors that are not correctable initiate a process kill event, which blocks
> the device access towards the host and the network, and a recovery process
> is started in the management FW and in the driver.
> 
> This series adds the support of this process in the qed core module and in
> the qede driver (patches 2 & 3).
> Patch 1 in the series revises the load sequence, to avoid PCI errors that
> might be observed during a recovery process.

Series applied.

^ permalink raw reply

* Re: [PATCH net v2] net/ipv6: don't return positive numbers when nothing was dumped
From: David Miller @ 2019-01-23  1:25 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: dsahern, netdev, oss-drivers
In-Reply-To: <20190122224719.7509-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 22 Jan 2019 14:47:19 -0800

> in6_dump_addrs() returns a positive 1 if there was nothing to dump.
> This return value can not be passed as return from inet6_dump_addr()
> as is, because it will confuse rtnetlink, resulting in NLMSG_DONE
> never getting set:
> 
> $ ip addr list dev lo
> EOF on netlink
> Dump terminated
> 
> v2: flip condition to avoid a new goto (DaveA)
> 
> Fixes: 7c1e8a3817c5 ("netlink: fixup regression in RTM_GETADDR")
> Reported-by: Brendan Galloway <brendan.galloway@netronome.com>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied, and since the RTM_GETADDR regression fix was backports I'll have
to -stable this as well.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] bridge: implement Multicast Router Discovery (RFC4286)
From: David Miller @ 2019-01-23  1:18 UTC (permalink / raw)
  To: linus.luessing
  Cc: netdev, roopa, nikolay, kuznet, yoshfuji, bridge, linux-kernel
In-Reply-To: <20190121062628.2710-1-linus.luessing@c0d3.blue>

From: Linus Lüssing <linus.luessing@c0d3.blue>
Date: Mon, 21 Jan 2019 07:26:24 +0100

> This patchset adds initial Multicast Router Discovery support to
> the Linux bridge (RFC4286). With MRD it is possible to detect multicast
> routers and mark bridge ports and forward multicast packets to such routers
> accordingly.
> 
> So far, multicast routers are detected via IGMP/MLD queries and PIM
> messages in the Linux bridge. As there is only one active, selected
> querier at a time RFC4541 ("Considerations for Internet Group Management
> Protocol (IGMP) and Multicast Listener Discovery (MLD) Snooping
> Switches") section 2.1.1.a) recommends snooping Multicast Router
> Advertisements as provided by MRD (RFC4286).
> 
> 
> The first two patches are refactoring some existing code which is reused
> for parsing the Multicast Router Advertisements later in the fourth
> patch. The third patch lets the bridge join the all-snoopers multicast
> address to be able to reliably receive the Multicast Router
> Advertisements.
 ...

Series applied, thanks!

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag
From: Peter Oskolkov @ 2019-01-23  1:13 UTC (permalink / raw)
  To: Tom Herbert
  Cc: David Miller, Linux Kernel Network Developers, Peter Oskolkov,
	Eric Dumazet, Florian Westphal
In-Reply-To: <CALx6S35fy6pkkhBs1ME7m+r58juwqZHsRdQF0872jVrk95VYVQ@mail.gmail.com>

On Tue, Jan 22, 2019 at 5:04 PM Tom Herbert <tom@herbertland.com> wrote:
>
> On Tue, Jan 22, 2019 at 4:53 PM Peter Oskolkov <posk@google.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 4:37 PM Tom Herbert <tom@herbertland.com> wrote:
> > >
> > > On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@google.com> wrote:
> > > >
> > > > Currently, IPv6 defragmentation code drops non-last fragments that
> > > > are smaller than 1280 bytes: see
> > > > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
> > > >
> > > Peter,
> > >
> > > I'm a bit confused on why so much code is required to undo this patch
> > > which was a whole three lines of code to begin with.
> >
> > I agree that if the goal is to just lower the 1280 IPv6 fragment
> > limit, but keep it reasonably high, then my patchset is not needed.
> > However, if the decision (ultimately by David Miller, I guess) is to
> > accept fragments of any size, as the standard seems to imply, then
> > something similar to what I suggested here is needed, I think. This
> > was done in IPv4...
>
> Peter,
>
> Sorry, I'm still not following. Before "ipv6: defrag: drop non-last
> frags smaller than min mtu" wouldn't we have accepted fragments of any
> size? Are you saying that things were previously broken otherwise?

Yes, before the patch you reference above, fragments of any size had
been accepted. And this had been identified as a DDOS threat and
mitigated in IPv6 by setting the 1280 fragment limit and in IPv4 by
using rbtrees. The IPv4/IPv6 difference was due to the belief, at the
time, that IPv6 standard actually required non-last fragments less
than 1280 to be dropped, while IPv4 standard required accepting
fragments of any size.

So things were broken in the sense of having this DDOS vulnerability,
and the vulnerability was fixed in IPv4 and IPv6 using different
approaches...

Thanks,
Peter

>
> Tom
>
> >
> > Thanks,
> > Peter
> >
> > >
> > > > This behavior is not specified in IPv6 RFCs and appears to break
> > > > compatibility with some IPv6 implemenations, as reported here:
> > > > https://www.spinics.net/lists/netdev/msg543846.html
> > > >
> > > I am not yet convinced that the patch should be reverted. While it is
> > > not conformant, it is a potential mitigation against DOS attack. For
> > > instance, a single fragmented packet could be composed of over 8000
> > > fragments with the minimum fragment size of eight bytes. Receiving
> > > packets like that attacks both memory and CPU, and I don't see any
> > > purpose to sending tiny fragments other than DOS attack. For practical
> > > implemenation, I believe a limit is justified. The 1280 MTU limit in
> > > the patch was too austere, I think it should be smaller than that.
> > > Also, any such limit should apply to fragments payload length and not
> > > length of the packet since a clever attacker could stuff the packet
> > > with various other extension headers and still end up sending tiny
> > > fragments. I have a patch along these lines with limit configurable by
> > > sysctl. I will post shortly.
> > >
> > > Tom
> > >
> > > > This patch re-uses common IP defragmentation queueing and reassembly
> > > > code in IPv6, removing the 1280 byte restriction.
> > > >
> > > > Signed-off-by: Peter Oskolkov <posk@google.com>
> > > > Reported-by: Tom Herbert <tom@herbertland.com>
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Cc: Florian Westphal <fw@strlen.de>
> > > > ---
> > > >  include/net/ipv6_frag.h |  11 +-
> > > >  net/ipv6/reassembly.c   | 233 +++++++++++-----------------------------
> > > >  2 files changed, 71 insertions(+), 173 deletions(-)
> > > >
> > > > diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
> > > > index 6ced1e6899b6..28aa9b30aece 100644
> > > > --- a/include/net/ipv6_frag.h
> > > > +++ b/include/net/ipv6_frag.h
> > > > @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
> > > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
> > > >
> > > >         /* Don't send error if the first segment did not arrive. */
> > > > -       head = fq->q.fragments;
> > > > -       if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
> > > > +       if (!(fq->q.flags & INET_FRAG_FIRST_IN))
> > > > +               goto out;
> > > > +
> > > > +       /* sk_buff::dev and sk_buff::rbnode are unionized. So we
> > > > +        * pull the head out of the tree in order to be able to
> > > > +        * deal with head->dev.
> > > > +        */
> > > > +       head = inet_frag_pull_head(&fq->q);
> > > > +       if (!head)
> > > >                 goto out;
> > > >
> > > >         head->dev = dev;
> > > > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> > > > index 36a3d8dc61f5..24264d0a4b85 100644
> > > > --- a/net/ipv6/reassembly.c
> > > > +++ b/net/ipv6/reassembly.c
> > > > @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
> > > >
> > > >  static struct inet_frags ip6_frags;
> > > >
> > > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > -                         struct net_device *dev);
> > > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > > +                         struct sk_buff *prev_tail, struct net_device *dev);
> > > >
> > > >  static void ip6_frag_expire(struct timer_list *t)
> > > >  {
> > > > @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > >                           struct frag_hdr *fhdr, int nhoff,
> > > >                           u32 *prob_offset)
> > > >  {
> > > > -       struct sk_buff *prev, *next;
> > > > -       struct net_device *dev;
> > > > -       int offset, end, fragsize;
> > > >         struct net *net = dev_net(skb_dst(skb)->dev);
> > > > +       int offset, end, fragsize;
> > > > +       struct sk_buff *prev_tail;
> > > > +       struct net_device *dev;
> > > > +       int err = -ENOENT;
> > > >         u8 ecn;
> > > >
> > > >         if (fq->q.flags & INET_FRAG_COMPLETE)
> > > >                 goto err;
> > > >
> > > > +       err = -EINVAL;
> > > >         offset = ntohs(fhdr->frag_off) & ~0x7;
> > > >         end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
> > > >                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
> > > >
> > > >         if ((unsigned int)end > IPV6_MAXPLEN) {
> > > >                 *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
> > > > +               /* note that if prob_offset is set, the skb is freed elsewhere,
> > > > +                * we do not free it here.
> > > > +                */
> > > >                 return -1;
> > > >         }
> > > >
> > > > @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > >         if (end == offset)
> > > >                 goto discard_fq;
> > > >
> > > > +       err = -ENOMEM;
> > > >         /* Point into the IP datagram 'data' part. */
> > > >         if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data))
> > > >                 goto discard_fq;
> > > >
> > > > -       if (pskb_trim_rcsum(skb, end - offset))
> > > > +       err = pskb_trim_rcsum(skb, end - offset);
> > > > +       if (err)
> > > >                 goto discard_fq;
> > > >
> > > > -       /* Find out which fragments are in front and at the back of us
> > > > -        * in the chain of fragments so far.  We must know where to put
> > > > -        * this fragment, right?
> > > > -        */
> > > > -       prev = fq->q.fragments_tail;
> > > > -       if (!prev || prev->ip_defrag_offset < offset) {
> > > > -               next = NULL;
> > > > -               goto found;
> > > > -       }
> > > > -       prev = NULL;
> > > > -       for (next = fq->q.fragments; next != NULL; next = next->next) {
> > > > -               if (next->ip_defrag_offset >= offset)
> > > > -                       break;  /* bingo! */
> > > > -               prev = next;
> > > > -       }
> > > > -
> > > > -found:
> > > > -       /* 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.
> > > > -        */
> > > > -
> > > > -       /* Check for overlap with preceding fragment. */
> > > > -       if (prev &&
> > > > -           (prev->ip_defrag_offset + prev->len) > offset)
> > > > -               goto discard_fq;
> > > > -
> > > > -       /* Look for overlap with succeeding segment. */
> > > > -       if (next && next->ip_defrag_offset < end)
> > > > -               goto discard_fq;
> > > > -
> > > > -       /* Note : skb->ip_defrag_offset and skb->sk share the same location */
> > > > +       /* Note : skb->rbnode and skb->dev share the same location. */
> > > >         dev = skb->dev;
> > > > -       if (dev)
> > > > -               fq->iif = dev->ifindex;
> > > >         /* Makes sure compiler wont do silly aliasing games */
> > > >         barrier();
> > > > -       skb->ip_defrag_offset = offset;
> > > >
> > > > -       /* Insert this fragment in the chain of fragments. */
> > > > -       skb->next = next;
> > > > -       if (!next)
> > > > -               fq->q.fragments_tail = skb;
> > > > -       if (prev)
> > > > -               prev->next = skb;
> > > > -       else
> > > > -               fq->q.fragments = skb;
> > > > +       prev_tail = fq->q.fragments_tail;
> > > > +       err = inet_frag_queue_insert(&fq->q, skb, offset, end);
> > > > +       if (err)
> > > > +               goto insert_error;
> > > > +
> > > > +       if (dev)
> > > > +               fq->iif = dev->ifindex;
> > > >
> > > >         fq->q.stamp = skb->tstamp;
> > > >         fq->q.meat += skb->len;
> > > > @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > > >
> > > >         if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > > >             fq->q.meat == fq->q.len) {
> > > > -               int res;
> > > >                 unsigned long orefdst = skb->_skb_refdst;
> > > >
> > > >                 skb->_skb_refdst = 0UL;
> > > > -               res = ip6_frag_reasm(fq, prev, dev);
> > > > +               err = ip6_frag_reasm(fq, skb, prev_tail, dev);
> > > >                 skb->_skb_refdst = orefdst;
> > > > -               return res;
> > > > +               return err;
> > > >         }
> > > >
> > > >         skb_dst_drop(skb);
> > > > -       return -1;
> > > > +       return -EINPROGRESS;
> > > >
> > > > +insert_error:
> > > > +       if (err == IPFRAG_DUP) {
> > > > +               kfree_skb(skb);
> > > > +               return -EINVAL;
> > > > +       }
> > > > +       err = -EINVAL;
> > > > +       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > > +                       IPSTATS_MIB_REASM_OVERLAPS);
> > > >  discard_fq:
> > > >         inet_frag_kill(&fq->q);
> > > > -err:
> > > >         __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > >                         IPSTATS_MIB_REASMFAILS);
> > > > +err:
> > > >         kfree_skb(skb);
> > > > -       return -1;
> > > > +       return err;
> > > >  }
> > > >
> > > >  /*
> > > >   *     Check if this packet is complete.
> > > > - *     Returns NULL on failure by any reason, and pointer
> > > > - *     to current nexthdr field in reassembled frame.
> > > >   *
> > > >   *     It is called with locked fq, and caller must check that
> > > >   *     queue is eligible for reassembly i.e. it is not COMPLETE,
> > > >   *     the last and the first frames arrived and all the bits are here.
> > > >   */
> > > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > > -                         struct net_device *dev)
> > > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > > +                         struct sk_buff *prev_tail, struct net_device *dev)
> > > >  {
> > > >         struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> > > > -       struct sk_buff *fp, *head = fq->q.fragments;
> > > > -       int    payload_len, delta;
> > > >         unsigned int nhoff;
> > > > -       int sum_truesize;
> > > > +       void *reasm_data;
> > > > +       int payload_len;
> > > >         u8 ecn;
> > > >
> > > >         inet_frag_kill(&fq->q);
> > > > @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > >         if (unlikely(ecn == 0xff))
> > > >                 goto out_fail;
> > > >
> > > > -       /* Make the one we just received the head. */
> > > > -       if (prev) {
> > > > -               head = prev->next;
> > > > -               fp = skb_clone(head, GFP_ATOMIC);
> > > > -
> > > > -               if (!fp)
> > > > -                       goto out_oom;
> > > > -
> > > > -               fp->next = head->next;
> > > > -               if (!fp->next)
> > > > -                       fq->q.fragments_tail = fp;
> > > > -               prev->next = fp;
> > > > -
> > > > -               skb_morph(head, fq->q.fragments);
> > > > -               head->next = fq->q.fragments->next;
> > > > -
> > > > -               consume_skb(fq->q.fragments);
> > > > -               fq->q.fragments = head;
> > > > -       }
> > > > -
> > > > -       WARN_ON(head == NULL);
> > > > -       WARN_ON(head->ip_defrag_offset != 0);
> > > > +       reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail);
> > > > +       if (!reasm_data)
> > > > +               goto out_oom;
> > > >
> > > > -       /* Unfragmented part is taken from the first segment. */
> > > > -       payload_len = ((head->data - skb_network_header(head)) -
> > > > +       payload_len = ((skb->data - skb_network_header(skb)) -
> > > >                        sizeof(struct ipv6hdr) + fq->q.len -
> > > >                        sizeof(struct frag_hdr));
> > > >         if (payload_len > IPV6_MAXPLEN)
> > > >                 goto out_oversize;
> > > >
> > > > -       delta = - head->truesize;
> > > > -
> > > > -       /* Head of list must not be cloned. */
> > > > -       if (skb_unclone(head, GFP_ATOMIC))
> > > > -               goto out_oom;
> > > > -
> > > > -       delta += head->truesize;
> > > > -       if (delta)
> > > > -               add_frag_mem_limit(fq->q.net, delta);
> > > > -
> > > > -       /* If the first fragment is fragmented itself, we split
> > > > -        * it to two chunks: the first with data and paged part
> > > > -        * and the second, holding only fragments. */
> > > > -       if (skb_has_frag_list(head)) {
> > > > -               struct sk_buff *clone;
> > > > -               int i, plen = 0;
> > > > -
> > > > -               clone = alloc_skb(0, GFP_ATOMIC);
> > > > -               if (!clone)
> > > > -                       goto out_oom;
> > > > -               clone->next = head->next;
> > > > -               head->next = clone;
> > > > -               skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
> > > > -               skb_frag_list_init(head);
> > > > -               for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
> > > > -                       plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
> > > > -               clone->len = clone->data_len = head->data_len - plen;
> > > > -               head->data_len -= clone->len;
> > > > -               head->len -= clone->len;
> > > > -               clone->csum = 0;
> > > > -               clone->ip_summed = head->ip_summed;
> > > > -               add_frag_mem_limit(fq->q.net, clone->truesize);
> > > > -       }
> > > > -
> > > >         /* We have to remove fragment header from datagram and to relocate
> > > >          * header in order to calculate ICV correctly. */
> > > >         nhoff = fq->nhoffset;
> > > > -       skb_network_header(head)[nhoff] = skb_transport_header(head)[0];
> > > > -       memmove(head->head + sizeof(struct frag_hdr), head->head,
> > > > -               (head->data - head->head) - sizeof(struct frag_hdr));
> > > > -       if (skb_mac_header_was_set(head))
> > > > -               head->mac_header += sizeof(struct frag_hdr);
> > > > -       head->network_header += sizeof(struct frag_hdr);
> > > > -
> > > > -       skb_reset_transport_header(head);
> > > > -       skb_push(head, head->data - skb_network_header(head));
> > > > -
> > > > -       sum_truesize = head->truesize;
> > > > -       for (fp = head->next; fp;) {
> > > > -               bool headstolen;
> > > > -               int delta;
> > > > -               struct sk_buff *next = fp->next;
> > > > -
> > > > -               sum_truesize += fp->truesize;
> > > > -               if (head->ip_summed != fp->ip_summed)
> > > > -                       head->ip_summed = CHECKSUM_NONE;
> > > > -               else if (head->ip_summed == CHECKSUM_COMPLETE)
> > > > -                       head->csum = csum_add(head->csum, fp->csum);
> > > > -
> > > > -               if (skb_try_coalesce(head, fp, &headstolen, &delta)) {
> > > > -                       kfree_skb_partial(fp, headstolen);
> > > > -               } else {
> > > > -                       fp->sk = NULL;
> > > > -                       if (!skb_shinfo(head)->frag_list)
> > > > -                               skb_shinfo(head)->frag_list = fp;
> > > > -                       head->data_len += fp->len;
> > > > -                       head->len += fp->len;
> > > > -                       head->truesize += fp->truesize;
> > > > -               }
> > > > -               fp = next;
> > > > -       }
> > > > -       sub_frag_mem_limit(fq->q.net, sum_truesize);
> > > > +       skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0];
> > > > +       memmove(skb->head + sizeof(struct frag_hdr), skb->head,
> > > > +               (skb->data - skb->head) - sizeof(struct frag_hdr));
> > > > +       if (skb_mac_header_was_set(skb))
> > > > +               skb->mac_header += sizeof(struct frag_hdr);
> > > > +       skb->network_header += sizeof(struct frag_hdr);
> > > > +
> > > > +       skb_reset_transport_header(skb);
> > > > +
> > > > +       inet_frag_reasm_finish(&fq->q, skb, reasm_data);
> > > >
> > > > -       skb_mark_not_on_list(head);
> > > > -       head->dev = dev;
> > > > -       head->tstamp = fq->q.stamp;
> > > > -       ipv6_hdr(head)->payload_len = htons(payload_len);
> > > > -       ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
> > > > -       IP6CB(head)->nhoff = nhoff;
> > > > -       IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
> > > > -       IP6CB(head)->frag_max_size = fq->q.max_size;
> > > > +       skb->dev = dev;
> > > > +       ipv6_hdr(skb)->payload_len = htons(payload_len);
> > > > +       ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn);
> > > > +       IP6CB(skb)->nhoff = nhoff;
> > > > +       IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
> > > > +       IP6CB(skb)->frag_max_size = fq->q.max_size;
> > > >
> > > >         /* Yes, and fold redundant checksum back. 8) */
> > > > -       skb_postpush_rcsum(head, skb_network_header(head),
> > > > -                          skb_network_header_len(head));
> > > > +       skb_postpush_rcsum(skb, skb_network_header(skb),
> > > > +                          skb_network_header_len(skb));
> > > >
> > > >         rcu_read_lock();
> > > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> > > > @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > >         fq->q.fragments = NULL;
> > > >         fq->q.rb_fragments = RB_ROOT;
> > > >         fq->q.fragments_tail = NULL;
> > > > +       fq->q.last_run_head = NULL;
> > > >         return 1;
> > > >
> > > >  out_oversize:
> > > > @@ -464,10 +358,6 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > > >                 return 1;
> > > >         }
> > > >
> > > > -       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> > > > -           fhdr->frag_off & htons(IP6_MF))
> > > > -               goto fail_hdr;
> > > > -
> > > >         iif = skb->dev ? skb->dev->ifindex : 0;
> > > >         fq = fq_find(net, fhdr->identification, hdr, iif);
> > > >         if (fq) {
> > > > @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > > >                 if (prob_offset) {
> > > >                         __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
> > > >                                         IPSTATS_MIB_INHDRERRORS);
> > > > +                       /* icmpv6_param_prob() calls kfree_skb(skb) */
> > > >                         icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
> > > >                 }
> > > >                 return ret;
> > > > --
> > > > 2.20.1.321.g9e740568ce-goog
> > > >

^ permalink raw reply

* Re: [PATCH bpf-next v5 03/12] bpf: verifier: remove dead code
From: Martin Lau @ 2019-01-23  1:10 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net,
	netdev@vger.kernel.org, oss-drivers@netronome.com, Yonghong Song
In-Reply-To: <20190122161251.799e1ebf@cakuba.netronome.com>

On Tue, Jan 22, 2019 at 04:12:51PM -0800, Jakub Kicinski wrote:
> > > +static int bpf_adj_linfo_after_remove(struct bpf_verifier_env *env, u32 off,
> > > +				      u32 cnt)
> > > +{
> > > +	struct bpf_prog *prog = env->prog;
> > > +	u32 i, l_off, l_cnt, nr_linfo;
> > > +	struct bpf_line_info *linfo;
> > > +
> > > +	nr_linfo = prog->aux->nr_linfo;
> > > +	if (!nr_linfo)
> > > +		return 0;
> > > +
> > > +	linfo = prog->aux->linfo;
> > > +
> > > +	/* find first line info to remove, count lines to be removed */
> > > +	for (i = 0; i < nr_linfo; i++)
> > > +		if (linfo[i].insn_off >= off)
> > > +			break;
> > > +
> > > +	l_off = i;
> > > +	l_cnt = 0;
> > > +	for (; i < nr_linfo; i++)
> > > +		if (linfo[i].insn_off < off + cnt)
> > > +			l_cnt++;
> > > +		else
> > > +			break;
> > > +
> > > +	/* First live insn doesn't match first live linfo, it needs to "inherit"
> > > +	 * last removed linfo.  prog is already modified, so prog->len == off
> > > +	 * means no live instructions after.
> > > +	 */
> > > +	if (prog->len != off && l_cnt &&
> > > +	    (i == nr_linfo || linfo[i].insn_off != off + cnt)) {
> > > +		l_cnt--;
> > > +		linfo[--i].insn_off = off + cnt;
> > > +	}
> > > +
> > > +	/* remove the line info which refers to the removed instructions */
> > > +	if (l_cnt) {
> > > +		memmove(linfo + l_off, linfo + i,
> > > +			sizeof(*linfo) * (nr_linfo - i));
> > > +
> > > +		prog->aux->nr_linfo -= l_cnt;
> > > +		nr_linfo = prog->aux->nr_linfo;
> > > +	}
> > > +
> > > +	/* pull all linfo[i].insn_off >= off + cnt in by cnt */
> > > +	for (i = l_off; i < nr_linfo; i++)
> > > +		linfo[i].insn_off -= cnt;
> > > +
> > > +	/* fix up all subprogs (incl. 'exit') which start >= off */
> > > +	for (i = 0; i <= env->subprog_cnt; i++)
> > > +		if (env->subprog_info[i].linfo_idx > l_off) {
> > > +			if (env->subprog_info[i].linfo_idx >= l_off + l_cnt)
> > > +				env->subprog_info[i].linfo_idx -= l_cnt;
> > > +			else
> > > +				env->subprog_info[i].linfo_idx = l_off;  
> > 
> > For l_off < linfo_idx < l_off + lcnt, had those subprog_info already been
> > removed in adjust_subprog_starts_after_remove()?
> 
> If we remove tail of one program and start of another this will set the
> linfo_idx to the new first instruction's linfo_idx.
Thanks for the explanation.  Make sense after another thought.
It would be very helpful to add another comment here.

In general, I feel the bpf_adj_line_after_remove() is quite
tricky to read....could be me slow only.

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag
From: Tom Herbert @ 2019-01-23  1:03 UTC (permalink / raw)
  To: Peter Oskolkov
  Cc: David Miller, Linux Kernel Network Developers, Peter Oskolkov,
	Eric Dumazet, Florian Westphal
In-Reply-To: <CAPNVh5cUh_yOTmnkwdxtRR4TQCGVnJUMOCvJoa49bvR-6J8bQg@mail.gmail.com>

On Tue, Jan 22, 2019 at 4:53 PM Peter Oskolkov <posk@google.com> wrote:
>
> On Tue, Jan 22, 2019 at 4:37 PM Tom Herbert <tom@herbertland.com> wrote:
> >
> > On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@google.com> wrote:
> > >
> > > Currently, IPv6 defragmentation code drops non-last fragments that
> > > are smaller than 1280 bytes: see
> > > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
> > >
> > Peter,
> >
> > I'm a bit confused on why so much code is required to undo this patch
> > which was a whole three lines of code to begin with.
>
> I agree that if the goal is to just lower the 1280 IPv6 fragment
> limit, but keep it reasonably high, then my patchset is not needed.
> However, if the decision (ultimately by David Miller, I guess) is to
> accept fragments of any size, as the standard seems to imply, then
> something similar to what I suggested here is needed, I think. This
> was done in IPv4...

Peter,

Sorry, I'm still not following. Before "ipv6: defrag: drop non-last
frags smaller than min mtu" wouldn't we have accepted fragments of any
size? Are you saying that things were previously broken otherwise?

Tom

>
> Thanks,
> Peter
>
> >
> > > This behavior is not specified in IPv6 RFCs and appears to break
> > > compatibility with some IPv6 implemenations, as reported here:
> > > https://www.spinics.net/lists/netdev/msg543846.html
> > >
> > I am not yet convinced that the patch should be reverted. While it is
> > not conformant, it is a potential mitigation against DOS attack. For
> > instance, a single fragmented packet could be composed of over 8000
> > fragments with the minimum fragment size of eight bytes. Receiving
> > packets like that attacks both memory and CPU, and I don't see any
> > purpose to sending tiny fragments other than DOS attack. For practical
> > implemenation, I believe a limit is justified. The 1280 MTU limit in
> > the patch was too austere, I think it should be smaller than that.
> > Also, any such limit should apply to fragments payload length and not
> > length of the packet since a clever attacker could stuff the packet
> > with various other extension headers and still end up sending tiny
> > fragments. I have a patch along these lines with limit configurable by
> > sysctl. I will post shortly.
> >
> > Tom
> >
> > > This patch re-uses common IP defragmentation queueing and reassembly
> > > code in IPv6, removing the 1280 byte restriction.
> > >
> > > Signed-off-by: Peter Oskolkov <posk@google.com>
> > > Reported-by: Tom Herbert <tom@herbertland.com>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Florian Westphal <fw@strlen.de>
> > > ---
> > >  include/net/ipv6_frag.h |  11 +-
> > >  net/ipv6/reassembly.c   | 233 +++++++++++-----------------------------
> > >  2 files changed, 71 insertions(+), 173 deletions(-)
> > >
> > > diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
> > > index 6ced1e6899b6..28aa9b30aece 100644
> > > --- a/include/net/ipv6_frag.h
> > > +++ b/include/net/ipv6_frag.h
> > > @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
> > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
> > >
> > >         /* Don't send error if the first segment did not arrive. */
> > > -       head = fq->q.fragments;
> > > -       if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
> > > +       if (!(fq->q.flags & INET_FRAG_FIRST_IN))
> > > +               goto out;
> > > +
> > > +       /* sk_buff::dev and sk_buff::rbnode are unionized. So we
> > > +        * pull the head out of the tree in order to be able to
> > > +        * deal with head->dev.
> > > +        */
> > > +       head = inet_frag_pull_head(&fq->q);
> > > +       if (!head)
> > >                 goto out;
> > >
> > >         head->dev = dev;
> > > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> > > index 36a3d8dc61f5..24264d0a4b85 100644
> > > --- a/net/ipv6/reassembly.c
> > > +++ b/net/ipv6/reassembly.c
> > > @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
> > >
> > >  static struct inet_frags ip6_frags;
> > >
> > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > -                         struct net_device *dev);
> > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > +                         struct sk_buff *prev_tail, struct net_device *dev);
> > >
> > >  static void ip6_frag_expire(struct timer_list *t)
> > >  {
> > > @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > >                           struct frag_hdr *fhdr, int nhoff,
> > >                           u32 *prob_offset)
> > >  {
> > > -       struct sk_buff *prev, *next;
> > > -       struct net_device *dev;
> > > -       int offset, end, fragsize;
> > >         struct net *net = dev_net(skb_dst(skb)->dev);
> > > +       int offset, end, fragsize;
> > > +       struct sk_buff *prev_tail;
> > > +       struct net_device *dev;
> > > +       int err = -ENOENT;
> > >         u8 ecn;
> > >
> > >         if (fq->q.flags & INET_FRAG_COMPLETE)
> > >                 goto err;
> > >
> > > +       err = -EINVAL;
> > >         offset = ntohs(fhdr->frag_off) & ~0x7;
> > >         end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
> > >                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
> > >
> > >         if ((unsigned int)end > IPV6_MAXPLEN) {
> > >                 *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
> > > +               /* note that if prob_offset is set, the skb is freed elsewhere,
> > > +                * we do not free it here.
> > > +                */
> > >                 return -1;
> > >         }
> > >
> > > @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > >         if (end == offset)
> > >                 goto discard_fq;
> > >
> > > +       err = -ENOMEM;
> > >         /* Point into the IP datagram 'data' part. */
> > >         if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data))
> > >                 goto discard_fq;
> > >
> > > -       if (pskb_trim_rcsum(skb, end - offset))
> > > +       err = pskb_trim_rcsum(skb, end - offset);
> > > +       if (err)
> > >                 goto discard_fq;
> > >
> > > -       /* Find out which fragments are in front and at the back of us
> > > -        * in the chain of fragments so far.  We must know where to put
> > > -        * this fragment, right?
> > > -        */
> > > -       prev = fq->q.fragments_tail;
> > > -       if (!prev || prev->ip_defrag_offset < offset) {
> > > -               next = NULL;
> > > -               goto found;
> > > -       }
> > > -       prev = NULL;
> > > -       for (next = fq->q.fragments; next != NULL; next = next->next) {
> > > -               if (next->ip_defrag_offset >= offset)
> > > -                       break;  /* bingo! */
> > > -               prev = next;
> > > -       }
> > > -
> > > -found:
> > > -       /* 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.
> > > -        */
> > > -
> > > -       /* Check for overlap with preceding fragment. */
> > > -       if (prev &&
> > > -           (prev->ip_defrag_offset + prev->len) > offset)
> > > -               goto discard_fq;
> > > -
> > > -       /* Look for overlap with succeeding segment. */
> > > -       if (next && next->ip_defrag_offset < end)
> > > -               goto discard_fq;
> > > -
> > > -       /* Note : skb->ip_defrag_offset and skb->sk share the same location */
> > > +       /* Note : skb->rbnode and skb->dev share the same location. */
> > >         dev = skb->dev;
> > > -       if (dev)
> > > -               fq->iif = dev->ifindex;
> > >         /* Makes sure compiler wont do silly aliasing games */
> > >         barrier();
> > > -       skb->ip_defrag_offset = offset;
> > >
> > > -       /* Insert this fragment in the chain of fragments. */
> > > -       skb->next = next;
> > > -       if (!next)
> > > -               fq->q.fragments_tail = skb;
> > > -       if (prev)
> > > -               prev->next = skb;
> > > -       else
> > > -               fq->q.fragments = skb;
> > > +       prev_tail = fq->q.fragments_tail;
> > > +       err = inet_frag_queue_insert(&fq->q, skb, offset, end);
> > > +       if (err)
> > > +               goto insert_error;
> > > +
> > > +       if (dev)
> > > +               fq->iif = dev->ifindex;
> > >
> > >         fq->q.stamp = skb->tstamp;
> > >         fq->q.meat += skb->len;
> > > @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > >
> > >         if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > >             fq->q.meat == fq->q.len) {
> > > -               int res;
> > >                 unsigned long orefdst = skb->_skb_refdst;
> > >
> > >                 skb->_skb_refdst = 0UL;
> > > -               res = ip6_frag_reasm(fq, prev, dev);
> > > +               err = ip6_frag_reasm(fq, skb, prev_tail, dev);
> > >                 skb->_skb_refdst = orefdst;
> > > -               return res;
> > > +               return err;
> > >         }
> > >
> > >         skb_dst_drop(skb);
> > > -       return -1;
> > > +       return -EINPROGRESS;
> > >
> > > +insert_error:
> > > +       if (err == IPFRAG_DUP) {
> > > +               kfree_skb(skb);
> > > +               return -EINVAL;
> > > +       }
> > > +       err = -EINVAL;
> > > +       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > > +                       IPSTATS_MIB_REASM_OVERLAPS);
> > >  discard_fq:
> > >         inet_frag_kill(&fq->q);
> > > -err:
> > >         __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > >                         IPSTATS_MIB_REASMFAILS);
> > > +err:
> > >         kfree_skb(skb);
> > > -       return -1;
> > > +       return err;
> > >  }
> > >
> > >  /*
> > >   *     Check if this packet is complete.
> > > - *     Returns NULL on failure by any reason, and pointer
> > > - *     to current nexthdr field in reassembled frame.
> > >   *
> > >   *     It is called with locked fq, and caller must check that
> > >   *     queue is eligible for reassembly i.e. it is not COMPLETE,
> > >   *     the last and the first frames arrived and all the bits are here.
> > >   */
> > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > > -                         struct net_device *dev)
> > > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > > +                         struct sk_buff *prev_tail, struct net_device *dev)
> > >  {
> > >         struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> > > -       struct sk_buff *fp, *head = fq->q.fragments;
> > > -       int    payload_len, delta;
> > >         unsigned int nhoff;
> > > -       int sum_truesize;
> > > +       void *reasm_data;
> > > +       int payload_len;
> > >         u8 ecn;
> > >
> > >         inet_frag_kill(&fq->q);
> > > @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > >         if (unlikely(ecn == 0xff))
> > >                 goto out_fail;
> > >
> > > -       /* Make the one we just received the head. */
> > > -       if (prev) {
> > > -               head = prev->next;
> > > -               fp = skb_clone(head, GFP_ATOMIC);
> > > -
> > > -               if (!fp)
> > > -                       goto out_oom;
> > > -
> > > -               fp->next = head->next;
> > > -               if (!fp->next)
> > > -                       fq->q.fragments_tail = fp;
> > > -               prev->next = fp;
> > > -
> > > -               skb_morph(head, fq->q.fragments);
> > > -               head->next = fq->q.fragments->next;
> > > -
> > > -               consume_skb(fq->q.fragments);
> > > -               fq->q.fragments = head;
> > > -       }
> > > -
> > > -       WARN_ON(head == NULL);
> > > -       WARN_ON(head->ip_defrag_offset != 0);
> > > +       reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail);
> > > +       if (!reasm_data)
> > > +               goto out_oom;
> > >
> > > -       /* Unfragmented part is taken from the first segment. */
> > > -       payload_len = ((head->data - skb_network_header(head)) -
> > > +       payload_len = ((skb->data - skb_network_header(skb)) -
> > >                        sizeof(struct ipv6hdr) + fq->q.len -
> > >                        sizeof(struct frag_hdr));
> > >         if (payload_len > IPV6_MAXPLEN)
> > >                 goto out_oversize;
> > >
> > > -       delta = - head->truesize;
> > > -
> > > -       /* Head of list must not be cloned. */
> > > -       if (skb_unclone(head, GFP_ATOMIC))
> > > -               goto out_oom;
> > > -
> > > -       delta += head->truesize;
> > > -       if (delta)
> > > -               add_frag_mem_limit(fq->q.net, delta);
> > > -
> > > -       /* If the first fragment is fragmented itself, we split
> > > -        * it to two chunks: the first with data and paged part
> > > -        * and the second, holding only fragments. */
> > > -       if (skb_has_frag_list(head)) {
> > > -               struct sk_buff *clone;
> > > -               int i, plen = 0;
> > > -
> > > -               clone = alloc_skb(0, GFP_ATOMIC);
> > > -               if (!clone)
> > > -                       goto out_oom;
> > > -               clone->next = head->next;
> > > -               head->next = clone;
> > > -               skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
> > > -               skb_frag_list_init(head);
> > > -               for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
> > > -                       plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
> > > -               clone->len = clone->data_len = head->data_len - plen;
> > > -               head->data_len -= clone->len;
> > > -               head->len -= clone->len;
> > > -               clone->csum = 0;
> > > -               clone->ip_summed = head->ip_summed;
> > > -               add_frag_mem_limit(fq->q.net, clone->truesize);
> > > -       }
> > > -
> > >         /* We have to remove fragment header from datagram and to relocate
> > >          * header in order to calculate ICV correctly. */
> > >         nhoff = fq->nhoffset;
> > > -       skb_network_header(head)[nhoff] = skb_transport_header(head)[0];
> > > -       memmove(head->head + sizeof(struct frag_hdr), head->head,
> > > -               (head->data - head->head) - sizeof(struct frag_hdr));
> > > -       if (skb_mac_header_was_set(head))
> > > -               head->mac_header += sizeof(struct frag_hdr);
> > > -       head->network_header += sizeof(struct frag_hdr);
> > > -
> > > -       skb_reset_transport_header(head);
> > > -       skb_push(head, head->data - skb_network_header(head));
> > > -
> > > -       sum_truesize = head->truesize;
> > > -       for (fp = head->next; fp;) {
> > > -               bool headstolen;
> > > -               int delta;
> > > -               struct sk_buff *next = fp->next;
> > > -
> > > -               sum_truesize += fp->truesize;
> > > -               if (head->ip_summed != fp->ip_summed)
> > > -                       head->ip_summed = CHECKSUM_NONE;
> > > -               else if (head->ip_summed == CHECKSUM_COMPLETE)
> > > -                       head->csum = csum_add(head->csum, fp->csum);
> > > -
> > > -               if (skb_try_coalesce(head, fp, &headstolen, &delta)) {
> > > -                       kfree_skb_partial(fp, headstolen);
> > > -               } else {
> > > -                       fp->sk = NULL;
> > > -                       if (!skb_shinfo(head)->frag_list)
> > > -                               skb_shinfo(head)->frag_list = fp;
> > > -                       head->data_len += fp->len;
> > > -                       head->len += fp->len;
> > > -                       head->truesize += fp->truesize;
> > > -               }
> > > -               fp = next;
> > > -       }
> > > -       sub_frag_mem_limit(fq->q.net, sum_truesize);
> > > +       skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0];
> > > +       memmove(skb->head + sizeof(struct frag_hdr), skb->head,
> > > +               (skb->data - skb->head) - sizeof(struct frag_hdr));
> > > +       if (skb_mac_header_was_set(skb))
> > > +               skb->mac_header += sizeof(struct frag_hdr);
> > > +       skb->network_header += sizeof(struct frag_hdr);
> > > +
> > > +       skb_reset_transport_header(skb);
> > > +
> > > +       inet_frag_reasm_finish(&fq->q, skb, reasm_data);
> > >
> > > -       skb_mark_not_on_list(head);
> > > -       head->dev = dev;
> > > -       head->tstamp = fq->q.stamp;
> > > -       ipv6_hdr(head)->payload_len = htons(payload_len);
> > > -       ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
> > > -       IP6CB(head)->nhoff = nhoff;
> > > -       IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
> > > -       IP6CB(head)->frag_max_size = fq->q.max_size;
> > > +       skb->dev = dev;
> > > +       ipv6_hdr(skb)->payload_len = htons(payload_len);
> > > +       ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn);
> > > +       IP6CB(skb)->nhoff = nhoff;
> > > +       IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
> > > +       IP6CB(skb)->frag_max_size = fq->q.max_size;
> > >
> > >         /* Yes, and fold redundant checksum back. 8) */
> > > -       skb_postpush_rcsum(head, skb_network_header(head),
> > > -                          skb_network_header_len(head));
> > > +       skb_postpush_rcsum(skb, skb_network_header(skb),
> > > +                          skb_network_header_len(skb));
> > >
> > >         rcu_read_lock();
> > >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> > > @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > >         fq->q.fragments = NULL;
> > >         fq->q.rb_fragments = RB_ROOT;
> > >         fq->q.fragments_tail = NULL;
> > > +       fq->q.last_run_head = NULL;
> > >         return 1;
> > >
> > >  out_oversize:
> > > @@ -464,10 +358,6 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > >                 return 1;
> > >         }
> > >
> > > -       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> > > -           fhdr->frag_off & htons(IP6_MF))
> > > -               goto fail_hdr;
> > > -
> > >         iif = skb->dev ? skb->dev->ifindex : 0;
> > >         fq = fq_find(net, fhdr->identification, hdr, iif);
> > >         if (fq) {
> > > @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> > >                 if (prob_offset) {
> > >                         __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
> > >                                         IPSTATS_MIB_INHDRERRORS);
> > > +                       /* icmpv6_param_prob() calls kfree_skb(skb) */
> > >                         icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
> > >                 }
> > >                 return ret;
> > > --
> > > 2.20.1.321.g9e740568ce-goog
> > >

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag
From: Peter Oskolkov @ 2019-01-23  0:53 UTC (permalink / raw)
  To: Tom Herbert
  Cc: David Miller, Linux Kernel Network Developers, Peter Oskolkov,
	Eric Dumazet, Florian Westphal
In-Reply-To: <CALx6S35aNoSgP5UyA70xE=3h_haWw8NU11ViaqUqG=uGbtzUgQ@mail.gmail.com>

On Tue, Jan 22, 2019 at 4:37 PM Tom Herbert <tom@herbertland.com> wrote:
>
> On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@google.com> wrote:
> >
> > Currently, IPv6 defragmentation code drops non-last fragments that
> > are smaller than 1280 bytes: see
> > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
> >
> Peter,
>
> I'm a bit confused on why so much code is required to undo this patch
> which was a whole three lines of code to begin with.

I agree that if the goal is to just lower the 1280 IPv6 fragment
limit, but keep it reasonably high, then my patchset is not needed.
However, if the decision (ultimately by David Miller, I guess) is to
accept fragments of any size, as the standard seems to imply, then
something similar to what I suggested here is needed, I think. This
was done in IPv4...

Thanks,
Peter

>
> > This behavior is not specified in IPv6 RFCs and appears to break
> > compatibility with some IPv6 implemenations, as reported here:
> > https://www.spinics.net/lists/netdev/msg543846.html
> >
> I am not yet convinced that the patch should be reverted. While it is
> not conformant, it is a potential mitigation against DOS attack. For
> instance, a single fragmented packet could be composed of over 8000
> fragments with the minimum fragment size of eight bytes. Receiving
> packets like that attacks both memory and CPU, and I don't see any
> purpose to sending tiny fragments other than DOS attack. For practical
> implemenation, I believe a limit is justified. The 1280 MTU limit in
> the patch was too austere, I think it should be smaller than that.
> Also, any such limit should apply to fragments payload length and not
> length of the packet since a clever attacker could stuff the packet
> with various other extension headers and still end up sending tiny
> fragments. I have a patch along these lines with limit configurable by
> sysctl. I will post shortly.
>
> Tom
>
> > This patch re-uses common IP defragmentation queueing and reassembly
> > code in IPv6, removing the 1280 byte restriction.
> >
> > Signed-off-by: Peter Oskolkov <posk@google.com>
> > Reported-by: Tom Herbert <tom@herbertland.com>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Florian Westphal <fw@strlen.de>
> > ---
> >  include/net/ipv6_frag.h |  11 +-
> >  net/ipv6/reassembly.c   | 233 +++++++++++-----------------------------
> >  2 files changed, 71 insertions(+), 173 deletions(-)
> >
> > diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
> > index 6ced1e6899b6..28aa9b30aece 100644
> > --- a/include/net/ipv6_frag.h
> > +++ b/include/net/ipv6_frag.h
> > @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
> >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
> >
> >         /* Don't send error if the first segment did not arrive. */
> > -       head = fq->q.fragments;
> > -       if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
> > +       if (!(fq->q.flags & INET_FRAG_FIRST_IN))
> > +               goto out;
> > +
> > +       /* sk_buff::dev and sk_buff::rbnode are unionized. So we
> > +        * pull the head out of the tree in order to be able to
> > +        * deal with head->dev.
> > +        */
> > +       head = inet_frag_pull_head(&fq->q);
> > +       if (!head)
> >                 goto out;
> >
> >         head->dev = dev;
> > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> > index 36a3d8dc61f5..24264d0a4b85 100644
> > --- a/net/ipv6/reassembly.c
> > +++ b/net/ipv6/reassembly.c
> > @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
> >
> >  static struct inet_frags ip6_frags;
> >
> > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > -                         struct net_device *dev);
> > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > +                         struct sk_buff *prev_tail, struct net_device *dev);
> >
> >  static void ip6_frag_expire(struct timer_list *t)
> >  {
> > @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> >                           struct frag_hdr *fhdr, int nhoff,
> >                           u32 *prob_offset)
> >  {
> > -       struct sk_buff *prev, *next;
> > -       struct net_device *dev;
> > -       int offset, end, fragsize;
> >         struct net *net = dev_net(skb_dst(skb)->dev);
> > +       int offset, end, fragsize;
> > +       struct sk_buff *prev_tail;
> > +       struct net_device *dev;
> > +       int err = -ENOENT;
> >         u8 ecn;
> >
> >         if (fq->q.flags & INET_FRAG_COMPLETE)
> >                 goto err;
> >
> > +       err = -EINVAL;
> >         offset = ntohs(fhdr->frag_off) & ~0x7;
> >         end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
> >                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
> >
> >         if ((unsigned int)end > IPV6_MAXPLEN) {
> >                 *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
> > +               /* note that if prob_offset is set, the skb is freed elsewhere,
> > +                * we do not free it here.
> > +                */
> >                 return -1;
> >         }
> >
> > @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> >         if (end == offset)
> >                 goto discard_fq;
> >
> > +       err = -ENOMEM;
> >         /* Point into the IP datagram 'data' part. */
> >         if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data))
> >                 goto discard_fq;
> >
> > -       if (pskb_trim_rcsum(skb, end - offset))
> > +       err = pskb_trim_rcsum(skb, end - offset);
> > +       if (err)
> >                 goto discard_fq;
> >
> > -       /* Find out which fragments are in front and at the back of us
> > -        * in the chain of fragments so far.  We must know where to put
> > -        * this fragment, right?
> > -        */
> > -       prev = fq->q.fragments_tail;
> > -       if (!prev || prev->ip_defrag_offset < offset) {
> > -               next = NULL;
> > -               goto found;
> > -       }
> > -       prev = NULL;
> > -       for (next = fq->q.fragments; next != NULL; next = next->next) {
> > -               if (next->ip_defrag_offset >= offset)
> > -                       break;  /* bingo! */
> > -               prev = next;
> > -       }
> > -
> > -found:
> > -       /* 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.
> > -        */
> > -
> > -       /* Check for overlap with preceding fragment. */
> > -       if (prev &&
> > -           (prev->ip_defrag_offset + prev->len) > offset)
> > -               goto discard_fq;
> > -
> > -       /* Look for overlap with succeeding segment. */
> > -       if (next && next->ip_defrag_offset < end)
> > -               goto discard_fq;
> > -
> > -       /* Note : skb->ip_defrag_offset and skb->sk share the same location */
> > +       /* Note : skb->rbnode and skb->dev share the same location. */
> >         dev = skb->dev;
> > -       if (dev)
> > -               fq->iif = dev->ifindex;
> >         /* Makes sure compiler wont do silly aliasing games */
> >         barrier();
> > -       skb->ip_defrag_offset = offset;
> >
> > -       /* Insert this fragment in the chain of fragments. */
> > -       skb->next = next;
> > -       if (!next)
> > -               fq->q.fragments_tail = skb;
> > -       if (prev)
> > -               prev->next = skb;
> > -       else
> > -               fq->q.fragments = skb;
> > +       prev_tail = fq->q.fragments_tail;
> > +       err = inet_frag_queue_insert(&fq->q, skb, offset, end);
> > +       if (err)
> > +               goto insert_error;
> > +
> > +       if (dev)
> > +               fq->iif = dev->ifindex;
> >
> >         fq->q.stamp = skb->tstamp;
> >         fq->q.meat += skb->len;
> > @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> >
> >         if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> >             fq->q.meat == fq->q.len) {
> > -               int res;
> >                 unsigned long orefdst = skb->_skb_refdst;
> >
> >                 skb->_skb_refdst = 0UL;
> > -               res = ip6_frag_reasm(fq, prev, dev);
> > +               err = ip6_frag_reasm(fq, skb, prev_tail, dev);
> >                 skb->_skb_refdst = orefdst;
> > -               return res;
> > +               return err;
> >         }
> >
> >         skb_dst_drop(skb);
> > -       return -1;
> > +       return -EINPROGRESS;
> >
> > +insert_error:
> > +       if (err == IPFRAG_DUP) {
> > +               kfree_skb(skb);
> > +               return -EINVAL;
> > +       }
> > +       err = -EINVAL;
> > +       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> > +                       IPSTATS_MIB_REASM_OVERLAPS);
> >  discard_fq:
> >         inet_frag_kill(&fq->q);
> > -err:
> >         __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> >                         IPSTATS_MIB_REASMFAILS);
> > +err:
> >         kfree_skb(skb);
> > -       return -1;
> > +       return err;
> >  }
> >
> >  /*
> >   *     Check if this packet is complete.
> > - *     Returns NULL on failure by any reason, and pointer
> > - *     to current nexthdr field in reassembled frame.
> >   *
> >   *     It is called with locked fq, and caller must check that
> >   *     queue is eligible for reassembly i.e. it is not COMPLETE,
> >   *     the last and the first frames arrived and all the bits are here.
> >   */
> > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> > -                         struct net_device *dev)
> > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> > +                         struct sk_buff *prev_tail, struct net_device *dev)
> >  {
> >         struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> > -       struct sk_buff *fp, *head = fq->q.fragments;
> > -       int    payload_len, delta;
> >         unsigned int nhoff;
> > -       int sum_truesize;
> > +       void *reasm_data;
> > +       int payload_len;
> >         u8 ecn;
> >
> >         inet_frag_kill(&fq->q);
> > @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> >         if (unlikely(ecn == 0xff))
> >                 goto out_fail;
> >
> > -       /* Make the one we just received the head. */
> > -       if (prev) {
> > -               head = prev->next;
> > -               fp = skb_clone(head, GFP_ATOMIC);
> > -
> > -               if (!fp)
> > -                       goto out_oom;
> > -
> > -               fp->next = head->next;
> > -               if (!fp->next)
> > -                       fq->q.fragments_tail = fp;
> > -               prev->next = fp;
> > -
> > -               skb_morph(head, fq->q.fragments);
> > -               head->next = fq->q.fragments->next;
> > -
> > -               consume_skb(fq->q.fragments);
> > -               fq->q.fragments = head;
> > -       }
> > -
> > -       WARN_ON(head == NULL);
> > -       WARN_ON(head->ip_defrag_offset != 0);
> > +       reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail);
> > +       if (!reasm_data)
> > +               goto out_oom;
> >
> > -       /* Unfragmented part is taken from the first segment. */
> > -       payload_len = ((head->data - skb_network_header(head)) -
> > +       payload_len = ((skb->data - skb_network_header(skb)) -
> >                        sizeof(struct ipv6hdr) + fq->q.len -
> >                        sizeof(struct frag_hdr));
> >         if (payload_len > IPV6_MAXPLEN)
> >                 goto out_oversize;
> >
> > -       delta = - head->truesize;
> > -
> > -       /* Head of list must not be cloned. */
> > -       if (skb_unclone(head, GFP_ATOMIC))
> > -               goto out_oom;
> > -
> > -       delta += head->truesize;
> > -       if (delta)
> > -               add_frag_mem_limit(fq->q.net, delta);
> > -
> > -       /* If the first fragment is fragmented itself, we split
> > -        * it to two chunks: the first with data and paged part
> > -        * and the second, holding only fragments. */
> > -       if (skb_has_frag_list(head)) {
> > -               struct sk_buff *clone;
> > -               int i, plen = 0;
> > -
> > -               clone = alloc_skb(0, GFP_ATOMIC);
> > -               if (!clone)
> > -                       goto out_oom;
> > -               clone->next = head->next;
> > -               head->next = clone;
> > -               skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
> > -               skb_frag_list_init(head);
> > -               for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
> > -                       plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
> > -               clone->len = clone->data_len = head->data_len - plen;
> > -               head->data_len -= clone->len;
> > -               head->len -= clone->len;
> > -               clone->csum = 0;
> > -               clone->ip_summed = head->ip_summed;
> > -               add_frag_mem_limit(fq->q.net, clone->truesize);
> > -       }
> > -
> >         /* We have to remove fragment header from datagram and to relocate
> >          * header in order to calculate ICV correctly. */
> >         nhoff = fq->nhoffset;
> > -       skb_network_header(head)[nhoff] = skb_transport_header(head)[0];
> > -       memmove(head->head + sizeof(struct frag_hdr), head->head,
> > -               (head->data - head->head) - sizeof(struct frag_hdr));
> > -       if (skb_mac_header_was_set(head))
> > -               head->mac_header += sizeof(struct frag_hdr);
> > -       head->network_header += sizeof(struct frag_hdr);
> > -
> > -       skb_reset_transport_header(head);
> > -       skb_push(head, head->data - skb_network_header(head));
> > -
> > -       sum_truesize = head->truesize;
> > -       for (fp = head->next; fp;) {
> > -               bool headstolen;
> > -               int delta;
> > -               struct sk_buff *next = fp->next;
> > -
> > -               sum_truesize += fp->truesize;
> > -               if (head->ip_summed != fp->ip_summed)
> > -                       head->ip_summed = CHECKSUM_NONE;
> > -               else if (head->ip_summed == CHECKSUM_COMPLETE)
> > -                       head->csum = csum_add(head->csum, fp->csum);
> > -
> > -               if (skb_try_coalesce(head, fp, &headstolen, &delta)) {
> > -                       kfree_skb_partial(fp, headstolen);
> > -               } else {
> > -                       fp->sk = NULL;
> > -                       if (!skb_shinfo(head)->frag_list)
> > -                               skb_shinfo(head)->frag_list = fp;
> > -                       head->data_len += fp->len;
> > -                       head->len += fp->len;
> > -                       head->truesize += fp->truesize;
> > -               }
> > -               fp = next;
> > -       }
> > -       sub_frag_mem_limit(fq->q.net, sum_truesize);
> > +       skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0];
> > +       memmove(skb->head + sizeof(struct frag_hdr), skb->head,
> > +               (skb->data - skb->head) - sizeof(struct frag_hdr));
> > +       if (skb_mac_header_was_set(skb))
> > +               skb->mac_header += sizeof(struct frag_hdr);
> > +       skb->network_header += sizeof(struct frag_hdr);
> > +
> > +       skb_reset_transport_header(skb);
> > +
> > +       inet_frag_reasm_finish(&fq->q, skb, reasm_data);
> >
> > -       skb_mark_not_on_list(head);
> > -       head->dev = dev;
> > -       head->tstamp = fq->q.stamp;
> > -       ipv6_hdr(head)->payload_len = htons(payload_len);
> > -       ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
> > -       IP6CB(head)->nhoff = nhoff;
> > -       IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
> > -       IP6CB(head)->frag_max_size = fq->q.max_size;
> > +       skb->dev = dev;
> > +       ipv6_hdr(skb)->payload_len = htons(payload_len);
> > +       ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn);
> > +       IP6CB(skb)->nhoff = nhoff;
> > +       IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
> > +       IP6CB(skb)->frag_max_size = fq->q.max_size;
> >
> >         /* Yes, and fold redundant checksum back. 8) */
> > -       skb_postpush_rcsum(head, skb_network_header(head),
> > -                          skb_network_header_len(head));
> > +       skb_postpush_rcsum(skb, skb_network_header(skb),
> > +                          skb_network_header_len(skb));
> >
> >         rcu_read_lock();
> >         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> > @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> >         fq->q.fragments = NULL;
> >         fq->q.rb_fragments = RB_ROOT;
> >         fq->q.fragments_tail = NULL;
> > +       fq->q.last_run_head = NULL;
> >         return 1;
> >
> >  out_oversize:
> > @@ -464,10 +358,6 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> >                 return 1;
> >         }
> >
> > -       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> > -           fhdr->frag_off & htons(IP6_MF))
> > -               goto fail_hdr;
> > -
> >         iif = skb->dev ? skb->dev->ifindex : 0;
> >         fq = fq_find(net, fhdr->identification, hdr, iif);
> >         if (fq) {
> > @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
> >                 if (prob_offset) {
> >                         __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
> >                                         IPSTATS_MIB_INHDRERRORS);
> > +                       /* icmpv6_param_prob() calls kfree_skb(skb) */
> >                         icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
> >                 }
> >                 return ret;
> > --
> > 2.20.1.321.g9e740568ce-goog
> >

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag
From: Tom Herbert @ 2019-01-23  0:37 UTC (permalink / raw)
  To: Peter Oskolkov
  Cc: David Miller, Linux Kernel Network Developers, Peter Oskolkov,
	Eric Dumazet, Florian Westphal
In-Reply-To: <20190122180253.128336-3-posk@google.com>

On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@google.com> wrote:
>
> Currently, IPv6 defragmentation code drops non-last fragments that
> are smaller than 1280 bytes: see
> commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu")
>
Peter,

I'm a bit confused on why so much code is required to undo this patch
which was a whole three lines of code to begin with.

> This behavior is not specified in IPv6 RFCs and appears to break
> compatibility with some IPv6 implemenations, as reported here:
> https://www.spinics.net/lists/netdev/msg543846.html
>
I am not yet convinced that the patch should be reverted. While it is
not conformant, it is a potential mitigation against DOS attack. For
instance, a single fragmented packet could be composed of over 8000
fragments with the minimum fragment size of eight bytes. Receiving
packets like that attacks both memory and CPU, and I don't see any
purpose to sending tiny fragments other than DOS attack. For practical
implemenation, I believe a limit is justified. The 1280 MTU limit in
the patch was too austere, I think it should be smaller than that.
Also, any such limit should apply to fragments payload length and not
length of the packet since a clever attacker could stuff the packet
with various other extension headers and still end up sending tiny
fragments. I have a patch along these lines with limit configurable by
sysctl. I will post shortly.

Tom

> This patch re-uses common IP defragmentation queueing and reassembly
> code in IPv6, removing the 1280 byte restriction.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> Reported-by: Tom Herbert <tom@herbertland.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Florian Westphal <fw@strlen.de>
> ---
>  include/net/ipv6_frag.h |  11 +-
>  net/ipv6/reassembly.c   | 233 +++++++++++-----------------------------
>  2 files changed, 71 insertions(+), 173 deletions(-)
>
> diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
> index 6ced1e6899b6..28aa9b30aece 100644
> --- a/include/net/ipv6_frag.h
> +++ b/include/net/ipv6_frag.h
> @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
>         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
>
>         /* Don't send error if the first segment did not arrive. */
> -       head = fq->q.fragments;
> -       if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
> +       if (!(fq->q.flags & INET_FRAG_FIRST_IN))
> +               goto out;
> +
> +       /* sk_buff::dev and sk_buff::rbnode are unionized. So we
> +        * pull the head out of the tree in order to be able to
> +        * deal with head->dev.
> +        */
> +       head = inet_frag_pull_head(&fq->q);
> +       if (!head)
>                 goto out;
>
>         head->dev = dev;
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 36a3d8dc61f5..24264d0a4b85 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
>
>  static struct inet_frags ip6_frags;
>
> -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> -                         struct net_device *dev);
> +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> +                         struct sk_buff *prev_tail, struct net_device *dev);
>
>  static void ip6_frag_expire(struct timer_list *t)
>  {
> @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>                           struct frag_hdr *fhdr, int nhoff,
>                           u32 *prob_offset)
>  {
> -       struct sk_buff *prev, *next;
> -       struct net_device *dev;
> -       int offset, end, fragsize;
>         struct net *net = dev_net(skb_dst(skb)->dev);
> +       int offset, end, fragsize;
> +       struct sk_buff *prev_tail;
> +       struct net_device *dev;
> +       int err = -ENOENT;
>         u8 ecn;
>
>         if (fq->q.flags & INET_FRAG_COMPLETE)
>                 goto err;
>
> +       err = -EINVAL;
>         offset = ntohs(fhdr->frag_off) & ~0x7;
>         end = offset + (ntohs(ipv6_hdr(skb)->payload_len) -
>                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
>
>         if ((unsigned int)end > IPV6_MAXPLEN) {
>                 *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
> +               /* note that if prob_offset is set, the skb is freed elsewhere,
> +                * we do not free it here.
> +                */
>                 return -1;
>         }
>
> @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>         if (end == offset)
>                 goto discard_fq;
>
> +       err = -ENOMEM;
>         /* Point into the IP datagram 'data' part. */
>         if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data))
>                 goto discard_fq;
>
> -       if (pskb_trim_rcsum(skb, end - offset))
> +       err = pskb_trim_rcsum(skb, end - offset);
> +       if (err)
>                 goto discard_fq;
>
> -       /* Find out which fragments are in front and at the back of us
> -        * in the chain of fragments so far.  We must know where to put
> -        * this fragment, right?
> -        */
> -       prev = fq->q.fragments_tail;
> -       if (!prev || prev->ip_defrag_offset < offset) {
> -               next = NULL;
> -               goto found;
> -       }
> -       prev = NULL;
> -       for (next = fq->q.fragments; next != NULL; next = next->next) {
> -               if (next->ip_defrag_offset >= offset)
> -                       break;  /* bingo! */
> -               prev = next;
> -       }
> -
> -found:
> -       /* 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.
> -        */
> -
> -       /* Check for overlap with preceding fragment. */
> -       if (prev &&
> -           (prev->ip_defrag_offset + prev->len) > offset)
> -               goto discard_fq;
> -
> -       /* Look for overlap with succeeding segment. */
> -       if (next && next->ip_defrag_offset < end)
> -               goto discard_fq;
> -
> -       /* Note : skb->ip_defrag_offset and skb->sk share the same location */
> +       /* Note : skb->rbnode and skb->dev share the same location. */
>         dev = skb->dev;
> -       if (dev)
> -               fq->iif = dev->ifindex;
>         /* Makes sure compiler wont do silly aliasing games */
>         barrier();
> -       skb->ip_defrag_offset = offset;
>
> -       /* Insert this fragment in the chain of fragments. */
> -       skb->next = next;
> -       if (!next)
> -               fq->q.fragments_tail = skb;
> -       if (prev)
> -               prev->next = skb;
> -       else
> -               fq->q.fragments = skb;
> +       prev_tail = fq->q.fragments_tail;
> +       err = inet_frag_queue_insert(&fq->q, skb, offset, end);
> +       if (err)
> +               goto insert_error;
> +
> +       if (dev)
> +               fq->iif = dev->ifindex;
>
>         fq->q.stamp = skb->tstamp;
>         fq->q.meat += skb->len;
> @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>
>         if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
>             fq->q.meat == fq->q.len) {
> -               int res;
>                 unsigned long orefdst = skb->_skb_refdst;
>
>                 skb->_skb_refdst = 0UL;
> -               res = ip6_frag_reasm(fq, prev, dev);
> +               err = ip6_frag_reasm(fq, skb, prev_tail, dev);
>                 skb->_skb_refdst = orefdst;
> -               return res;
> +               return err;
>         }
>
>         skb_dst_drop(skb);
> -       return -1;
> +       return -EINPROGRESS;
>
> +insert_error:
> +       if (err == IPFRAG_DUP) {
> +               kfree_skb(skb);
> +               return -EINVAL;
> +       }
> +       err = -EINVAL;
> +       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> +                       IPSTATS_MIB_REASM_OVERLAPS);
>  discard_fq:
>         inet_frag_kill(&fq->q);
> -err:
>         __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
>                         IPSTATS_MIB_REASMFAILS);
> +err:
>         kfree_skb(skb);
> -       return -1;
> +       return err;
>  }
>
>  /*
>   *     Check if this packet is complete.
> - *     Returns NULL on failure by any reason, and pointer
> - *     to current nexthdr field in reassembled frame.
>   *
>   *     It is called with locked fq, and caller must check that
>   *     queue is eligible for reassembly i.e. it is not COMPLETE,
>   *     the last and the first frames arrived and all the bits are here.
>   */
> -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
> -                         struct net_device *dev)
> +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> +                         struct sk_buff *prev_tail, struct net_device *dev)
>  {
>         struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
> -       struct sk_buff *fp, *head = fq->q.fragments;
> -       int    payload_len, delta;
>         unsigned int nhoff;
> -       int sum_truesize;
> +       void *reasm_data;
> +       int payload_len;
>         u8 ecn;
>
>         inet_frag_kill(&fq->q);
> @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
>         if (unlikely(ecn == 0xff))
>                 goto out_fail;
>
> -       /* Make the one we just received the head. */
> -       if (prev) {
> -               head = prev->next;
> -               fp = skb_clone(head, GFP_ATOMIC);
> -
> -               if (!fp)
> -                       goto out_oom;
> -
> -               fp->next = head->next;
> -               if (!fp->next)
> -                       fq->q.fragments_tail = fp;
> -               prev->next = fp;
> -
> -               skb_morph(head, fq->q.fragments);
> -               head->next = fq->q.fragments->next;
> -
> -               consume_skb(fq->q.fragments);
> -               fq->q.fragments = head;
> -       }
> -
> -       WARN_ON(head == NULL);
> -       WARN_ON(head->ip_defrag_offset != 0);
> +       reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail);
> +       if (!reasm_data)
> +               goto out_oom;
>
> -       /* Unfragmented part is taken from the first segment. */
> -       payload_len = ((head->data - skb_network_header(head)) -
> +       payload_len = ((skb->data - skb_network_header(skb)) -
>                        sizeof(struct ipv6hdr) + fq->q.len -
>                        sizeof(struct frag_hdr));
>         if (payload_len > IPV6_MAXPLEN)
>                 goto out_oversize;
>
> -       delta = - head->truesize;
> -
> -       /* Head of list must not be cloned. */
> -       if (skb_unclone(head, GFP_ATOMIC))
> -               goto out_oom;
> -
> -       delta += head->truesize;
> -       if (delta)
> -               add_frag_mem_limit(fq->q.net, delta);
> -
> -       /* If the first fragment is fragmented itself, we split
> -        * it to two chunks: the first with data and paged part
> -        * and the second, holding only fragments. */
> -       if (skb_has_frag_list(head)) {
> -               struct sk_buff *clone;
> -               int i, plen = 0;
> -
> -               clone = alloc_skb(0, GFP_ATOMIC);
> -               if (!clone)
> -                       goto out_oom;
> -               clone->next = head->next;
> -               head->next = clone;
> -               skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list;
> -               skb_frag_list_init(head);
> -               for (i = 0; i < skb_shinfo(head)->nr_frags; i++)
> -                       plen += skb_frag_size(&skb_shinfo(head)->frags[i]);
> -               clone->len = clone->data_len = head->data_len - plen;
> -               head->data_len -= clone->len;
> -               head->len -= clone->len;
> -               clone->csum = 0;
> -               clone->ip_summed = head->ip_summed;
> -               add_frag_mem_limit(fq->q.net, clone->truesize);
> -       }
> -
>         /* We have to remove fragment header from datagram and to relocate
>          * header in order to calculate ICV correctly. */
>         nhoff = fq->nhoffset;
> -       skb_network_header(head)[nhoff] = skb_transport_header(head)[0];
> -       memmove(head->head + sizeof(struct frag_hdr), head->head,
> -               (head->data - head->head) - sizeof(struct frag_hdr));
> -       if (skb_mac_header_was_set(head))
> -               head->mac_header += sizeof(struct frag_hdr);
> -       head->network_header += sizeof(struct frag_hdr);
> -
> -       skb_reset_transport_header(head);
> -       skb_push(head, head->data - skb_network_header(head));
> -
> -       sum_truesize = head->truesize;
> -       for (fp = head->next; fp;) {
> -               bool headstolen;
> -               int delta;
> -               struct sk_buff *next = fp->next;
> -
> -               sum_truesize += fp->truesize;
> -               if (head->ip_summed != fp->ip_summed)
> -                       head->ip_summed = CHECKSUM_NONE;
> -               else if (head->ip_summed == CHECKSUM_COMPLETE)
> -                       head->csum = csum_add(head->csum, fp->csum);
> -
> -               if (skb_try_coalesce(head, fp, &headstolen, &delta)) {
> -                       kfree_skb_partial(fp, headstolen);
> -               } else {
> -                       fp->sk = NULL;
> -                       if (!skb_shinfo(head)->frag_list)
> -                               skb_shinfo(head)->frag_list = fp;
> -                       head->data_len += fp->len;
> -                       head->len += fp->len;
> -                       head->truesize += fp->truesize;
> -               }
> -               fp = next;
> -       }
> -       sub_frag_mem_limit(fq->q.net, sum_truesize);
> +       skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0];
> +       memmove(skb->head + sizeof(struct frag_hdr), skb->head,
> +               (skb->data - skb->head) - sizeof(struct frag_hdr));
> +       if (skb_mac_header_was_set(skb))
> +               skb->mac_header += sizeof(struct frag_hdr);
> +       skb->network_header += sizeof(struct frag_hdr);
> +
> +       skb_reset_transport_header(skb);
> +
> +       inet_frag_reasm_finish(&fq->q, skb, reasm_data);
>
> -       skb_mark_not_on_list(head);
> -       head->dev = dev;
> -       head->tstamp = fq->q.stamp;
> -       ipv6_hdr(head)->payload_len = htons(payload_len);
> -       ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn);
> -       IP6CB(head)->nhoff = nhoff;
> -       IP6CB(head)->flags |= IP6SKB_FRAGMENTED;
> -       IP6CB(head)->frag_max_size = fq->q.max_size;
> +       skb->dev = dev;
> +       ipv6_hdr(skb)->payload_len = htons(payload_len);
> +       ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn);
> +       IP6CB(skb)->nhoff = nhoff;
> +       IP6CB(skb)->flags |= IP6SKB_FRAGMENTED;
> +       IP6CB(skb)->frag_max_size = fq->q.max_size;
>
>         /* Yes, and fold redundant checksum back. 8) */
> -       skb_postpush_rcsum(head, skb_network_header(head),
> -                          skb_network_header_len(head));
> +       skb_postpush_rcsum(skb, skb_network_header(skb),
> +                          skb_network_header_len(skb));
>
>         rcu_read_lock();
>         __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
>         fq->q.fragments = NULL;
>         fq->q.rb_fragments = RB_ROOT;
>         fq->q.fragments_tail = NULL;
> +       fq->q.last_run_head = NULL;
>         return 1;
>
>  out_oversize:
> @@ -464,10 +358,6 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>                 return 1;
>         }
>
> -       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> -           fhdr->frag_off & htons(IP6_MF))
> -               goto fail_hdr;
> -
>         iif = skb->dev ? skb->dev->ifindex : 0;
>         fq = fq_find(net, fhdr->identification, hdr, iif);
>         if (fq) {
> @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>                 if (prob_offset) {
>                         __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev),
>                                         IPSTATS_MIB_INHDRERRORS);
> +                       /* icmpv6_param_prob() calls kfree_skb(skb) */
>                         icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
>                 }
>                 return ret;
> --
> 2.20.1.321.g9e740568ce-goog
>

^ permalink raw reply

* Re: net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs
From: Andrew Lunn @ 2019-01-23  0:22 UTC (permalink / raw)
  To: John David Anglin; +Cc: Russell King, Vivien Didelot, Florian Fainelli, netdev
In-Reply-To: <e74f35e2-d81c-7f86-a6b3-7cecce035ce9@bell.net>

> > It does not need to. There are two options here:
> >
> > 1) The PHY has no interrupt. phylib will poll the PHY once per second
> >    for link changes.
> >
> > 2) The PHY has in interrupt. Link changes will cause the interrupt to
> >    fire, and the phylib will then read the current state.
> >
> > For PHYs embedded within a switch driver by mv88e6xxx interrupts
> > should always be used.

Hi Dave

From my Espressobin

cat /proc/interrupts
...
 44:          0          0  mv88e6xxx-g1   3 Edge      mv88e6xxx-g1-atu-prob
 46:          0          0  mv88e6xxx-g1   5 Edge      mv88e6xxx-g1-vtu-prob
 48:         38         24  mv88e6xxx-g1   7 Edge      mv88e6xxx-g2
 51:          0          1  mv88e6xxx-g2   1 Edge      !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:11
 52:          0          0  mv88e6xxx-g2   2 Edge      !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:12
 53:         38         23  mv88e6xxx-g2   3 Edge      !soc!internal-regs@d0000000!mdio@32004!switch0@1!mdio:13

These are PHY interrupts.

> I don't think option 2) is implemented.  Didn't see any irq code in phy.c.

You would not. All the interrupt code is in the PHY core and the PHY
driver. drivers/net/dsa/mv88e6xxx/phy.c is just a bunch of helpers
which allow the mdio bus driver to access phy registers. The PHY
driver itself is drivers/net/phy/marvell.c, and the interrupt handling
is spread between that and drivers/net/phy/phy.c

> If I remember correctly, one needs to use clause 45 accesses to get at
> the PHY registers in the 88E6341.

Nope. The PHYs are c22 devices. The SERDES are probably C45, but those
are not being used here.

      Andrew

^ permalink raw reply

* Re: [PATCH bpf-next v5 03/12] bpf: verifier: remove dead code
From: Jakub Kicinski @ 2019-01-23  0:12 UTC (permalink / raw)
  To: Martin Lau
  Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net,
	netdev@vger.kernel.org, oss-drivers@netronome.com, Yonghong Song
In-Reply-To: <20190122234632.5ripepyqqeqtsbmx@kafai-mbp.dhcp.thefacebook.com>

On Tue, 22 Jan 2019 23:46:34 +0000, Martin Lau wrote:
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 71949c163b7a..fa8011409c51 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -6431,6 +6431,144 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
> >  	return new_prog;
> >  }
> >  
> > +static int adjust_subprog_starts_after_remove(struct bpf_verifier_env *env,
> > +					      u32 off, u32 cnt)
> > +{
> > +	int i, j;
> > +
> > +	/* find first prog starting at or after off (first to remove) */
> > +	for (i = 0; i < env->subprog_cnt; i++)
> > +		if (env->subprog_info[i].start >= off)
> > +			break;
> > +	/* find first prog starting at or after off + cnt (first to stay) */
> > +	for (j = i; j < env->subprog_cnt; j++)
> > +		if (env->subprog_info[j].start >= off + cnt)
> > +			break;
> > +	/* if j doesn't start exactly at off + cnt, we are just removing
> > +	 * the front of previous prog
> > +	 */
> > +	if (env->subprog_info[j].start != off + cnt)
> > +		j--;
> > +
> > +	if (j > i) {
> > +		struct bpf_prog_aux *aux = env->prog->aux;
> > +		int move;
> > +
> > +		/* move fake 'exit' subprog as well */
> > +		move = env->subprog_cnt + 1 - j;
> > +
> > +		memmove(env->subprog_info + i,
> > +			env->subprog_info + j,
> > +			sizeof(*env->subprog_info) * move);
> > +		env->subprog_cnt -= j - i;
> > +
> > +		/* remove func_info */  
> 
> It would be helpful to add a comment here to explain
> that func_info->insn_off is set later in adjust_btf_func(),
> so no need to adjust it here.

Will do!

> > +		if (aux->func_info) {
> > +			move = aux->func_info_cnt - j;
> > +
> > +			memmove(aux->func_info + i,
> > +				aux->func_info + j,
> > +				sizeof(*aux->func_info) * move);
> > +			aux->func_info_cnt -= j - i;
> > +		}
> > +	} else {
> > +		/* convert i from "first prog to remove" to "first to adjust" */
> > +		if (env->subprog_info[i].start == off)
> > +			i++;
> > +	}
> > +
> > +	/* update fake 'exit' subprog as well */
> > +	for (; i <= env->subprog_cnt; i++)
> > +		env->subprog_info[i].start -= cnt;
> > +
> > +	return 0;
> > +}
> > +
> > +static int bpf_adj_linfo_after_remove(struct bpf_verifier_env *env, u32 off,
> > +				      u32 cnt)
> > +{
> > +	struct bpf_prog *prog = env->prog;
> > +	u32 i, l_off, l_cnt, nr_linfo;
> > +	struct bpf_line_info *linfo;
> > +
> > +	nr_linfo = prog->aux->nr_linfo;
> > +	if (!nr_linfo)
> > +		return 0;
> > +
> > +	linfo = prog->aux->linfo;
> > +
> > +	/* find first line info to remove, count lines to be removed */
> > +	for (i = 0; i < nr_linfo; i++)
> > +		if (linfo[i].insn_off >= off)
> > +			break;
> > +
> > +	l_off = i;
> > +	l_cnt = 0;
> > +	for (; i < nr_linfo; i++)
> > +		if (linfo[i].insn_off < off + cnt)
> > +			l_cnt++;
> > +		else
> > +			break;
> > +
> > +	/* First live insn doesn't match first live linfo, it needs to "inherit"
> > +	 * last removed linfo.  prog is already modified, so prog->len == off
> > +	 * means no live instructions after.
> > +	 */
> > +	if (prog->len != off && l_cnt &&
> > +	    (i == nr_linfo || linfo[i].insn_off != off + cnt)) {
> > +		l_cnt--;
> > +		linfo[--i].insn_off = off + cnt;
> > +	}
> > +
> > +	/* remove the line info which refers to the removed instructions */
> > +	if (l_cnt) {
> > +		memmove(linfo + l_off, linfo + i,
> > +			sizeof(*linfo) * (nr_linfo - i));
> > +
> > +		prog->aux->nr_linfo -= l_cnt;
> > +		nr_linfo = prog->aux->nr_linfo;
> > +	}
> > +
> > +	/* pull all linfo[i].insn_off >= off + cnt in by cnt */
> > +	for (i = l_off; i < nr_linfo; i++)
> > +		linfo[i].insn_off -= cnt;
> > +
> > +	/* fix up all subprogs (incl. 'exit') which start >= off */
> > +	for (i = 0; i <= env->subprog_cnt; i++)
> > +		if (env->subprog_info[i].linfo_idx > l_off) {
> > +			if (env->subprog_info[i].linfo_idx >= l_off + l_cnt)
> > +				env->subprog_info[i].linfo_idx -= l_cnt;
> > +			else
> > +				env->subprog_info[i].linfo_idx = l_off;  
> 
> For l_off < linfo_idx < l_off + lcnt, had those subprog_info already been
> removed in adjust_subprog_starts_after_remove()?

If we remove tail of one program and start of another this will set the
linfo_idx to the new first instruction's linfo_idx.

> > +		}
> > +
> > +	return 0;
> > +}

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox