Re: [PATCH net v2 1/2] sctp: avoid auth_enable sysctl UAF during netns teardown
Xin Long <[email protected]> Mon, 13 Jul 2026 12:31:04 -0400
| Newsgroups | org.kernel.vger.linux-sctp,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CADvbK_fCkj-WGok_yV8kGD9w-H=UWCG2TcRqSn5+wEhoAsTohw@mail.gmail.com> |
On Sat, Jul 11, 2026 at 12:22=E2=80=AFAM Ren Wei <[email protected]> wrote: > > From: Zhiling Zou <[email protected]> > > proc_sctp_do_auth() updates the SCTP control socket after changing > net.sctp.auth_enable. The handler gets the per-net SCTP state from > ctl->data, so an already opened sysctl file can still target a network > namespace while that namespace is being torn down. > > SCTP previously registered its per-net sysctls from sctp_defaults_init(), > while the control socket is created later from sctp_ctrlsock_init(). This > exposed a window during initialization where auth_enable was writable > before net->sctp.ctl_sock existed, and a teardown window where auth_enabl= e > stayed writable after inet_ctl_sock_destroy() had released the control > socket. > > Move the per-net SCTP sysctl registration into sctp_ctrlsock_init() after > sctp_ctl_sock_init() succeeds, and unregister the sysctl table before > destroying the control socket in sctp_ctrlsock_exit(). If sysctl > registration fails after the control socket was created, destroy the > control socket in the same init path. > > Make sctp_sysctl_net_unregister() tolerate a missing header and clear the > saved pointer so init-error and exit paths can safely share the unregiste= r > helper. > > Fixes: 15649fd5415e ("sctp: sysctl: auth_enable: avoid using current->nsp= roxy") > Cc: [email protected] > Reported-by: Yuan Tan <[email protected]> > Reported-by: Yifan Wu <[email protected]> > Reported-by: Juefei Pu <[email protected]> > Reported-by: Xin Liu <[email protected]> > Co-developed-by: Qi Tang <[email protected]> > Signed-off-by: Qi Tang <[email protected]> > Signed-off-by: Zhiling Zou <[email protected]> > Signed-off-by: Ren Wei <[email protected]> > --- > net/sctp/protocol.c | 17 ++++++++++------- > net/sctp/sysctl.c | 9 +++++++-- > 2 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index 587b0017a67d..f5fe6ddf0d7d 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -1382,10 +1382,6 @@ static int __net_init sctp_defaults_init(struct ne= t *net) > net->sctp.l3mdev_accept =3D 1; > #endif > > - status =3D sctp_sysctl_net_register(net); > - if (status) > - goto err_sysctl_register; > - > /* Allocate and initialise sctp mibs. */ > status =3D init_sctp_mibs(net); > if (status) > @@ -1419,8 +1415,6 @@ static int __net_init sctp_defaults_init(struct net= *net) > cleanup_sctp_mibs(net); > #endif > err_init_mibs: > - sctp_sysctl_net_unregister(net); > -err_sysctl_register: > return status; > } > > @@ -1435,7 +1429,6 @@ static void __net_exit sctp_defaults_exit(struct ne= t *net) > net->sctp.proc_net_sctp =3D NULL; > #endif > cleanup_sctp_mibs(net); > - sctp_sysctl_net_unregister(net); > } > > static struct pernet_operations sctp_defaults_ops =3D { > @@ -1451,14 +1444,24 @@ static int __net_init sctp_ctrlsock_init(struct n= et *net) > status =3D sctp_ctl_sock_init(net); > if (status) > pr_err("Failed to initialize the SCTP control sock\n"); > + else > + status =3D sctp_sysctl_net_register(net); > + > + if (status && net->sctp.ctl_sock) { > + inet_ctl_sock_destroy(net->sctp.ctl_sock); > + net->sctp.ctl_sock =3D NULL; > + } I think the Linux style here should be: /* Initialize the control inode/socket for handling OOTB packets. = */ status =3D sctp_ctl_sock_init(net); if (status) { pr_err("Failed to initialize the SCTP control sock\n"); return status; } status =3D sctp_sysctl_net_register(net); if (status) { inet_ctl_sock_destroy(net->sctp.ctl_sock); net->sctp.ctl_sock =3D NULL; } Thanks. > > return status; > } > > static void __net_exit sctp_ctrlsock_exit(struct net *net) > { > + sctp_sysctl_net_unregister(net); > + > /* Free the control endpoint. */ > inet_ctl_sock_destroy(net->sctp.ctl_sock); > + net->sctp.ctl_sock =3D NULL; > } > > static struct pernet_operations sctp_ctrlsock_ops =3D { > diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c > index 15e7db9a3ab2..fca840484ebf 100644 > --- a/net/sctp/sysctl.c > +++ b/net/sctp/sysctl.c > @@ -615,11 +615,16 @@ int sctp_sysctl_net_register(struct net *net) > > void sctp_sysctl_net_unregister(struct net *net) > { > + struct ctl_table_header *header =3D net->sctp.sysctl_header; > const struct ctl_table *table; > > - table =3D net->sctp.sysctl_header->ctl_table_arg; > - unregister_net_sysctl_table(net->sctp.sysctl_header); > + if (!header) > + return; > + > + table =3D header->ctl_table_arg; > + unregister_net_sysctl_table(header); > kfree(table); > + net->sctp.sysctl_header =3D NULL; > } > > static struct ctl_table_header *sctp_sysctl_header; > -- > 2.43.0 >