[PATCH net] sctp: check asoc->base.dead before the RCV_SHUTDOWN break in sctp_wait_for_connect()

[Jun Yang]

sctp_wait_for_connect() sleeps with the socket lock dropped, so the
association can be destroyed underneath it: an out-of-the-blue ABORT
answering the INIT is drained from the backlog by that release_sock() and
ends in sctp_association_free().  The loop does revalidate the association
after waking, but the (sk->sk_shutdown & RCV_SHUTDOWN) break is evaluated
before all of those checks and leaves the loop with err == 0.  A concurrent
shutdown(fd, SHUT_RD) - inert on a one-to-many socket, where it only sets
the flag - therefore makes the function report a dead association as
connected, and the sctp_association_put() on the way out drops the last
reference.

sctp_sendmsg_to_asoc() trusts that 0.  Commit c863850ce22e ("sctp: not free
the new asoc when sctp_wait_for_connect returns err") turns a failure into
-ESRCH to stop exactly this double free, but it only covers err != 0.  The
freed association is then used by sctp_datamsg_from_user() and
sctp_set_owner_w(), and destroyed a second time by sctp_sendmsg().

Test asoc->base.dead at the top of the loop, as commit ca3af4dd28cf ("sctp:
do not free asoc when it is already dead in sctp_sendmsg") already does in
sctp_wait_for_sndbuf(), so a destroyed association always leaves via
do_error.  Keep the existing return values rather than adding an -ESRCH
escape, since sctp_wait_for_connect() is also the return value of connect().
base.dead is false on the first iteration in both callers - the socket lock
is held until the first release_sock() - so nothing changes for a live
association.

Reproduced on v7.3-rc2.

  refcount_t: addition on 0; use-after-free.
  WARNING: lib/refcount.c:25 at refcount_warn_saturate+0xf5/0x110
  CPU: 0 UID: 1000 PID: 261 Comm: poc 7.3.0-rc2 #1
   sctp_association_hold+0x96/0xa0
   sctp_sendmsg_to_asoc+0xc76/0x1c60
   sctp_sendmsg+0x11c0/0x1ee0
   __sys_sendto+0x3e7/0x470

  list_del corruption, ff11000104fa2070->next is LIST_POISON1
  kernel BUG at lib/list_debug.c:56!
   sctp_association_free+0x80/0x7c0
   sctp_sendmsg+0x199f/0x1ee0
   __sys_sendto+0x3e7/0x470

Fixes: 668c9beb9020 ("sctp: implement assign_number for sctp_stream_interleave")
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: TencentOS Corvus AI <corvus@xxxxxxxxxxx>
Assisted-by: tencentos-corvus-ai:hy4-preview
Signed-off-by: Jun Yang <junvyyang@xxxxxxxxxxx>
---
A reproducer for this issue is available if requested.

 net/sctp/socket.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index c7b9e325ec1c..de4bae94c9fa 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -9400,12 +9400,16 @@ static int sctp_wait_for_connect(struct sctp_association *asoc, long *timeo_p)
 	for (;;) {
 		prepare_to_wait_exclusive(&asoc->wait, &wait,
 					  TASK_INTERRUPTIBLE);
+		/* The asoc can be destroyed while sleeping below, and the
+		 * RCV_SHUTDOWN break reports success, so check it first.
+		 */
+		if (asoc->base.dead)
+			goto do_error;
 		if (!*timeo_p)
 			goto do_nonblock;
 		if (sk->sk_shutdown & RCV_SHUTDOWN)
 			break;
-		if (sk->sk_err || asoc->state >= SCTP_STATE_SHUTDOWN_PENDING ||
-		    asoc->base.dead)
+		if (sk->sk_err || asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)
 			goto do_error;
 		if (signal_pending(current))
 			goto do_interrupted;
--
2.43.7