mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] sctp: re-point retained control chunks on association migration
@ 2026-07-30  9:05 Jun Yang
  2026-08-01  1:56 ` Xin Long
  0 siblings, 1 reply; 2+ messages in thread
From: Jun Yang @ 2026-07-30  9:05 UTC (permalink / raw)
  To: netdev
  Cc: Jun Yang, stable, TencentOS Corvus AI, Marcelo Ricardo Leitner,
	Xin Long, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, linux-sctp, linux-kernel

From: Jun Yang <junvyyang@tencent.com>

sctp_control_set_owner_w() records the owning socket in a control chunk's
skb->sk as a bare pointer.  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.

Re-point every control chunk the association still holds at the new
socket right after sctp_assoc_migrate(), as is already done for DATA
chunks.  Control chunks carry no socket wmem charge and their skb
destructor does not use skb->sk, so a bare owner update is sufficient.

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 <corvus@tencent.com>
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
 net/sctp/socket.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index c7b9e325ec1c..9a6da4e0d741 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -9522,6 +9522,30 @@ static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
 	sctp_skb_set_owner_r(skb, sk);
 }
 
+/* Control chunks record their owning socket in skb->sk as a bare pointer and
+ * are not reached by sctp_for_each_tx_datachunk(); re-point the ones the
+ * association still holds so migration does not leave them on the old socket.
+ */
+static void sctp_ctrl_set_owner_w(struct sctp_chunk *chunk, struct sock *sk)
+{
+	if (chunk && chunk->skb)
+		chunk->skb->sk = sk;
+}
+
+static void sctp_for_each_tx_ctrlchunk(struct sctp_association *assoc, struct sock *sk)
+{
+	struct sctp_chunk *chunk;
+
+	list_for_each_entry(chunk, &assoc->outqueue.control_chunk_list, list)
+		sctp_ctrl_set_owner_w(chunk, sk);
+	list_for_each_entry(chunk, &assoc->asconf_ack_list, transmitted_list)
+		sctp_ctrl_set_owner_w(chunk, sk);
+	list_for_each_entry(chunk, &assoc->addip_chunk_list, list)
+		sctp_ctrl_set_owner_w(chunk, sk);
+	sctp_ctrl_set_owner_w(assoc->strreset_chunk, sk);
+	sctp_ctrl_set_owner_w(assoc->addip_last_asconf, sk);
+}
+
 /* Populate the fields of the newsk from the oldsk and migrate the assoc
  * and its messages to the newsk.
  */
@@ -9633,6 +9657,7 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 	sctp_for_each_tx_datachunk(assoc, true, sctp_clear_owner_w);
 	sctp_assoc_migrate(assoc, newsk);
 	sctp_for_each_tx_datachunk(assoc, false, sctp_set_owner_w);
+	sctp_for_each_tx_ctrlchunk(assoc, newsk);
 
 	/* If the association on the newsk is already closed before accept()
 	 * is called, set RCV_SHUTDOWN flag.
-- 
2.55.0


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

* Re: [PATCH net] sctp: re-point retained control chunks on association migration
  2026-07-30  9:05 [PATCH net] sctp: re-point retained control chunks on association migration Jun Yang
@ 2026-08-01  1:56 ` Xin Long
  0 siblings, 0 replies; 2+ messages in thread
From: Xin Long @ 2026-08-01  1:56 UTC (permalink / raw)
  To: Jun Yang
  Cc: netdev, Jun Yang, stable, TencentOS Corvus AI,
	Marcelo Ricardo Leitner, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-sctp,
	linux-kernel

On Thu, Jul 30, 2026 at 5:05 AM Jun Yang <juny24602@gmail.com> wrote:
>
> From: Jun Yang <junvyyang@tencent.com>
>
> sctp_control_set_owner_w() records the owning socket in a control chunk's
> skb->sk as a bare pointer.  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.
>
> Re-point every control chunk the association still holds at the new
> socket right after sctp_assoc_migrate(), as is already done for DATA
> chunks.  Control chunks carry no socket wmem charge and their skb
> destructor does not use skb->sk, so a bare owner update is sufficient.
>
> 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 <corvus@tencent.com>
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> ---
>  net/sctp/socket.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c7b9e325ec1c..9a6da4e0d741 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -9522,6 +9522,30 @@ static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
>         sctp_skb_set_owner_r(skb, sk);
>  }
>
> +/* Control chunks record their owning socket in skb->sk as a bare pointer and
> + * are not reached by sctp_for_each_tx_datachunk(); re-point the ones the
> + * association still holds so migration does not leave them on the old socket.
> + */
> +static void sctp_ctrl_set_owner_w(struct sctp_chunk *chunk, struct sock *sk)
> +{
> +       if (chunk && chunk->skb)
> +               chunk->skb->sk = sk;
> +}
> +
> +static void sctp_for_each_tx_ctrlchunk(struct sctp_association *assoc, struct sock *sk)
> +{
> +       struct sctp_chunk *chunk;
> +
> +       list_for_each_entry(chunk, &assoc->outqueue.control_chunk_list, list)
> +               sctp_ctrl_set_owner_w(chunk, sk);
> +       list_for_each_entry(chunk, &assoc->asconf_ack_list, transmitted_list)
> +               sctp_ctrl_set_owner_w(chunk, sk);
> +       list_for_each_entry(chunk, &assoc->addip_chunk_list, list)
> +               sctp_ctrl_set_owner_w(chunk, sk);
> +       sctp_ctrl_set_owner_w(assoc->strreset_chunk, sk);
> +       sctp_ctrl_set_owner_w(assoc->addip_last_asconf, sk);
> +}
> +
>  /* Populate the fields of the newsk from the oldsk and migrate the assoc
>   * and its messages to the newsk.
>   */
> @@ -9633,6 +9657,7 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>         sctp_for_each_tx_datachunk(assoc, true, sctp_clear_owner_w);
>         sctp_assoc_migrate(assoc, newsk);
>         sctp_for_each_tx_datachunk(assoc, false, sctp_set_owner_w);
> +       sctp_for_each_tx_ctrlchunk(assoc, newsk);
>
>         /* If the association on the newsk is already closed before accept()
>          * is called, set RCV_SHUTDOWN flag.
> --
> 2.55.0
>

Can you rename sctp_for_each_tx_datachunk() to sctp_for_each_tx_chunk() and
move the control chunk traversal there?

        list_for_each_entry(chunk, &q->control_chunk_list, list)
                cb(chunk);

        list_for_each_entry(chunk, &asoc->asconf_ack_list, transmitted_list)
                cb(chunk);

        list_for_each_entry(chunk, &asoc->addip_chunk_list, list)
                cb(chunk);

        if (asoc->strreset_chunk)
                cb(asoc->strreset_chunk);

        if (asoc->addip_last_asconf)
                cb(asoc->addip_last_asconf);

The in-progress patch:

https://lore.kernel.org/netdev/20260731120558.558957-1-david.lee@trailofbits.com/

will use sctp_set_owner_w_migrate() as the callback for
sctp_for_each_tx_datachunk(). Based on that, you can simply handle control
chunks (msg is NULL) in sctp_set_owner_w_migrate():

        if (chunk->msg)
                sctp_set_owner_w(chunk);
        else
                sctp_control_set_owner_w(chunk);

This avoids introducing additional helpers such as
sctp_for_each_tx_ctrlchunk() and sctp_ctrl_set_owner_w().

Thanks.

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

end of thread, other threads:[~2026-08-01  1:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-30  9:05 [PATCH net] sctp: re-point retained control chunks on association migration Jun Yang
2026-08-01  1:56 ` Xin Long

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

all inboxes | Powered by JetHome®