[PATCH net 0/5] tcp: more robust ooo handling

[PATCH net 0/5] tcp: more robust ooo handling

[Eric Dumazet]

Juha-Matti Tilli reported that malicious peers could inject tiny
packets in out_of_order_queue, forcing very expensive calls
to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
every incoming packet.

With tcp_rmem[2] default of 6MB, the ooo queue could
contain ~7000 nodes.

This patch series makes sure we cut cpu cycles enough to
render the attack not critical.

We might in the future go further, like disconnecting
or black-holing proven malicious flows.

Eric Dumazet (5):
  tcp: free batches of packets in tcp_prune_ofo_queue()
  tcp: avoid collapses in tcp_prune_queue() if possible
  tcp: detect malicious patterns in tcp_collapse_ofo_queue()
  tcp: call tcp_drop() from tcp_data_queue_ofo()
  tcp: add tcp_ooo_try_coalesce() helper

 net/ipv4/tcp_input.c | 62 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 12 deletions(-)

-- 
2.18.0.233.g985f88cf7e-goog

[PATCH net 1/5] tcp: free batches of packets in tcp_prune_ofo_queue()

[Eric Dumazet]

Juha-Matti Tilli reported that malicious peers could inject tiny
packets in out_of_order_queue, forcing very expensive calls
to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
every incoming packet. out_of_order_queue rb-tree can contain
thousands of nodes, iterating over all of them is not nice.

Before linux-4.9, we would have pruned all packets in ofo_queue
in one go, every XXXX packets. XXXX depends on sk_rcvbuf and skbs
truesize, but is about 7000 packets with tcp_rmem[2] default of 6 MB.

Since we plan to increase tcp_rmem[2] in the future to cope with
modern BDP, can not revert to the old behavior, without great pain.

Strategy taken in this patch is to purge ~12.5 % of the queue capacity.

Fixes: 36a6503fedda ("tcp: refine tcp_prune_ofo_queue() to not drop all packets")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
 net/ipv4/tcp_input.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 6bade06aaf72afea474ec83677d63b23e531ab68..64e45b279431886a50c8097593b9dbc9e5d75cc1 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4942,6 +4942,7 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
  * 2) not add too big latencies if thousands of packets sit there.
  *    (But if application shrinks SO_RCVBUF, we could still end up
  *     freeing whole queue here)
+ * 3) Drop at least 12.5 % of sk_rcvbuf to avoid malicious attacks.
  *
  * Return true if queue has shrunk.
  */
@@ -4949,20 +4950,26 @@ static bool tcp_prune_ofo_queue(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct rb_node *node, *prev;
+	int goal;
 
 	if (RB_EMPTY_ROOT(&tp->out_of_order_queue))
 		return false;
 
 	NET_INC_STATS(sock_net(sk), LINUX_MIB_OFOPRUNED);
+	goal = sk->sk_rcvbuf >> 3;
 	node = &tp->ooo_last_skb->rbnode;
 	do {
 		prev = rb_prev(node);
 		rb_erase(node, &tp->out_of_order_queue);
+		goal -= rb_to_skb(node)->truesize;
 		tcp_drop(sk, rb_to_skb(node));
-		sk_mem_reclaim(sk);
-		if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
-		    !tcp_under_memory_pressure(sk))
-			break;
+		if (!prev || goal <= 0) {
+			sk_mem_reclaim(sk);
+			if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
+			    !tcp_under_memory_pressure(sk))
+				break;
+			goal = sk->sk_rcvbuf >> 3;
+		}
 		node = prev;
 	} while (node);
 	tp->ooo_last_skb = rb_to_skb(prev);
-- 
2.18.0.233.g985f88cf7e-goog

[PATCH net 2/5] tcp: avoid collapses in tcp_prune_queue() if possible

[Eric Dumazet]

Right after a TCP flow is created, receiving tiny out of order
packets allways hit the condition :

if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
	tcp_clamp_window(sk);

tcp_clamp_window() increases sk_rcvbuf to match sk_rmem_alloc
(guarded by tcp_rmem[2])

Calling tcp_collapse_ofo_queue() in this case is not useful,
and offers a O(N^2) surface attack to malicious peers.

Better not attempt anything before full queue capacity is reached,
forcing attacker to spend lots of resource and allow us to more
easily detect the abuse.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 64e45b279431886a50c8097593b9dbc9e5d75cc1..53289911362a2dea6b1e9d9ce630b29eed87ebb9 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5004,6 +5004,9 @@ static int tcp_prune_queue(struct sock *sk)
 	else if (tcp_under_memory_pressure(sk))
 		tp->rcv_ssthresh = min(tp->rcv_ssthresh, 4U * tp->advmss);
 
+	if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf)
+		return 0;
+
 	tcp_collapse_ofo_queue(sk);
 	if (!skb_queue_empty(&sk->sk_receive_queue))
 		tcp_collapse(sk, &sk->sk_receive_queue, NULL,
-- 
2.18.0.233.g985f88cf7e-goog

[PATCH net 3/5] tcp: detect malicious patterns in tcp_collapse_ofo_queue()

[Eric Dumazet]

In case an attacker feeds tiny packets completely out of order,
tcp_collapse_ofo_queue() might scan the whole rb-tree, performing
expensive copies, but not changing socket memory usage at all.

1) Do not attempt to collapse tiny skbs.
2) Add logic to exit early when too many tiny skbs are detected.

We prefer not doing aggressive collapsing (which copies packets)
for pathological flows, and revert to tcp_prune_ofo_queue() which
will be less expensive.

In the future, we might add the possibility of terminating flows
that are proven to be malicious.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
 net/ipv4/tcp_input.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 53289911362a2dea6b1e9d9ce630b29eed87ebb9..78068b902e7bca6e60cbe562f1554fc33b43c872 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4902,6 +4902,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 static void tcp_collapse_ofo_queue(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
+	u32 range_truesize, sum_tiny = 0;
 	struct sk_buff *skb, *head;
 	u32 start, end;
 
@@ -4913,6 +4914,7 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
 	}
 	start = TCP_SKB_CB(skb)->seq;
 	end = TCP_SKB_CB(skb)->end_seq;
+	range_truesize = skb->truesize;
 
 	for (head = skb;;) {
 		skb = skb_rb_next(skb);
@@ -4923,11 +4925,20 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
 		if (!skb ||
 		    after(TCP_SKB_CB(skb)->seq, end) ||
 		    before(TCP_SKB_CB(skb)->end_seq, start)) {
-			tcp_collapse(sk, NULL, &tp->out_of_order_queue,
-				     head, skb, start, end);
+			/* Do not attempt collapsing tiny skbs */
+			if (range_truesize != head->truesize ||
+			    end - start >= SKB_WITH_OVERHEAD(SK_MEM_QUANTUM)) {
+				tcp_collapse(sk, NULL, &tp->out_of_order_queue,
+					     head, skb, start, end);
+			} else {
+				sum_tiny += range_truesize;
+				if (sum_tiny > sk->sk_rcvbuf >> 3)
+					return;
+			}
 			goto new_range;
 		}
 
+		range_truesize += skb->truesize;
 		if (unlikely(before(TCP_SKB_CB(skb)->seq, start)))
 			start = TCP_SKB_CB(skb)->seq;
 		if (after(TCP_SKB_CB(skb)->end_seq, end))
-- 
2.18.0.233.g985f88cf7e-goog

[PATCH net 4/5] tcp: call tcp_drop() from tcp_data_queue_ofo()

[Eric Dumazet]

In order to be able to give better diagnostics and detect
malicious traffic, we need to have better sk->sk_drops tracking.

Fixes: 9f5afeae5152 ("tcp: use an RB tree for ooo receive queue")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 78068b902e7bca6e60cbe562f1554fc33b43c872..b062a76922384f6199563af7cf30a30c5baa7601 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4510,7 +4510,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 				/* All the bits are present. Drop. */
 				NET_INC_STATS(sock_net(sk),
 					      LINUX_MIB_TCPOFOMERGE);
-				__kfree_skb(skb);
+				tcp_drop(sk, skb);
 				skb = NULL;
 				tcp_dsack_set(sk, seq, end_seq);
 				goto add_sack;
@@ -4529,7 +4529,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 						 TCP_SKB_CB(skb1)->end_seq);
 				NET_INC_STATS(sock_net(sk),
 					      LINUX_MIB_TCPOFOMERGE);
-				__kfree_skb(skb1);
+				tcp_drop(sk, skb1);
 				goto merge_right;
 			}
 		} else if (tcp_try_coalesce(sk, skb1,
-- 
2.18.0.233.g985f88cf7e-goog

[PATCH net 5/5] tcp: add tcp_ooo_try_coalesce() helper

[Eric Dumazet]

In case skb in out_or_order_queue is the result of
multiple skbs coalescing, we would like to get a proper gso_segs
counter tracking, so that future tcp_drop() can report an accurate
number.

I chose to not implement this tracking for skbs in receive queue,
since they are not dropped, unless socket is disconnected.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b062a76922384f6199563af7cf30a30c5baa7601..3bcd30a2ba06827e061d86ba22680986824e3ee4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4358,6 +4358,23 @@ static bool tcp_try_coalesce(struct sock *sk,
 	return true;
 }
 
+static bool tcp_ooo_try_coalesce(struct sock *sk,
+			     struct sk_buff *to,
+			     struct sk_buff *from,
+			     bool *fragstolen)
+{
+	bool res = tcp_try_coalesce(sk, to, from, fragstolen);
+
+	/* In case tcp_drop() is called later, update to->gso_segs */
+	if (res) {
+		u32 gso_segs = max_t(u16, 1, skb_shinfo(to)->gso_segs) +
+			       max_t(u16, 1, skb_shinfo(from)->gso_segs);
+
+		skb_shinfo(to)->gso_segs = min_t(u32, gso_segs, 0xFFFF);
+	}
+	return res;
+}
+
 static void tcp_drop(struct sock *sk, struct sk_buff *skb)
 {
 	sk_drops_add(sk, skb);
@@ -4481,8 +4498,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	/* In the typical case, we are adding an skb to the end of the list.
 	 * Use of ooo_last_skb avoids the O(Log(N)) rbtree lookup.
 	 */
-	if (tcp_try_coalesce(sk, tp->ooo_last_skb,
-			     skb, &fragstolen)) {
+	if (tcp_ooo_try_coalesce(sk, tp->ooo_last_skb,
+				 skb, &fragstolen)) {
 coalesce_done:
 		tcp_grow_window(sk, skb);
 		kfree_skb_partial(skb, fragstolen);
@@ -4532,8 +4549,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 				tcp_drop(sk, skb1);
 				goto merge_right;
 			}
-		} else if (tcp_try_coalesce(sk, skb1,
-					    skb, &fragstolen)) {
+		} else if (tcp_ooo_try_coalesce(sk, skb1,
+						skb, &fragstolen)) {
 			goto coalesce_done;
 		}
 		p = &parent->rb_right;
-- 
2.18.0.233.g985f88cf7e-goog

Re: [PATCH net 0/5] tcp: more robust ooo handling

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Mon, 23 Jul 2018 09:28:16 -0700

> Juha-Matti Tilli reported that malicious peers could inject tiny
> packets in out_of_order_queue, forcing very expensive calls
> to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
> every incoming packet.
> 
> With tcp_rmem[2] default of 6MB, the ooo queue could
> contain ~7000 nodes.
> 
> This patch series makes sure we cut cpu cycles enough to
> render the attack not critical.
> 
> We might in the future go further, like disconnecting
> or black-holing proven malicious flows.

Sucky...

It took me a while to understand the sums_tiny logic, every
time I read that function I forget that we reset all of the
state and restart the loop after a coalesce inside the loop.

Series applied, and queued up for -stable.

Thanks!

Re: [PATCH net 0/5] tcp: more robust ooo handling

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

On Mon, 2018-07-23 at 12:03 -0700, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Mon, 23 Jul 2018 09:28:16 -0700
> 
> > Juha-Matti Tilli reported that malicious peers could inject tiny
> > packets in out_of_order_queue, forcing very expensive calls
> > to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
> > every incoming packet.
> > 
> > With tcp_rmem[2] default of 6MB, the ooo queue could
> > contain ~7000 nodes.
> > 
> > This patch series makes sure we cut cpu cycles enough to
> > render the attack not critical.
> > 
> > We might in the future go further, like disconnecting
> > or black-holing proven malicious flows.
> 
> Sucky...
> 
> It took me a while to understand the sums_tiny logic, every
> time I read that function I forget that we reset all of the
> state and restart the loop after a coalesce inside the loop.
> 
> Series applied, and queued up for -stable.

I see the first four in 4.9.116 but not the fifth (adding
tcp_ooo_try_coalesce()).

Is that intentional? 

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH net 0/5] tcp: more robust ooo handling

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 03 Aug 2018 11:55:37 +0100

> I see the first four in 4.9.116 but not the fifth (adding
> tcp_ooo_try_coalesce()).
> 
> Is that intentional? 

I don't work on the 4.9 -stable backports, so I personally have
no idea.

I submitted for 4.17 and 4.14

Re: [PATCH net 0/5] tcp: more robust ooo handling

[Greg KH]

On Fri, Aug 03, 2018 at 04:53:27PM -0700, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Fri, 03 Aug 2018 11:55:37 +0100
> 
> > I see the first four in 4.9.116 but not the fifth (adding
> > tcp_ooo_try_coalesce()).
> > 
> > Is that intentional? 
> 
> I don't work on the 4.9 -stable backports, so I personally have
> no idea.
> 
> I submitted for 4.17 and 4.14

Ok, then it's my fault :)

Odds are it did not apply and so I didn't backport it.  If you think it
should be there, please provide a working backport.

thanks,

greg k-h

Re: [PATCH net 0/5] tcp: more robust ooo handling

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

On Sat, 2018-08-04 at 09:05 +0200, Greg KH wrote:
> 
> Ok, then it's my fault :)
> 
> Odds are it did not apply and so I didn't backport it.  If you think it
> should be there, please provide a working backport.

It has whitespace issues but that's about it. Will send a version which
applies cleanly...

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

[PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[David Woodhouse]

From: Eric Dumazet <edumazet@google.com>

commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.

In case skb in out_or_order_queue is the result of
multiple skbs coalescing, we would like to get a proper gso_segs
counter tracking, so that future tcp_drop() can report an accurate
number.

I chose to not implement this tracking for skbs in receive queue,
since they are not dropped, unless socket is disconnected.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 44d136f..ea2583c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4366,6 +4366,23 @@ static bool tcp_try_coalesce(struct sock *sk,
 	return true;
 }
 
+static bool tcp_ooo_try_coalesce(struct sock *sk,
+			     struct sk_buff *to,
+			     struct sk_buff *from,
+			     bool *fragstolen)
+{
+	bool res = tcp_try_coalesce(sk, to, from, fragstolen);
+
+	/* In case tcp_drop() is called later, update to->gso_segs */
+	if (res) {
+		u32 gso_segs = max_t(u16, 1, skb_shinfo(to)->gso_segs) +
+			       max_t(u16, 1, skb_shinfo(from)->gso_segs);
+
+		skb_shinfo(to)->gso_segs = min_t(u32, gso_segs, 0xFFFF);
+	}
+	return res;
+}
+
 static void tcp_drop(struct sock *sk, struct sk_buff *skb)
 {
 	sk_drops_add(sk, skb);
@@ -4489,7 +4506,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	/* In the typical case, we are adding an skb to the end of the list.
 	 * Use of ooo_last_skb avoids the O(Log(N)) rbtree lookup.
 	 */
-	if (tcp_try_coalesce(sk, tp->ooo_last_skb, skb, &fragstolen)) {
+	if (tcp_ooo_try_coalesce(sk, tp->ooo_last_skb,
+				 skb, &fragstolen)) {
 coalesce_done:
 		tcp_grow_window(sk, skb);
 		kfree_skb_partial(skb, fragstolen);
@@ -4539,7 +4557,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 				tcp_drop(sk, skb1);
 				goto merge_right;
 			}
-		} else if (tcp_try_coalesce(sk, skb1, skb, &fragstolen)) {
+		} else if (tcp_ooo_try_coalesce(sk, skb1,
+						skb, &fragstolen)) {
 			goto coalesce_done;
 		}
 		p = &parent->rb_right;
-- 
2.7.4

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[Greg KH]

On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
> 
> In case skb in out_or_order_queue is the result of
> multiple skbs coalescing, we would like to get a proper gso_segs
> counter tracking, so that future tcp_drop() can report an accurate
> number.
> 
> I chose to not implement this tracking for skbs in receive queue,
> since they are not dropped, unless socket is disconnected.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

Now applied, thanks,

greg k-h

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[maowenan]

On 2018/8/7 21:22, Greg KH wrote:
> On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
>>
>> In case skb in out_or_order_queue is the result of
>> multiple skbs coalescing, we would like to get a proper gso_segs
>> counter tracking, so that future tcp_drop() can report an accurate
>> number.
>>
>> I chose to not implement this tracking for skbs in receive queue,
>> since they are not dropped, unless socket is disconnected.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
>> Acked-by: Yuchung Cheng <ycheng@google.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>> ---
>>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> Now applied, thanks,
> 
> greg k-h
> 
> .
> 

Hello,

There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible

test results:
with fix patch： 78.2%   ksoftirqd
no fix patch：   90%     ksoftirqd

there is %0 when no attack packets.

so please help verify that fixed patches are enough in linux-stable 4.4.

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[Greg KH]

On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
> 
> 
> On 2018/8/7 21:22, Greg KH wrote:
> > On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
> >> From: Eric Dumazet <edumazet@google.com>
> >>
> >> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
> >>
> >> In case skb in out_or_order_queue is the result of
> >> multiple skbs coalescing, we would like to get a proper gso_segs
> >> counter tracking, so that future tcp_drop() can report an accurate
> >> number.
> >>
> >> I chose to not implement this tracking for skbs in receive queue,
> >> since they are not dropped, unless socket is disconnected.
> >>
> >> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> >> Acked-by: Yuchung Cheng <ycheng@google.com>
> >> Signed-off-by: David S. Miller <davem@davemloft.net>
> >> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> >> ---
> >>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
> >>  1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > Now applied, thanks,
> > 
> > greg k-h
> > 
> > .
> > 
> 
> Hello,
> 
> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> 
> test results:
> with fix patch： 78.2%   ksoftirqd
> no fix patch：   90%     ksoftirqd
> 
> there is %0 when no attack packets.
> 
> so please help verify that fixed patches are enough in linux-stable 4.4.
> 

I do not know, I am not a network developer.  Please try to reproduce
the same thing on a newer kernel release and see if the result is the
same or not.  If you can find a change that I missed, please let me know
and I will be glad to apply it.

thnaks,

greg k-h

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

On Thu, 2018-08-09 at 14:47 +0200, Greg KH wrote:
> On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
> > There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
> > dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
> > 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
> > 
> > test results:
> > with fix patch： 78.2%   ksoftirqd
> > no fix patch：   90%     ksoftirqd
> > 
> > there is %0 when no attack packets.
> > 
> > so please help verify that fixed patches are enough in linux-stable 4.4.
> > 
> 
> I do not know, I am not a network developer.  Please try to reproduce
> the same thing on a newer kernel release and see if the result is the
> same or not.  If you can find a change that I missed, please let me know
> and I will be glad to apply it.

maowenan, there were five patches in the original upstream set to
address SegmentSmack:

      tcp: free batches of packets in tcp_prune_ofo_queue()
      tcp: avoid collapses in tcp_prune_queue() if possible
      tcp: detect malicious patterns in tcp_collapse_ofo_queue()
      t
cp: call tcp_drop() from tcp_data_queue_ofo()
      tcp: add
tcp_ooo_try_coalesce() helper

I believe that the first one, "free batches of packets..." is not
needed in 4.4 because we only have a simple queue of packets there
anyway, so we're dropping everything each time and don't need the
heuristics for how many to drop.

That leaves two more which have so far not been backported to 4.4; can
you try applying them and see if it resolves the problem for you?

Thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[maowenan]

On 2018/8/9 20:47, Greg KH wrote:
> On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
>>
>>
>> On 2018/8/7 21:22, Greg KH wrote:
>>> On Sat, Aug 04, 2018 at 10:10:00AM +0100, David Woodhouse wrote:
>>>> From: Eric Dumazet <edumazet@google.com>
>>>>
>>>> commit 58152ecbbcc6a0ce7fddd5bf5f6ee535834ece0c upstream.
>>>>
>>>> In case skb in out_or_order_queue is the result of
>>>> multiple skbs coalescing, we would like to get a proper gso_segs
>>>> counter tracking, so that future tcp_drop() can report an accurate
>>>> number.
>>>>
>>>> I chose to not implement this tracking for skbs in receive queue,
>>>> since they are not dropped, unless socket is disconnected.
>>>>
>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
>>>> Acked-by: Yuchung Cheng <ycheng@google.com>
>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>>>> ---
>>>>  net/ipv4/tcp_input.c | 23 +++++++++++++++++++++--
>>>>  1 file changed, 21 insertions(+), 2 deletions(-)
>>>
>>> Now applied, thanks,
>>>
>>> greg k-h
>>>
>>> .
>>>
>>
>> Hello,
>>
>> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
>> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
>>
>> test results:
>> with fix patch： 78.2%   ksoftirqd
>> no fix patch：   90%     ksoftirqd
>>
>> there is %0 when no attack packets.
>>
>> so please help verify that fixed patches are enough in linux-stable 4.4.
>>
> 
> I do not know, I am not a network developer.  Please try to reproduce
> the same thing on a newer kernel release and see if the result is the
> same or not.  If you can find a change that I missed, please let me know
> and I will be glad to apply it.

I have verified that in linux 4.18-rc3(no fixed patches), and 4.18 rc7(with 5 fixed patches),
it works well and cpu usage drops from 95% to 27%.

> 
> thnaks,
> 
> greg k-h
> 
> .
> 

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[maowenan]

On 2018/8/9 20:52, David Woodhouse wrote:
> On Thu, 2018-08-09 at 14:47 +0200, Greg KH wrote:
>> On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
>>> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
>>> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>>> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
>>>  
>>> test results:
>>> with fix patch： 78.2%   ksoftirqd
>>> no fix patch：   90%     ksoftirqd
>>>  
>>> there is %0 when no attack packets.
>>>  
>>> so please help verify that fixed patches are enough in linux-stable 4.4.
>>>  
>>
>> I do not know, I am not a network developer.  Please try to reproduce
>> the same thing on a newer kernel release and see if the result is the
>> same or not.  If you can find a change that I missed, please let me know
>> and I will be glad to apply it.
> 
> maowenan, there were five patches in the original upstream set to
> address SegmentSmack:
> 
>       tcp: free batches of packets in tcp_prune_ofo_queue()
>       tcp: avoid collapses in tcp_prune_queue() if possible
>       tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>       t
> cp: call tcp_drop() from tcp_data_queue_ofo()
>       tcp: add
> tcp_ooo_try_coalesce() helper
> 
> I believe that the first one, "free batches of packets..." is not
> needed in 4.4 because we only have a simple queue of packets there
> anyway, so we're dropping everything each time and don't need the
> heuristics for how many to drop.
> 
> That leaves two more which have so far not been backported to 4.4; can
> you try applying them and see if it resolves the problem for you?
ok, i will try.
> 
> Thanks.
> 

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[maowenan]

On 2018/8/9 20:52, David Woodhouse wrote:
> On Thu, 2018-08-09 at 14:47 +0200, Greg KH wrote:
>> On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
>>> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
>>> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>>> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
>>>  
>>> test results:
>>> with fix patch： 78.2%   ksoftirqd
>>> no fix patch：   90%     ksoftirqd
>>>  
>>> there is %0 when no attack packets.
>>>  
>>> so please help verify that fixed patches are enough in linux-stable 4.4.
>>>  
>>
>> I do not know, I am not a network developer.  Please try to reproduce
>> the same thing on a newer kernel release and see if the result is the
>> same or not.  If you can find a change that I missed, please let me know
>> and I will be glad to apply it.
> 
> maowenan, there were five patches in the original upstream set to
> address SegmentSmack:
> 
>       tcp: free batches of packets in tcp_prune_ofo_queue()
>       tcp: avoid collapses in tcp_prune_queue() if possible
>       tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>       t
> cp: call tcp_drop() from tcp_data_queue_ofo()
>       tcp: add
> tcp_ooo_try_coalesce() helper
> 
> I believe that the first one, "free batches of packets..." is not
> needed in 4.4 because we only have a simple queue of packets there
> anyway, so we're dropping everything each time and don't need the
> heuristics for how many to drop.
> 
> That leaves two more which have so far not been backported to 4.4; can
> you try applying them and see if it resolves the problem for you?

I have tried to add below two patches in 4.4 stable, since I can't apply
tcp: add tcp_ooo_try_coalesce() helper because conflicts, it has the same result
after testing, and the cpu usage has not obviously been improved.

tcp: call tcp_drop() from tcp_data_queue_ofo()
tcp: increment sk_drops for dropped rx packets

@Eric Dumazet, do you have any comments about this, and shall we apply which patches
to fix in stable branch?

> 
> Thanks.
> 

Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper

[maowenan]

On 2018/8/10 14:26, maowenan wrote:
> 
> 
> On 2018/8/9 20:52, David Woodhouse wrote:
>> On Thu, 2018-08-09 at 14:47 +0200, Greg KH wrote:
>>> On Thu, Aug 09, 2018 at 08:37:13PM +0800, maowenan wrote:
>>>> There are two patches in stable branch linux-4.4, but I have tested with below patches, and found that the cpu usage was very high.
>>>> dc6ae4d tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>>>> 5fbec48 tcp: avoid collapses in tcp_prune_queue() if possible
>>>>  
>>>> test results:
>>>> with fix patch： 78.2%   ksoftirqd
>>>> no fix patch：   90%     ksoftirqd
>>>>  
>>>> there is %0 when no attack packets.
>>>>  
>>>> so please help verify that fixed patches are enough in linux-stable 4.4.
>>>>  
>>>
>>> I do not know, I am not a network developer.  Please try to reproduce
>>> the same thing on a newer kernel release and see if the result is the
>>> same or not.  If you can find a change that I missed, please let me know
>>> and I will be glad to apply it.
>>
>> maowenan, there were five patches in the original upstream set to
>> address SegmentSmack:
>>
>>       tcp: free batches of packets in tcp_prune_ofo_queue()
>>       tcp: avoid collapses in tcp_prune_queue() if possible
>>       tcp: detect malicious patterns in tcp_collapse_ofo_queue()
>>       t
>> cp: call tcp_drop() from tcp_data_queue_ofo()
>>       tcp: add
>> tcp_ooo_try_coalesce() helper
>>
>> I believe that the first one, "free batches of packets..." is not
>> needed in 4.4 because we only have a simple queue of packets there
>> anyway, so we're dropping everything each time and don't need the
>> heuristics for how many to drop.
>>
>> That leaves two more which have so far not been backported to 4.4; can
>> you try applying them and see if it resolves the problem for you?
> 
> I have tried to add below two patches in 4.4 stable, since I can't apply
> tcp: add tcp_ooo_try_coalesce() helper because conflicts, it has the same result
> after testing, and the cpu usage has not obviously been improved.
> 
> tcp: call tcp_drop() from tcp_data_queue_ofo()
> tcp: increment sk_drops for dropped rx packets
> 
> @Eric Dumazet, do you have any comments about this, and shall we apply which patches
> to fix in stable branch?

I have checked [PATCH net 3/5] tcp: detect malicious patterns in tcp_collapse_ofo_queue(),
and found that stable branch 4.4 and 3.18 are tiny different from latest mainline,
stable 4.4 branch:
range_truesize = skb->truesize;
latest mainline code:
range_truesize += skb->truesize;

I wonder know why there is some difference here, anything I have ignored?
thank you.

@@ -4923,11 +4925,20 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
 		if (!skb ||
 		    after(TCP_SKB_CB(skb)->seq, end) ||
 		    before(TCP_SKB_CB(skb)->end_seq, start)) {
-			tcp_collapse(sk, NULL, &tp->out_of_order_queue,
-				     head, skb, start, end);
+			/* Do not attempt collapsing tiny skbs */
+			if (range_truesize != head->truesize ||
+			    end - start >= SKB_WITH_OVERHEAD(SK_MEM_QUANTUM)) {
+				tcp_collapse(sk, NULL, &tp->out_of_order_queue,
+					     head, skb, start, end);
+			} else {
+				sum_tiny += range_truesize;
+				if (sum_tiny > sk->sk_rcvbuf >> 3)
+					return;
+			}
 			goto new_range;
 		}

+		range_truesize += skb->truesize;                 //stable 4.4 is different from mainline.
 		if (unlikely(before(TCP_SKB_CB(skb)->seq, start)))
 			start = TCP_SKB_CB(skb)->seq;
 		if (after(TCP_SKB_CB(skb)->end_seq, end))


> 
>>
>> Thanks.
>>
> 
> 
> .
>