<!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;">
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</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>
#pfptBannerfkk2jfb { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important;
background-color: #c2d4d4 !important;
max-width: none !important; max-height: none !important }
.pfptPrimaryButtonfkk2jfb:hover, .pfptPrimaryButtonfkk2jfb:focus {
background-color: #a2b1b1 !important; }
.pfptPrimaryButtonfkk2jfb: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">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
</pre></body></html>