netdev - Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 10 Aug 2018 18:10:44 +0800
From:   maowenan <maowenan@...wei.com>
To:     David Woodhouse <dwmw2@...radead.org>,
        Greg KH <gregkh@...ux-foundation.org>
CC:     <davem@...emloft.net>, <edumazet@...gle.com>,
        <juha-matti.tilli@....fi>, <ycheng@...gle.com>,
        <soheil@...gle.com>, <netdev@...r.kernel.org>,
        <eric.dumazet@...il.com>, <jdw@...zon.de>
Subject: Re: [PATCH 4.9-stable] tcp: add tcp_ooo_try_coalesce() helper



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.
>>
> 
> 
> .
>