Re: [PATCH net-next v2] rds: synchronize info callbacks with module unload

From: Allison Henderson

Date: Thu Jul 30 2026 - 14:28:35 EST


On Thu, 2026-07-30 at 11:55 +0200, Paolo Abeni wrote:
> On 7/27/26 7:48 PM, Chengfeng Ye wrote:
> > rds_info_getsockopt() reads a callback from rds_info_funcs and invokes it
> > without protecting the callback's lifetime. Transport modules register
> > functions stored in this array. For example, rds_tcp.ko registers
> > rds_tcp_tc_info() for RDS_INFO_TCP_SOCKETS.
> >
> > This permits the following interleaving:
> >
> > CPU0 CPU1
> > rds_info_getsockopt()
> > func = rds_tcp_tc_info
> > rmmod rds_tcp
> > rds_tcp_exit()
> > rds_info_deregister_func()
> > rds_info_funcs[offset] = NULL
> > free rds_tcp module text
> > func()
> >
> > The reader can therefore branch to an address in unloaded module text.
> >
> > Protect callback invocation with SRCU. Enter the SRCU read-side critical
> > section before loading the callback and leave it only after the callback
> > returns. Clear the callback with release semantics and call
> > synchronize_srcu() before deregistration returns, preventing module unload
> > from freeing its text while an old reader is still executing it. SRCU is
> > required because callbacks such as RDS_INFO_COUNTERS can sleep.
> >
> > Keep the callback array unannotated and use acquire and release operations
> > for publication so sparse does not have to apply __rcu through the
> > function-pointer typedef. Replace the two callback-slot BUG_ON() checks
> > with WARN_ON_ONCE() and return without changing the slot on mismatch.
> >
> > Fixes: 70041088e3b9 ("RDS: Add TCP transport to RDS")
> > Link: https://lore.kernel.org/netdev/20260720184955.3008978-1-nicoyip.dev@xxxxxxxxx/
> > Suggested-by: Allison Henderson <achender@xxxxxxxxxx>
> > Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> @Alison: I understand you prefer this on net-next, and I'm fine with
> such target, but the current guidance for net-next patches is no fixes
> tag unless the buggy code is on net-next only.
>
> I'm going to strip the fixes tag while applying this one.
>
> Thanks,
>
> Paolo
>
I see, sure that would be fine. Thank you!

Allison