[rds-devel] [External] : Re: rds_wq UAF

[Allison Henderson]

On Tue, 2026-10-20 at 15:09 +0200, Oleg Nesterov wrote:
> (change subject)
> 
> On 10/19, Allison Henderson wrote:
> > 
> > On Mon, 2026-10-19 at 17:35 +0200, Oleg Nesterov wrote:
> > > > 
> > > > But. rds_init() can also fail even if CONFIG_SYSCTL=y, after
> > > > rds_sysctl_init(), so to me this looks like a bug in net/rds/.
> > > > 
> > 
> > Yes that's a separate bug, so I'll send a separate fix for that.
> 
> Let me try to summarize.
> 
> If rds_init() fails after rds_threads_init() for any reason,
> rds_threads_exit() will destroy/free rds_wq.
> 
> If rds_rdma_init() is called after that, and rds_ib_init() fails
> after rds_ib_mr_init(), rds_ib_unregister_client() will try to
> flush the already freed rds_wq.
> 
> But we have
> 
> 	module_init(rds_init);		// net/rds/af_rds.c
> 	module_init(rds_rdma_init);	// net/rds/rdma_transport.c
> 
> and IIUC the order of these initcalls is not defined. So if
> rds_rdma_init() -> rds_ib_init() is called before rds_init(), it
> can do flush_workqueue(rds_wq) when rds_wq == NULL on failure.
It is sort of defined.  When built as modules the order is enforced by
symbol dependencies. rds_rdma and rds_tcp need symbols exported by rds
(rds_wq itself is one of them), so they can't be loaded unless rds_init()
succeeded.  So the rds_wq == NULL case can't happen.

What can happen is what was reported: built in, rds_init() fails,
do_initcalls() keeps going, and rds_rdma_init()/rds_tcp_init() run against
a core that has already been torn down.


> 
> With or without CONFIG_SYSCTL. Right?
> 
> But it seems that rds_ib_init() doesn't need to flush rds_wq on failure,
> so how about the patch below? Of course, I have no idea if net/rds has
> more similar issues with rds_wq.
> 
> Oleg.
> ---
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 786f39169bc1..cde2d1bfabbd 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -619,7 +619,7 @@ int rds_ib_init(void)
>  out_sysctl:
>  	rds_ib_sysctl_exit();
>  out_ibreg:
> -	rds_ib_unregister_client();
> +	ib_unregister_client(&rds_ib_client);

This works for when there are no IB devices, as in the report, but the flush
is what ensures rds_ib_dev_free() has finished before rds_rdma.ko's text goes away.  

If there is a device,  ib_register_client() has already called rds_ib_add_one().
Then ib_unregister_client() calls rds_ib_remove_one(), which drops the last
reference.  And then rds_ib_dev_put() does queue_work(rds_wq, ...) on the
freed workqueue anyway.  

So I think the fix belongs at the transport init: if the core didn't
come up, rds_rdma_init() and rds_tcp_init() should return early,
before they register anything.  I can send that along with the
sysctl change tomorrow.

Thanks for working on this!
Allison



>  out_mr_exit:
>  	rds_ib_mr_exit();
>  out:
>