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);