netdev - Re: [PATCH] ipv4: set the tcp_min_rtt_wlen range from 0 to u32_max_div

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:   Tue, 16 Apr 2019 09:48:41 +0800
From:   "zhangxiaoxu (A)" <zhangxiaoxu5@...wei.com>
To:     Eric Dumazet <eric.dumazet@...il.com>, <davem@...emloft.net>,
        <kuznet@....inr.ac.ru>, <yoshfuji@...ux-ipv6.org>,
        <netdev@...r.kernel.org>
CC:     Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH] ipv4: set the tcp_min_rtt_wlen range from 0 to
 u32_max_div_HZ

Thank you very much.

I have updated the patch:
https://lore.kernel.org/netdev/1555379244-71866-1-git-send-email-zhangxiaoxu5@huawei.com/

On 4/16/2019 1:53 AM, Eric Dumazet wrote:
> 
> 
> On 04/15/2019 12:15 AM, ZhangXiaoxu wrote:
>> There is a UBSAN report as below:
>> UBSAN: Undefined behaviour in net/ipv4/tcp_input.c:2877:56
>> signed integer overflow:
>> 2147483647 * 1000 cannot be represented in type 'int'
>> CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.1.0-rc4-00058-g582549e #1
>> Call Trace:
>>   <IRQ>
>>   dump_stack+0x8c/0xba
>>   ubsan_epilogue+0x11/0x60
>>   handle_overflow+0x12d/0x170
>>   ? ttwu_do_wakeup+0x21/0x320
>>   __ubsan_handle_mul_overflow+0x12/0x20
>>   tcp_ack_update_rtt+0x76c/0x780
>>   tcp_clean_rtx_queue+0x499/0x14d0
>>   tcp_ack+0x69e/0x1240
>>   ? __wake_up_sync_key+0x2c/0x50
>>   ? update_group_capacity+0x50/0x680
>>   tcp_rcv_established+0x4e2/0xe10
>>   tcp_v4_do_rcv+0x22b/0x420
>>   tcp_v4_rcv+0xfe8/0x1190
>>   ip_protocol_deliver_rcu+0x36/0x180
>>   ip_local_deliver+0x15b/0x1a0
>>   ip_rcv+0xac/0xd0
>>   __netif_receive_skb_one_core+0x7f/0xb0
>>   __netif_receive_skb+0x33/0xc0
>>   netif_receive_skb_internal+0x84/0x1c0
>>   napi_gro_receive+0x2a0/0x300
>>   receive_buf+0x3d4/0x2350
>>   ? detach_buf_split+0x159/0x390
>>   virtnet_poll+0x198/0x840
>>   ? reweight_entity+0x243/0x4b0
>>   net_rx_action+0x25c/0x770
>>   __do_softirq+0x19b/0x66d
>>   irq_exit+0x1eb/0x230
>>   do_IRQ+0x7a/0x150
>>   common_interrupt+0xf/0xf
>>   </IRQ>
>>
>> It can be reproduced by:
>>    echo 2147483647 > /proc/sys/net/ipv4/tcp_min_rtt_wlen
>>
>> Fixes: f672258391b42 ("tcp: track min RTT using windowed min-filter")
>>
>> Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@...wei.com>
>> ---
>>   net/ipv4/sysctl_net_ipv4.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
>> index ba0fc4b..56306bc 100644
>> --- a/net/ipv4/sysctl_net_ipv4.c
>> +++ b/net/ipv4/sysctl_net_ipv4.c
>> @@ -1151,7 +1151,9 @@ static struct ctl_table ipv4_net_table[] = {
>>   		.data		= &init_net.ipv4.sysctl_tcp_min_rtt_wlen,
>>   		.maxlen		= sizeof(int),
>>   		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec
>> +		.proc_handler	= proc_dointvec_minmax,
>> +		.extra1		= &zero,
>> +		.extra2		= &u32_max_div_HZ
>>   	},
>>   	{
>>   		.procname	= "tcp_autocorking",
>>
> 
> Thanks for the patch, but I believe we need more sensible limits,
> otherwise we hide a real problem.
> 
> Please take a look at minmax_running_min() implementation.
> 
> If we allow @win to be close of 0xFFFFFFFF, then (val.t - m->s[2].t > win)
> will never be true.
> 
> One day limit would be more than enough I think.
> 
> 
> 
> 
> .
>