Re: [PATCH net 1/1] sctp: diag: reject stale associations in dump_one path
Xin Long <[email protected]> Mon, 1 Jun 2026 13:43:34 -0400
| Newsgroups | org.kernel.vger.linux-sctp,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CADvbK_cXhGqgEua975KuP+Vu81s7ZjYjE=kTiMrwxcBKLdb0VA@mail.gmail.com> |
On Sat, May 30, 2026 at 11:57 AM Ren Wei <[email protected]> wrote: > > From: Zhao Zhang <[email protected]> > > The SCTP exact sock_diag lookup can hold a transport reference, block on > lock_sock(sk), and then resume after sctp_association_free() has marked > the association dead and freed its bind address list. > > When that happens, inet_assoc_attr_size() and > inet_diag_msg_sctpasoc_fill() can still dereference association state > that is no longer valid for reporting. In particular, > inet_diag_msg_sctpasoc_fill() may read an empty bind-address list as a > real sctp_sockaddr_entry and trigger an out-of-bounds read from > unrelated association memory. > > Reject the association after taking the socket lock if it has been > reaped or detached from the endpoint, and report the lookup as stale. > This keeps the exact dump-one path from formatting torn association > state. > > Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file") > Cc: [email protected] > Reported-by: Yuan Tan <[email protected]> > Reported-by: Yifan Wu <[email protected]> > Reported-by: Juefei Pu <[email protected]> > Reported-by: Zhengchuan Liang <[email protected]> > Reported-by: Xin Liu <[email protected]> > Assisted-by: Codex:GPT-5.4 > Signed-off-by: Zhao Zhang <[email protected]> > Signed-off-by: Ren Wei <[email protected]> > --- > net/sctp/diag.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/net/sctp/diag.c b/net/sctp/diag.c > index 2afb376299fe..d758f5c3e06e 100644 > --- a/net/sctp/diag.c > +++ b/net/sctp/diag.c > @@ -266,15 +266,15 @@ static int sctp_sock_dump_one(struct sctp_endpoint *ep, struct sctp_transport *t > > lock_sock(sk); > > - rep = nlmsg_new(inet_assoc_attr_size(sk, assoc), GFP_KERNEL); > - if (!rep) { > - release_sock(sk); > - return -ENOMEM; > + if (ep != assoc->ep || assoc->base.dead) { > + err = -ESTALE; > + goto out_unlock; > } > > - if (ep != assoc->ep) { > - err = -EAGAIN; > - goto out; > + rep = nlmsg_new(inet_assoc_attr_size(sk, assoc), GFP_KERNEL); > + if (!rep) { > + err = -ENOMEM; > + goto out_unlock; > } > > err = inet_sctp_diag_fill(sk, assoc, rep, req, sk_user_ns(NETLINK_CB(skb).sk), > @@ -289,8 +289,9 @@ static int sctp_sock_dump_one(struct sctp_endpoint *ep, struct sctp_transport *t > return nlmsg_unicast(sock_net(skb->sk)->diag_nlsk, rep, NETLINK_CB(skb).portid); > > out: > - release_sock(sk); > kfree_skb(rep); > +out_unlock: > + release_sock(sk); > return err; > } > > -- > 2.47.3 > Thanks for the fix. Acked-by: Xin Long <[email protected]> Note that the issue reported in https://sashiko.dev/#/patchset/fac6043fa20a2ff68e12958c431836f692c51268.1780113823.git.zzhan461%40ucr.edu. I don't think it exists, as sctp_sock_filter() is called via sctp_transport_traverse_process() where sctp_transport_get_next() only returns primary_path's transport, not each transport.