netdev - Re: Compare length of req sock queue with sk_max_ack

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:   Fri, 2 Feb 2018 09:55:00 +0800
From:   Tonghao Zhang <xiangxia.m.yue@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: Compare length of req sock queue with sk_max_ack_backlog

On Thu, Feb 1, 2018 at 10:00 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Thu, 2018-02-01 at 20:34 +0800, Tonghao Zhang wrote:
>> Hi Eric
>> One question for you, In the patch ef547f2ac16 ("tcp: remove
>> max_qlen_log"),  why we compared the length of req sock queue with
>> sk_max_ack_backlog. If we remove the max_qlen_log, we should check the
>> length of req sock queue with tcp_max_syn_backlog, right ?
>>
>> With this patch, the option "/proc/sys/net/ipv4/tcp_max_syn_backlog"
>> will be unsed anymore, right ?
>
> Not right.
>
> Please "git grep -n sysctl_max_syn_backlog" to convince it is still
> used.
>
In the tcp_conn_request(), we call the  inet_csk_reqsk_queue_is_full()
to check the length of req sock queue. But
inet_csk_reqsk_queue_is_full()
compares it with sk_max_ack_backlog.

inet_csk_reqsk_queue_is_full
         inet_csk_reqsk_queue_len(sk) >= sk->sk_max_ack_backlog;


inet_csk_reqsk_queue_len
         reqsk_queue_len(&inet_csk(sk)->icsk_accept_queue);

If we set sysctl_max_syn_backlog to 1024 via
/proc/sys/net/ipv4/tcp_max_syn_backlog, and
backlog(sk_max_ack_backlog) to 128 via listen() , tcp_conn_request
will drop the packets when
length of req sock queue > 128, but not 1024.

tcp_conn_request
        if ((net->ipv4.sysctl_tcp_syncookies == 2 ||
             inet_csk_reqsk_queue_is_full(sk)) && !isn) {
                want_cookie = tcp_syn_flood_action(sk, skb, rsk_ops->slab_name);
                if (!want_cookie)
                        goto drop;  // drop the packets
        }


>> I hope we should
>> 1. Add a variate in sock struct, such as sk_max_syn_backlog. Then req
>> sock queue and accept sock queue can control their max value. When we
>> create the sock, we can store the tcp_max_syn_backlog value to
>> sk_max_syn_backlog.
>>
>
> There is a single /proc/sys/net/ipv4/tcp_max_syn_backlog value shared
> by all sockets of a given network namespace.
>
> But you can have thousands of TCP listeners, each of them having a
> distinct listen() backlog
>
>> 2. Update the backlog, we can update it via ioctl.
>
> No need to add an ugly ioctl.
>
> Simply call listen() again with another backlog value.
>
>