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

[Aohan Mei <ljp1205831794@gmail.com>]

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