mirror of
https://github.com/torvalds/linux
synced 2024-11-05 18:23:50 +00:00
tcp: avoid fastopen API to be used on AF_UNSPEC
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: cf60af03ca
("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>
This commit is contained in:
parent
7c3f1875c6
commit
ba615f6752
1 changed files with 5 additions and 2 deletions
|
@ -1084,9 +1084,12 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
|
|||
{
|
||||
struct tcp_sock *tp = tcp_sk(sk);
|
||||
struct inet_sock *inet = inet_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)
|
||||
return -EALREADY; /* Another Fast Open is in progress */
|
||||
|
@ -1108,7 +1111,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
|
|||
}
|
||||
}
|
||||
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, 1);
|
||||
/* fastopen_req could already be freed in __inet_stream_connect
|
||||
* if the connection times out or gets rst
|
||||
|
|
Loading…
Reference in a new issue