[PATCH net] sctp: avoid livelock while updating retransmit path

[Yiqi Sun]

sctp_assoc_update_retran_path() walks the association transport list from
the current retransmit path's successor and stops once it reaches the
current retransmit path again. However, the loop skips transports in
SCTP_UNCONFIRMED state before checking for the wraparound condition.

This makes the loop non-terminating when the association contains only
UNCONFIRMED transports at that point and asoc->peer.retran_path is also
UNCONFIRMED. One way to reach that state is through ASCONF wildcard
DEL-IP processing after an unconfirmed address is selected as the
primary transport. sctp_assoc_del_nonprimary_peers() then removes the
other transports one by one; when removing the current retran_path,
sctp_assoc_rm_peer() calls sctp_assoc_update_retran_path() before
unlinking it. If the remaining candidate and the current retran_path are
both UNCONFIRMED, the loop repeatedly continues before it can observe
that it has completed a full pass.

The same reproducer that exercised the bug fixed by commit 9b2854f86f0b
("sctp: don't free the ASCONF's own transport in DEL-IP processing") can
still trigger this CPU stall after that fix is applied. With the UAF
prevented, the ASCONF processing no longer dereferences the freed
transport, but it can still reach the retransmit-path update described
above and spin in the all-UNCONFIRMED case.

Fix this by remembering whether the current transport is the original
retran_path, still considering it as a candidate when it is not
UNCONFIRMED, and then breaking after the candidate logic. This preserves
the existing fallback semantics while making the full-pass termination
independent of the transport state.

Also restore the NULL guard around the retran_path assignment. In the
all-UNCONFIRMED case there is no eligible replacement transport, and
installing NULL would leave later retransmit-path users and the debug
print with a NULL path.

Fixes: 4c47af4d5eb2 ("net: sctp: rework multihoming retransmission path selection to rfc4960")
Signed-off-by: Yiqi Sun <sunyiqixm@xxxxxxxxx>
---
 net/sctp/associola.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index c0512c827d0f..6f19eb0b01e2 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1272,6 +1272,7 @@ void sctp_assoc_update_retran_path(struct sctp_association *asoc)
 {
 	struct sctp_transport *trans = asoc->peer.retran_path;
 	struct sctp_transport *trans_next = NULL;
+	bool last = false;
 
 	/* We're done as we only have the one and only path. */
 	if (asoc->peer.transport_count == 1)
@@ -1289,18 +1290,20 @@ void sctp_assoc_update_retran_path(struct sctp_association *asoc)
 		/* Manually skip the head element. */
 		if (&trans->transports == &asoc->peer.transport_addr_list)
 			continue;
-		if (trans->state == SCTP_UNCONFIRMED)
-			continue;
-		trans_next = sctp_trans_elect_best(trans, trans_next);
-		/* Active is good enough for immediate return. */
-		if (trans_next->state == SCTP_ACTIVE)
-			break;
+		last = trans == asoc->peer.retran_path;
+		if (trans->state != SCTP_UNCONFIRMED) {
+			trans_next = sctp_trans_elect_best(trans, trans_next);
+			/* Active is good enough for immediate return. */
+			if (trans_next->state == SCTP_ACTIVE)
+				break;
+		}
 		/* We've reached the end, time to update path. */
-		if (trans == asoc->peer.retran_path)
+		if (last)
 			break;
 	}
 
-	asoc->peer.retran_path = trans_next;
+	if (trans_next)
+		asoc->peer.retran_path = trans_next;
 
 	pr_debug("%s: association:%p updated new path to addr:%pISpc\n",
 		 __func__, asoc, &asoc->peer.retran_path->ipaddr.sa);
-- 
2.34.1