Re: [PATCH net 1/1] sctp: avoid auth_enable sysctl UAF during netns teardown

On Mon, Jun 29, 2026 at 10:23 PM Xin Long <[email protected]> wrote:
>
> On Mon, Jun 29, 2026 at 10:04 AM Xin Long <[email protected]> wrote:
> >
> > On Sun, Jun 28, 2026 at 4:40 AM 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 unregisters its per-net sysctls from sctp_defaults_exit(), but
> > > sctp_ctrlsock_exit() runs earlier because the control-socket pernet ops
> > > are registered after the defaults ops.  This leaves a teardown window
> > > where auth_enable is still writable after inet_ctl_sock_destroy() has
> > > released net->sctp.ctl_sock, leading to a use-after-free when the sysctl
> > > handler locks and dereferences the stale socket.
> > >
> > > Unregister the per-net SCTP sysctl table before destroying the control
> > > socket.  Make sctp_sysctl_net_unregister() tolerate a missing header and
> > > clear the saved pointer so the later defaults exit path and init-error
> > > path can safely share the same unregister helper.
> > >
> > > Fixes: 15649fd5415e ("sctp: sysctl: auth_enable: avoid using current->nsproxy")
> > > 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]>
> > > Assisted-by: Codex:gpt-5.4
> > > Signed-off-by: Zhiling Zou <[email protected]>
> > > Signed-off-by: Ren Wei <[email protected]>
> > > ---
> > >  net/sctp/protocol.c | 3 +++
> > >  net/sctp/sysctl.c   | 9 +++++++--
> > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> > > index 587b0017a67d..ae381d304bd5 100644
> > > --- a/net/sctp/protocol.c
> > > +++ b/net/sctp/protocol.c
> > > @@ -1457,8 +1457,11 @@ static int __net_init sctp_ctrlsock_init(struct net *net)
> > >
> > >  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 = NULL;
> > >  }
> > >
> > >  static struct pernet_operations sctp_ctrlsock_ops = {
> > > 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 = net->sctp.sysctl_header;
> > >         const struct ctl_table *table;
> > >
> > > -       table = net->sctp.sysctl_header->ctl_table_arg;
> > > -       unregister_net_sysctl_table(net->sctp.sysctl_header);
> > > +       if (!header)
> > > +               return;
> > > +
> > > +       table = header->ctl_table_arg;
> > > +       unregister_net_sysctl_table(header);
> > >         kfree(table);
> > > +       net->sctp.sysctl_header = NULL;
> > >  }
> > >
> > >  static struct ctl_table_header *sctp_sysctl_header;
> > > --
> > > 2.43.0
> > >
> >
> > Please also move sctp_sysctl_net_register() to sctp_ctrlsock_init(), and call
> > it AFTER sctp_ctl_sock_init().
> >
> > This is not just for being symmetric, but also fixes two problems:
> >
> > 1. A regression caused by this patch:
> >
> > If sctp_v4_protosw_init() or sctp_v6_protosw_init() fails in sctp_init(),
> > there's no place to call sctp_sysctl_net_unregister() on the err path.
> >
> > 2. A pre-existing issue reported by sashiko-gemini:
> >
> > > diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> > > index 15e7db9a3ab2e..fca840484ebf7 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 = net->sctp.sysctl_header;
> > > const struct ctl_table *table;
> > This is a pre-existing issue, but I noticed a potential race condition
> > during SCTP module initialization related to the sysctls modified here.
> > During sctp_init(), sctp_defaults_ops registers the sysctls globally before
> > sctp_ctrlsock_ops allocates net->sctp.ctl_sock:
> > sctp_init() {
> > ...
> > status = register_pernet_subsys(&sctp_defaults_ops);
> > if (status)
> > goto err_register_defaults;
> > ...
> > status = register_pernet_subsys(&sctp_ctrlsock_ops);
> > ...
> > }
> > If userspace accesses the sysctls in this window, proc_sctp_do_auth() could
> > dereference a NULL pointer since it assumes ctl_sock is ready:
> > proc_sctp_do_auth() {
> > ...
> > struct sock *sk = net->sctp.ctl_sock;
> > net->sctp.auth_enable = new_value;
> > /* Update the value in the control socket */
> > lock_sock(sk);
> > ...
> > }
> > Can we hit a kernel panic here if the sysctl is modified during automatic
> > module loading?
> > [...]
> >
>
> Also, if you don't mind, please try to address another issue reported
> in sashiko-gemini:
>
> > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> > index 587b0017a67d5..ae381d304bd53 100644
> > --- a/net/sctp/protocol.c
> > +++ b/net/sctp/protocol.c
> > @@ -1457,8 +1457,11 @@ static int __net_init sctp_ctrlsock_init(struct net *net)
> >
> > static void __net_exit sctp_ctrlsock_exit(struct net *net)
> > {
> This isn't a bug introduced by this patch, but while reviewing the netns
> teardown sequence, I noticed the SCTP UDP tunnel sockets appear to leak.
> In sctp_defaults_exit():
> sctp_defaults_exit() {
> /* Free the local address list */
> sctp_free_addr_wq(net);
> sctp_free_local_addr_list(net);
> ...
> }
> Should sctp_defaults_exit() call sctp_udp_sock_stop(net) to ensure the
> UDP tunnel sockets are closed?
> If a user creates a network namespace, writes to the net.sctp.udp_port sysctl
> to allocate the sockets, and then destroys the namespace, could these sockets
> remain active and cause a use-after-free of struct net when packets arrive?
> [...]
>
> maybe by adding sctp_udp_sock_stop() in sctp_ctrlsock_exit(), and call it AFTER
> sctp_sysctl_net_unregister() in a separate patch.
>
> Thanks.

Thanks for the review.

I addressed both comments in v2. Patch 1 moves the per-net SCTP sysctl

registration after sctp_ctl_sock_init() and keeps the unregister before

destroying the control socket. Patch 2 separately stops the SCTP UDP tunnel

sockets after sysctl unregistration during netns teardown.

I will send the v2 series as a new threaded 0/2, 1/2, 2/2 patch series.

Thanks,

Zhiling