netdev - Re: [PATCH] tcp: Use BPF timeout setting for SYN ACK RTO

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]

Message-ID: <87d9c47b-1797-3f9a-9707-48d2b398dba3@gmail.com>
Date:   Mon, 25 Oct 2021 13:48:12 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Alexander Azimov <mitradir@...dex-team.ru>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     zeil@...dex-team.ru, Lawrence Brakmo <brakmo@...com>,
        Akhmat Karakotov <hmukos@...dex-team.ru>,
        netdev@...r.kernel.org, Neal Cardwell <ncardwell@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH] tcp: Use BPF timeout setting for SYN ACK RTO

On 10/25/21 12:26 PM, Alexander Azimov wrote:
> Hi Eric,
>  
> Can you please clarify why do you think that SYN RTO should be accessible through BPF and SYN ACK RTO should be bound to TCP_TIMEOUT_INIT constant?
> I can't see the reason for such asymmetry, please advise.
>  
> I also wonder what kind of existing BPF programs can suffer from these changes. Please give us your insights.
> 
> ps: this is a copy of the original message, hope this one will come in a plain text
> 

When commit 8550f328f45db6d37981eb2041bc465810245c03
("bpf: Support for per connection SYN/SYN-ACK RTOs")
was added, tcp_timeout_init() (and potential eBPF prog)
would be called from tcp_conn_request() and tcp_connect_init()

So some users are now using this eBPF program, expecting
it to have an effect only from these call sites.

If you are adding more call sites, suddenly the behavior
of TCP stack for these users will change. You have to
document if bad things could happen for them, like unexpected
max acceptable delays for connection establishment being severely reduced.

In any case, I would prefer adding a new @timeout field
in struct request_sock to carry the base timeout
instead of calling a BPF program all the times,
otherwise we could have weird behavior (eg PAWS checks)
if the return from eBPF program is variable for one 5-tuple.

Also, have you checked if TCP syn cookies would still work
if tcp_timeout_init() returns a small value like 5ms ?

tcp_check_req()
...
tmp_opt.ts_recent_stamp = ktime_get_seconds() - ((tcp_timeout_init((struct sock *)req)/HZ)<<req->num_timeout);

-> tmp_opt.ts_recent_stamp = ktime_get_seconds()