[PATCH net] sctp: don't re-register a removed transport as last_data_from

[PATCH net] sctp: don't re-register a removed transport as last_data_from

[Aohan Mei]

From: Aohan Mei <henrymei@tencent.com>

When an association is in COOKIE-ECHOED state and the peer sends a
bundled [ERROR(Stale Cookie)][DATA] packet from one of its non-primary
addresses, processing the ERROR chunk takes the non-fatal stale-cookie
retry path sctp_sf_do_5_2_6_stale(), which queues
SCTP_CMD_DEL_NON_PRIMARY while keeping the association alive.
sctp_cmd_del_non_primary() removes every non-primary transport -
including the very transport this packet arrived on, which is still
referenced by the receive lookup and shared by all chunks of the
packet via chunk->transport.

sctp_assoc_rm_peer() does redirect asoc->peer.last_data_from away from
the removed transport, but right afterwards the bundled DATA chunk
makes sctp_assoc_bh_rcv() re-register
asoc->peer.last_data_from = chunk->transport unconditionally, undoing
the redirection with the just-removed transport.

Once the packet is done, the receive reference is dropped and the
transport is RCU-freed, while the surviving association keeps the
dangling last_data_from.  A later FWD-TSN (or the delayed SACK timer)
makes sctp_gen_sack() dereference it (->param_flags and friends), and
sctp_make_sack()/sctp_outq_select_transport() may write to the freed
object and link it into the live transport list.  This is a
use-after-free triggerable by any malicious SCTP peer (or a local
unprivileged user acting as one) with no capabilities required:

  BUG: KASAN: slab-use-after-free in sctp_do_sm+0x498a/0x5660
  Read of size 4 at addr ffff88800e1e356c by task poc/115
  Call Trace: sctp_do_sm <- sctp_assoc_bh_rcv <- sctp_inq_push <-
              sctp_rcv <- ip_protocol_deliver_rcu <- ip_rcv
  Allocated: sctp_transport_new <- sctp_assoc_add_peer <-
             sctp_process_init (INIT-ACK processing)
  Freed: kfree <- sctp_transport_destroy_rcu <- rcu_core
         (call_rcu queued by sctp_transport_put at end of sctp_rcv)
  The buggy address is located 364 bytes inside of freed 1024-byte
  region [ffff88800e1e3400, ffff88800e1e3800), cache kmalloc-1k

Related is commit 03a9d10ecf71 ("sctp: drop a chunk if its transport was
removed"), which only covers the window between the receive lookup and
the chunk processing (e.g. an ASCONF DEL-IP racing the socket backlog);
here the transport is removed *while* the packet is being processed, by
an earlier chunk of the same packet, so the drop in sctp_inq_push() does
not reach this path.  Verified with the bundled [ERROR(Stale
Cookie)][DATA] + FWD-TSN reproducer: the KASAN report above still fires
with that commit applied, and is gone with this patch on top.

Fix it by never registering a dead transport as last_data_from:
sctp_transport_free() sets ->dead when the transport is removed, so
both re-registration sites (the association and the endpoint backlog
paths) can simply skip it, keeping the redirection done by
sctp_assoc_rm_peer() in effect.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---
 net/sctp/associola.c   | 16 ++++++++++++----
 net/sctp/endpointola.c | 10 ++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 5b0ae616e1ff9..ecef09959630d 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1023,11 +1023,19 @@ static void sctp_assoc_bh_rcv(struct work_struct *work)
 			continue;
 
 		/* Remember where the last DATA chunk came from so we
-		 * know where to send the SACK.
+		 * know where to send the SACK.  chunk->transport may have
+		 * been removed while processing an earlier chunk of this
+		 * same packet (e.g. a stale-cookie ERROR chunk queues
+		 * SCTP_CMD_DEL_NON_PRIMARY, which removes the non-primary
+		 * transport this packet arrived on), so never register a
+		 * dead transport; otherwise last_data_from would be left
+		 * dangling once the receive reference is dropped and the
+		 * transport is freed.
 		 */
-		if (sctp_chunk_is_data(chunk))
-			asoc->peer.last_data_from = chunk->transport;
-		else {
+		if (sctp_chunk_is_data(chunk)) {
+			if (!chunk->transport || !chunk->transport->dead)
+				asoc->peer.last_data_from = chunk->transport;
+		} else {
 			SCTP_INC_STATS(net, SCTP_MIB_INCTRLCHUNKS);
 			asoc->stats.ictrlchunks++;
 			if (chunk->chunk_hdr->type == SCTP_CID_SACK)
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index dfb1719275dba..f9d4f318e132e 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -392,11 +392,13 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work)
 			continue;
 
 		/* Remember where the last DATA chunk came from so we
-		 * know where to send the SACK.
+		 * know where to send the SACK.  As in sctp_assoc_bh_rcv(),
+		 * never register a dead (already removed) transport.
 		 */
-		if (asoc && sctp_chunk_is_data(chunk))
-			asoc->peer.last_data_from = chunk->transport;
-		else {
+		if (asoc && sctp_chunk_is_data(chunk)) {
+			if (!chunk->transport || !chunk->transport->dead)
+				asoc->peer.last_data_from = chunk->transport;
+		} else {
 			SCTP_INC_STATS(ep->base.net, SCTP_MIB_INCTRLCHUNKS);
 			if (asoc)
 				asoc->stats.ictrlchunks++;
-- 
2.43.7

Re: [PATCH net] sctp: don't re-register a removed transport as last_data_from

[netdev-bot+sashiko]

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

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

- [High] Incomplete fix: the patch only sanitizes the two
  `asoc->peer.last_data_from` re-registration sites, but leaves the…
- [Medium] The new use-after-free guard reads `transport->dead`, which is
  bit 0 of an unlocked, non-atomically updated `__u32` bitfield in…

--

sctp: don't re-register a removed transport as last_data_from

This patch stops sctp_assoc_bh_rcv() and sctp_endpoint_bh_rcv() from storing
a transport into asoc->peer.last_data_from when that transport has already
been marked dead, which happens when an earlier chunk of the same packet
removes the transport the packet arrived on. The intent is to preserve the
redirection that sctp_assoc_rm_peer() performed, so last_data_from is not
left pointing at a transport that gets RCU-freed at the end of sctp_rcv().

A couple of questions below.

> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index c0512c827d0f5..e14fd88e69d6b 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1030,11 +1030,19 @@ static void sctp_assoc_bh_rcv(struct work_struct *work)
>  			continue;
>  
>  		/* Remember where the last DATA chunk came from so we
> -		 * know where to send the SACK.
> +		 * know where to send the SACK.  chunk->transport may have
> +		 * been removed while processing an earlier chunk of this
> +		 * same packet (e.g. a stale-cookie ERROR chunk queues
> +		 * SCTP_CMD_DEL_NON_PRIMARY, which removes the non-primary
> +		 * transport this packet arrived on), so never register a
> +		 * dead transport; otherwise last_data_from would be left
> +		 * dangling once the receive reference is dropped and the
> +		 * transport is freed.
>  		 */
> -		if (sctp_chunk_is_data(chunk))
> -			asoc->peer.last_data_from = chunk->transport;
> -		else {
> +		if (sctp_chunk_is_data(chunk)) {
> +			if (!chunk->transport || !chunk->transport->dead)
> +				asoc->peer.last_data_from = chunk->transport;
> +		} else {

[Severity: High]

Is guarding only the last_data_from store enough here?  After the earlier
chunk removed the arrival transport, the rest of this same loop iteration in
sctp_assoc_bh_rcv() still uses the dead transport:

	if (chunk->transport)
		chunk->transport->last_time_heard = ktime_get();

	error = sctp_do_sm(net, SCTP_EVENT_T_CHUNK, subtype,
			   state, ep, asoc, chunk, GFP_ATOMIC);

so chunk->transport is handed to the state machine for every remaining chunk
of the packet, and other sinks copy it into association-lifetime state.

The redirection in sctp_assoc_rm_peer() looks like a one-shot snapshot:

	if (asoc->peer.last_data_from == peer)
		asoc->peer.last_data_from = transport;
	...
	list_for_each_entry(ch, &asoc->outqueue.control_chunk_list, list)
		if (ch->transport == peer)
			ch->transport = NULL;

Only references that already exist at removal time are rewritten, so any
reference created by a later chunk of the same packet is never cleaned,
because the transport is already unhashed and off the transport list.

One such later reference is in sctp_cmd_setup_t2(), which has no dead test:

	if (chunk->transport)
		t = chunk->transport;
	...
	asoc->shutdown_last_sent_to = t;
	asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = t->rto;

sctp_sf_t2_timer_expire() later does SCTP_CMD_STRIKE on
asoc->shutdown_last_sent_to, and sctp_do_8_2_transport_strike() reads and
writes the transport from timer context, after the receive reference is gone.

Is that path reachable mid-packet?  sctp_assoc_rm_peer() is also called from
sctp_assoc_update() on the COOKIE-ECHO restart path, and that leaves the
association ESTABLISHED for the following bundled chunks, so a bundled
[COOKIE ECHO][SHUTDOWN] appears to reach sctp_sf_do_9_2_shutdown() and then
SCTP_CMD_SETUP_T2 with the removed transport.

A second sink is the reply chunks, for example sctp_make_heartbeat_ack():

	if (chunk)
		retval->transport = chunk->transport;

The same assignment exists in sctp_make_op_error(), sctp_make_abort*(),
sctp_make_shutdown*(), sctp_make_cookie_echo() and sctp_make_cwr().  Such a
chunk is queued on asoc->outqueue.control_chunk_list after
sctp_assoc_rm_peer() already sanitized that list, so it is not covered by the
redirection.

Can that chunk outlive the packet?  In sctp_outq_flush_ctrl() a one_packet
control chunk that returns SCTP_XMIT_PMTU_FULL is put back:

			if (status != SCTP_XMIT_OK) {
				/* put the chunk back */
				list_add(&chunk->list, &ctx->q->control_chunk_list);

and the src_out_of_asoc_ok branch skips non-ASCONF chunks:

		if (ctx->asoc->src_out_of_asoc_ok && chunk->chunk_hdr->type != SCTP_CID_ASCONF)
			continue;

sctp_packet_will_fit() returns PMTU_FULL for an oversized control chunk once
the packet is non-empty, and the peer controls the HEARTBEAT payload size.
On the next flush, sctp_outq_select_transport() reads new_transport->state
with no dead check, keeps the transport for HEARTBEAT/HEARTBEAT_ACK/
ASCONF_ACK, then writes into it and links it into the live list:

		if (list_empty(&ctx->transport->send_ready))
			list_add_tail(&ctx->transport->send_ready, &ctx->transport_list);
		sctp_packet_config(ctx->packet, ...);
		sctp_transport_burst_limited(ctx->transport);

That is the same read/write plus live-list linking the commit message
describes for the reported case.

This also seems to go against the policy the commit message cites from commit
03a9d10ecf71, where sctp_inq_push() frees such a chunk outright:

	if (chunk->rcvr->dead ||
	    (chunk->transport && chunk->transport->dead)) {
		sctp_chunk_free(chunk);
		return;
	}

Would discarding the remaining chunks of the packet, or clearing
chunk->transport when the arrival transport is removed mid-packet, be more
consistent with that and cover the other sinks too?  Note that the ASCONF
DEL-IP path already refuses to remove the arrival transport at all:

	if (peer == asconf->transport)
		return SCTP_ERROR_REQ_REFUSED;

Related to that, the commit message says the fix works because "both
re-registration sites (the association and the endpoint backlog paths) can
simply skip it, keeping the redirection done by sctp_assoc_rm_peer() in
effect".  Is that accurate for anything other than last_data_from?  The
sctp_cmd_setup_t2() and reply-chunk references above are created after
sctp_assoc_rm_peer() has run and are not redirected by it.

> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index a15b599b20b76..f1e2acf8d9bcb 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -396,11 +396,13 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work)
>  			continue;
>  
>  		/* Remember where the last DATA chunk came from so we
> -		 * know where to send the SACK.
> +		 * know where to send the SACK.  As in sctp_assoc_bh_rcv(),
> +		 * never register a dead (already removed) transport.
>  		 */
> -		if (asoc && sctp_chunk_is_data(chunk))
> -			asoc->peer.last_data_from = chunk->transport;
> -		else {
> +		if (asoc && sctp_chunk_is_data(chunk)) {
> +			if (!chunk->transport || !chunk->transport->dead)
> +				asoc->peer.last_data_from = chunk->transport;
> +		} else {

[Severity: Medium]

Can the dead bit read by this new guard, in both sctp_endpoint_bh_rcv() and
sctp_assoc_bh_rcv(), be lost to a concurrent bitfield update?  In
struct sctp_transport the flag shares one storage unit with the others:

	__u32	dead:1,
		rto_pending:1,
		hb_sent:1,
		pmtu_pending:1,
		dst_pending_confirm:1,
		sack_generation:1;

The two writers of that word do not share a lock.  sctp_transport_free()
does transport->dead = 1, reached in the scenario of this commit from
sctp_assoc_bh_rcv() -> sctp_do_sm() -> sctp_cmd_del_non_primary() ->
sctp_assoc_rm_peer().  When that runs from backlog processing,
__release_sock() has already dropped sk->sk_lock.slock before calling
sk_backlog_rcv(), so only the owned flag is held.

Meanwhile sctp_icmp_frag_needed(), from sctp_v4_err()/sctp_v6_err() in
softirq, holds only bh_lock_sock() and writes another bit of the same word,
in exactly the branch taken when a process-context owner is running:

	if (sock_owned_by_user(sk)) {
		atomic_set(&t->mtu_info, pmtu);
		asoc->pmtu_pending = 1;
		t->pmtu_pending = 1;
		return;
	}

sctp_err_lookup() holds a transport reference from
__sctp_lookup_association(), so both CPUs can legitimately be touching the
same object.  A single-bit assignment compiles to a non-atomic
read-modify-write of the containing byte or word, so the softirq store can
write back a copy loaded before dead = 1 became visible.

If dead is reset that way, this guard evaluates to true, the removed
transport is re-registered in asoc->peer.last_data_from, and the
use-after-free is back; the existing sctp_inq_push() drop on the same flag
would be defeated the same way.  The neighbouring atomic_set() on t->mtu_info
suggests this field is already known to be touched cross-context.

Would putting dead in its own word with WRITE_ONCE()/READ_ONCE(), or a fix
that does not read the flag at all (clearing chunk->transport when the
arrival transport is removed mid-packet), avoid depending on this?

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

Re: [PATCH net] sctp: don't re-register a removed transport as last_data_from

[Xin Long]

On Thu, Sep 17, 2026 at 7:10 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
>
> - [High] Incomplete fix: the patch only sanitizes the two
>   `asoc->peer.last_data_from` re-registration sites, but leaves the…
> - [Medium] The new use-after-free guard reads `transport->dead`, which is
>   bit 0 of an unlocked, non-atomically updated `__u32` bitfield in…
>
> --
>
> sctp: don't re-register a removed transport as last_data_from
>
> This patch stops sctp_assoc_bh_rcv() and sctp_endpoint_bh_rcv() from storing
> a transport into asoc->peer.last_data_from when that transport has already
> been marked dead, which happens when an earlier chunk of the same packet
> removes the transport the packet arrived on. The intent is to preserve the
> redirection that sctp_assoc_rm_peer() performed, so last_data_from is not
> left pointing at a transport that gets RCU-freed at the end of sctp_rcv().
>
> A couple of questions below.
>
> > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > index c0512c827d0f5..e14fd88e69d6b 100644
> > --- a/net/sctp/associola.c
> > +++ b/net/sctp/associola.c
> > @@ -1030,11 +1030,19 @@ static void sctp_assoc_bh_rcv(struct work_struct *work)
> >                       continue;
> >
> >               /* Remember where the last DATA chunk came from so we
> > -              * know where to send the SACK.
> > +              * know where to send the SACK.  chunk->transport may have
> > +              * been removed while processing an earlier chunk of this
> > +              * same packet (e.g. a stale-cookie ERROR chunk queues
> > +              * SCTP_CMD_DEL_NON_PRIMARY, which removes the non-primary
> > +              * transport this packet arrived on), so never register a
> > +              * dead transport; otherwise last_data_from would be left
> > +              * dangling once the receive reference is dropped and the
> > +              * transport is freed.
> >                */
> > -             if (sctp_chunk_is_data(chunk))
> > -                     asoc->peer.last_data_from = chunk->transport;
> > -             else {
> > +             if (sctp_chunk_is_data(chunk)) {
> > +                     if (!chunk->transport || !chunk->transport->dead)
> > +                             asoc->peer.last_data_from = chunk->transport;
> > +             } else {
>
> [Severity: High]
>
> Is guarding only the last_data_from store enough here?  After the earlier
> chunk removed the arrival transport, the rest of this same loop iteration in
> sctp_assoc_bh_rcv() still uses the dead transport:
>
>         if (chunk->transport)
>                 chunk->transport->last_time_heard = ktime_get();
>
>         error = sctp_do_sm(net, SCTP_EVENT_T_CHUNK, subtype,
>                            state, ep, asoc, chunk, GFP_ATOMIC);
>
> so chunk->transport is handed to the state machine for every remaining chunk
> of the packet, and other sinks copy it into association-lifetime state.
>
> The redirection in sctp_assoc_rm_peer() looks like a one-shot snapshot:
>
>         if (asoc->peer.last_data_from == peer)
>                 asoc->peer.last_data_from = transport;
>         ...
>         list_for_each_entry(ch, &asoc->outqueue.control_chunk_list, list)
>                 if (ch->transport == peer)
>                         ch->transport = NULL;
>
> Only references that already exist at removal time are rewritten, so any
> reference created by a later chunk of the same packet is never cleaned,
> because the transport is already unhashed and off the transport list.
>
> One such later reference is in sctp_cmd_setup_t2(), which has no dead test:
>
>         if (chunk->transport)
>                 t = chunk->transport;
>         ...
>         asoc->shutdown_last_sent_to = t;
>         asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = t->rto;
>
> sctp_sf_t2_timer_expire() later does SCTP_CMD_STRIKE on
> asoc->shutdown_last_sent_to, and sctp_do_8_2_transport_strike() reads and
> writes the transport from timer context, after the receive reference is gone.
>
> Is that path reachable mid-packet?  sctp_assoc_rm_peer() is also called from
> sctp_assoc_update() on the COOKIE-ECHO restart path, and that leaves the
> association ESTABLISHED for the following bundled chunks, so a bundled
> [COOKIE ECHO][SHUTDOWN] appears to reach sctp_sf_do_9_2_shutdown() and then
> SCTP_CMD_SETUP_T2 with the removed transport.
>
> A second sink is the reply chunks, for example sctp_make_heartbeat_ack():
>
>         if (chunk)
>                 retval->transport = chunk->transport;
>
> The same assignment exists in sctp_make_op_error(), sctp_make_abort*(),
> sctp_make_shutdown*(), sctp_make_cookie_echo() and sctp_make_cwr().  Such a
> chunk is queued on asoc->outqueue.control_chunk_list after
> sctp_assoc_rm_peer() already sanitized that list, so it is not covered by the
> redirection.
>
> Can that chunk outlive the packet?  In sctp_outq_flush_ctrl() a one_packet
> control chunk that returns SCTP_XMIT_PMTU_FULL is put back:
>
>                         if (status != SCTP_XMIT_OK) {
>                                 /* put the chunk back */
>                                 list_add(&chunk->list, &ctx->q->control_chunk_list);
>
> and the src_out_of_asoc_ok branch skips non-ASCONF chunks:
>
>                 if (ctx->asoc->src_out_of_asoc_ok && chunk->chunk_hdr->type != SCTP_CID_ASCONF)
>                         continue;
>
> sctp_packet_will_fit() returns PMTU_FULL for an oversized control chunk once
> the packet is non-empty, and the peer controls the HEARTBEAT payload size.
> On the next flush, sctp_outq_select_transport() reads new_transport->state
> with no dead check, keeps the transport for HEARTBEAT/HEARTBEAT_ACK/
> ASCONF_ACK, then writes into it and links it into the live list:
>
>                 if (list_empty(&ctx->transport->send_ready))
>                         list_add_tail(&ctx->transport->send_ready, &ctx->transport_list);
>                 sctp_packet_config(ctx->packet, ...);
>                 sctp_transport_burst_limited(ctx->transport);
>
> That is the same read/write plus live-list linking the commit message
> describes for the reported case.
>
> This also seems to go against the policy the commit message cites from commit
> 03a9d10ecf71, where sctp_inq_push() frees such a chunk outright:
>
>         if (chunk->rcvr->dead ||
>             (chunk->transport && chunk->transport->dead)) {
>                 sctp_chunk_free(chunk);
>                 return;
>         }
>
> Would discarding the remaining chunks of the packet, or clearing
> chunk->transport when the arrival transport is removed mid-packet, be more
> consistent with that and cover the other sinks too?  Note that the ASCONF
> DEL-IP path already refuses to remove the arrival transport at all:
>
>         if (peer == asconf->transport)
>                 return SCTP_ERROR_REQ_REFUSED;
>
> Related to that, the commit message says the fix works because "both
> re-registration sites (the association and the endpoint backlog paths) can
> simply skip it, keeping the redirection done by sctp_assoc_rm_peer() in
> effect".  Is that accurate for anything other than last_data_from?  The
> sctp_cmd_setup_t2() and reply-chunk references above are created after
> sctp_assoc_rm_peer() has run and are not redirected by it.
>
> > diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> > index a15b599b20b76..f1e2acf8d9bcb 100644
> > --- a/net/sctp/endpointola.c
> > +++ b/net/sctp/endpointola.c
> > @@ -396,11 +396,13 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work)
> >                       continue;
> >
> >               /* Remember where the last DATA chunk came from so we
> > -              * know where to send the SACK.
> > +              * know where to send the SACK.  As in sctp_assoc_bh_rcv(),
> > +              * never register a dead (already removed) transport.
> >                */
> > -             if (asoc && sctp_chunk_is_data(chunk))
> > -                     asoc->peer.last_data_from = chunk->transport;
> > -             else {
> > +             if (asoc && sctp_chunk_is_data(chunk)) {
> > +                     if (!chunk->transport || !chunk->transport->dead)
> > +                             asoc->peer.last_data_from = chunk->transport;
> > +             } else {
>
> [Severity: Medium]
>
> Can the dead bit read by this new guard, in both sctp_endpoint_bh_rcv() and
> sctp_assoc_bh_rcv(), be lost to a concurrent bitfield update?  In
> struct sctp_transport the flag shares one storage unit with the others:
>
>         __u32   dead:1,
>                 rto_pending:1,
>                 hb_sent:1,
>                 pmtu_pending:1,
>                 dst_pending_confirm:1,
>                 sack_generation:1;
>
> The two writers of that word do not share a lock.  sctp_transport_free()
> does transport->dead = 1, reached in the scenario of this commit from
> sctp_assoc_bh_rcv() -> sctp_do_sm() -> sctp_cmd_del_non_primary() ->
> sctp_assoc_rm_peer().  When that runs from backlog processing,
> __release_sock() has already dropped sk->sk_lock.slock before calling
> sk_backlog_rcv(), so only the owned flag is held.
>
> Meanwhile sctp_icmp_frag_needed(), from sctp_v4_err()/sctp_v6_err() in
> softirq, holds only bh_lock_sock() and writes another bit of the same word,
> in exactly the branch taken when a process-context owner is running:
>
>         if (sock_owned_by_user(sk)) {
>                 atomic_set(&t->mtu_info, pmtu);
>                 asoc->pmtu_pending = 1;
>                 t->pmtu_pending = 1;
>                 return;
>         }
>
> sctp_err_lookup() holds a transport reference from
> __sctp_lookup_association(), so both CPUs can legitimately be touching the
> same object.  A single-bit assignment compiles to a non-atomic
> read-modify-write of the containing byte or word, so the softirq store can
> write back a copy loaded before dead = 1 became visible.
>
> If dead is reset that way, this guard evaluates to true, the removed
> transport is re-registered in asoc->peer.last_data_from, and the
> use-after-free is back; the existing sctp_inq_push() drop on the same flag
> would be defeated the same way.  The neighbouring atomic_set() on t->mtu_info
> suggests this field is already known to be touched cross-context.
>
> Would putting dead in its own word with WRITE_ONCE()/READ_ONCE(), or a fix
> that does not read the flag at all (clearing chunk->transport when the
> arrival transport is removed mid-packet), avoid depending on this?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916080955.1019050-1-ljp1205831794%40gmail.com

Can you try to fix this issue by discarding the packet in
sctp_sf_do_5_2_6_stale() instead? like:

@@ -2653,6 +2653,7 @@ static enum sctp_disposition sctp_sf_do_5_2_6_stale(
                        SCTP_TO(SCTP_EVENT_TIMEOUT_T1_INIT));

        sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
+       sctp_add_cmd_sf(commands, SCTP_CMD_DISCARD_PACKET, SCTP_NULL());

        return SCTP_DISPOSITION_CONSUME;

Please also share the PoC with the maintainers if there's one.

Thanks.

[PATCH net v2] sctp: discard the rest of the packet on a stale-cookie error

[Aohan Mei]

From: Aohan Mei <henrymei@tencent.com>

When an association is in COOKIE-ECHOED state and the peer sends a
bundled [ERROR(Stale Cookie)][DATA] packet from one of its non-primary
addresses, processing the ERROR chunk takes the non-fatal stale-cookie
retry path sctp_sf_do_5_2_6_stale(), which queues
SCTP_CMD_DEL_NON_PRIMARY while keeping the association alive.
sctp_cmd_del_non_primary() removes every non-primary transport -
including the very transport this packet arrived on, which is still
referenced by the receive lookup and shared by all chunks of the
packet via chunk->transport.

sctp_assoc_rm_peer() does redirect asoc->peer.last_data_from away from
the removed transport, but right afterwards the bundled DATA chunk
makes sctp_assoc_bh_rcv() re-register
asoc->peer.last_data_from = chunk->transport unconditionally, undoing
the redirection with the just-removed transport.

Once the packet is done, the receive reference is dropped and the
transport is RCU-freed, while the surviving association keeps the
dangling last_data_from.  A later FWD-TSN (or the delayed SACK timer)
makes sctp_gen_sack() dereference it (->param_flags and friends), and
sctp_make_sack()/sctp_outq_select_transport() may write to the freed
object and link it into the live transport list.  This is a
use-after-free triggerable by any malicious SCTP peer (or a local
unprivileged user acting as one) with no capabilities required:

  BUG: KASAN: slab-use-after-free in sctp_do_sm+0x498a/0x5660
  Read of size 4 at addr ffff88800e1e356c by task poc/115
  Call Trace: sctp_do_sm <- sctp_assoc_bh_rcv <- sctp_inq_push <-
              sctp_rcv <- ip_protocol_deliver_rcu <- ip_rcv
  Allocated: sctp_transport_new <- sctp_assoc_add_peer <-
             sctp_process_init (INIT-ACK processing)
  Freed: kfree <- sctp_transport_destroy_rcu <- rcu_core
         (call_rcu queued by sctp_transport_put at end of sctp_rcv)
  The buggy address is located 364 bytes inside of freed 1024-byte
  region [ffff88800e1e3400, ffff88800e1e3800), cache kmalloc-1k

Note that commit 03a9d10ecf71 ("sctp: drop a chunk if its transport
was removed") only covers the window between the receive lookup and
the chunk processing (e.g. an ASCONF DEL-IP racing the socket backlog);
here the transport is removed *while* the packet is being processed,
by an earlier chunk of the same packet, so the drop in sctp_inq_push()
does not reach this path.  Verified with the bundled [ERROR(Stale
Cookie)][DATA] + FWD-TSN reproducer: the KASAN report above still
fires with that commit applied, and is gone with this patch on top.

Fix it by discarding the rest of the packet on this path, as suggested
by Xin.  After the stale-cookie ERROR has sent the association back to
COOKIE-WAIT and removed the non-primary transports, the remaining
chunks of the packet can only run against the restarted handshake
while referencing the removed arrival transport through
chunk->transport: besides the last_data_from registration above,
sctp_cmd_setup_t2() and the sctp_make_*() reply builders would also
copy that pointer into association-lifetime state that
sctp_assoc_rm_peer() has already sanitized.  Let the peer retransmit
them, in line with what sctp_inq_push() does for chunks whose
transport was removed before processing.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Xin Long <lucien.xin@gmail.com>
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---

Hi Xin,

Thanks a lot for the review and the suggestion.  Implemented in v2:
SCTP_CMD_DISCARD_PACKET queued at the end of the stale-cookie retry.

Verified with the reproducer: the kprobe trace now shows the ERROR
chunk still removing the transport (sctp_assoc_rm_peer/
sctp_transport_free fire as before) but no further chunk of the packet
being processed, and asoc->peer.last_data_from keeps the redirection
done by sctp_assoc_rm_peer().  The KASAN use-after-free is gone and
the association completes the retry handshake normally.

This also takes care of the other same-packet sinks the sashiko review
pointed out (sctp_cmd_setup_t2() and the sctp_make_*() reply chunks
copying chunk->transport into association-lifetime state): the
remaining chunks are now dropped before they can run, in line with
what sctp_inq_push() does for chunks whose transport was removed
before processing.

Regarding the cookie-echo restart path the review mentioned
(sctp_assoc_update() removing the arrival transport mid-packet, with a
bundled [COOKIE ECHO][SHUTDOWN] reaching SCTP_CMD_SETUP_T2): discarding
the packet does not look like an option there, as bundling DATA with
COOKIE-ECHO is a legitimate fast path.  I can look into clearing the
in-progress chunk's transport on removal, or a loop-level check, as a
follow-up - please let me know if you'd rather have it handled here.

On the bitfield concern: v2 no longer reads transport->dead at all.
The race described (sctp_icmp_frag_needed() in softirq doing a
non-atomic read-modify-write of pmtu_pending racing dead = 1) would
also defeat the existing transport->dead check in sctp_inq_push() from
03a9d10ecf71, so it might be worth moving that flag into its own word
separately.

As for the reproducer: it is a single static binary that plays both
roles over a TUN device - the victim side is a plain SCTP client
socket, while the peer side answers the INIT-ACK advertising a primary
and a non-primary address plus FWD-TSN support, injects the bundled
[ERROR(Stale Cookie)][DATA] from the non-primary address while the
association is COOKIE-ECHOED, completes the retry handshake, and sends
an FWD-TSN one second later to consume the dangling pointer.  It was
verified on tlinux 6.6.119 and on mainline v7.2-rc6-343, where it
still fires with 03a9d10ecf71f applied and is clean with this patch on
top.

Since it is a ready-to-run trigger for the bug, I'd prefer not to post
it to the public list - would it be OK if I send it to you and Marcelo
off-list instead?  Happy to share it in whatever way you prefer.

Thanks again!

 net/sctp/sm_statefuns.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 708fa07d5fffc..43ebceb5e15f5 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2654,6 +2654,8 @@ static enum sctp_disposition sctp_sf_do_5_2_6_stale(
 
 	sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
 
+	sctp_add_cmd_sf(commands, SCTP_CMD_DISCARD_PACKET, SCTP_NULL());
+
 	return SCTP_DISPOSITION_CONSUME;
 
 nomem:
-- 
2.43.7

Re: [PATCH net v2] sctp: discard the rest of the packet on a stale-cookie error

[Xin Long]

On Mon, Sep 21, 2026 at 5:37 AM Aohan Mei <ljp1205831794@gmail.com> wrote:
>
> From: Aohan Mei <henrymei@tencent.com>
>
> When an association is in COOKIE-ECHOED state and the peer sends a
> bundled [ERROR(Stale Cookie)][DATA] packet from one of its non-primary
> addresses, processing the ERROR chunk takes the non-fatal stale-cookie
> retry path sctp_sf_do_5_2_6_stale(), which queues
> SCTP_CMD_DEL_NON_PRIMARY while keeping the association alive.
> sctp_cmd_del_non_primary() removes every non-primary transport -
> including the very transport this packet arrived on, which is still
> referenced by the receive lookup and shared by all chunks of the
> packet via chunk->transport.
>
> sctp_assoc_rm_peer() does redirect asoc->peer.last_data_from away from
> the removed transport, but right afterwards the bundled DATA chunk
> makes sctp_assoc_bh_rcv() re-register
> asoc->peer.last_data_from = chunk->transport unconditionally, undoing
> the redirection with the just-removed transport.
>
> Once the packet is done, the receive reference is dropped and the
> transport is RCU-freed, while the surviving association keeps the
> dangling last_data_from.  A later FWD-TSN (or the delayed SACK timer)
> makes sctp_gen_sack() dereference it (->param_flags and friends), and
> sctp_make_sack()/sctp_outq_select_transport() may write to the freed
> object and link it into the live transport list.  This is a
> use-after-free triggerable by any malicious SCTP peer (or a local
> unprivileged user acting as one) with no capabilities required:
>
>   BUG: KASAN: slab-use-after-free in sctp_do_sm+0x498a/0x5660
>   Read of size 4 at addr ffff88800e1e356c by task poc/115
>   Call Trace: sctp_do_sm <- sctp_assoc_bh_rcv <- sctp_inq_push <-
>               sctp_rcv <- ip_protocol_deliver_rcu <- ip_rcv
>   Allocated: sctp_transport_new <- sctp_assoc_add_peer <-
>              sctp_process_init (INIT-ACK processing)
>   Freed: kfree <- sctp_transport_destroy_rcu <- rcu_core
>          (call_rcu queued by sctp_transport_put at end of sctp_rcv)
>   The buggy address is located 364 bytes inside of freed 1024-byte
>   region [ffff88800e1e3400, ffff88800e1e3800), cache kmalloc-1k
>
> Note that commit 03a9d10ecf71 ("sctp: drop a chunk if its transport
> was removed") only covers the window between the receive lookup and
> the chunk processing (e.g. an ASCONF DEL-IP racing the socket backlog);
> here the transport is removed *while* the packet is being processed,
> by an earlier chunk of the same packet, so the drop in sctp_inq_push()
> does not reach this path.  Verified with the bundled [ERROR(Stale
> Cookie)][DATA] + FWD-TSN reproducer: the KASAN report above still
> fires with that commit applied, and is gone with this patch on top.
>
> Fix it by discarding the rest of the packet on this path, as suggested
> by Xin.  After the stale-cookie ERROR has sent the association back to
> COOKIE-WAIT and removed the non-primary transports, the remaining
> chunks of the packet can only run against the restarted handshake
> while referencing the removed arrival transport through
> chunk->transport: besides the last_data_from registration above,
> sctp_cmd_setup_t2() and the sctp_make_*() reply builders would also
> copy that pointer into association-lifetime state that
> sctp_assoc_rm_peer() has already sanitized.  Let the peer retransmit
> them, in line with what sctp_inq_push() does for chunks whose
> transport was removed before processing.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Xin Long <lucien.xin@gmail.com>
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Cc: stable@vger.kernel.org
> Assisted-by: CodeBuddy:Kimi-K3
> Signed-off-by: Aohan Mei <henrymei@tencent.com>
> ---
>
> Hi Xin,
>
> Thanks a lot for the review and the suggestion.  Implemented in v2:
> SCTP_CMD_DISCARD_PACKET queued at the end of the stale-cookie retry.
>
> Verified with the reproducer: the kprobe trace now shows the ERROR
> chunk still removing the transport (sctp_assoc_rm_peer/
> sctp_transport_free fire as before) but no further chunk of the packet
> being processed, and asoc->peer.last_data_from keeps the redirection
> done by sctp_assoc_rm_peer().  The KASAN use-after-free is gone and
> the association completes the retry handshake normally.
>
> This also takes care of the other same-packet sinks the sashiko review
> pointed out (sctp_cmd_setup_t2() and the sctp_make_*() reply chunks
> copying chunk->transport into association-lifetime state): the
> remaining chunks are now dropped before they can run, in line with
> what sctp_inq_push() does for chunks whose transport was removed
> before processing.
>
> Regarding the cookie-echo restart path the review mentioned
> (sctp_assoc_update() removing the arrival transport mid-packet, with a
> bundled [COOKIE ECHO][SHUTDOWN] reaching SCTP_CMD_SETUP_T2): discarding
> the packet does not look like an option there, as bundling DATA with
> COOKIE-ECHO is a legitimate fast path.  I can look into clearing the
> in-progress chunk's transport on removal, or a loop-level check, as a
> follow-up - please let me know if you'd rather have it handled here.
>
> On the bitfield concern: v2 no longer reads transport->dead at all.
> The race described (sctp_icmp_frag_needed() in softirq doing a
> non-atomic read-modify-write of pmtu_pending racing dead = 1) would
> also defeat the existing transport->dead check in sctp_inq_push() from
> 03a9d10ecf71, so it might be worth moving that flag into its own word
> separately.
>
> As for the reproducer: it is a single static binary that plays both
> roles over a TUN device - the victim side is a plain SCTP client
> socket, while the peer side answers the INIT-ACK advertising a primary
> and a non-primary address plus FWD-TSN support, injects the bundled
> [ERROR(Stale Cookie)][DATA] from the non-primary address while the
> association is COOKIE-ECHOED, completes the retry handshake, and sends
> an FWD-TSN one second later to consume the dangling pointer.  It was
> verified on tlinux 6.6.119 and on mainline v7.2-rc6-343, where it
> still fires with 03a9d10ecf71f applied and is clean with this patch on
> top.
>
> Since it is a ready-to-run trigger for the bug, I'd prefer not to post
> it to the public list - would it be OK if I send it to you and Marcelo
> off-list instead?  Happy to share it in whatever way you prefer.
>
> Thanks again!
>
>  net/sctp/sm_statefuns.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 708fa07d5fffc..43ebceb5e15f5 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -2654,6 +2654,8 @@ static enum sctp_disposition sctp_sf_do_5_2_6_stale(
>
>         sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
>
> +       sctp_add_cmd_sf(commands, SCTP_CMD_DISCARD_PACKET, SCTP_NULL());
> +
>         return SCTP_DISPOSITION_CONSUME;
>
>  nomem:
> --
> 2.43.7
>
Acked-by: Xin Long <lucien.xin@gmail.com>

Re: [PATCH net v2] sctp: discard the rest of the packet on a stale-cookie error

[Xin Long]

On Mon, Sep 21, 2026 at 5:37 AM Aohan Mei <ljp1205831794@gmail.com> wrote:
>
> Hi Xin,
>
> Thanks a lot for the review and the suggestion.  Implemented in v2:
> SCTP_CMD_DISCARD_PACKET queued at the end of the stale-cookie retry.
>
> Verified with the reproducer: the kprobe trace now shows the ERROR
> chunk still removing the transport (sctp_assoc_rm_peer/
> sctp_transport_free fire as before) but no further chunk of the packet
> being processed, and asoc->peer.last_data_from keeps the redirection
> done by sctp_assoc_rm_peer().  The KASAN use-after-free is gone and
> the association completes the retry handshake normally.
>
> This also takes care of the other same-packet sinks the sashiko review
> pointed out (sctp_cmd_setup_t2() and the sctp_make_*() reply chunks
> copying chunk->transport into association-lifetime state): the
> remaining chunks are now dropped before they can run, in line with
> what sctp_inq_push() does for chunks whose transport was removed
> before processing.
>
> Regarding the cookie-echo restart path the review mentioned
> (sctp_assoc_update() removing the arrival transport mid-packet, with a
> bundled [COOKIE ECHO][SHUTDOWN] reaching SCTP_CMD_SETUP_T2): discarding
> the packet does not look like an option there, as bundling DATA with
> COOKIE-ECHO is a legitimate fast path.  I can look into clearing the
> in-progress chunk's transport on removal, or a loop-level check, as a
> follow-up - please let me know if you'd rather have it handled here.
>
Thanks for checking the AI comments from the last version.

First, you need to check if it's possible to remove a transport that is
the same as chunk->transport in sctp_assoc_update(). As far as I know,
the chunk->transport should always be included in the new asoc.

> On the bitfield concern: v2 no longer reads transport->dead at all.
> The race described (sctp_icmp_frag_needed() in softirq doing a
> non-atomic read-modify-write of pmtu_pending racing dead = 1) would
> also defeat the existing transport->dead check in sctp_inq_push() from
> 03a9d10ecf71, so it might be worth moving that flag into its own word
> separately.
>
Sounds good to me.

But move pmtu_pending instead, as I think it's the only one bit that might
get updated without holding the sock lock.

Also, split this __u32 field to two __u16 fields without changing the size
of struct sctp_transport.

> As for the reproducer: it is a single static binary that plays both
> roles over a TUN device - the victim side is a plain SCTP client
> socket, while the peer side answers the INIT-ACK advertising a primary
> and a non-primary address plus FWD-TSN support, injects the bundled
> [ERROR(Stale Cookie)][DATA] from the non-primary address while the
> association is COOKIE-ECHOED, completes the retry handshake, and sends
> an FWD-TSN one second later to consume the dangling pointer.  It was
> verified on tlinux 6.6.119 and on mainline v7.2-rc6-343, where it
> still fires with 03a9d10ecf71f applied and is clean with this patch on
> top.
>
> Since it is a ready-to-run trigger for the bug, I'd prefer not to post
> it to the public list - would it be OK if I send it to you and Marcelo
> off-list instead?  Happy to share it in whatever way you prefer.
>
Sure, sending to the maintainers is okay to me.

Thanks.

Re: [PATCH net v2] sctp: discard the rest of the packet on a stale-cookie error

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 21 Sep 2026 17:37:04 +0800 you wrote:
> From: Aohan Mei <henrymei@tencent.com>
> 
> When an association is in COOKIE-ECHOED state and the peer sends a
> bundled [ERROR(Stale Cookie)][DATA] packet from one of its non-primary
> addresses, processing the ERROR chunk takes the non-fatal stale-cookie
> retry path sctp_sf_do_5_2_6_stale(), which queues
> SCTP_CMD_DEL_NON_PRIMARY while keeping the association alive.
> sctp_cmd_del_non_primary() removes every non-primary transport -
> including the very transport this packet arrived on, which is still
> referenced by the receive lookup and shared by all chunks of the
> packet via chunk->transport.
> 
> [...]

Here is the summary with links:
  - [net,v2] sctp: discard the rest of the packet on a stale-cookie error
    https://git.kernel.org/netdev/net/c/4498467a8af0

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