git: 636b19ead43a - main - tcp: Disallow re-connection of a connected socket

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Tue, 14 Feb 2023 15:14:26 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=636b19ead43a722d8434776e0bd185bfc97819b6

commit 636b19ead43a722d8434776e0bd185bfc97819b6
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-02-14 14:27:47 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-02-14 15:07:19 +0000

    tcp: Disallow re-connection of a connected socket
    
    soconnectat() tries to ensure that one cannot connect a connected
    socket.  However, the check is racy and does not really prevent two
    threads from attempting to connect the same TCP socket.
    
    Modify tcp_connect() and tcp6_connect() to perform the check again, this
    time synchronized by the inpcb lock, under which we call
    soisconnecting().
    
    Reported by:    syzkaller
    Reviewed by:    glebius
    MFC after:      2 weeks
    Sponsored by:   Klara, Inc.
    Sponsored by:   Modirum MDPay
    Differential Revision:  https://reviews.freebsd.org/D38507
---
 sys/kern/uipc_socket.c   |  4 ++++
 sys/netinet/tcp_usrreq.c | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 9b7f63a81617..1d2b06311c8c 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1345,10 +1345,14 @@ soconnectat(int fd, struct socket *so, struct sockaddr *nam, struct thread *td)
 	int error;
 
 	CURVNET_SET(so->so_vnet);
+
 	/*
 	 * If protocol is connection-based, can only connect once.
 	 * Otherwise, if connected, try to disconnect first.  This allows
 	 * user to disconnect by connecting to, e.g., a null address.
+	 *
+	 * Note, this check is racy and may need to be re-evaluated at the
+	 * protocol layer.
 	 */
 	if (so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING) &&
 	    ((so->so_proto->pr_flags & PR_CONNREQUIRED) ||
diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c
index 43f2fcfb097a..5c98e969c5ce 100644
--- a/sys/netinet/tcp_usrreq.c
+++ b/sys/netinet/tcp_usrreq.c
@@ -1401,6 +1401,10 @@ tcp_connect(struct tcpcb *tp, struct sockaddr_in *sin, struct thread *td)
 	NET_EPOCH_ASSERT();
 	INP_WLOCK_ASSERT(inp);
 
+	if (__predict_false((so->so_state &
+	    (SS_ISCONNECTING | SS_ISCONNECTED)) != 0))
+		return (EISCONN);
+
 	INP_HASH_WLOCK(&V_tcbinfo);
 	error = in_pcbconnect(inp, sin, td->td_ucred, true);
 	INP_HASH_WUNLOCK(&V_tcbinfo);
@@ -1433,11 +1437,16 @@ static int
 tcp6_connect(struct tcpcb *tp, struct sockaddr_in6 *sin6, struct thread *td)
 {
 	struct inpcb *inp = tptoinpcb(tp);
+	struct socket *so = tptosocket(tp);
 	int error;
 
 	NET_EPOCH_ASSERT();
 	INP_WLOCK_ASSERT(inp);
 
+	if (__predict_false((so->so_state &
+	    (SS_ISCONNECTING | SS_ISCONNECTED)) != 0))
+		return (EISCONN);
+
 	INP_HASH_WLOCK(&V_tcbinfo);
 	error = in6_pcbconnect(inp, sin6, td->td_ucred, true);
 	INP_HASH_WUNLOCK(&V_tcbinfo);
@@ -1449,7 +1458,7 @@ tcp6_connect(struct tcpcb *tp, struct sockaddr_in6 *sin6, struct thread *td)
 	    (TCP_MAXWIN << tp->request_r_scale) < sb_max)
 		tp->request_r_scale++;
 
-	soisconnecting(inp->inp_socket);
+	soisconnecting(so);
 	TCPSTAT_INC(tcps_connattempt);
 	tcp_state_change(tp, TCPS_SYN_SENT);
 	tp->iss = tcp_new_isn(&inp->inp_inc);