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

NeilBrown <[email protected]>
Newsgroups gmane.linux.nfs,gmane.linux.kernel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.