<!DOCTYPE html>
<!-- BaNnErBlUrFlE-BoDy-start -->
<!-- Preheader Text : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;">
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: </div>
<!-- Preheader Text : END -->
<!-- Email Banner : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;"></div>
<!-- Email Banner : END -->
<!-- BaNnErBlUrFlE-BoDy-end -->
<html>
<head><!-- BaNnErBlUrFlE-HeAdEr-start -->
<style>
#pfptBanners7bud40 { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important;
background-color: #c2d4d4 !important;
max-width: none !important; max-height: none !important }
.pfptPrimaryButtons7bud40:hover, .pfptPrimaryButtons7bud40:focus {
background-color: #a2b1b1 !important; }
.pfptPrimaryButtons7bud40:active {
background-color: #828e8e !important; }
html:root, html:root>body { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important; }
</style>
<!-- BaNnErBlUrFlE-HeAdEr-end -->
</head><body><pre style="font-family: sans-serif; font-size: 100%; white-space: pre-wrap; word-wrap: break-word">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
</pre></body></html>