[ Note well: Updated Bruce's email address. ]
> On May 19, 2022, at 10:31 PM, Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx> wrote:
>
> KASAN report null-ptr-deref as follows:
>
> BUG: KASAN: null-ptr-deref in nfsd_fill_super+0xc6/0xe0 [nfsd]
> Write of size 8 at addr 000000000000005d by task a.out/852
>
> CPU: 7 PID: 852 Comm: a.out Not tainted 5.18.0-rc7-dirty #66
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1.fc33 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x34/0x44
> kasan_report+0xab/0x120
> ? nfsd_mkdir+0x71/0x1c0 [nfsd]
> ? nfsd_fill_super+0xc6/0xe0 [nfsd]
> nfsd_fill_super+0xc6/0xe0 [nfsd]
> ? nfsd_mkdir+0x1c0/0x1c0 [nfsd]
> get_tree_keyed+0x8e/0x100
> vfs_get_tree+0x41/0xf0
> __do_sys_fsconfig+0x590/0x670
> ? fscontext_read+0x180/0x180
> ? anon_inode_getfd+0x4f/0x70
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> This can be reproduce by concurrent operations:
> 1. fsopen(nfsd)/fsconfig
> 2. insmod/rmmod nfsd
>
> Since the nfsd file system is registered before than nfsd_net allocated,
> the caller may get the file_system_type and use the nfsd_net before it
> allocated, then null-ptr-deref occured.
>
> So should allocate the nfsd_net firstly, other than register file system.
IIUC, I suggest: "So init_nfsd() should call register_filesystem() last."
> Fixes: bd5ae9288d64 ("nfsd: register pernet ops last, unregister first")
> Cc: stable@xxxxxxxxxx
> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx>
I think this looks right. Bruce, as author of bd5ae9288d64, any
thoughts?
I need a v2 of this, though. The current version conflicts with the
courteous server patches already in my for-next branch. See:
https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/log/?h=for-next
> ---
> fs/nfsd/nfsctl.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 16920e4512bd..e17100e90e19 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1535,20 +1535,20 @@ static int __init init_nfsd(void)
> retval = create_proc_exports_entry();
> if (retval)
> goto out_free_lockd;
> - retval = register_filesystem(&nfsd_fs_type);
> - if (retval)
> - goto out_free_exports;
> retval = register_pernet_subsys(&nfsd_net_ops);
> if (retval < 0)
> - goto out_free_filesystem;
> + goto out_free_exports;
> retval = register_cld_notifier();
> + if (retval)
> + goto out_free_subsys;
> + retval = register_filesystem(&nfsd_fs_type);
> if (retval)
> goto out_free_all;
> return 0;
> out_free_all:
> + unregister_cld_notifier();
> +out_free_subsys:
> unregister_pernet_subsys(&nfsd_net_ops);
> -out_free_filesystem:
> - unregister_filesystem(&nfsd_fs_type);
> out_free_exports:
> remove_proc_entry("fs/nfs/exports", NULL);
> remove_proc_entry("fs/nfs", NULL);
> @@ -1566,6 +1566,7 @@ static int __init init_nfsd(void)
>
> static void __exit exit_nfsd(void)
> {
> + unregister_filesystem(&nfsd_fs_type);
> unregister_cld_notifier();
> unregister_pernet_subsys(&nfsd_net_ops);
> nfsd_drc_slab_free();
> @@ -1575,7 +1576,6 @@ static void __exit exit_nfsd(void)
> nfsd_lockd_shutdown();
> nfsd4_free_slabs();
> nfsd4_exit_pnfs();
> - unregister_filesystem(&nfsd_fs_type);
> }
>
> MODULE_AUTHOR("Olaf Kirch <okir@xxxxxxxxxxxx>");
> --
> 2.31.1
>
--
Chuck Lever
