From: Alexander Sverdlin <[email protected]>
Commit bd5ae9288d64 ("nfsd: register pernet ops last, unregister first")
has re-opened rpc_pipefs_event() race against nfsd_net_id registration
(register_pernet_subsys()) which has been fixed by commit bb7ffbf29e76
("nfsd: fix nsfd startup race triggering BUG_ON").
Restore the order of register_pernet_subsys() vs register_cld_notifier().
Add WARN_ON() to prevent a future regression.
Crash info:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000012
CPU: 8 PID: 345 Comm: mount Not tainted 5.4.144-... #1
pc : rpc_pipefs_event+0x54/0x120 [nfsd]
lr : rpc_pipefs_event+0x48/0x120 [nfsd]
Call trace:
rpc_pipefs_event+0x54/0x120 [nfsd]
blocking_notifier_call_chain
rpc_fill_super
get_tree_keyed
rpc_fs_get_tree
vfs_get_tree
do_mount
ksys_mount
__arm64_sys_mount
el0_svc_handler
el0_svc
Fixes: bd5ae9288d64 ("nfsd: register pernet ops last, unregister first")
Cc: [email protected]
Signed-off-by: Alexander Sverdlin <[email protected]>
---
fs/nfsd/nfs4recover.c | 1 +
fs/nfsd/nfsctl.c | 14 +++++++-------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 6fedc49..4d829cf 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -2156,6 +2156,7 @@ static struct notifier_block nfsd4_cld_block = {
int
register_cld_notifier(void)
{
+ WARN_ON(!nfsd_net_id);
return rpc_pipefs_notifier_register(&nfsd4_cld_block);
}
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index af8531c..51a49e0 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1521,12 +1521,9 @@ static int __init init_nfsd(void)
int retval;
printk(KERN_INFO "Installing knfsd (copyright (C) 1996 [email protected]).\n");
- retval = register_cld_notifier();
- if (retval)
- return retval;
retval = nfsd4_init_slabs();
if (retval)
- goto out_unregister_notifier;
+ return retval;
retval = nfsd4_init_pnfs();
if (retval)
goto out_free_slabs;
@@ -1545,9 +1542,14 @@ static int __init init_nfsd(void)
goto out_free_exports;
retval = register_pernet_subsys(&nfsd_net_ops);
if (retval < 0)
+ goto out_free_filesystem;
+ retval = register_cld_notifier();
+ if (retval)
goto out_free_all;
return 0;
out_free_all:
+ unregister_pernet_subsys(&nfsd_net_ops);
+out_free_filesystem:
unregister_filesystem(&nfsd_fs_type);
out_free_exports:
remove_proc_entry("fs/nfs/exports", NULL);
@@ -1561,13 +1563,12 @@ static int __init init_nfsd(void)
nfsd4_exit_pnfs();
out_free_slabs:
nfsd4_free_slabs();
-out_unregister_notifier:
- unregister_cld_notifier();
return retval;
}
static void __exit exit_nfsd(void)
{
+ unregister_cld_notifier();
unregister_pernet_subsys(&nfsd_net_ops);
nfsd_drc_slab_free();
remove_proc_entry("fs/nfs/exports", NULL);
@@ -1577,7 +1578,6 @@ static void __exit exit_nfsd(void)
nfsd4_free_slabs();
nfsd4_exit_pnfs();
unregister_filesystem(&nfsd_fs_type);
- unregister_cld_notifier();
}
MODULE_AUTHOR("Olaf Kirch <[email protected]>");
--
2.10.2
On Tue, Dec 07, 2021 at 03:00:39PM +0100, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin <[email protected]>
>
> Commit bd5ae9288d64 ("nfsd: register pernet ops last, unregister first")
> has re-opened rpc_pipefs_event() race against nfsd_net_id registration
> (register_pernet_subsys()) which has been fixed by commit bb7ffbf29e76
> ("nfsd: fix nsfd startup race triggering BUG_ON").
>
> Restore the order of register_pernet_subsys() vs register_cld_notifier().
> Add WARN_ON() to prevent a future regression.
That makes sense, thanks. I'll pass it along for 5.16.
--b.
>
> Crash info:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000012
> CPU: 8 PID: 345 Comm: mount Not tainted 5.4.144-... #1
> pc : rpc_pipefs_event+0x54/0x120 [nfsd]
> lr : rpc_pipefs_event+0x48/0x120 [nfsd]
> Call trace:
> rpc_pipefs_event+0x54/0x120 [nfsd]
> blocking_notifier_call_chain
> rpc_fill_super
> get_tree_keyed
> rpc_fs_get_tree
> vfs_get_tree
> do_mount
> ksys_mount
> __arm64_sys_mount
> el0_svc_handler
> el0_svc
>
> Fixes: bd5ae9288d64 ("nfsd: register pernet ops last, unregister first")
> Cc: [email protected]
> Signed-off-by: Alexander Sverdlin <[email protected]>
> ---
> fs/nfsd/nfs4recover.c | 1 +
> fs/nfsd/nfsctl.c | 14 +++++++-------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 6fedc49..4d829cf 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -2156,6 +2156,7 @@ static struct notifier_block nfsd4_cld_block = {
> int
> register_cld_notifier(void)
> {
> + WARN_ON(!nfsd_net_id);
> return rpc_pipefs_notifier_register(&nfsd4_cld_block);
> }
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index af8531c..51a49e0 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1521,12 +1521,9 @@ static int __init init_nfsd(void)
> int retval;
> printk(KERN_INFO "Installing knfsd (copyright (C) 1996 [email protected]).\n");
>
> - retval = register_cld_notifier();
> - if (retval)
> - return retval;
> retval = nfsd4_init_slabs();
> if (retval)
> - goto out_unregister_notifier;
> + return retval;
> retval = nfsd4_init_pnfs();
> if (retval)
> goto out_free_slabs;
> @@ -1545,9 +1542,14 @@ static int __init init_nfsd(void)
> goto out_free_exports;
> retval = register_pernet_subsys(&nfsd_net_ops);
> if (retval < 0)
> + goto out_free_filesystem;
> + retval = register_cld_notifier();
> + if (retval)
> goto out_free_all;
> return 0;
> out_free_all:
> + unregister_pernet_subsys(&nfsd_net_ops);
> +out_free_filesystem:
> unregister_filesystem(&nfsd_fs_type);
> out_free_exports:
> remove_proc_entry("fs/nfs/exports", NULL);
> @@ -1561,13 +1563,12 @@ static int __init init_nfsd(void)
> nfsd4_exit_pnfs();
> out_free_slabs:
> nfsd4_free_slabs();
> -out_unregister_notifier:
> - unregister_cld_notifier();
> return retval;
> }
>
> static void __exit exit_nfsd(void)
> {
> + unregister_cld_notifier();
> unregister_pernet_subsys(&nfsd_net_ops);
> nfsd_drc_slab_free();
> remove_proc_entry("fs/nfs/exports", NULL);
> @@ -1577,7 +1578,6 @@ static void __exit exit_nfsd(void)
> nfsd4_free_slabs();
> nfsd4_exit_pnfs();
> unregister_filesystem(&nfsd_fs_type);
> - unregister_cld_notifier();
> }
>
> MODULE_AUTHOR("Olaf Kirch <[email protected]>");
> --
> 2.10.2