Thread (5 messages) 5 messages, 2 authors, 8d ago

Re: [PATCH net] rds: tcp: unregister sysctl before tearing down listen socket

From: Allison Henderson <achender@kernel.org>
Date: 2026-07-19 08:13:53
Also in: linux-rdma, lkml

On Sat, 2026-07-18 at 14:34 -0400, Cen Zhang (Microsoft) wrote:
rds_tcp_exit_net() frees the per-netns RDS TCP listen socket via
rds_tcp_kill_sock() before unregistering the per-netns sysctl table.  Since
rds_tcp_skbuf_handler() derives the netns from rtn->rds_tcp_listen_sock->sk,
a concurrent sysctl write can race with netns teardown and dereference the
freed socket/sk.
Hi Cen,

Thanks for working on this.  The race is real and the analysis is right.
Some comments below:
KASAN reports the race as:

  BUG: KASAN: slab-use-after-free in rds_tcp_skbuf_handler+0x2aa/0x2e0
  rds_tcp_skbuf_handler              net/rds/tcp.c:721
  proc_sys_call_handler              fs/proc/proc_sysctl.c
  vfs_write                          fs/read_write.c
  __x64_sys_pwrite64                 fs/read_write.c
Was this stack actually observed or was it derived from an analysis? If it was
derived that's fine but just note it somewhere so that someone doesnt end up
chasing a synthesized stack trace.  If you did actually hit it though, please
include the full report and a reproducer if you have it.
Fix this by unregistering the RDS TCP sysctl table before calling
rds_tcp_kill_sock().  unregister_net_sysctl_table() prevents new sysctl
handlers from starting and waits for in-flight handlers to finish, so
the listen socket can then be released safely.

Fixes: 7f5611cbc487 ("rds: sysctl: rds_tcp_{rcv,snd}buf: avoid using current->nsproxy")
Reported-by: AutonomousCodeSecurity@microsoft.com
Check patch generates a warning here for a Closes: or Link: tag.  
If the reporting tool you're using generates a public report, please include the link here
Signed-off-by: Cen Zhang (Microsoft) <redacted>
Other than that I think the fix is ok.  With the above fixed, you can add my rvb:
Reviewed-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison
quoted hunk ↗ jump to hunk
---
 net/rds/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index a1de114d5e2e..453d4077a85e 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -655,13 +655,13 @@ static void __net_exit rds_tcp_exit_net(struct net *net)
 {
 	struct rds_tcp_net *rtn = net_generic(net, rds_tcp_netid);
 
-	rds_tcp_kill_sock(net);
-
 	if (rtn->rds_tcp_sysctl)
 		unregister_net_sysctl_table(rtn->rds_tcp_sysctl);
 
 	if (net != &init_net)
 		kfree(rtn->ctl_table);
+
+	rds_tcp_kill_sock(net);
 }
 
 static struct pernet_operations rds_tcp_net_ops = {
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help