* Re: [PATCH 1/1] fs/nfsd/nfsctl.c: fix race between nfsd registration and exports_proc
2025-03-05 9:32 ` [PATCH 1/1] fs/nfsd/nfsctl.c: fix race between nfsd registration and exports_proc Maninder Singh
@ 2025-03-05 14:06 ` Chuck Lever
0 siblings, 0 replies; 2+ messages in thread
From: Chuck Lever @ 2025-03-05 14:06 UTC (permalink / raw)
To: Maninder Singh, jlayton, neilb, okorniev, Dai.Ngo, tom
Cc: linux-nfs, linux-kernel, Shubham Rana
On 3/5/25 4:32 AM, Maninder Singh wrote:
> 1) As of now nfsd calls create_proc_exports_entry() at start of init_nfsd
> and cleanup by remove_proc_entry() at last of exit_nfsd.
>
> Which causes kernel OOPS if there is race between below 2 operations:
> (i) exportfs -r
> (ii) mount -t nfsd none /proc/fs/nfsd
>
> for 5.4 kernel ARM64:
>
> CPU 1:
> el1_irq+0xbc/0x180
> arch_counter_get_cntvct+0x14/0x18
> running_clock+0xc/0x18
> preempt_count_add+0x88/0x110
> prep_new_page+0xb0/0x220
> get_page_from_freelist+0x2d8/0x1778
> __alloc_pages_nodemask+0x15c/0xef0
> __vmalloc_node_range+0x28c/0x478
> __vmalloc_node_flags_caller+0x8c/0xb0
> kvmalloc_node+0x88/0xe0
> nfsd_init_net+0x6c/0x108 [nfsd]
> ops_init+0x44/0x170
> register_pernet_operations+0x114/0x270
> register_pernet_subsys+0x34/0x50
> init_nfsd+0xa8/0x718 [nfsd]
> do_one_initcall+0x54/0x2e0
>
> CPU 2 :
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
>
> PC is at : exports_net_open+0x50/0x68 [nfsd]
>
> Call trace:
> exports_net_open+0x50/0x68 [nfsd]
> exports_proc_open+0x2c/0x38 [nfsd]
> proc_reg_open+0xb8/0x198
> do_dentry_open+0x1c4/0x418
> vfs_open+0x38/0x48
> path_openat+0x28c/0xf18
> do_filp_open+0x70/0xe8
> do_sys_open+0x154/0x248
> exports_net_open+0x50/0x68 [nfsd]
> exports_proc_open+0x2c/0x38 [nfsd]
>
> Sometimes it crashes at exports_net_open() and sometimes cache_seq_next_rcu().
>
> and same is happening on latest 6.14 kernel as well:
>
> [ 0.000000] Linux version 6.14.0-rc5-next-20250304-dirty
> ...
> [ 285.455918] Unable to handle kernel paging request at virtual address 00001f4800001f48
> ...
> [ 285.464902] pc : cache_seq_next_rcu+0x78/0xa4
> ...
> [ 285.469695] Call trace:
> [ 285.470083] cache_seq_next_rcu+0x78/0xa4 (P)
> [ 285.470488] seq_read+0xe0/0x11c
> [ 285.470675] proc_reg_read+0x9c/0xf0
> [ 285.470874] vfs_read+0xc4/0x2fc
> [ 285.471057] ksys_read+0x6c/0xf4
> [ 285.471231] __arm64_sys_read+0x1c/0x28
> [ 285.471428] invoke_syscall+0x44/0x100
> [ 285.471633] el0_svc_common.constprop.0+0x40/0xe0
> [ 285.471870] do_el0_svc_compat+0x1c/0x34
> [ 285.472073] el0_svc_compat+0x2c/0x80
> [ 285.472265] el0t_32_sync_handler+0x90/0x140
> [ 285.472473] el0t_32_sync+0x19c/0x1a0
> [ 285.472887] Code: f9400885 93407c23 937d7c27 11000421 (f86378a3)
> [ 285.473422] ---[ end trace 0000000000000000 ]---
>
> It reproduced simply with below script:
> while [ 1 ]
> do
> /exportfs -r
> done &
>
> while [ 1 ]
> do
> insmod /nfsd.ko
> mount -t nfsd none /proc/fs/nfsd
> umount /proc/fs/nfsd
> rmmod nfsd
> done &
>
> So exporting interfaces to user space shall be done at last and
> cleanup at first place.
>
> With change no Kernel OOPs.
>
> 2) Also unregister of register_filesystem() was missed in case
> genl_register_family() fails.
> Corrected that also.
Thanks for the report.
You observed the original problem on 5.4, so the fix for that should be
constructed so it can be backported to LTS kernels.
The fix for 2) is only applicable for recent kernels that have NFSD
netlink support.
Can you therefore split these two fixes into separate patches so they
can each be handled appropriately by stable@?
> Co-developed-by: Shubham Rana <s9.rana@samsung.com>
> Signed-off-by: Shubham Rana <s9.rana@samsung.com>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> ---
> fs/nfsd/nfsctl.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index ac265d6fde35..d936a99ada2a 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -2291,12 +2291,10 @@ static int __init init_nfsd(void)
> if (retval)
> goto out_free_pnfs;
> nfsd_lockd_init(); /* lockd->nfsd callbacks */
> - retval = create_proc_exports_entry();
> - if (retval)
> - goto out_free_lockd;
> +
> retval = register_pernet_subsys(&nfsd_net_ops);
> if (retval < 0)
> - goto out_free_exports;
> + goto out_free_lockd;
> retval = register_cld_notifier();
> if (retval)
> goto out_free_subsys;
> @@ -2305,22 +2303,28 @@ static int __init init_nfsd(void)
> goto out_free_cld;
> retval = register_filesystem(&nfsd_fs_type);
> if (retval)
> - goto out_free_all;
> + goto out_free_nfsd4;
> retval = genl_register_family(&nfsd_nl_family);
> + if (retval)
> + goto out_free_filesystem;
> + retval = create_proc_exports_entry();
> if (retval)
> goto out_free_all;
> +
> nfsd_localio_ops_init();
>
> return 0;
> +
> out_free_all:
> + genl_unregister_family(&nfsd_nl_family);
> +out_free_filesystem:
> + unregister_filesystem(&nfsd_fs_type);
> +out_free_nfsd4:
> nfsd4_destroy_laundry_wq();
> out_free_cld:
> unregister_cld_notifier();
> out_free_subsys:
> unregister_pernet_subsys(&nfsd_net_ops);
> -out_free_exports:
> - remove_proc_entry("fs/nfs/exports", NULL);
> - remove_proc_entry("fs/nfs", NULL);
> out_free_lockd:
> nfsd_lockd_shutdown();
> nfsd_drc_slab_free();
> @@ -2333,14 +2337,14 @@ static int __init init_nfsd(void)
>
> static void __exit exit_nfsd(void)
> {
> + remove_proc_entry("fs/nfs/exports", NULL);
> + 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);
> nfsd_drc_slab_free();
> - remove_proc_entry("fs/nfs/exports", NULL);
> - remove_proc_entry("fs/nfs", NULL);
> nfsd_lockd_shutdown();
> nfsd4_free_slabs();
> nfsd4_exit_pnfs();
--
Chuck Lever
^ permalink raw reply [flat|nested] 2+ messages in thread