The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
@ 2026-05-08  0:14 joycathacker
  2026-05-08 20:35 ` Xin Long
  2026-05-09  1:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: joycathacker @ 2026-05-08  0:14 UTC (permalink / raw)
  To: marcelo.leitner, lucien.xin
  Cc: davem, edumazet, kuba, pabeni, horms, linux-sctp, netdev,
	linux-kernel, security, Ben Morris, stable

From: Ben Morris <bmorris@anthropic.com>

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: stable@vger.kernel.org
Assisted-by: claude:mythos
Signed-off-by: Ben Morris <bmorris@anthropic.com>
---
 net/sctp/socket.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 58d0d9747f0b..1d2568bb6bc2 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1986,6 +1986,15 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
 				goto out_unlock;

 			iov_iter_revert(&msg->msg_iter, err);
+
+			/* sctp_sendmsg_to_asoc() may have released the socket
+			 * lock (sctp_wait_for_sndbuf), during which other
+			 * associations on ep->asocs could have been peeled
+			 * off or freed.  @asoc itself is revalidated by the
+			 * base.dead and base.sk checks in sctp_wait_for_sndbuf,
+			 * so re-derive the cached cursor from it.
+			 */
+			tmp = list_next_entry(asoc, asocs);
 		}

 		goto out_unlock;
--
2.43.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
  2026-05-08  0:14 [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL joycathacker
@ 2026-05-08 20:35 ` Xin Long
  2026-05-09  1:20   ` Jakub Kicinski
  2026-05-09  1:30 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 5+ messages in thread
From: Xin Long @ 2026-05-08 20:35 UTC (permalink / raw)
  To: joycathacker
  Cc: marcelo.leitner, davem, edumazet, kuba, pabeni, horms, linux-sctp,
	netdev, linux-kernel, security, Ben Morris, stable

On Thu, May 7, 2026 at 8:15 PM <joycathacker@gmail.com> wrote:
>
> From: Ben Morris <bmorris@anthropic.com>
>
> 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: stable@vger.kernel.org
> Assisted-by: claude:mythos
> Signed-off-by: Ben Morris <bmorris@anthropic.com>

Acked-by: Xin Long <lucien.xin@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
  2026-05-08 20:35 ` Xin Long
@ 2026-05-09  1:20   ` Jakub Kicinski
  2026-05-11 20:52     ` Xin Long
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2026-05-09  1:20 UTC (permalink / raw)
  To: Xin Long
  Cc: joycathacker, marcelo.leitner, davem, edumazet, pabeni, horms,
	linux-sctp, netdev, linux-kernel, security, Ben Morris, stable

On Fri, 8 May 2026 16:35:21 -0400 Xin Long wrote:
> On Thu, May 7, 2026 at 8:15 PM <joycathacker@gmail.com> wrote:
> >
> > From: Ben Morris <bmorris@anthropic.com>
> >
> > 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: stable@vger.kernel.org
> > Assisted-by: claude:mythos
> > Signed-off-by: Ben Morris <bmorris@anthropic.com>  
> 
> Acked-by: Xin Long <lucien.xin@gmail.com>

FWIW sashiko says there's more?

https://sashiko.dev/#/patchset/20260508001455.3137-1-joycathacker%40gmail.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
  2026-05-08  0:14 [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL joycathacker
  2026-05-08 20:35 ` Xin Long
@ 2026-05-09  1:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-05-09  1:30 UTC (permalink / raw)
  To: None
  Cc: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni, horms,
	linux-sctp, netdev, linux-kernel, security, bmorris, stable

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  7 May 2026 17:14:55 -0700 you wrote:
> From: Ben Morris <bmorris@anthropic.com>
> 
> 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().
> 
> [...]

Here is the summary with links:
  - [net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
    https://git.kernel.org/netdev/net/c/abb5f36771cc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL
  2026-05-09  1:20   ` Jakub Kicinski
@ 2026-05-11 20:52     ` Xin Long
  0 siblings, 0 replies; 5+ messages in thread
From: Xin Long @ 2026-05-11 20:52 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: joycathacker, marcelo.leitner, davem, edumazet, pabeni, horms,
	linux-sctp, netdev, linux-kernel, security, Ben Morris, stable

On Fri, May 8, 2026 at 9:20 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 8 May 2026 16:35:21 -0400 Xin Long wrote:
> > On Thu, May 7, 2026 at 8:15 PM <joycathacker@gmail.com> wrote:
> > >
> > > From: Ben Morris <bmorris@anthropic.com>
> > >
> > > 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: stable@vger.kernel.org
> > > Assisted-by: claude:mythos
> > > Signed-off-by: Ben Morris <bmorris@anthropic.com>
> >
> > Acked-by: Xin Long <lucien.xin@gmail.com>
>
> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-11 20:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08  0:14 [PATCH net] sctp: revalidate list cursor after sctp_sendmsg_to_asoc() in SCTP_SENDALL joycathacker
2026-05-08 20:35 ` Xin Long
2026-05-09  1:20   ` Jakub Kicinski
2026-05-11 20:52     ` Xin Long
2026-05-09  1:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox