On Fri, May 20, 2022 at 03:22:51PM +0000, Chuck Lever III wrote:
> [ 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'm not seeing any problem with the patch.
Reviewed-by: J. Bruce Fields <bfields@xxxxxxxxxxxx>
--b.
>
> 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
>
>
