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

[Allison Henderson <achender@kernel.org>]

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) <blbllhy@gmail.com>

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
> ---
>  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 = {