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.