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

On Thu, Aug 27, 2026 at 3:50 AM, Xin Long wrote:

quoted

After removing the continue, I think you can keep using
if (trans == asoc->peer.retran_path) here without 'last' needed.

Yes. The v1 'last' variable was redundant once the SCTP_UNCONFIRMED
path no longer uses continue. Drop it in this revision and retain the
original wraparound comparison after the candidate-selection block.

The reproducer is attached.

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.

Fix this by considering a transport only when it is not UNCONFIRMED,
then checking whether the walk has returned to retran_path. This makes
the full-pass termination independent of the transport state while
preserving the existing fallback selection semantics.

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 <redacted>
---
Changes in v2:
- Drop the redundant 'last' variable as suggested by Xin Long.
- Link to v1: https://lore.kernel.org/r/20260827075006.3979566-1-sunyiqixm@gmail.com/ (local)
---
 net/sctp/associola.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index c0512c827d0f..4521be3bd85a 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c

@@ -1289,18 +1289,19 @@ 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;
+               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)
                        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