diff mbox series

[1/1] fs/nfsd/nfsctl.c: fix race between nfsd registration and exports_proc

Message ID 20250305093222.1051935-1-maninder1.s@samsung.com (mailing list archive)
State Changes Requested
Delegated to: Chuck Lever
Headers show
Series [1/1] fs/nfsd/nfsctl.c: fix race between nfsd registration and exports_proc | expand

Commit Message

Maninder Singh March 5, 2025, 9:32 a.m. UTC 
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.

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(-)

Comments

Chuck Lever March 5, 2025, 2:06 p.m. UTC | #1 
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();
diff mbox series

Patch

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();