Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH] rds: synchronize info callbacks with connection teardown
@ 2026-07-20 18:49 Chengfeng Ye
  2026-07-23  4:20 ` Allison Henderson
  2026-08-13 17:20 ` kernel test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-07-20 18:49 UTC (permalink / raw)
  To: Allison Henderson, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Santosh Shilimkar, Ka-Cheong Poon,
	Dan Carpenter
  Cc: netdev, linux-rdma, rds-devel, linux-kernel, Chengfeng Ye, stable

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.

Fixes: 7d0a06586b26 ("net/rds: Fix info leak in rds6_inc_info_copy()")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 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
@@ -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
@@ -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;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] rds: synchronize info callbacks with connection teardown
  2026-07-20 18:49 [PATCH] rds: synchronize info callbacks with connection teardown Chengfeng Ye
@ 2026-07-23  4:20 ` Allison Henderson
  2026-07-23 16:00   ` Chengfeng Ye
  2026-08-13 17:20 ` kernel test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Allison Henderson @ 2026-07-23  4:20 UTC (permalink / raw)
  To: Chengfeng Ye, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Santosh Shilimkar, Ka-Cheong Poon,
	Dan Carpenter
  Cc: netdev, linux-rdma, rds-devel, linux-kernel, 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:


> 
> Fixes: 7d0a06586b26 ("net/rds: Fix info leak in rds6_inc_info_copy()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
>  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.


> @@ -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

> @@ -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;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rds: synchronize info callbacks with connection teardown
  2026-07-23  4:20 ` Allison Henderson
@ 2026-07-23 16:00   ` Chengfeng Ye
  0 siblings, 0 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-07-23 16:00 UTC (permalink / raw)
  To: Allison Henderson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Santosh Shilimkar, Ka-Cheong Poon, Dan Carpenter,
	netdev, linux-rdma, rds-devel, linux-kernel, stable

On Thu, Jul 23, 2026 at 12:20 PM Allison Henderson <achender@kernel.org> wrote:
>
> 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:
>
>
> >
> > Fixes: 7d0a06586b26 ("net/rds: Fix info leak in rds6_inc_info_copy()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> > ---
> >  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.
>
>
> > @@ -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
>
> > @@ -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;
>

Hi Allison,

Thank you for the detailed review and for confirming the bug.

I would prefer to leave the connection-reference port to you, since it
depends on the new refcounting infrastructure. Please feel free to carry
the fix in your series with:

Reported-by: Chengfeng Ye <nicoyip.dev@gmail.com>

I will resend the info.c changes against net-next.

Best regards,
Chengfeng

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] rds: synchronize info callbacks with connection teardown
  2026-07-20 18:49 [PATCH] rds: synchronize info callbacks with connection teardown Chengfeng Ye
  2026-07-23  4:20 ` Allison Henderson
@ 2026-08-13 17:20 ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-08-13 17:20 UTC (permalink / raw)
  To: Chengfeng Ye, Allison Henderson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Santosh Shilimkar,
	Ka-Cheong Poon, Dan Carpenter
  Cc: oe-kbuild-all, netdev, linux-rdma, rds-devel, linux-kernel,
	Chengfeng Ye, stable

Hi Chengfeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]
[also build test WARNING on linus/master horms-ipvs/master v7.2-rc7]
[cannot apply to net-next/main next-20260812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chengfeng-Ye/rds-synchronize-info-callbacks-with-connection-teardown/20260812-225931
base:   net/main
patch link:    https://lore.kernel.org/r/20260720184955.3008978-1-nicoyip.dev%40gmail.com
patch subject: [PATCH] rds: synchronize info callbacks with connection teardown
config: mips-randconfig-r134-20260813 (https://download.01.org/0day-ci/archive/20260814/202608140151.MOvRf1jc-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.5.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260814/202608140151.MOvRf1jc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608140151.MOvRf1jc-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> net/rds/info.c:83:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/rds/info.c:83:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/rds/info.c:83:9: sparse:    void ( [noderef] __rcu * )( ... )
   net/rds/info.c:83:9: sparse:    void ( * )( ... )
   net/rds/info.c:84:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/rds/info.c:84:9: sparse:    void ( [noderef] __rcu * )( ... )
   net/rds/info.c:84:9: sparse:    void ( * )( ... )
   net/rds/info.c:96:9: sparse: sparse: cast removes address space '__rcu' of expression
   net/rds/info.c:96:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/rds/info.c:96:9: sparse:    void ( [noderef] __rcu * )( ... )
   net/rds/info.c:96:9: sparse:    void ( * )( ... )
   net/rds/info.c:97:9: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/rds/info.c:97:9: sparse:    void ( [noderef] __rcu * )( ... )
   net/rds/info.c:97:9: sparse:    void ( * )( ... )
   net/rds/info.c:222:16: sparse: sparse: cast removes address space '__rcu' of expression
   net/rds/info.c:222:16: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/rds/info.c:222:16: sparse:    void ( [noderef] __rcu * )( ... )
   net/rds/info.c:222:16: sparse:    void ( * )( ... )

vim +/__rcu +83 net/rds/info.c

    75	
    76	void rds_info_register_func(int optname, rds_info_func func)
    77	{
    78		int offset = optname - RDS_INFO_FIRST;
    79	
    80		BUG_ON(optname < RDS_INFO_FIRST || optname > RDS_INFO_LAST);
    81	
    82		spin_lock(&rds_info_lock);
  > 83		BUG_ON(rcu_access_pointer(rds_info_funcs[offset]));
    84		rcu_assign_pointer(rds_info_funcs[offset], func);
    85		spin_unlock(&rds_info_lock);
    86	}
    87	EXPORT_SYMBOL_GPL(rds_info_register_func);
    88	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-13 17:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 18:49 [PATCH] rds: synchronize info callbacks with connection teardown Chengfeng Ye
2026-07-23  4:20 ` Allison Henderson
2026-07-23 16:00   ` Chengfeng Ye
2026-08-13 17:20 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox