Thank you very much.

I have updated the patch:
https://lore.kernel.org/netdev/1555379244-71866-1-git-send-email-zhangxiao...@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 <zhangxiao...@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.




.


Reply via email to