Re: [PATCH net] sctp: re-point the GSO head_skb socket on association migration

[Xin Long]

On Thu, Jul 30, 2026 at 7:06 AM Jun Yang <juny24602@xxxxxxxxx> wrote:
>
> From: Jun Yang <junvyyang@xxxxxxxxxxx>
>
> sctp_ulpevent_set_owner() stashes the receiving socket on the GSO "cover
> letter" skb as a bare pointer (net/sctp/ulpevent.c:90):
>
>         if (chunk && chunk->head_skb && !chunk->head_skb->sk)
>                 chunk->head_skb->sk = asoc->base.sk;
>
> That store takes no reference and installs no destructor, unlike the
> sctp_skb_set_owner_r() applied to the event skb just above it.
>
> When an association is moved to another socket -- SCTP_SOCKOPT_PEELOFF, or
> accept() on a TCP-style socket -- sctp_sock_migrate() re-owns the queued
> skbs with sctp_skb_set_owner_r_frag().  That walks the receive, lobby and
> reassembly queues, but chunk->head_skb is on none of them: it is reachable
> only through event->chunk->head_skb.  Its ->sk therefore keeps pointing at
> the original socket.  Once that socket is closed and freed, the dangling
> pointer is dereferenced on the very next receive, in sctp_recvmsg()
> (net/sctp/socket.c:2155):
>
>         sp->pf->skb_msgname(head_skb, msg->msg_name, &msg->msg_namelen);
>
> which for IPv6 reads sctp_sk(skb->sk)->v4mapped (net/sctp/ipv6.c:894).
>
> Re-point the cover-letter skb during migration as well.  The event lives in
> skb->cb of the event skb only, so this cannot be folded into
> sctp_skb_set_owner_r_frag(), which also recurses over fragment skbs; add a
> small wrapper and use it at the three migration call sites.
>
>   BUG: KASAN: slab-use-after-free in sctp_inet6_skb_msgname+0x633/0xb30
>   Read of size 2 at addr ff1100010aafc8e0 by task gso_uaf/6387
>   CPU: 1 UID: 1000 PID: 6387 Comm: gso_uaf
>    sctp_inet6_skb_msgname+0x633/0xb30
>    sctp_recvmsg+0x481/0xa00
>    sock_recvmsg+0x121/0x170
>   Freed by task 0:
>    __sk_destruct+0x39d/0x4c0
>    sctp_endpoint_destroy_rcu+0x8b/0xc0
>   The buggy address belongs to the object at ff1100010aafc380
>    which belongs to the cache SCTPv6 of size 1560
>
> Fixes: 90017accff61 ("sctp: Add GSO support")
> Cc: stable@xxxxxxxxxx
> Reported-by: TencentOS Corvus AI <corvus@xxxxxxxxxxx>
> Signed-off-by: Jun Yang <junvyyang@xxxxxxxxxxx>
> ---
>  net/sctp/socket.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c7b9e325ec1c..ade413bbec56 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -9522,6 +9522,27 @@ static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
>         sctp_skb_set_owner_r(skb, sk);
>  }
>
> +/* Re-own a queued event's skbs, including the GSO "cover letter" skb.
> + *
> + * sctp_ulpevent_set_owner() stashes the socket on chunk->head_skb as a bare
> + * pointer, with no reference and no destructor.  That skb is not on any of the
> + * queues walked during a migration, so it has to be re-pointed explicitly.
> + *
> + * Only the event skb itself carries a struct sctp_ulpevent in ->cb, so this
> + * must not be folded into the recursive helper above, which also visits
> + * fragment skbs.
> + */
> +static void sctp_skb_set_owner_r_event(struct sk_buff *skb, struct sock *sk)
> +{
Please use a consistent name with sctp_set_owner_w_migrate(), like:
sctp_skb_set_owner_r_migrate().

> +       struct sctp_ulpevent *event = sctp_skb2event(skb);
> +
> +       sctp_skb_set_owner_r_frag(skb, sk);
> +
> +       if (event->chunk && event->chunk->head_skb &&
> +           event->chunk->head_skb != skb)
> +               event->chunk->head_skb->sk = sk;
I think 'event->chunk->head_skb != skb' check is not needed, head_skb is
either NULL or the head skb of a GSO packet, and it should never be
queued on these receive-related queues if it's not NULL.

BTW, if you have PoCs for these four issues, could you also share them in
the corresponding threads? It would be good to include links to the
reproducers/PoCs below the --- marker, as Jakub suggested here:

https://lore.kernel.org/netdev/20260723103515.0059802b@xxxxxxxxxx/

Thanks.

> +}
> +
>  /* Populate the fields of the newsk from the oldsk and migrate the assoc
>   * and its messages to the newsk.
>   */
> @@ -9571,7 +9592,7 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>                 if (event->asoc == assoc) {
>                         __skb_unlink(skb, &oldsk->sk_receive_queue);
>                         __skb_queue_tail(&newsk->sk_receive_queue, skb);
> -                       sctp_skb_set_owner_r_frag(skb, newsk);
> +                       sctp_skb_set_owner_r_event(skb, newsk);
>                 }
>         }
>
> @@ -9600,7 +9621,7 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>                         if (event->asoc == assoc) {
>                                 __skb_unlink(skb, &oldsp->pd_lobby);
>                                 __skb_queue_tail(queue, skb);
> -                               sctp_skb_set_owner_r_frag(skb, newsk);
> +                               sctp_skb_set_owner_r_event(skb, newsk);
>                         }
>                 }
>
> @@ -9612,7 +9633,7 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>
>         }
>
> -       sctp_for_each_rx_skb(assoc, newsk, sctp_skb_set_owner_r_frag);
> +       sctp_for_each_rx_skb(assoc, newsk, sctp_skb_set_owner_r_event);
>
>         /* Set the type of socket to indicate that it is peeled off from the
>          * original UDP-style socket or created with the accept() call on a
> --
> 2.43.7