Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
Jakub Kicinski <[email protected]> Fri, 8 May 2026 18:20:44 -0700
| Newsgroups | org.kernel.vger.linux-sctp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On Fri, 8 May 2026 16:35:21 -0400 Xin Long wrote: > On Thu, May 7, 2026 at 8:15 PM <[email protected]> wrote: > > > > From: Ben Morris <[email protected]> > > > > The SCTP_SENDALL path in sctp_sendmsg() iterates ep->asocs with > > list_for_each_entry_safe(), which caches the next entry in @tmp before > > the loop body runs. The body calls sctp_sendmsg_to_asoc(), which may > > drop the socket lock inside sctp_wait_for_sndbuf(). > > > > While the lock is dropped, another thread can SCTP_SOCKOPT_PEELOFF the > > association cached in @tmp, migrating it to a new endpoint via > > sctp_sock_migrate() (list_del_init() + list_add_tail() to > > newep->asocs), and optionally close the new socket which frees the > > association via kfree_rcu(). The cached @tmp can also be freed by a > > network ABORT for that association, processed in softirq while the > > lock is dropped. > > > > sctp_wait_for_sndbuf() revalidates @asoc (the current entry) on re-lock > > via the "sk != asoc->base.sk" and "asoc->base.dead" checks, but nothing > > revalidates @tmp. After a successful return, the iterator advances to > > the stale @tmp, yielding either a use-after-free (if the peeled socket > > was closed) or a list-walk onto the new endpoint's list head (type > > confusion of &newep->asocs as a struct sctp_association *). > > > > Both are reachable from CapEff=0; the type-confusion path gives > > controlled indirect call via the outqueue.sched->init_sid pointer. > > > > Fix by re-deriving @tmp from @asoc after sctp_sendmsg_to_asoc() > > returns. @asoc is known to still be on ep->asocs at that point: the > > only callers that list_del an association from ep->asocs are > > sctp_association_free() (which sets asoc->base.dead) and > > sctp_assoc_migrate() (which changes asoc->base.sk), and > > sctp_wait_for_sndbuf() checks both under the lock before any > > successful return; a tripped check propagates as err < 0 and the loop > > bails before the re-derive. > > > > The SCTP_ABORT path in sctp_sendmsg_check_sflags() returns 0 and the > > loop hits 'continue' before sctp_sendmsg_to_asoc() is ever called, so > > the @tmp cached by list_for_each_entry_safe() still covers the > > lock-held free that ba59fb027307 ("sctp: walk the list of asoc > > safely") was added for. > > > > Fixes: 4910280503f3 ("sctp: add support for snd flag SCTP_SENDALL process in sendmsg") > > Cc: [email protected] > > Assisted-by: claude:mythos > > Signed-off-by: Ben Morris <[email protected]> > > Acked-by: Xin Long <[email protected]> FWIW sashiko says there's more? https://sashiko.dev/#/patchset/20260508001455.3137-1-joycathacker%40gmail.com