Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Zhang Xiaoxu <zhangxiaoxu5@huawei.com>,
        Bruce Fields <bfields@fieldses.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "yi.zhang@huawei.com" <yi.zhang@huawei.com>,
        "luomeng12@huawei.com" <luomeng12@huawei.com>
Subject: Re: [PATCH] nfsd: Fix null-ptr-deref in nfsd_fill_super()
Thread-Topic: [PATCH] nfsd: Fix null-ptr-deref in nfsd_fill_super()
Thread-Index: AQHYa+t1veW/n8zz1U6eDJiqYomDp60n4xgA
Date:   Fri, 20 May 2022 15:22:51 +0000
Message-ID: <EB2E6FAD-AEC6-48B4-AC02-634941D80687@oracle.com>
References: <20220520023106.6651-1-zhangxiaoxu5@huawei.com>
In-Reply-To: <20220520023106.6651-1-zhangxiaoxu5@huawei.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?sGp+GLkU3k8t6/Aj6gy2Ixu8lWjdRyTK5WIq3TtCeKR7vKC+f55nvHYtXOOU?=
 =?us-ascii?Q?UfwY3gXeIyWkIirXP/sVfWzleHccMI3H6fh/ZhbC24IIvmaMgPWGQOVkfEEo?=
 =?us-ascii?Q?0mmo2kc/49dBcsHHUNj5cl/G3tE4DDyFxI/ZYW4+hSkl+jjY8vm0T+x/ONAa?=
 =?us-ascii?Q?Bk6dDQYwKIzkGsklmsWGuWbCL/R56VoktUGjLvXpIPOoDO97t/3M60fS/yZZ?=
 =?us-ascii?Q?q0bQaXpStQT9vp50h1ZBIySHguIHuvgUywqRZ+BquY5XtV8ALbEeLkiiDBlz?=
 =?us-ascii?Q?lM610F3nL2uvvSlsm2h9Tq6Cg1vo+884UOpPjJQdSs4RQUrzTGOCeI/n8Nyd?=
 =?us-ascii?Q?D7BNsILw17fS3UXBaVLGSosJxIlveTLGCC+37Ib8tVPw2p65WCKq4ruFKiCP?=
 =?us-ascii?Q?CVDpw8KWZRD3l4+OBBwV0SqqQOjfxSExtJvkrdxx9w1JCQJmRtcwVtLnpyn2?=
 =?us-ascii?Q?lDX7e/CL2M8WZtHycYMBgUIS8Hvhn0zpSNxhiPMHXmh/u+mXEPhKUXy45f3j?=
 =?us-ascii?Q?uV5zT/gePNjiPuS1fO9dhdqRnoJkCyutUu8luHa3c8eN1psf42wSEZTufCdM?=
 =?us-ascii?Q?g9Lo6Jwe5tGuiB9XifNoz0SWHzcfwOpxSjuoc8fXqvbMNufH1sbRnCUrTMQa?=
 =?us-ascii?Q?0N453Uos0TolWkUH13d2nbWkUT8/ZSXtgIfG7Vxj4TTL3pK52jmInxxiE/07?=
 =?us-ascii?Q?3XzBsqcYfDg67rD4716oG2wb3pI1stQi4eOiO3wPw2DNhFwuYI+OWhDxh2sN?=
 =?us-ascii?Q?JcUL5kqlyQyFKLnmYnmvOCRZ65lNJcBBXOEiC+VrjjtrQhpG4hVFOid0y5/5?=
 =?us-ascii?Q?+Df0DLBZ+S+28zbXINqKioTyTbd3Z1D35j58ehi4mBc2xTJzVvSpdK36MYQ7?=
 =?us-ascii?Q?IDUnA++pD96fFvGYnJxUvqrc1GEAi/br8bmCAoKyz1szlNEw2SpDxPEQU7aa?=
 =?us-ascii?Q?83L0FXhcqEJmfyI17+G2WYXeauJo8QmJyFDWKtDgk6fll8LtyNvJyOFShYfS?=
 =?us-ascii?Q?K7HJRwYSpLh83GCL8EYSZwNOjYltJ5RikU5pfuqfTzwnqXcLbwekqk/VOBet?=
 =?us-ascii?Q?8RdltANup0qFpEoIxNH+JjKsSJ/E5U1ORgBTbWHq3NnYhEo+xbZuedQHJrCJ?=
 =?us-ascii?Q?HYQwbiSEXIZgMTog+km70DdsXYidrocCKATv5WJT3ZdHhUalnMGpldTV+ne9?=
 =?us-ascii?Q?xxwI4qp2gVdtS/amjaw72Y3+/VcRl9ftT3Db79V6trPybaT8vuJV/JllsMR0?=
 =?us-ascii?Q?C0M7OHMAs2gDmdb/TMXH2F3kXGDDcdwfy0MceSNo6mwqjC9GE92DxVOiqdFo?=
 =?us-ascii?Q?hD9xuM3O0r/qbRVTv6AYC4ujV7+SUrcNfoWR2rIsX7oFhBHdqVnGfXQqrUuj?=
 =?us-ascii?Q?7VIQkY5ofYGLBGmp34yFvbJGC8Ux1/2Ame4hwTY7DOhCu3kOxbSDCMXX3hNk?=
 =?us-ascii?Q?C7qjlX+318u9EZveMVEgk1qUBVOn+/7CSae+v0a5FyISStCJf2I8CDnZ23dX?=
 =?us-ascii?Q?TDIVP9VZStIR+Ug3fmYr1Nm0PRHhzEvg6UNxjY3HEFiKrEp365wEs+tRrcTW?=
 =?us-ascii?Q?R+bnor0nX7uvrjCuEIi9yqYfkfsNG0i/xSN5fhoUD/1ePgtOEfV4OJa1bAzF?=
 =?us-ascii?Q?mqad3JgiX89BXuzMXutHPnU+JgxT2sCy8O/djQYO9yYIMcfHsu1/WH5ST4w3?=
 =?us-ascii?Q?iZq5rFAT4/7U9m4oaF84y7XnccdSbl1DjWxmIjCJ2o9c/iQSW5prTXJdvEkl?=
 =?us-ascii?Q?wRfQgI8bG5wFAI7cySiiUGKidMM0IHE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <95C355B334464F41BEE4189191233287@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d14a3c39-2ffd-421d-c83e-08da3a749b1d
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 May 2022 15:22:51.8759
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: tp2lLW7+fzGMSCsEesg3e1GyyeCKrE+eRM74ShSWlDV/OsY5DhQfNiYQq+yjB+TwHbTQ7OMmdyAmidshWujJIA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR10MB6016
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.874
 definitions=2022-05-20_04:2022-05-20,2022-05-20 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 malwarescore=0
 suspectscore=0 mlxlogscore=999 adultscore=0 phishscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205200104
X-Proofpoint-GUID: ZPsm-wRigchkSi8QLDU_vg5waYzped8E
X-Proofpoint-ORIG-GUID: ZPsm-wRigchkSi8QLDU_vg5waYzped8E
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

[ Note well: Updated Bruce's email address. ]


> On May 19, 2022, at 10:31 PM, Zhang Xiaoxu <zhangxiaoxu5@huawei.com> wrot=
e:
>=20
> KASAN report null-ptr-deref as follows:
>=20
>  BUG: KASAN: null-ptr-deref in nfsd_fill_super+0xc6/0xe0 [nfsd]
>  Write of size 8 at addr 000000000000005d by task a.out/852
>=20
>  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.fc3=
3 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
>=20
> This can be reproduce by concurrent operations:
> 	1. fsopen(nfsd)/fsconfig
> 	2. insmod/rmmod nfsd
>=20
> 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.
>=20
> 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@kernel.org
> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>

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=3Dfor-=
next


> ---
> fs/nfsd/nfsctl.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>=20
> 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 =3D create_proc_exports_entry();
> 	if (retval)
> 		goto out_free_lockd;
> -	retval =3D register_filesystem(&nfsd_fs_type);
> -	if (retval)
> -		goto out_free_exports;
> 	retval =3D register_pernet_subsys(&nfsd_net_ops);
> 	if (retval < 0)
> -		goto out_free_filesystem;
> +		goto out_free_exports;
> 	retval =3D register_cld_notifier();
> +	if (retval)
> +		goto out_free_subsys;
> +	retval =3D 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)
>=20
> 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);
> }
>=20
> MODULE_AUTHOR("Olaf Kirch <okir@monad.swb.de>");
> --=20
> 2.31.1
>=20

--
Chuck Lever