Re: [PATCH 5/4] sunrpc: protect the svc_pool_map pool_to[] array with RCU

[NeilBrown]

On Tue, 30 Jun 2026, Jeff Layton wrote:
> svc_pool_map_get_node() reads the global svc_pool_map without holding
> svc_pool_map_mutex and dereferences m->pool_to[]. The array was both
> published and torn down without any synchronisation against that
> lockless reader:
> 
>  - svc_pool_map_get() incremented m->count to one before
>    svc_pool_map_init_pernode() allocated and filled the arrays, so a
>    reader observing the map as "in use" could see a NULL (or partially
>    built) pool_to[] and oops.
> 
>  - svc_pool_map_put() freed the arrays as soon as the last reference
>    went away, so a reader that had already started dereferencing
>    pool_to[] could use it after free.
> 
> svc_new_thread() takes this lockless path for every service, including
> unpooled ones that hold no map reference, so the reader genuinely can
> run concurrently with another service's startup or shutdown.
> 
> Publish pool_to[] with rcu_assign_pointer() only after it is fully
> built in a private allocation, and have svc_pool_map_get_node()
> dereference it under rcu_read_lock(). On teardown, clear the pointer
> and defer the free past a grace period with kfree_rcu_mightsleep().
> 
> svc_pool_map_set_cpumask() also reads pool_to[], but its caller holds a
> map reference (it checks sv_nrpools > 1) so the array is stable; it uses
> rcu_dereference_protected() rather than taking the read lock.
> 
> to_pool[] needs no such treatment: it is only read by services that
> hold a map reference, so it cannot be freed under a reader.

I don't think this is the best approach.
The problem only occurs when svc_create() is used as when
svc_create_pooled() is used, svc_pool_map_get() is called early so a
reference is held to svc_pool_map.

svc_new_thread() is the only caller of svc_pool_map_get_node(), and it
can easily check pool->sv_is_pooled and only call
svc_pool_map_get_node() is sv_is_pooled is true.  When false it should
probably use NUMA_NO_NODE.
e.g.

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index ae9ec4bf34f7..063826702f46 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -800,7 +800,10 @@ int svc_new_thread(struct svc_serv *serv, struct svc_pool *pool)
 	int node;
 	int err = 0;
 
-	node = svc_pool_map_get_node(pool->sp_id);
+	if (serv->sv_is_pooled)
+		node = svc_pool_map_get_node(pool->sp_id);
+	else
+		node = NUMA_NO_NODE;
 
 	rqstp = svc_prepare_thread(serv, pool, node);
 	if (!rqstp)


NeilBrown