Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL

On Fri, May 8, 2026 at 9:20 PM Jakub Kicinski <[email protected]> wrote:
>
> 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

I will try to verify this and submit a fix if it could be reproduced.

Thanks.