| Message ID | 1555312526-69165-1-git-send-email-zhangxiaoxu5@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipv4: set the tcp_min_rtt_wlen range from 0 to u32_max_div_HZ | expand |
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@huawei.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.
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@huawei.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. > > > > > . >
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",
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@huawei.com> --- net/ipv4/sysctl_net_ipv4.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)