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

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/[email protected]/
> Suggested-by: Allison Henderson <[email protected]>
> Signed-off-by: Chengfeng Ye <[email protected]>
@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