lkml.org 
[lkml]   [2017]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.18 15/33] tcp: avoid fastopen API to be used on AF_UNSPEC
    Date
    3.18-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Wei Wang <weiwan@google.com>


    [ Upstream commit ba615f675281d76fd19aa03558777f81fb6b6084 ]

    Fastopen API should be used to perform fastopen operations on the TCP
    socket. It does not make sense to use fastopen API to perform disconnect
    by calling it with AF_UNSPEC. The fastopen data path is also prone to
    race conditions and bugs when using with AF_UNSPEC.

    One issue reported and analyzed by Vegard Nossum is as follows:
    +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
    Thread A: Thread B:
    ------------------------------------------------------------------------
    sendto()
    - tcp_sendmsg()
    - sk_stream_memory_free() = 0
    - goto wait_for_sndbuf
    - sk_stream_wait_memory()
    - sk_wait_event() // sleep
    | sendto(flags=MSG_FASTOPEN, dest_addr=AF_UNSPEC)
    | - tcp_sendmsg()
    | - tcp_sendmsg_fastopen()
    | - __inet_stream_connect()
    | - tcp_disconnect() //because of AF_UNSPEC
    | - tcp_transmit_skb()// send RST
    | - return 0; // no reconnect!
    | - sk_stream_wait_connect()
    | - sock_error()
    | - xchg(&sk->sk_err, 0)
    | - return -ECONNRESET
    - ... // wake up, see sk->sk_err == 0
    - skb_entail() on TCP_CLOSE socket

    If the connection is reopened then we will send a brand new SYN packet
    after thread A has already queued a buffer. At this point I think the
    socket internal state (sequence numbers etc.) becomes messed up.

    When the new connection is closed, the FIN-ACK is rejected because the
    sequence number is outside the window. The other side tries to
    retransmit,
    but __tcp_retransmit_skb() calls tcp_trim_head() on an empty skb which
    corrupts the skb data length and hits a BUG() in copy_and_csum_bits().
    +++++++++++++++++++++++++++++++++++++++++++++++++++++++++

    Hence, this patch adds a check for AF_UNSPEC in the fastopen data path
    and return EOPNOTSUPP to user if such case happens.

    Fixes: cf60af03ca4e7 ("tcp: Fast Open client - sendmsg(MSG_FASTOPEN)")
    Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
    Signed-off-by: Wei Wang <weiwan@google.com>
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/ipv4/tcp.c | 7 +++++--
    1 file changed, 5 insertions(+), 2 deletions(-)

    --- a/net/ipv4/tcp.c
    +++ b/net/ipv4/tcp.c
    @@ -1066,9 +1066,12 @@ static int tcp_sendmsg_fastopen(struct s
    int *copied, size_t size)
    {
    struct tcp_sock *tp = tcp_sk(sk);
    + struct sockaddr *uaddr = msg->msg_name;
    int err, flags;

    - if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE))
    + if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
    + (uaddr && msg->msg_namelen >= sizeof(uaddr->sa_family) &&
    + uaddr->sa_family == AF_UNSPEC))
    return -EOPNOTSUPP;
    if (tp->fastopen_req != NULL)
    return -EALREADY; /* Another Fast Open is in progress */
    @@ -1081,7 +1084,7 @@ static int tcp_sendmsg_fastopen(struct s
    tp->fastopen_req->size = size;

    flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
    - err = __inet_stream_connect(sk->sk_socket, msg->msg_name,
    + err = __inet_stream_connect(sk->sk_socket, uaddr,
    msg->msg_namelen, flags);
    *copied = tp->fastopen_req->copied;
    tcp_free_fastopen_req(tp);

    \
     
     \ /
      Last update: 2017-06-12 00:30    [W:2.854 / U:0.516 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site