All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Yu <raise.sail@gmail.com>
To: netdev <netdev@vger.kernel.org>
Cc: David Miller <davem@davemloft.net>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Bruce Curtis <brutus@google.com>
Subject: Re: v3 for tcp friends?
Date: Wed, 23 Jan 2013 17:39:51 +0800	[thread overview]
Message-ID: <50FFAFE7.5030707@gmail.com> (raw)
In-Reply-To: <50FF9836.4060106@gmail.com>

于 2013年01月23日 15:58, Li Yu 写道:
> 于 2013年01月23日 15:21, Li Yu 写道:
>> 于 2013年01月23日 14:46, Eric Dumazet 写道:
>>> On Wed, 2013-01-23 at 14:12 +0800, Li Yu wrote:
>>>> Oops, this hang is not since TCP friends patch!
>>>>
>>>> sk_sndbuf_get() is broken by 32 bits integer overflow
>>>> because of so large value in net.ipv4.tcp_{rmem,wmem}.
>>>>
>>>> but this hang also can be found in net-next.git
>>>> (3.8.0-rc3+), if we run below commands, then all new
>>>> TCP connections stop working!
>>>>
>>>> # when TCP friends is disabled
>>>> sysctl -w net.ipv4.tcp_rmem="4096 4294967296 4294967296" # 4GB
>>>> sysctl -w net.ipv4.tcp_wmem="4096 4294967296 4294967296"
>>>
>>> Right we need to make sure we dont overflow.
>>>
>>> Try the following fix :
>>>
>>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
>>> index a25e1d2..1459145 100644
>>> --- a/net/ipv4/sysctl_net_ipv4.c
>>> +++ b/net/ipv4/sysctl_net_ipv4.c
>>> @@ -549,14 +549,16 @@ static struct ctl_table ipv4_table[] = {
>>>           .data        = &sysctl_tcp_wmem,
>>>           .maxlen        = sizeof(sysctl_tcp_wmem),
>>>           .mode        = 0644,
>>> -        .proc_handler    = proc_dointvec
>>> +        .extra1        = &zero,

If we added below:

+static int one = 1;
+static int int_max = INT_MAX;
....
+               .extra1     = &one,
+		.extra2	    = &int_max,

The bug is fixed without TCP friends.

Maybe, TCP friends need to use long integer to record result
of "sk->sk_sndbuf + sk->sk_friend->sk_rcvbuf" ?

BTW: This overflow problem also breaks UDP sockets.

Thanks

Yu

>>> +        .proc_handler    = proc_dointvec_minmax
>>>       },
>>>       {
>>>           .procname    = "tcp_rmem",
>>>           .data        = &sysctl_tcp_rmem,
>>>           .maxlen        = sizeof(sysctl_tcp_rmem),
>>>           .mode        = 0644,
>>> -        .proc_handler    = proc_dointvec
>>> +        .extra1        = &zero,
>>> +        .proc_handler    = proc_dointvec_minmax
>>>       },
>>>       {
>>>           .procname    = "tcp_app_win",
>>>
>>>
>>>
>> Thanks for so quick reply, I will test it soon.
>>
>> however I suspect whether this patch could fix overflow if we merged TCP
>> friends patch in future.
>>
>
> Tested on 3.7.0-rc1+, but bug is still alive
> with disabled TCP friends ...
>
> Thanks
>
> Yu
>
>> With TCP friends, we have source like below, or TCP friends should care
>> this ?
>>
>> static inline int sk_sndbuf_get(const struct sock *sk)
>> {
>>          if (sk->sk_friend)
>>                  return sk->sk_sndbuf + sk->sk_friend->sk_rcvbuf;
>>          else
>>                  return sk->sk_sndbuf;
>> }
>>
>> static inline bool sk_stream_memory_free(const struct sock *sk)
>> {
>>          return sk_wmem_queued_get(sk) < sk_sndbuf_get(sk);
>> }
>>
>> So sk_sndbuf_get() still may be overflow when we have right value in
>> net.ipv4.tcp_{rmem,wmem}.
>>
>> Thanks
>

  reply	other threads:[~2013-01-23  9:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-03 19:48 v3 for tcp friends? David Miller
2012-09-04 15:10 ` Bruce Curtis
2012-09-04 16:58   ` David Miller
2013-01-21  7:29     ` Li Yu
2013-01-21  7:31       ` Li Yu
     [not found]         ` <CA+WLrf8xuJ3UXK-tZQykuTPMXu67WsNKWNuNzBKk7MBAidW-CQ@mail.gmail.com>
     [not found]           ` <CAEkNxbH0MAAU9oiwwrFTbMJP1yzfNdxD5NDdZOqGnvhTLvodoQ@mail.gmail.com>
2013-01-23  3:14             ` Li Yu
2013-01-23  6:12               ` Li Yu
2013-01-23  6:46                 ` Eric Dumazet
2013-01-23  7:21                   ` Li Yu
2013-01-23  7:58                     ` Li Yu
2013-01-23  9:39                       ` Li Yu [this message]
2013-01-23  9:52                         ` Li Yu
2013-01-23 13:16                           ` Weiping Pan
2013-01-24  2:04                         ` Xiaotian Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50FFAFE7.5030707@gmail.com \
    --to=raise.sail@gmail.com \
    --cc=brutus@google.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.