netdev - Re: [PATCH net-next 4/6] tcp: remove header prediction

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

Message-ID: <CADVnQy=PTR+uPTaEBrTt37j0v-2+-YBxkP0PdOnG7dAwPQEHcg@mail.gmail.com>
Date:   Tue, 1 Aug 2017 13:23:55 -0400
From:   Neal Cardwell <ncardwell@...gle.com>
To:     Florian Westphal <fw@...len.de>
Cc:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>
Subject: Re: [PATCH net-next 4/6] tcp: remove header prediction

On Sat, Jul 29, 2017 at 9:57 PM, Florian Westphal <fw@...len.de> wrote:
> @@ -5519,11 +5347,10 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
>         if (sock_flag(sk, SOCK_KEEPOPEN))
>                 inet_csk_reset_keepalive_timer(sk, keepalive_time_when(tp));
>
> -       if (!tp->rx_opt.snd_wscale)
> -               __tcp_fast_path_on(tp, tp->snd_wnd);
> -       else
> -               tp->pred_flags = 0;
> -
> +       if (!sock_flag(sk, SOCK_DEAD)) {
> +               sk->sk_state_change(sk);
> +               sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
> +       }
>  }
>

This patch hunk seems like it introduces a minor bug. It seems like
after this change, the sk_state_change() and sk_wake_async() calls for
a completed active connection happen twice: once in this new spot
inside tcp_finish_connect() and once in the existing code in
tcp_rcv_synsent_state_process() immediately after it calls
tcp_finish_connect().

I would vote for removing this new code snippet and retaining the old
one, in case there are subtle interactions with
tcp_rcv_fastopen_synack(), which happens in between the new wake-up
location and the old wake-up location.

neal