git: ee4731179cb8 - main - sctp: Fix a lock order reversal in sctp_swap_inpcb_for_listen()

[Mark Johnston]

The branch main has been updated by markj:

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

commit ee4731179cb818570c8f708e77d238bf094d556c
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-09-08 03:20:21 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-09-08 15:41:19 +0000

    sctp: Fix a lock order reversal in sctp_swap_inpcb_for_listen()
    
    When port reuse is enabled in a one-to-one-style socket, sctp_listen()
    may call sctp_swap_inpcb_for_listen() to move the PCB out of the "TCP
    pool".  In so doing it will drop the PCB lock, yielding an LOR since we
    now hold several socket locks.  Reorder sctp_listen() so that it
    performs this operation before beginning the conversion to a listening
    socket.  Also modify sctp_swap_inpcb_for_listen() to return with PCB
    write-locked, since that's what sctp_listen() expects now.
    
    Reviewed by:    tuexen
    Fixes:  bd4a39cc93d9 ("socket: Properly interlock when transitioning to a listening socket")
    MFC after:      1 month
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31879
---
 sys/netinet/sctp_pcb.c    |  5 ++---
 sys/netinet/sctp_usrreq.c | 15 ++++++---------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/sys/netinet/sctp_pcb.c b/sys/netinet/sctp_pcb.c
index cdecb9f4bc9c..ee5308ff6192 100644
--- a/sys/netinet/sctp_pcb.c
+++ b/sys/netinet/sctp_pcb.c
@@ -1846,6 +1846,7 @@ sctp_swap_inpcb_for_listen(struct sctp_inpcb *inp)
 	struct sctp_inpcb *tinp, *ninp;
 
 	SCTP_INP_INFO_WLOCK_ASSERT();
+	SCTP_INP_WLOCK_ASSERT(inp);
 
 	if (sctp_is_feature_off(inp, SCTP_PCB_FLAGS_PORTREUSE)) {
 		/* only works with port reuse on */
@@ -1854,7 +1855,7 @@ sctp_swap_inpcb_for_listen(struct sctp_inpcb *inp)
 	if ((inp->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL) == 0) {
 		return (0);
 	}
-	SCTP_INP_RUNLOCK(inp);
+	SCTP_INP_WUNLOCK(inp);
 	head = &SCTP_BASE_INFO(sctp_ephash)[SCTP_PCBHASH_ALLADDR(inp->sctp_lport,
 	    SCTP_BASE_INFO(hashmark))];
 	/* Kick out all non-listeners to the TCP hash */
@@ -1884,8 +1885,6 @@ sctp_swap_inpcb_for_listen(struct sctp_inpcb *inp)
 	inp->sctp_flags &= ~SCTP_PCB_FLAGS_IN_TCPPOOL;
 	head = &SCTP_BASE_INFO(sctp_ephash)[SCTP_PCBHASH_ALLADDR(inp->sctp_lport, SCTP_BASE_INFO(hashmark))];
 	LIST_INSERT_HEAD(head, inp, sctp_hash);
-	SCTP_INP_WUNLOCK(inp);
-	SCTP_INP_RLOCK(inp);
 	return (0);
 }
 
diff --git a/sys/netinet/sctp_usrreq.c b/sys/netinet/sctp_usrreq.c
index 7079cdf7c403..309cce10fe9b 100644
--- a/sys/netinet/sctp_usrreq.c
+++ b/sys/netinet/sctp_usrreq.c
@@ -7208,12 +7208,6 @@ sctp_listen(struct socket *so, int backlog, struct thread *p)
 		sctp_log_lock(inp, (struct sctp_tcb *)NULL, SCTP_LOG_LOCK_SOCK);
 	}
 #endif
-	SOCK_LOCK(so);
-	error = solisten_proto_check(so);
-	if (error) {
-		SOCK_UNLOCK(so);
-		goto out;
-	}
 	if ((sctp_is_feature_on(inp, SCTP_PCB_FLAGS_PORTREUSE)) &&
 	    (inp->sctp_flags & SCTP_PCB_FLAGS_IN_TCPPOOL)) {
 		/*
@@ -7223,14 +7217,17 @@ sctp_listen(struct socket *so, int backlog, struct thread *p)
 		 * move the guy that was listener to the TCP Pool.
 		 */
 		if (sctp_swap_inpcb_for_listen(inp)) {
-			SOCK_UNLOCK(so);
-			solisten_proto_abort(so);
 			error = EADDRINUSE;
 			SCTP_LTRACE_ERR_RET(inp, NULL, NULL, SCTP_FROM_SCTP_USRREQ, error);
 			goto out;
 		}
 	}
-
+	SOCK_LOCK(so);
+	error = solisten_proto_check(so);
+	if (error) {
+		SOCK_UNLOCK(so);
+		goto out;
+	}
 	if ((inp->sctp_flags & SCTP_PCB_FLAGS_TCPTYPE) &&
 	    (inp->sctp_flags & SCTP_PCB_FLAGS_CONNECTED)) {
 		SOCK_UNLOCK(so);