From: Jun Yang sctp_control_set_owner_w() records the owning socket in a control chunk's skb->sk. sctp_sock_migrate() re-owns the association's DATA chunks via sctp_for_each_tx_datachunk(), but that walk keys off chunk->msg and so skips control chunks: any control chunk the association still holds (for example the saved stream-reset request asoc->strreset_chunk, the ASCONF request/ack lists, or asoc->addip_last_asconf) keeps pointing at the old socket after the association is moved to the new one. Once the old socket is freed, a later retransmit reaches sctp_packet_transmit() -> skb_set_owner_w(head, chunk->skb->sk) and operates on the freed socket -- refcount_add() on its sk_wmem_alloc, then sk->sk_write_space() from sock_wfree() -- a use-after-free of struct sock. Rename sctp_for_each_tx_datachunk() to sctp_for_each_tx_chunk() and walk the control chunks the association retains there as well, so migration re-owns them with the same clear/set bracketing already used for DATA chunks. sctp_set_owner_w_migrate() picks the right owner helper by testing chunk->msg, which is NULL for control chunks. The per-chunk owner test that traverse_and_process() already applies is split out into sctp_process_tx_chunk() and reused for the control lists. A chunk can sit on two of them at once -- asoc->strreset_chunk and asoc->addip_last_asconf both stay queued on outqueue.control_chunk_list until they are flushed -- and the test keeps such a chunk from being cleared or re-owned twice, which would otherwise leak an shkey reference. sctp_control_set_owner_w() re-reads chunk->shkey from asoc->shkey, so sctp_set_owner_w_migrate() releases the reference sctp_clear_owner_w() took by value instead of re-reading chunk->shkey, which would drop the wrong key if the active key changed while the chunk was queued. Fixes: d04adf1b3551 ("sctp: reset owner sk for data chunks on out queues when migrating a sock") Cc: stable@kernel.org Reported-by: TencentOS Corvus AI Assisted-by: tencentos-corvus-ai:kimi-k3 Signed-off-by: Jun Yang --- This is based on David Lee's [PATCH] sctp: hold shkey across socket migration https://lore.kernel.org/netdev/20260731120558.558957-1-david.lee@trailofbits.com/ which adds sctp_set_owner_w_migrate() v2: - Rename sctp_for_each_tx_datachunk() to sctp_for_each_tx_chunk() and move the control-chunk traversal into it, rather than adding a separate sctp_for_each_tx_ctrlchunk() helper (Xin Long). - Handle control chunks in sctp_set_owner_w_migrate() by testing chunk->msg, dropping the sctp_ctrl_set_owner_w() helper (Xin Long). Control chunks now go through the full clear/set bracketing instead of a bare skb->sk store, so sctp_control_set_owner_w() is no longer static. - Factor the existing owner test out of traverse_and_process() into sctp_process_tx_chunk() so the control lists get it too. v1: https://lore.kernel.org/netdev/20260730090537.27629-1-juny24602@gmail.com/ include/net/sctp/sm.h | 1 + net/sctp/sm_make_chunk.c | 2 +- net/sctp/socket.c | 53 ++++++++++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h index 3bfd261a53cc..76605d1ee839 100644 --- a/include/net/sctp/sm.h +++ b/include/net/sctp/sm.h @@ -252,6 +252,7 @@ struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc, struct sctp_fwdtsn_skip *skiplist); struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc, __u16 key_id); +void sctp_control_set_owner_w(struct sctp_chunk *chunk); struct sctp_chunk *sctp_make_strreset_req(const struct sctp_association *asoc, __u16 stream_num, __be16 *stream_list, bool out, bool in); diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 0ae30c3c8913..7684686798cf 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -94,7 +94,7 @@ static void sctp_control_release_owner(struct sk_buff *skb) } } -static void sctp_control_set_owner_w(struct sctp_chunk *chunk) +void sctp_control_set_owner_w(struct sctp_chunk *chunk) { struct sctp_association *asoc = chunk->asoc; struct sk_buff *skb = chunk->skb; diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 4a08023d52aa..d09b9f139070 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -155,9 +155,24 @@ static void sctp_clear_owner_w(struct sctp_chunk *chunk) static void sctp_set_owner_w_migrate(struct sctp_chunk *chunk) { - sctp_set_owner_w(chunk); - if (chunk->shkey) - sctp_auth_shkey_release(chunk->shkey); + struct sctp_shared_key *shkey = chunk->shkey; + + if (chunk->msg) + sctp_set_owner_w(chunk); + else + sctp_control_set_owner_w(chunk); + + if (shkey) + sctp_auth_shkey_release(shkey); +} + +static void sctp_process_tx_chunk(struct sctp_association *asoc, + struct sctp_chunk *chunk, bool clear, + void (*cb)(struct sctp_chunk *)) +{ + if ((clear && asoc->base.sk == chunk->skb->sk) || + (!clear && asoc->base.sk != chunk->skb->sk)) + cb(chunk); } #define traverse_and_process() \ @@ -165,17 +180,14 @@ do { \ msg = chunk->msg; \ if (msg == prev_msg) \ continue; \ - list_for_each_entry(c, &msg->chunks, frag_list) { \ - if ((clear && asoc->base.sk == c->skb->sk) || \ - (!clear && asoc->base.sk != c->skb->sk)) \ - cb(c); \ - } \ + list_for_each_entry(c, &msg->chunks, frag_list) \ + sctp_process_tx_chunk(asoc, c, clear, cb); \ prev_msg = msg; \ } while (0) -static void sctp_for_each_tx_datachunk(struct sctp_association *asoc, - bool clear, - void (*cb)(struct sctp_chunk *)) +static void sctp_for_each_tx_chunk(struct sctp_association *asoc, + bool clear, + void (*cb)(struct sctp_chunk *)) { struct sctp_datamsg *msg, *prev_msg = NULL; @@ -198,6 +210,21 @@ static void sctp_for_each_tx_datachunk(struct sctp_association *asoc, list_for_each_entry(chunk, &q->out_chunk_list, list) traverse_and_process(); + + list_for_each_entry(chunk, &q->control_chunk_list, list) + sctp_process_tx_chunk(asoc, chunk, clear, cb); + + list_for_each_entry(chunk, &asoc->asconf_ack_list, transmitted_list) + sctp_process_tx_chunk(asoc, chunk, clear, cb); + + list_for_each_entry(chunk, &asoc->addip_chunk_list, list) + sctp_process_tx_chunk(asoc, chunk, clear, cb); + + if (asoc->strreset_chunk) + sctp_process_tx_chunk(asoc, asoc->strreset_chunk, clear, cb); + + if (asoc->addip_last_asconf) + sctp_process_tx_chunk(asoc, asoc->addip_last_asconf, clear, cb); } static void sctp_for_each_rx_skb(struct sctp_association *asoc, struct sock *sk, @@ -9640,9 +9667,9 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk, * paths won't try to lock it and then oldsk. */ lock_sock_nested(newsk, SINGLE_DEPTH_NESTING); - sctp_for_each_tx_datachunk(assoc, true, sctp_clear_owner_w); + sctp_for_each_tx_chunk(assoc, true, sctp_clear_owner_w); sctp_assoc_migrate(assoc, newsk); - sctp_for_each_tx_datachunk(assoc, false, sctp_set_owner_w_migrate); + sctp_for_each_tx_chunk(assoc, false, sctp_set_owner_w_migrate); /* If the association on the newsk is already closed before accept() * is called, set RCV_SHUTDOWN flag. -- 2.55.0