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
>