Re: [PATCH net] sctp: re-point retained control chunks on association migration

Xin Long <[email protected]> Fri, 31 Jul 2026 21:56:38 -0400
Newsgroups org.kernel.vger.linux-sctp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <CADvbK_cucNJp9pDs94o4BDAH4hi-Jtpk_k5i-thSW4XhVZQgFQ@mail.gmail.com>
On Thu, Jul 30, 2026 at 5:05=E2=80=AFAM Jun Yang <[email protected]> wrot=
e:
>
> From: Jun Yang <[email protected]>
>
> 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: [email protected]
> Reported-by: TencentOS Corvus AI <[email protected]>
> Signed-off-by: Jun Yang <[email protected]>
> ---
>  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_bu=
ff *skb, struct sock *sk)
>         sctp_skb_set_owner_r(skb, sk);
>  }
>
> +/* Control chunks record their owning socket in skb->sk as a bare pointe=
r and
> + * are not reached by sctp_for_each_tx_datachunk(); re-point the ones th=
e
> + * association still holds so migration does not leave them on the old s=
ocket.
> + */
> +static void sctp_ctrl_set_owner_w(struct sctp_chunk *chunk, struct sock =
*sk)
> +{
> +       if (chunk && chunk->skb)
> +               chunk->skb->sk =3D sk;
> +}
> +
> +static void sctp_for_each_tx_ctrlchunk(struct sctp_association *assoc, s=
truct sock *sk)
> +{
> +       struct sctp_chunk *chunk;
> +
> +       list_for_each_entry(chunk, &assoc->outqueue.control_chunk_list, l=
ist)
> +               sctp_ctrl_set_owner_w(chunk, sk);
> +       list_for_each_entry(chunk, &assoc->asconf_ack_list, transmitted_l=
ist)
> +               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, st=
ruct 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 accep=
t()
>          * 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@trailofbit=
s.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.