Re: [PATCH] rds: synchronize info callbacks with connection teardown
From: Allison Henderson <achender@kernel.org>
Date: 2026-07-23 04:20:42
Also in:
linux-rdma, lkml, stable
On Tue, 2026-07-21 at 02:49 +0800, Chengfeng Ye wrote:
rds_info_getsockopt() reads an info callback without holding
rds_info_lock across its invocation. At the same time, rds_exit()
destroys all connections before unregistering the socket info callbacks.
This permits the following interleaving:
CPU0 CPU1
rds_info_getsockopt()
func = rds6_sock_inc_info
rds_exit()
rds_conn_exit()
rds_conn_destroy()
kmem_cache_free(conn)
func()
rds6_inc_info_copy()
inc->i_conn->c_tos
The receive queue lock keeps the incoming message on its socket queue,
but it does not keep inc->i_conn alive. KASAN reports:
BUG: KASAN: slab-use-after-free in rds6_inc_info_copy+0x459/0x530 [rds]
Read of size 1 at addr ffff888106031c50 by task poc/101
Call Trace:
rds6_inc_info_copy+0x459/0x530 [rds]
rds6_sock_inc_info+0x2b9/0x3c0 [rds]
rds_info_getsockopt+0x19d/0x380 [rds]
do_sock_getsockopt+0x2ac/0x480
__sys_getsockopt+0x128/0x210
Allocated by task 99:
kmem_cache_alloc_noprof+0x11a/0x360
__rds_conn_create+0x7e/0xd60 [rds]
rds_conn_create_outgoing+0x61/0x80 [rds]
rds_sendmsg+0xb83/0x1d00 [rds]
Freed by task 102:
kmem_cache_free+0x1b5/0x3d0
rds_conn_destroy+0x484/0x600 [rds]
rds_loop_exit_net+0x32/0x50 [rds]
unregister_pernet_device+0x2c/0x50
rds_conn_exit+0x13/0xa0 [rds]
rds_exit+0x1a/0xc40 [rds]
__do_sys_delete_module+0x30a/0x4d0
Publish and clear callback pointers with SRCU, and keep the SRCU read-side
critical section across callback execution. SRCU permits callbacks such as
RDS_INFO_COUNTERS to sleep, unlike classic RCU. After clearing a slot,
synchronize_srcu() waits for every in-flight callback.
Move socket callback deregistration ahead of protocol and connection
teardown. This prevents callbacks from reaching a connection after it is
freed.Hi Chengfeng, Thanks for the report and the patch, this is a great start. The UAF analysis is correct, but there are a few bumps to smooth out before this can go in. First, the build bots caught some sparse errors that need to be addressed: https://netdev-ctrl.bots.linux.dev/logs/build/1131097/14698942/build_allmodconfig_warn/stderr Sparse can't apply the __rcu notation through a function-pointer typedef, so all of the rcu_*() macros trip over it. The simplest fix is to drop the __rcu annotation and just use smp_load_acquire/smp_store_release on a plain array in register/deregister. Also, checkpatch warns about the two BUG_ON()s you touched. We can clean those up by converting them to a WARN_ON_ONCE(). On to the fix itself: this patch has two separate changes going on that I think would be better done as two separate patches. Comments below:
quoted hunk ↗ jump to hunk
Fixes: 7d0a06586b26 ("net/rds: Fix info leak in rds6_inc_info_copy()") Cc: stable@vger.kernel.org Signed-off-by: Chengfeng Ye <redacted> --- net/rds/af_rds.c | 12 ++++++------ net/rds/info.c | 20 ++++++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-)diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c index d5defe9172e3..b3fe2bd6ea35 100644 --- a/net/rds/af_rds.c +++ b/net/rds/af_rds.c
First, the changes in af_rds.c changes don't quite work the way you might think. net/rds builds as three separate modules: rds.ko, rds_tcp.ko and rds_rdma.ko. rds_exit() only runs for "rmmod rds". rds_tcp_exit() only runs for "rmmod rds_tcp". "rmmod rds_tcp" deregisters RDS_INFO_TCP_SOCKETS, and then destroys and frees all the TCP connections. The rds module stays loaded with RDS_INFO_RECV_MESSAGES still registered, so "rmmod rds_tcp" still reproduces the splat since rds_exit() never runs in that case. Ideally what we want here is a ref counting infrastructure that isn't upstream yet. The good news is, I've been working on one, so I can push a tag for you to base a revised fix on. https://github.com/allisonhenderson/rds_work/releases/tag/rds-conn-kref-base-v1 The branch may still see some review churn, but I'll leave this tag here for you to base your fix on. On top of it, the fix is just to make every holder of inc->i_conn own a reference. Really what we're doing here is porting a small UEK patch: https://github.com/oracle/linux-uek/commit/99b9a3715419 Upstream's assignment sites differ a little from UEK's, but basically the idea is to use rds_conn_get() everywhere i_conn is assigned. This can target net-next since every remaining trigger needs CAP_SYS_MODULE or physical device removal. So it isn't reachable by an unprivileged attacker. Once you've worked out the patch, I'll carry it with yourself as the author as part of that series so it lands in the right order. Alternately if you'd rather not take on a port, that's fine too, just let me know and I would tag you for the Reported-by.
quoted hunk ↗ jump to hunk
@@ -925,6 +925,12 @@ static void rds6_sock_info(struct socket *sock, unsigned int len, static void rds_exit(void) { sock_unregister(rds_family_ops.family); + rds_info_deregister_func(RDS_INFO_SOCKETS, rds_sock_info); + rds_info_deregister_func(RDS_INFO_RECV_MESSAGES, rds_sock_inc_info); +#if IS_ENABLED(CONFIG_IPV6) + rds_info_deregister_func(RDS6_INFO_SOCKETS, rds6_sock_info); + rds_info_deregister_func(RDS6_INFO_RECV_MESSAGES, rds6_sock_inc_info); +#endif proto_unregister(&rds_proto); rds_conn_exit(); rds_cong_exit();@@ -933,12 +939,6 @@ static void rds_exit(void) rds_stats_exit(); rds_page_exit(); rds_bind_lock_destroy(); - rds_info_deregister_func(RDS_INFO_SOCKETS, rds_sock_info); - rds_info_deregister_func(RDS_INFO_RECV_MESSAGES, rds_sock_inc_info); -#if IS_ENABLED(CONFIG_IPV6) - rds_info_deregister_func(RDS6_INFO_SOCKETS, rds6_sock_info); - rds_info_deregister_func(RDS6_INFO_RECV_MESSAGES, rds6_sock_inc_info); -#endif } module_exit(rds_exit);diff --git a/net/rds/info.c b/net/rds/info.c index 21b32eb16559..7d6f3552d65b 100644 --- a/net/rds/info.c +++ b/net/rds/info.c
These SRCU conversions in info.c are actually fixing a completely separate bug independent of the connections. A getsockopt(RDS_INFO_TCP_SOCKETS) racing against a "rmmod rds_tcp" could call the callback pointer of a freed module. Which this patch fixes. So please resend the info.c changes against net-next, with the build fixes, and a commit message describing that race. I think that about covers it. Let me know if you get stuck or have questions Thanks again, this was a good find. Allison
quoted hunk ↗ jump to hunk
@@ -32,6 +32,7 @@ */ #include <linux/percpu.h> #include <linux/seq_file.h> +#include <linux/srcu.h> #include <linux/slab.h> #include <linux/proc_fs.h> #include <linux/export.h>@@ -68,8 +69,9 @@ struct rds_info_iterator { unsigned long offset; }; +DEFINE_STATIC_SRCU(rds_info_srcu); static DEFINE_SPINLOCK(rds_info_lock); -static rds_info_func rds_info_funcs[RDS_INFO_LAST - RDS_INFO_FIRST + 1]; +static rds_info_func __rcu rds_info_funcs[RDS_INFO_LAST - RDS_INFO_FIRST + 1]; void rds_info_register_func(int optname, rds_info_func func) {@@ -78,8 +80,8 @@ void rds_info_register_func(int optname, rds_info_func func) BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST); spin_lock(&rds_info_lock); - BUG_ON(rds_info_funcs[offset]); - rds_info_funcs[offset] = func; + BUG_ON(rcu_access_pointer(rds_info_funcs[offset])); + rcu_assign_pointer(rds_info_funcs[offset], func); spin_unlock(&rds_info_lock); } EXPORT_SYMBOL_GPL(rds_info_register_func);@@ -91,9 +93,10 @@ void rds_info_deregister_func(int optname, rds_info_func func) BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST); spin_lock(&rds_info_lock); - BUG_ON(rds_info_funcs[offset] != func); - rds_info_funcs[offset] = NULL; + BUG_ON(rcu_access_pointer(rds_info_funcs[offset]) != func); + RCU_INIT_POINTER(rds_info_funcs[offset], NULL); spin_unlock(&rds_info_lock); + synchronize_srcu(&rds_info_srcu); } EXPORT_SYMBOL_GPL(rds_info_deregister_func);@@ -165,6 +168,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, sockopt_t *opt) int npages = 0; int ret; int len; + int srcu_idx; int total; len = opt->optlen;@@ -214,8 +218,11 @@ int rds_info_getsockopt(struct socket *sock, int optname, sockopt_t *opt) rdsdebug("len %d nr_pages %lu\n", len, nr_pages); call_func: - func = rds_info_funcs[optname - RDS_INFO_FIRST]; + srcu_idx = srcu_read_lock(&rds_info_srcu); + func = srcu_dereference(rds_info_funcs[optname - RDS_INFO_FIRST], + &rds_info_srcu); if (!func) { + srcu_read_unlock(&rds_info_srcu, srcu_idx); ret = -ENOPROTOOPT; goto out; }@@ -225,6 +232,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, sockopt_t *opt) iter.offset = offset0; func(sock, len, &iter, &lens); + srcu_read_unlock(&rds_info_srcu, srcu_idx); BUG_ON(lens.each == 0); total = lens.nr * lens.each;