netdev - Re: v3 for tcp friends?

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 23 Jan 2013 17:39:51 +0800
From:	Li Yu <raise.sail@...il.com>
To:	netdev <netdev@...r.kernel.org>
CC:	David Miller <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Bruce Curtis <brutus@...gle.com>
Subject: Re: v3 for tcp friends?

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html