[PATCH] rds: synchronize info callbacks with connection teardown

[PATCH] rds: synchronize info callbacks with connection teardown

[Chengfeng Ye]

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