git: 636b19ead43a - main - tcp: Disallow re-connection of a connected socket
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);