tcp: avoid fastopen API to be used on AF_UNSPEC
[ 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: 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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
d7ed7fcecf
commit
fe22b60055
1 changed files with 5 additions and 2 deletions
|
@ -1071,9 +1071,12 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
|
|||
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)
|
||||
return -EALREADY; /* Another Fast Open is in progress */
|
||||
|
@ -1086,7 +1089,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
|
|||
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);
|
||||
|
|
Loading…
Add table
Reference in a new issue