git: f6379f7fdec0 - main - socket: Fix a race between kevent(2) and listen(2)

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Thu, 16 Jun 2022 14:36:26 UTC
The branch main has been updated by markj:

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

commit f6379f7fdec030e2ede75454cb61d25517606eaa
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-06-16 14:10:45 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-06-16 14:20:04 +0000

    socket: Fix a race between kevent(2) and listen(2)
    
    When locking the knote list for a socket, we check whether the socket is
    a listening socket in order to select the appropriate mutex; a listening
    socket uses the socket lock, while data sockets use socket buffer
    mutexes.
    
    If SOLISTENING(so) is false and the knote lock routine locks a socket
    buffer, then it must re-check whether the socket is a listening socket
    since solisten_proto() could have changed the socket's identity while we
    were blocked on the socket buffer lock.
    
    Reported by:    syzkaller
    Reviewed by:    glebius
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D35492
---
 sys/kern/uipc_socket.c | 36 ++++++++++++++++++++++++------------
 sys/sys/socketvar.h    |  5 +++++
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 2a01f656d258..a2241464e35b 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -4273,10 +4273,16 @@ so_rdknl_lock(void *arg)
 {
 	struct socket *so = arg;
 
-	if (SOLISTENING(so))
-		SOCK_LOCK(so);
-	else
+retry:
+	if (SOLISTENING(so)) {
+		SOLISTEN_LOCK(so);
+	} else {
 		SOCK_RECVBUF_LOCK(so);
+		if (__predict_false(SOLISTENING(so))) {
+			SOCK_RECVBUF_UNLOCK(so);
+			goto retry;
+		}
+	}
 }
 
 static void
@@ -4285,7 +4291,7 @@ so_rdknl_unlock(void *arg)
 	struct socket *so = arg;
 
 	if (SOLISTENING(so))
-		SOCK_UNLOCK(so);
+		SOLISTEN_UNLOCK(so);
 	else
 		SOCK_RECVBUF_UNLOCK(so);
 }
@@ -4297,12 +4303,12 @@ so_rdknl_assert_lock(void *arg, int what)
 
 	if (what == LA_LOCKED) {
 		if (SOLISTENING(so))
-			SOCK_LOCK_ASSERT(so);
+			SOLISTEN_LOCK_ASSERT(so);
 		else
 			SOCK_RECVBUF_LOCK_ASSERT(so);
 	} else {
 		if (SOLISTENING(so))
-			SOCK_UNLOCK_ASSERT(so);
+			SOLISTEN_UNLOCK_ASSERT(so);
 		else
 			SOCK_RECVBUF_UNLOCK_ASSERT(so);
 	}
@@ -4313,10 +4319,16 @@ so_wrknl_lock(void *arg)
 {
 	struct socket *so = arg;
 
-	if (SOLISTENING(so))
-		SOCK_LOCK(so);
-	else
+retry:
+	if (SOLISTENING(so)) {
+		SOLISTEN_LOCK(so);
+	} else {
 		SOCK_SENDBUF_LOCK(so);
+		if (__predict_false(SOLISTENING(so))) {
+			SOCK_SENDBUF_UNLOCK(so);
+			goto retry;
+		}
+	}
 }
 
 static void
@@ -4325,7 +4337,7 @@ so_wrknl_unlock(void *arg)
 	struct socket *so = arg;
 
 	if (SOLISTENING(so))
-		SOCK_UNLOCK(so);
+		SOLISTEN_UNLOCK(so);
 	else
 		SOCK_SENDBUF_UNLOCK(so);
 }
@@ -4337,12 +4349,12 @@ so_wrknl_assert_lock(void *arg, int what)
 
 	if (what == LA_LOCKED) {
 		if (SOLISTENING(so))
-			SOCK_LOCK_ASSERT(so);
+			SOLISTEN_LOCK_ASSERT(so);
 		else
 			SOCK_SENDBUF_LOCK_ASSERT(so);
 	} else {
 		if (SOLISTENING(so))
-			SOCK_UNLOCK_ASSERT(so);
+			SOLISTEN_UNLOCK_ASSERT(so);
 		else
 			SOCK_SENDBUF_UNLOCK_ASSERT(so);
 	}
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index 249e0800f915..85aa7cbfea0f 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -254,6 +254,11 @@ struct socket {
 	KASSERT(SOLISTENING(sol),					\
 	    ("%s: %p not listening", __func__, (sol)));			\
 } while (0)
+#define	SOLISTEN_UNLOCK_ASSERT(sol)	do {				\
+	mtx_assert(&(sol)->so_lock, MA_NOTOWNED);			\
+	KASSERT(SOLISTENING(sol),					\
+	    ("%s: %p not listening", __func__, (sol)));			\
+} while (0)
 
 /*
  * Socket buffer locks.  These are strongly preferred over SOCKBUF_LOCK(sb)