netdev - Re: [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 4 Apr 2018 15:22:30 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Alexey Kodanev <alexey.kodanev@...cle.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>,
        Soheil Hassas Yeganeh <soheil.kdev@...il.com>,
        Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [RFC PATCH net] tcp: allow to use TCP Fastopen with MSG_ZEROCOPY

On Tue, Apr 3, 2018 at 2:43 PM, Alexey Kodanev
<alexey.kodanev@...cle.com> wrote:
> With TCP Fastopen we can have the following cases, which could also
> use MSG_ZEROCOPY flag with send() and sendto():
>
> * sendto() + MSG_FASTOPEN flag, sk state can be in TCP_CLOSE at
>   the start of tcp_sendmsg()
>
> * set socket option TCP_FASTOPEN_CONNECT, then connect()
>   and send(), sk state in TCP_SYN_SENT
>
> Currently, both cases with tcp_sendmsg() and MSG_ZEROCOPY flag results
> to EINVAL error, because of the check for TCP_ESTABLISHED sk state in
> the beginning of tcp_sendmsg().
>
> Both conditions require two more checks there: !tp->fastopen_connect
> and !(flags & MSG_FASTOPEN).  It looks like we could remove the original
> check altogether for this unlikely event instead. That way tcp_sendmsg()
> without TFO should fail with EPIPE on sk_stream_wait_connect(), as
> before the introduction of MSG_ZEROCOPY there. And work smoothly for
> the TFO cases.
>
> Fixes: f214f915e7db ("tcp: enable MSG_ZEROCOPY")

This patch adds MSG_ZEROCOPY support for TFO sockets. It is not
a fix that needs to go to stable.

> Signed-off-by: Alexey Kodanev <alexey.kodanev@...cle.com>
> ---
>
> Is there something that I've overlooked and we can't use it here, and
> we should handle this type of error, while using sendto() + TFO,
> in userspace?
>
>  net/ipv4/tcp.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 9225610..768f02c 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1193,11 +1193,6 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
>         flags = msg->msg_flags;
>
>         if (flags & MSG_ZEROCOPY && size) {
> -               if (sk->sk_state != TCP_ESTABLISHED) {
> -                       err = -EINVAL;
> -                       goto out_err;
> -               }
> -
>                 skb = tcp_write_queue_tail(sk);
>                 uarg = sock_zerocopy_realloc(sk, size, skb_zcopy(skb));
>                 if (!uarg) {
> --
> 1.8.3.1
>