git: a8aa6f1f784b - main - socket: Avoid clearing SS_ISCONNECTING if soconnect() fails

[Mark Johnston]

The branch main has been updated by markj:

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

commit a8aa6f1f784b91acb4ef9387a28c78311493eb66
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-09-07 18:51:54 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-09-07 21:12:09 +0000

    socket: Avoid clearing SS_ISCONNECTING if soconnect() fails
    
    This behaviour appears to date from the 4.4 BSD import.  It has two
    problems:
    
    1. The update to so_state is not protected by the socket lock, so
       concurrent updates to so_state may be lost.
    2. Suppose two threads race to call connect(2) on a socket, and one
       succeeds while the other fails.  Then the failing thread may
       incorrectly clear SS_ISCONNECTING, confusing the state machine.
    
    Simply remove the update.  It does not appear to be necessary:
    pru_connect implementations which call soisconnecting() only do so after
    all failure modes have been handled.  For instance, tcp_connect() and
    tcp6_connect() will never return an error after calling soisconnected().
    However, we cannot correctly assert that SS_ISCONNECTED is not set after
    an error from soconnect() since the socket lock is not held across the
    pru_connect call, so a concurrent connect(2) may have set the flag.
    
    MFC after:      1 month
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31699
---
 sys/kern/uipc_syscalls.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index 3208dc0491dd..44079bae1e9b 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -484,7 +484,7 @@ kern_connectat(struct thread *td, int dirfd, int fd, struct sockaddr *sa)
 {
 	struct socket *so;
 	struct file *fp;
-	int error, interrupted = 0;
+	int error;
 
 #ifdef CAPABILITY_MODE
 	if (IN_CAPABILITY_MODE(td) && (dirfd == AT_FDCWD))
@@ -522,11 +522,8 @@ kern_connectat(struct thread *td, int dirfd, int fd, struct sockaddr *sa)
 	while ((so->so_state & SS_ISCONNECTING) && so->so_error == 0) {
 		error = msleep(&so->so_timeo, &so->so_lock, PSOCK | PCATCH,
 		    "connec", 0);
-		if (error != 0) {
-			if (error == EINTR || error == ERESTART)
-				interrupted = 1;
+		if (error != 0)
 			break;
-		}
 	}
 	if (error == 0) {
 		error = so->so_error;
@@ -534,8 +531,6 @@ kern_connectat(struct thread *td, int dirfd, int fd, struct sockaddr *sa)
 	}
 	SOCK_UNLOCK(so);
 bad:
-	if (!interrupted)
-		so->so_state &= ~SS_ISCONNECTING;
 	if (error == ERESTART)
 		error = EINTR;
 done1: