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
This commit is contained in:
parent
d9bb583c25
commit
2d8a2b51dc
|
@ -1662,6 +1662,7 @@ zfs_umount(struct super_block *sb)
|
||||||
}
|
}
|
||||||
|
|
||||||
zfsvfs_free(zfsvfs);
|
zfsvfs_free(zfsvfs);
|
||||||
|
sb->s_fs_info = NULL;
|
||||||
return (0);
|
return (0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -277,8 +277,6 @@ zpl_test_super(struct super_block *s, void *data)
|
||||||
{
|
{
|
||||||
zfsvfs_t *zfsvfs = s->s_fs_info;
|
zfsvfs_t *zfsvfs = s->s_fs_info;
|
||||||
objset_t *os = data;
|
objset_t *os = data;
|
||||||
int match;
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If the os doesn't match the z_os in the super_block, assume it is
|
* 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
|
* 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
|
* that changes the z_os, e.g., rollback, where the match will be
|
||||||
* missed, but in that case the user will get an EBUSY.
|
* missed, but in that case the user will get an EBUSY.
|
||||||
*/
|
*/
|
||||||
if (zfsvfs == NULL || os != zfsvfs->z_os)
|
return (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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static struct super_block *
|
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);
|
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_long_rele(dmu_objset_ds(os), FTAG);
|
||||||
dsl_dataset_rele(dmu_objset_ds(os), FTAG);
|
dsl_dataset_rele(dmu_objset_ds(os), FTAG);
|
||||||
|
|
||||||
if (IS_ERR(s))
|
if (IS_ERR(s))
|
||||||
return (ERR_CAST(s));
|
return (ERR_CAST(s));
|
||||||
|
|
||||||
|
if (err) {
|
||||||
|
deactivate_locked_super(s);
|
||||||
|
return (ERR_PTR(err));
|
||||||
|
}
|
||||||
|
|
||||||
if (s->s_root == NULL) {
|
if (s->s_root == NULL) {
|
||||||
err = zpl_fill_super(s, zm, flags & SB_SILENT ? 1 : 0);
|
err = zpl_fill_super(s, zm, flags & SB_SILENT ? 1 : 0);
|
||||||
if (err) {
|
if (err) {
|
||||||
|
|
Loading…
Reference in New Issue