Fix zpl_test_super race with zfs_umount

We cannot call zpl_enter in zpl_test_super, because zpl_test_super is
under spinlock so we can't sleep, and also because zpl_test_super is
called without sb->s_umount taken, so it's possible we would race with
zfs_umount and call zpl_enter on freed zfsvfs.

Here's an stack trace when this happens:
[ 2379.114837] VERIFY(cvp->cv_magic == CV_MAGIC) failed
[ 2379.114845] PANIC at spl-condvar.c:497:__cv_broadcast()
[ 2379.114854] Kernel panic - not syncing: VERIFY(cvp->cv_magic == CV_MAGIC) failed
[ 2379.115012] Call Trace:
[ 2379.115019]  dump_stack+0x74/0x96
[ 2379.115024]  panic+0x114/0x2f6
[ 2379.115035]  spl_panic+0xcf/0xfc [spl]
[ 2379.115477]  __cv_broadcast+0x68/0xa0 [spl]
[ 2379.115585]  rrw_exit+0xb8/0x310 [zfs]
[ 2379.115696]  rrm_exit+0x4a/0x80 [zfs]
[ 2379.115808]  zpl_test_super+0xa9/0xd0 [zfs]
[ 2379.115920]  sget+0xd1/0x230
[ 2379.116033]  zpl_mount+0xdc/0x230 [zfs]
[ 2379.116037]  legacy_get_tree+0x28/0x50
[ 2379.116039]  vfs_get_tree+0x27/0xc0
[ 2379.116045]  path_mount+0x2fe/0xa70
[ 2379.116048]  do_mount+0x80/0xa0
[ 2379.116050]  __x64_sys_mount+0x8b/0xe0
[ 2379.116052]  do_syscall_64+0x35/0x50
[ 2379.116054]  entry_SYSCALL_64_after_hwframe+0x61/0xc6
[ 2379.116057] RIP: 0033:0x7f9912e8b26a

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes #15077

diff --git a/module/os/linux/zfs/zfs_vfsops.c b/module/os/linux/zfs/zfs_vfsops.c
index 6b6293b9e482bd75e1ced5bf92f2222c9a8181bc..87c4e6dcaf7d7d7749ce6929b2f32054b0590ffb 100644 (file)
--- a/module/os/linux/zfs/zfs_vfsops.c
+++ b/module/os/linux/zfs/zfs_vfsops.c
@@ -1662,6 +1662,7 @@ zfs_umount(struct super_block *sb)
        }
 
        zfsvfs_free(zfsvfs);
+       sb->s_fs_info = NULL;
        return (0);
 }
 

diff --git a/module/os/linux/zfs/zpl_super.c b/module/os/linux/zfs/zpl_super.c
index c5c230bee1448a09a66c7f71ccd33c7a31187cfc..ad52a11aada092b7d7c1e7db709909e856152bb7 100644 (file)
--- a/module/os/linux/zfs/zpl_super.c
+++ b/module/os/linux/zfs/zpl_super.c
@@ -277,8 +277,6 @@ zpl_test_super(struct super_block *s, void *data)
 {
        zfsvfs_t *zfsvfs = s->s_fs_info;
        objset_t *os = data;
-       int match;
-
        /*
         * If the os doesn't match the z_os in the super_block, assume it is
         * not a match. Matching would imply a multimount of a dataset. It is
@@ -286,19 +284,7 @@ zpl_test_super(struct super_block *s, void *data)
         * that changes the z_os, e.g., rollback, where the match will be
         * missed, but in that case the user will get an EBUSY.
         */
-       if (zfsvfs == NULL || os != zfsvfs->z_os)
-               return (0);
-
-       /*
-        * If they do match, recheck with the lock held to prevent mounting the
-        * wrong dataset since z_os can be stale when the teardown lock is held.
-        */
-       if (zpl_enter(zfsvfs, FTAG) != 0)
-               return (0);
-       match = (os == zfsvfs->z_os);
-       zpl_exit(zfsvfs, FTAG);
-
-       return (match);
+       return (zfsvfs != NULL && os == zfsvfs->z_os);
 }
 
 static struct super_block *
@@ -324,12 +310,35 @@ zpl_mount_impl(struct file_system_type *fs_type, int flags, zfs_mnt_t *zm)
 
        s = sget(fs_type, zpl_test_super, set_anon_super, flags, os);
 
+       /*
+        * Recheck with the lock held to prevent mounting the wrong dataset
+        * since z_os can be stale when the teardown lock is held.
+        *
+        * We can't do this in zpl_test_super in since it's under spinlock and
+        * also s_umount lock is not held there so it would race with
+        * zfs_umount and zfsvfs can be freed.
+        */
+       if (!IS_ERR(s) && s->s_fs_info != NULL) {
+               zfsvfs_t *zfsvfs = s->s_fs_info;
+               if (zpl_enter(zfsvfs, FTAG) == 0) {
+                       if (os != zfsvfs->z_os)
+                               err = -SET_ERROR(EBUSY);
+                       zpl_exit(zfsvfs, FTAG);
+               } else {
+                       err = -SET_ERROR(EBUSY);
+               }
+       }
        dsl_dataset_long_rele(dmu_objset_ds(os), FTAG);
        dsl_dataset_rele(dmu_objset_ds(os), FTAG);
 
        if (IS_ERR(s))
                return (ERR_CAST(s));
 
+       if (err) {
+               deactivate_locked_super(s);
+               return (ERR_PTR(err));
+       }
+
        if (s->s_root == NULL) {
                err = zpl_fill_super(s, zm, flags & SB_SILENT ? 1 : 0);
                if (err) {