[PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

sctp_wait_for_connect() drops the socket lock while it sleeps. An
out-of-the-blue ABORT can then be processed from the socket backlog and
unlink the association. If a concurrent shutdown(fd, SHUT_RD) sets
RCV_SHUTDOWN, the waiter breaks with err == 0 before checking
asoc->base.dead. Its final sctp_association_put() can then free the
association, leaving sctp_sendmsg_to_asoc() to continue with a dangling
pointer.

Check RCV_SHUTDOWN along with the wait error in sctp_sendmsg_to_asoc()
before using the association again. The check only accesses the socket,
so it needs no additional association reference. Return the existing
-ESRCH so that sctp_sendmsg() skips freeing a new association that may
already have been destroyed.

Keep sctp_wait_for_connect() unchanged to preserve its behavior for the
connect() caller.

Fixes: 668c9beb9020 ("sctp: implement assign_number for sctp_stream_interleave")
Cc: stable@vger.kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Assisted-by: tencentos-corvus-ai:hy4-preview
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
v3:
- Check RCV_SHUTDOWN after the sendmsg connect wait, as suggested by
  Xin Long, instead of taking an extra association reference and checking
  asoc->base.dead.
- Keep the shared wait helper unchanged and retain the -ESRCH error path.
- Link to v2: https://lore.kernel.org/all/20260911090039.75646-1-juny24602@gmail.com/
- Review: https://lore.kernel.org/all/CADvbK_f=S-2waid_92PbOCh-LWjU5oiZT+__xWfttVeTHEPQTg@mail.gmail.com/

v2:
- Leave sctp_wait_for_connect() unchanged to preserve connect() semantics.
- Hold and validate the association in the sendmsg-only caller.

 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index c7b9e325ec1c..4652fd90d9a6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1844,7 +1844,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		if (asoc->ep->intl_enable) {
 			timeo = sock_sndtimeo(sk, 0);
 			err = sctp_wait_for_connect(asoc, &timeo);
-			if (err) {
+			if (err || sk->sk_shutdown & RCV_SHUTDOWN) {
 				err = -ESRCH;
 				goto err;
 			}
-- 
2.54.0 (Apple Git-157)

Re: [PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In sctp_sendmsg_to_asoc() (net/sctp/socket.c:1847), the new
  condition `if (err || sk->sk_shutdown & RCV_SHUTDOWN)` returns -ESRCH…

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but it sits on
  the same post-wait path the patch hardens.

--

sctp: check RCV_SHUTDOWN after the sendmsg connect wait

On the intl_enable path of sctp_sendmsg_to_asoc(), this patch checks
sk->sk_shutdown & RCV_SHUTDOWN after sctp_wait_for_connect() and returns
-ESRCH if the bit is set. The aim is to stop the send from continuing with
an association that was freed while the socket lock was dropped. That can
happen when a concurrent SHUT_RD makes the wait break with err == 0.

> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c7b9e325ec1cd..4652fd90d9a6c 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1844,7 +1844,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
>  		if (asoc->ep->intl_enable) {
>  			timeo = sock_sndtimeo(sk, 0);
>  			err = sctp_wait_for_connect(asoc, &timeo);
> -			if (err) {
> +			if (err || sk->sk_shutdown & RCV_SHUTDOWN) {
>  				err = -ESRCH;
>  				goto err;
>  			}

[Severity: Medium]
Can this reject sends where the association is still alive and no race
happened?

The new condition only looks at the socket-wide RCV_SHUTDOWN bit. It does
not check whether the asoc actually died.

One case needs no race: shutdown(fd, SHUT_RD) on an unconnected socket.
inet_shutdown() falls through from TCP_CLOSE and sets the bit permanently,
even though it returns -ENOTCONN:

net/ipv4/af_inet.c:inet_shutdown() {
    ...
	case TCP_CLOSE:
		err = -ENOTCONN;
		fallthrough;
	default:
		WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | how);
    ...
}

sctp_shutdown() does nothing for SHUT_RD alone. A later sendmsg() to a
new peer then goes through sctp_primitive_ASSOCIATE(). On its first
iteration, sctp_wait_for_connect() breaks with err == 0 without sleeping:

		if (sk->sk_shutdown & RCV_SHUTDOWN)
			break;

A concurrent SHUT_RD that arrives after the handshake, on a healthy
ESTABLISHED asoc, would also hit this check.

Before this patch, both cases went on to sctp_primitive_SEND() and returned
msg_len. Now the data is dropped and userspace gets ESRCH. sctp_sendmsg()
reads -ESRCH as "the asoc may be gone":

	if (err < 0 && err != -ESRCH && new)
		sctp_association_free(asoc);

So the new asoc stays on ep->asocs with its INIT already sent. A retry then
succeeds because the asoc is no longer CLOSED.

This also looks inconsistent with how other code handles the same wait
result:

- __sctp_connect() returns it as connect() success.
- The non-intl wait_connect branch in this function sends and returns
  msg_len.
- The comment above sctp_shutdown() says SHUT_RD "Disables further
  receive operations. No SCTP protocol action is taken". Disabling sends
  is left to SHUT_WR and SHUT_RDWR.

The commit message only describes closing the concurrent SHUT_RD + ABORT
race. It doesn't mention the new errno from sendmsg() or the association
left behind.

Would a narrower check work here? One option is to hold the asoc across
the wait and test asoc->base.dead. Another is to treat RCV_SHUTDOWN as an
error only when the asoc is actually dead.

[Severity: Critical]
This is a pre-existing issue, but it sits on the same post-wait path this
patch hardens. Can the transport pointer also be stale after this wait?

sctp_wait_for_connect() drops the socket lock but only pins the asoc. It
takes no reference on the caller's transport and never checks
transport->dead afterwards. sctp_wait_for_sndbuf() does both.

When the wait returns 0 because the asoc reached ESTABLISHED,
sctp_sendmsg_to_asoc() goes on to:

	list_for_each_entry(chunk, &datamsg->chunks, frag_list) {
		sctp_chunk_hold(chunk);
		sctp_set_owner_w(chunk);
		chunk->transport = transport;
	}

	err = sctp_primitive_SEND(net, asoc, datamsg);

This sequence appears to free the transport while the lock is dropped:

1. sctp_sendmsg_new_asoc() adds the primary daddr as SCTP_UNKNOWN through
   sctp_connect_new_asoc(). Extra SCTP_DSTADDRV4/V6 cmsg addresses are
   added the same way through sctp_connect_add_peer().

2. sctp_sendmsg() keeps the transport for TCP-style sockets, or when
   SCTP_ADDR_OVER is set.

3. An INIT-ACK arrives from one of the extra addresses with the correct
   vtag. Its address parameters do not list the primary.

4. sctp_process_init() removes every transport still in SCTP_UNKNOWN:

	list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) {
		transport = list_entry(pos, struct sctp_transport, transports);
		if (transport->state == SCTP_UNKNOWN) {
			sctp_assoc_rm_peer(asoc, transport);
		}
	}

   In COOKIE_WAIT nothing else pins the primary transport. So
   sctp_transport_free() drops the last reference, and the transport is
   freed after an RCU grace period.

5. COOKIE-ACK moves the asoc to ESTABLISHED. The waiter returns 0 with
   RCV_SHUTDOWN clear, so the new check passes.

6. The flush that sctp_primitive_SEND() triggers reaches
   sctp_outq_select_transport(). That function reads new_transport->state,
   then does list_add_tail() on ctx->transport->send_ready and calls
   sctp_packet_config() on the freed transport.

A local socket needs net.sctp.intl_enable, SCTP_FRAGMENT_INTERLEAVE and
SCTP_INTERLEAVING_SUPPORTED for this path. The INIT-ACK contents and source
address are controlled by the remote peer. The problem seems to date back
to when the intl path started waiting for the connection before sending.

Does this path need a sctp_transport_hold() and a transport->dead check
across the wait, as in sctp_wait_for_sndbuf()? Re-deriving the transport
after the wait would be another option.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260926095606.68601-1-juny24602%40gmail.com

Re: [PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[Xin Long]

On Tue, Sep 29, 2026 at 8:58 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] In sctp_sendmsg_to_asoc() (net/sctp/socket.c:1847), the new
>   condition `if (err || sk->sk_shutdown & RCV_SHUTDOWN)` returns -ESRCH…
>
> Pre-existing issues:
> - [Critical] This problem wasn't introduced by this patch, but it sits on
>   the same post-wait path the patch hardens.
>
> --
>
> sctp: check RCV_SHUTDOWN after the sendmsg connect wait
>
> On the intl_enable path of sctp_sendmsg_to_asoc(), this patch checks
> sk->sk_shutdown & RCV_SHUTDOWN after sctp_wait_for_connect() and returns
> -ESRCH if the bit is set. The aim is to stop the send from continuing with
> an association that was freed while the socket lock was dropped. That can
> happen when a concurrent SHUT_RD makes the wait break with err == 0.
>
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index c7b9e325ec1cd..4652fd90d9a6c 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -1844,7 +1844,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
> >               if (asoc->ep->intl_enable) {
> >                       timeo = sock_sndtimeo(sk, 0);
> >                       err = sctp_wait_for_connect(asoc, &timeo);
> > -                     if (err) {
> > +                     if (err || sk->sk_shutdown & RCV_SHUTDOWN) {
> >                               err = -ESRCH;
> >                               goto err;
> >                       }
>
> [Severity: Medium]
> Can this reject sends where the association is still alive and no race
> happened?
>
> The new condition only looks at the socket-wide RCV_SHUTDOWN bit. It does
> not check whether the asoc actually died.
>
> One case needs no race: shutdown(fd, SHUT_RD) on an unconnected socket.
> inet_shutdown() falls through from TCP_CLOSE and sets the bit permanently,
> even though it returns -ENOTCONN:
>
> net/ipv4/af_inet.c:inet_shutdown() {
>     ...
>         case TCP_CLOSE:
>                 err = -ENOTCONN;
>                 fallthrough;
>         default:
>                 WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | how);
>     ...
> }
>
> sctp_shutdown() does nothing for SHUT_RD alone. A later sendmsg() to a
> new peer then goes through sctp_primitive_ASSOCIATE(). On its first
> iteration, sctp_wait_for_connect() breaks with err == 0 without sleeping:
>
>                 if (sk->sk_shutdown & RCV_SHUTDOWN)
>                         break;
>
> A concurrent SHUT_RD that arrives after the handshake, on a healthy
> ESTABLISHED asoc, would also hit this check.
>
> Before this patch, both cases went on to sctp_primitive_SEND() and returned
> msg_len. Now the data is dropped and userspace gets ESRCH. sctp_sendmsg()
> reads -ESRCH as "the asoc may be gone":
>
>         if (err < 0 && err != -ESRCH && new)
>                 sctp_association_free(asoc);
>
> So the new asoc stays on ep->asocs with its INIT already sent. A retry then
> succeeds because the asoc is no longer CLOSED.
>
> This also looks inconsistent with how other code handles the same wait
> result:
>
> - __sctp_connect() returns it as connect() success.
> - The non-intl wait_connect branch in this function sends and returns
>   msg_len.
> - The comment above sctp_shutdown() says SHUT_RD "Disables further
>   receive operations. No SCTP protocol action is taken". Disabling sends
>   is left to SHUT_WR and SHUT_RDWR.
>
> The commit message only describes closing the concurrent SHUT_RD + ABORT
> race. It doesn't mention the new errno from sendmsg() or the association
> left behind.
>
> Would a narrower check work here? One option is to hold the asoc across
> the wait and test asoc->base.dead. Another is to treat RCV_SHUTDOWN as an
> error only when the asoc is actually dead.
>
intl_enable case differs from the common case: chunks must not be enqueued
until the connection is established in sctp_sendmsg(). Therefore, we must
ensure the connection is established before continuing the send.

If shutdown(SHUT_RD) is called during the handshake in such a case.
sctp_wait_for_connect() may return 0 after seeing RCV_SHUTDOWN, without
confirming that the association reached ESTABLISHED. In this rare case,
it's safer to fail the send rather than enqueueing chunks based only on
!asoc->base.dead check.

So I would prefer the fix in this patch.

> [Severity: Critical]
> This is a pre-existing issue, but it sits on the same post-wait path this
> patch hardens. Can the transport pointer also be stale after this wait?
>
> sctp_wait_for_connect() drops the socket lock but only pins the asoc. It
> takes no reference on the caller's transport and never checks
> transport->dead afterwards. sctp_wait_for_sndbuf() does both.
>
> When the wait returns 0 because the asoc reached ESTABLISHED,
> sctp_sendmsg_to_asoc() goes on to:
>
>         list_for_each_entry(chunk, &datamsg->chunks, frag_list) {
>                 sctp_chunk_hold(chunk);
>                 sctp_set_owner_w(chunk);
>                 chunk->transport = transport;
>         }
>
>         err = sctp_primitive_SEND(net, asoc, datamsg);
>
> This sequence appears to free the transport while the lock is dropped:
>
> 1. sctp_sendmsg_new_asoc() adds the primary daddr as SCTP_UNKNOWN through
>    sctp_connect_new_asoc(). Extra SCTP_DSTADDRV4/V6 cmsg addresses are
>    added the same way through sctp_connect_add_peer().
>
> 2. sctp_sendmsg() keeps the transport for TCP-style sockets, or when
>    SCTP_ADDR_OVER is set.
>
> 3. An INIT-ACK arrives from one of the extra addresses with the correct
>    vtag. Its address parameters do not list the primary.
>
> 4. sctp_process_init() removes every transport still in SCTP_UNKNOWN:
>
>         list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) {
>                 transport = list_entry(pos, struct sctp_transport, transports);
>                 if (transport->state == SCTP_UNKNOWN) {
>                         sctp_assoc_rm_peer(asoc, transport);
>                 }
>         }
>
>    In COOKIE_WAIT nothing else pins the primary transport. So
>    sctp_transport_free() drops the last reference, and the transport is
>    freed after an RCU grace period.
>
> 5. COOKIE-ACK moves the asoc to ESTABLISHED. The waiter returns 0 with
>    RCV_SHUTDOWN clear, so the new check passes.
>
> 6. The flush that sctp_primitive_SEND() triggers reaches
>    sctp_outq_select_transport(). That function reads new_transport->state,
>    then does list_add_tail() on ctx->transport->send_ready and calls
>    sctp_packet_config() on the freed transport.
>
> A local socket needs net.sctp.intl_enable, SCTP_FRAGMENT_INTERLEAVE and
> SCTP_INTERLEAVING_SUPPORTED for this path. The INIT-ACK contents and source
> address are controlled by the remote peer. The problem seems to date back
> to when the intl path started waiting for the connection before sending.
>
> Does this path need a sctp_transport_hold() and a transport->dead check
> across the wait, as in sctp_wait_for_sndbuf()? Re-deriving the transport
> after the wait would be another option.
>
Seung already posted a patch for this pre-existing issue:
https://lore.kernel.org/netdev/20260928152132.3705137-1-guncraft2000@naver.com/

Thanks.

Re: [PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[Xin Long]

On Sat, Sep 26, 2026 at 5:56 AM Jun Yang <juny24602@gmail.com> wrote:
>
> From: Jun Yang <junvyyang@tencent.com>
>
> sctp_wait_for_connect() drops the socket lock while it sleeps. An
> out-of-the-blue ABORT can then be processed from the socket backlog and
> unlink the association. If a concurrent shutdown(fd, SHUT_RD) sets
> RCV_SHUTDOWN, the waiter breaks with err == 0 before checking
> asoc->base.dead. Its final sctp_association_put() can then free the
> association, leaving sctp_sendmsg_to_asoc() to continue with a dangling
> pointer.
>
> Check RCV_SHUTDOWN along with the wait error in sctp_sendmsg_to_asoc()
> before using the association again. The check only accesses the socket,
> so it needs no additional association reference. Return the existing
> -ESRCH so that sctp_sendmsg() skips freeing a new association that may
> already have been destroyed.
>
> Keep sctp_wait_for_connect() unchanged to preserve its behavior for the
> connect() caller.
>
> Fixes: 668c9beb9020 ("sctp: implement assign_number for sctp_stream_interleave")
> Cc: stable@vger.kernel.org
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Assisted-by: tencentos-corvus-ai:hy4-preview
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> ---
> v3:
> - Check RCV_SHUTDOWN after the sendmsg connect wait, as suggested by
>   Xin Long, instead of taking an extra association reference and checking
>   asoc->base.dead.
> - Keep the shared wait helper unchanged and retain the -ESRCH error path.
> - Link to v2: https://lore.kernel.org/all/20260911090039.75646-1-juny24602@gmail.com/
> - Review: https://lore.kernel.org/all/CADvbK_f=S-2waid_92PbOCh-LWjU5oiZT+__xWfttVeTHEPQTg@mail.gmail.com/
>
> v2:
> - Leave sctp_wait_for_connect() unchanged to preserve connect() semantics.
> - Hold and validate the association in the sendmsg-only caller.
>
>  net/sctp/socket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c7b9e325ec1c..4652fd90d9a6 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1844,7 +1844,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
>                 if (asoc->ep->intl_enable) {
>                         timeo = sock_sndtimeo(sk, 0);
>                         err = sctp_wait_for_connect(asoc, &timeo);
> -                       if (err) {
> +                       if (err || sk->sk_shutdown & RCV_SHUTDOWN) {
>                                 err = -ESRCH;
>                                 goto err;
>                         }
> --
> 2.54.0 (Apple Git-157)

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

Re: [PATCH net v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait

[patchwork-bot+netdevbpf]

Hello:

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

On Sat, 26 Sep 2026 17:55:15 +0800 you wrote:
> From: Jun Yang <junvyyang@tencent.com>
> 
> sctp_wait_for_connect() drops the socket lock while it sleeps. An
> out-of-the-blue ABORT can then be processed from the socket backlog and
> unlink the association. If a concurrent shutdown(fd, SHUT_RD) sets
> RCV_SHUTDOWN, the waiter breaks with err == 0 before checking
> asoc->base.dead. Its final sctp_association_put() can then free the
> association, leaving sctp_sendmsg_to_asoc() to continue with a dangling
> pointer.
> 
> [...]

Here is the summary with links:
  - [net,v3] sctp: check RCV_SHUTDOWN after the sendmsg connect wait
    https://git.kernel.org/netdev/net/c/4f1da630d13d

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