[PATCH v2] nfsd: pin the module while a server exists

[Shuangpeng Bai]

Since commit 924f4fb003ba ("NFSD: convert write_threads to netlink
command") the server can be started over netlink without mounting the
nfsd filesystem. But that mount was the only thing pinning the module
while a server exists (commit f49169c97fce ("NFSD: Remove
svc_serv_ops::svo_module") removed the per-thread reference).

So when the server has been started via netlink, a plain "rmmod nfsd"
succeeds while the threads are still running. exit_nfsd() then frees
struct nfsd_net in unregister_pernet_subsys(), destroys the DRC slab
and finally unmaps the module text underneath the running threads:

  BUG: KASAN: slab-use-after-free in nfsd_file_net_dispose (fs/nfsd/filecache.c:483) nfsd
  Read of size 8 at addr ffff888109fe92d0 by task nfsd/300
  Call Trace:
   nfsd_file_net_dispose (fs/nfsd/filecache.c:483) nfsd
   nfsd (fs/nfsd/nfssvc.c:957) nfsd
   kthread (kernel/kthread.c:436)
   ret_from_fork (arch/x86/kernel/process.c:158)
  Freed by task 311:
   kfree (mm/slub.c:6802)
   ops_undo_list (net/core/net_namespace.c:257)
   unregister_pernet_operations (net/core/net_namespace.c:1412)
   unregister_pernet_subsys (net/core/net_namespace.c:1459)
   cleanup_module nfsd
   __se_sys_delete_module (kernel/module/main.c:804)

followed by a general protection fault in nfsd_cache_lookup() as soon
as a thread allocates from the destroyed drc_slab.

Take a module reference in nfsd_create_serv() and drop it in
nfsd_destroy_serv(), so that "rmmod nfsd" fails with EBUSY while a
server exists, just as it does while the nfsd filesystem is mounted.

Use try_module_get() rather than __module_get(): a netlink command
runs without a reference on the nfsd module, and exit_nfsd() only
waits for it in genl_unregister_family(). A THREADS_SET or
LISTENER_SET request that is already running when "rmmod nfsd" starts
would otherwise still create a server after module unload has begun.
With try_module_get() that request fails with ENODEV instead.

Pinning the module also keeps the assumption behind the
__module_get() calls in fs/locks.c (via lm_mod_owner) and in
nfs_common/nfslocalio.c valid: they rely on an active server keeping
the module alive.

To reproduce: start the server with nfsdctl (or any other netlink
client) without mounting the nfsd filesystem, then run "rmmod nfsd".

Fixes: 924f4fb003ba ("NFSD: convert write_threads to netlink command")
Cc: stable@xxxxxxxxxxxxxxx # v6.10+
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@xxxxxxxxx>
---
Changes since v1:
- Pin the module while a server exists instead of shutting the server
  down in nfsd_net_exit(). As the sashiko review pointed out, shutting
  it down from exit_nfsd() still lets every __module_get(THIS_MODULE)
  taken during module exit (fs/locks.c via lm_mod_owner, nfslocalio)
  outlive free_module(). With the reference held, exit_nfsd() never
  runs while a server exists, so the laundry_wq / cld notifier ordering
  changes from v1 are not needed either.
- Take the reference with try_module_get(). A netlink command holds no
  reference on nfsd, so with __module_get() a LISTENER_SET/THREADS_SET
  that was already running when "rmmod nfsd" started could still bring
  up a server, and its threads then ran on the freed module text.
- Not changed: an exiting nfsd thread still executes a few instructions
  of module code after svc_stop_kthreads() has been woken up. That
  window exists on the umount path as well and is independent of this
  fix.

v1: https://lore.kernel.org/linux-nfs/20261026235626.649844-1-shuangpeng.kernel@xxxxxxxxx/

Tested on top of 7.3-rc7 with KASAN: with the server started over
netlink, "rmmod nfsd" now fails with EBUSY, also while a request is in
flight and with only a listener configured; after setting the thread
count to 0 the module unloads cleanly; starting and stopping the server
through the nfsd filesystem is unchanged. With a delay injected at the
top of nfsd_create_serv() to hold a netlink start open across "rmmod
nfsd", the start now fails with ENODEV and the module unloads cleanly
(with __module_get() the threads came up and oopsed in freed module
text).
 fs/nfsd/nfssvc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 067543da3ca0..6ae040c50987 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -579,6 +579,7 @@ void nfsd_destroy_serv(struct net *net)
 	svc_xprt_destroy_all(serv, net, true);
 	nfsd_shutdown_net(net);
 	svc_destroy(&serv);
+	module_put(THIS_MODULE);
 }
 
 void nfsd_reset_versions(struct nfsd_net *nn)
@@ -662,10 +663,18 @@ int nfsd_create_serv(struct net *net, bool no_rpcbind)
 	if (nn->nfsd_serv)
 		return 0;
 
+	/*
+	 * Keep the module loaded while the server exists, as a mounted
+	 * nfsd filesystem does. A netlink caller holds no module
+	 * reference, so this fails if module unload has already begun.
+	 */
+	if (!try_module_get(THIS_MODULE))
+		return -ENODEV;
+
 	error = percpu_ref_init(&nn->nfsd_net_ref, nfsd_net_free,
 				0, GFP_KERNEL);
 	if (error)
-		return error;
+		goto out_put;
 	init_completion(&nn->nfsd_net_free_done);
 	init_completion(&nn->nfsd_net_confirm_done);
 
@@ -677,7 +686,8 @@ int nfsd_create_serv(struct net *net, bool no_rpcbind)
 				 nn->max_blksize, nfsd);
 	if (serv == NULL) {
 		percpu_ref_exit(&nn->nfsd_net_ref);
-		return -ENOMEM;
+		error = -ENOMEM;
+		goto out_put;
 	}
 	serv->sv_tls = true;
 
@@ -688,7 +698,7 @@ int nfsd_create_serv(struct net *net, bool no_rpcbind)
 	if (error < 0) {
 		svc_destroy(&serv);
 		percpu_ref_exit(&nn->nfsd_net_ref);
-		return error;
+		goto out_put;
 	}
 	/*
 	 * Register before publishing nn->nfsd_serv.  Namespaces are only
@@ -704,6 +714,10 @@ int nfsd_create_serv(struct net *net, bool no_rpcbind)
 
 	nfsd_reset_write_verifier(nn);
 	return 0;
+
+out_put:
+	module_put(THIS_MODULE);
+	return error;
 }
 
 int nfsd_nrpools(struct net *net)
-- 
2.43.0