Re: [PATCH net] sctp: re-point the GSO head_skb socket on association migration
Xin Long <[email protected]> Fri, 31 Jul 2026 22:14:21 -0400
| Newsgroups | org.kernel.vger.linux-sctp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CADvbK_eiuRw5g6T_Nk=cVZ1z=W=GYZ0y9DdDm6Gomgq0+McV3Q@mail.gmail.com> |
On Thu, Jul 30, 2026 at 7:06=E2=80=AFAM Jun Yang <[email protected]> wrot= e: > > From: Jun Yang <[email protected]> > > 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 =3D 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, o= r > 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 reachabl= e > only through event->chunk->head_skb. Its ->sk therefore keeps pointing a= t > 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: [email protected] > Reported-by: TencentOS Corvus AI <[email protected]> > Signed-off-by: Jun Yang <[email protected]> > --- > 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_bu= ff *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 explicit= ly. > + * > + * 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 =3D sctp_skb2event(skb); > + > + sctp_skb_set_owner_r_frag(skb, sk); > + > + if (event->chunk && event->chunk->head_skb && > + event->chunk->head_skb !=3D skb) > + event->chunk->head_skb->sk =3D sk; I think 'event->chunk->head_skb !=3D 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/[email protected]/ 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, st= ruct sock *newsk, > if (event->asoc =3D=3D 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, st= ruct sock *newsk, > if (event->asoc =3D=3D 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, st= ruct 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