From 028682e1b645dc15a9fcc650a4708a5db2a4798d Mon Sep 17 00:00:00 2001 From: Chunwei Chen <david.chen@osnexus.com> Date: Tue, 19 Jul 2016 12:02:33 -0700 Subject: [PATCH] Fix NULL pointer in zfs_preumount from 1d9b3bd When zfs_domount fails zsb will be freed, and its caller mount_nodev/get_sb_nodev will do deactivate_locked_super and calls into zfs_preumount. In order to make sure we don't touch any nonexistent stuff, we must make sure s_fs_info is NULL in the fail path so zfs_preumount can easily check that. Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes #4867 Issue #4854 --- module/zfs/zfs_vfsops.c | 49 ++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/module/zfs/zfs_vfsops.c b/module/zfs/zfs_vfsops.c index 2e3069e30405..3475707ffdba 100644 --- a/module/zfs/zfs_vfsops.c +++ b/module/zfs/zfs_vfsops.c @@ -1442,7 +1442,8 @@ zfs_domount(struct super_block *sb, zfs_mntopts_t *zmo, int silent) dmu_objset_set_user(zsb->z_os, zsb); mutex_exit(&zsb->z_os->os_user_ptr_lock); } else { - error = zfs_sb_setup(zsb, B_TRUE); + if ((error = zfs_sb_setup(zsb, B_TRUE))) + goto out; } /* Allocate a root inode for the filesystem. */ @@ -1468,6 +1469,11 @@ zfs_domount(struct super_block *sb, zfs_mntopts_t *zmo, int silent) if (error) { dmu_objset_disown(zsb->z_os, zsb); zfs_sb_free(zsb); + /* + * make sure we don't have dangling sb->s_fs_info which + * zfs_preumount will use. + */ + sb->s_fs_info = NULL; } return (error); @@ -1486,26 +1492,29 @@ zfs_preumount(struct super_block *sb) { zfs_sb_t *zsb = sb->s_fs_info; - if (zsb) + /* zsb is NULL when zfs_domount fails during mount */ + if (zsb) { zfsctl_destroy(sb->s_fs_info); - /* - * Wait for iput_async before entering evict_inodes in - * generic_shutdown_super. The reason we must finish before - * evict_inodes is when lazytime is on, or when zfs_purgedir calls - * zfs_zget, iput would bump i_count from 0 to 1. This would race - * with the i_count check in evict_inodes. This means it could - * destroy the inode while we are still using it. - * - * We wait for two passes. xattr directories in the first pass may - * add xattr entries in zfs_purgedir, so in the second pass we wait - * for them. We don't use taskq_wait here because it is a pool wide - * taskq. Other mounted filesystems can constantly do iput_async - * and there's no guarantee when taskq will be empty. - */ - taskq_wait_outstanding(dsl_pool_iput_taskq( - dmu_objset_pool(zsb->z_os)), 0); - taskq_wait_outstanding(dsl_pool_iput_taskq( - dmu_objset_pool(zsb->z_os)), 0); + /* + * Wait for iput_async before entering evict_inodes in + * generic_shutdown_super. The reason we must finish before + * evict_inodes is when lazytime is on, or when zfs_purgedir + * calls zfs_zget, iput would bump i_count from 0 to 1. This + * would race with the i_count check in evict_inodes. This means + * it could destroy the inode while we are still using it. + * + * We wait for two passes. xattr directories in the first pass + * may add xattr entries in zfs_purgedir, so in the second pass + * we wait for them. We don't use taskq_wait here because it is + * a pool wide taskq. Other mounted filesystems can constantly + * do iput_async and there's no guarantee when taskq will be + * empty. + */ + taskq_wait_outstanding(dsl_pool_iput_taskq( + dmu_objset_pool(zsb->z_os)), 0); + taskq_wait_outstanding(dsl_pool_iput_taskq( + dmu_objset_pool(zsb->z_os)), 0); + } } EXPORT_SYMBOL(zfs_preumount);