Skip to contentSkip to conversation

sctp: avoid auth_enable sysctl UAF during netns teardown

PATCH Superseded Ren Wei 3 participants 4 messages Jun 28 – Jun 29 1 message not in the archive

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ren Wei

author
#1 lore raw

PATCH 1/1 sctp: avoid auth_enable sysctl UAF during netns teardown

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")
Assisted-by Codex:gpt-5.4
---
 net/sctp/protocol.c | 3 +++
 net/sctp/sysctl.c   | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
net/sctp/protocol.c +3 -0
index 587b0017a67d..ae381d304bd5 100644
@@ -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 = {
net/sctp/sysctl.c +7 -2
index 15e7db9a3ab2..fca840484ebf 100644
@@ -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;
Signature · 1 line
2.43.0
maintainer
#2 lore raw
Quoted 74 lines · Ren Wei

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")
Assisted-by Codex:gpt-5.4
---
 net/sctp/protocol.c | 3 +++
 net/sctp/sysctl.c   | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
net/sctp/protocol.c +3 -0
index 587b0017a67d..ae381d304bd5 100644
@@ -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 = {
net/sctp/sysctl.c +7 -3
index 15e7db9a3ab2..fca840484ebf 100644
@@ -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;
--
Quoted 2 lines

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:

net/sctp/sysctl.c int sctp_sysctl_net_register(struct net *net) · line 619

Quoted 10 lines
net/sctp/sysctl.c +1 -0
index 15e7db9a3ab2e..fca840484ebf7 100644
@@ -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?
[...]

Thanks.

maintainer
#3 lore raw
Quoted 119 lines · Xin Long
Quoted 74 lines · Ren Wei

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")
Assisted-by Codex:gpt-5.4
---
 net/sctp/protocol.c | 3 +++
 net/sctp/sysctl.c   | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
net/sctp/protocol.c +3 -0
index 587b0017a67d..ae381d304bd5 100644
@@ -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 = {
net/sctp/sysctl.c +7 -3
index 15e7db9a3ab2..fca840484ebf 100644
@@ -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;
--
Quoted 2 lines

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:

net/sctp/sysctl.c int sctp_sysctl_net_register(struct net *net) · line 619

Quoted 10 lines
net/sctp/sysctl.c +1 -0
index 15e7db9a3ab2e..fca840484ebf7 100644
@@ -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;

Quoted 9 lines

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:

net/sctp/protocol.c static int __net_init sctp_ctrlsock_init(struct net *net) · line 1459

Quoted 8 lines
net/sctp/protocol.c +0 -0
index 587b0017a67d5..ae381d304bd53 100644
@@ -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.

#4 lore raw
Quoted 158 lines · Xin Long
Quoted 119 lines · Xin Long
Quoted 74 lines · Ren Wei

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")
Assisted-by Codex:gpt-5.4
---
 net/sctp/protocol.c | 3 +++
 net/sctp/sysctl.c   | 9 +++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
net/sctp/protocol.c +3 -0
index 587b0017a67d..ae381d304bd5 100644
@@ -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 = {
net/sctp/sysctl.c +7 -3
index 15e7db9a3ab2..fca840484ebf 100644
@@ -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;
--
Quoted 2 lines

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:

net/sctp/sysctl.c int sctp_sysctl_net_register(struct net *net) · line 619

Quoted 10 lines
net/sctp/sysctl.c +1 -0
index 15e7db9a3ab2e..fca840484ebf7 100644
@@ -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;

Quoted 9 lines

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:

net/sctp/protocol.c static int __net_init sctp_ctrlsock_init(struct net *net) · line 1459

Quoted 8 lines
net/sctp/protocol.c +0 -0
index 587b0017a67d5..ae381d304bd53 100644
@@ -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? [...]

Quoted 4 lines

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