Re: [PATCH] nfsd: shut down running servers in nfsd_net_exit()

[Shuangpeng <shuangpeng.kernel@gmail.com>]

Thanks for the review.

I agree with the comment about the __module_get() users: stopping the
server from nfsd_net_exit() only covers nfsd's own threads, while
fs/locks.c (via lm_mod_owner) and nfs_common/nfslocalio.c take
__module_get(THIS_MODULE) on the assumption that an active server
keeps the module alive, and nothing stops free_module() once
exit_nfsd() has started.

So v2 takes a module reference while a server exists instead, which
makes "rmmod nfsd" fail with EBUSY as it does while the nfsd
filesystem is mounted. The laundry_wq and cld notifier ordering
concerns go away with that, since exit_nfsd() can no longer run while
a server exists.

The remaining point, an exiting nfsd thread still running a few
instructions of module code after svc_stop_kthreads() has been woken
up, is a pre-existing window on the umount path as well and is the
same with or without this patch, so I left it out of scope.

I'll send v2 shortly.


> On Oct 27, 2026, at 06:48, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2026-10-26 at 19:56 -0400, Shuangpeng Bai wrote:
>> 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 threads run (commit f49169c97fce ("NFSD: Remove
>> svc_serv_ops::svo_module") removed the per-thread reference), and
>> nfsd_umount() is the only caller of nfsd_shutdown_threads().
>> 
>> 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.
>> 
>> Shut down a server that is still running for a net namespace in
>> nfsd_net_exit(), which is invoked for every net namespace when the
>> module is unloaded. nfsd_shutdown_threads() is a no-op when no server
>> exists, so nothing changes for the common case. As the laundromat
>> work is queued on laundry_wq until the server is shut down, destroy
>> that workqueue after unregister_pernet_subsys().
>> 
>> 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@vger.kernel.org # v6.10+
>> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
>> ---
>> Tested on top of 7.3-rc7 with KASAN and with PROVE_LOCKING: with the
>> server started over netlink and an NFSv3 request in flight, "rmmod nfsd"
>> now waits for the request to complete and unloads cleanly; starting and
>> stopping the server via netlink and via the nfsd filesystem still works.
>> 
>> An alternative would be to take a module reference while a server
>> exists so that rmmod fails with EBUSY, as it does while the nfsd
>> filesystem is mounted. Stopping the server in nfsd_net_exit() seemed
>> simpler and mirrors what nfsd_umount() does, but I am happy to rework
>> it if the other behaviour is preferred.
>> fs/nfsd/nfsctl.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index f32311f2d7cf..0a9679114b58 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -3113,6 +3113,7 @@ static __net_exit void nfsd_net_exit(struct net *net)
>> {
>> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>> 
>> + nfsd_shutdown_threads(net);
>> kfree_sensitive(nn->fh_key);
>> nfsd_net_cb_shutdown(nn);
>> nfsd_proc_stat_shutdown(net);
>> @@ -3200,9 +3201,9 @@ static void __exit exit_nfsd(void)
>> remove_proc_entry("fs/nfs", NULL);
>> genl_unregister_family(&nfsd_nl_family);
>> unregister_filesystem(&nfsd_fs_type);
>> - nfsd4_destroy_laundry_wq();
>> unregister_cld_notifier();
>> unregister_pernet_subsys(&nfsd_net_ops);
>> + nfsd4_destroy_laundry_wq();
>> nfsd_drc_slab_free();
>> nfsd_lockd_shutdown();
>> nfsd4_exit_pnfs();
> 
> 
> I think I'm OK with either behavior (shutting down threads on rmmod or
> taking a reference), but sashiko has some comments on this patch that
> should be addressed:
> 
> https://sashiko.dev/#/patchset/20261026235626.649844-1-shuangpeng.kernel%40gmail.com
> 
> -- 
> Jeff Layton <jlayton@kernel.org>