git: dfcef8771484 - main - socket: Fix a use-after-free in soclose()

[Mark Johnston]

The branch main has been updated by markj:

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

commit dfcef8771484271f2bccdb1dbc088a838441c5a7
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-09-17 16:26:06 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-09-17 18:19:05 +0000

    socket: Fix a use-after-free in soclose()
    
    After releasing the fd reference to a socket "so", we should avoid
    testing SOLISTENING(so) since the socket may have been freed.  Instead,
    directly test whether the list of unaccepted sockets is empty.
    
    Fixes:          f4bb1869ddd2 ("Consistently use the SOLISTENING() macro")
    Pointy hat:     markj
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31973
---
 sys/kern/uipc_socket.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 95222a1ecf7b..f1a3b5acc827 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1223,6 +1223,7 @@ int
 soclose(struct socket *so)
 {
 	struct accept_queue lqueue;
+	struct socket *sp, *tsp;
 	int error = 0;
 
 	KASSERT(!(so->so_state & SS_NOFDREF), ("soclose: SS_NOFDREF on enter"));
@@ -1257,11 +1258,9 @@ drop:
 	if (so->so_proto->pr_usrreqs->pru_close != NULL)
 		(*so->so_proto->pr_usrreqs->pru_close)(so);
 
+	TAILQ_INIT(&lqueue);
 	SOCK_LOCK(so);
 	if (SOLISTENING(so)) {
-		struct socket *sp;
-
-		TAILQ_INIT(&lqueue);
 		TAILQ_SWAP(&lqueue, &so->sol_incomp, socket, so_list);
 		TAILQ_CONCAT(&lqueue, &so->sol_comp, so_list);
 
@@ -1279,17 +1278,14 @@ drop:
 	KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF"));
 	so->so_state |= SS_NOFDREF;
 	sorele(so);
-	if (SOLISTENING(so)) {
-		struct socket *sp, *tsp;
-
-		TAILQ_FOREACH_SAFE(sp, &lqueue, so_list, tsp) {
-			SOCK_LOCK(sp);
-			if (sp->so_count == 0) {
-				SOCK_UNLOCK(sp);
-				soabort(sp);
-			} else
-				/* sp is now in sofree() */
-				SOCK_UNLOCK(sp);
+	TAILQ_FOREACH_SAFE(sp, &lqueue, so_list, tsp) {
+		SOCK_LOCK(sp);
+		if (refcount_load(&sp->so_count) == 0) {
+			SOCK_UNLOCK(sp);
+			soabort(sp);
+		} else {
+			/* sp is now in sofree() */
+			SOCK_UNLOCK(sp);
 		}
 	}
 	CURVNET_RESTORE();