d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MFV r308987: 7180 potential race between zfs_suspend_fs+zfs_resume_fs

Browse Source 
and zfs_ioc_rename

illumos/illumos-gate@690041b9ca
690041b9ca

https://www.illumos.org/issues/7180
  If a filesystem is not unmounted while the rename is being performed, then, for
  example, a concurrect zfs rollback may call zfs_suspend_fs followed by
  zfs_resume_fs on the same filesystem.
  The latter takes the filesystem's name as an argument. If the filesystem name
  changes as a result of the rename, then dmu_objset_hold(osname, zfsvfs, &os)
  call in zfs_resume_fs would fail resulting in a kernel panic.
  So far I have been able to reproduce this problem on FreeBSD where zfs rename
  has -u option that skips the unmounting before doing the renaming.
  But I think that in theory the same problem can occur on illumos as well,
  because the unmounting is done in userland before invoking the rename ioctl and
  there could be a race with, e.g., zfs mount.
  panic: solaris assert: dmu_objset_hold(osname, zfsvfs, &zfsvfs->z_os) == 0 (0x2
  == 0x0), file: /usr/devel/svn/head/sys/cddl/contrib/opensolaris/uts/common/fs/
  zfs/zfs_vfsops.c, line: 2210
  KDB: stack backtrace:
  db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe004df30710
  vpanic() at vpanic+0x182/frame 0xfffffe004df30790
  panic() at panic+0x43/frame 0xfffffe004df307f0
  assfail3() at assfail3+0x2c/frame 0xfffffe004df30810
  zfs_resume_fs() at zfs_resume_fs+0xb9/frame 0xfffffe004df30860
  zfs_ioc_rollback() at zfs_ioc_rollback+0x61/frame 0xfffffe004df308a0
  zfsdev_ioctl() at zfsdev_ioctl+0x65c/frame 0xfffffe004df30940
  devfs_ioctl_f() at devfs_ioctl_f+0x156/frame 0xfffffe004df309a0
  kern_ioctl() at kern_ioctl+0x246/frame 0xfffffe004df30a00
  sys_ioctl() at sys_ioctl+0x171/frame 0xfffffe004df30ae0
  amd64_syscall() at amd64_syscall+0x2db/frame 0xfffffe004df30bf0
  Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe004df30bf0

Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

MFC after:	2 weeks
This commit is contained in:
avg 2016-11-24 10:21:22 +00:00
parent 84b3058dd8
commit e95b1e137c
3 changed files with 18 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_vfsops.h
View File

@ -142,7 +142,7 @@ extern uint_t zfs_fsyncer_key;
extern int zfs_super_owner;
extern int zfs_suspend_fs(zfsvfs_t *zfsvfs);
extern int zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname);
extern int zfs_resume_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds);
extern int zfs_userspace_one(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type,
const char *domain, uint64_t rid, uint64_t *valuep);
extern int zfs_userspace_many(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type,

14
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
View File

@ -3789,12 +3789,15 @@ zfs_ioc_rollback(const char *fsname, nvlist_t *args, nvlist_t *outnvl)
int error;
if (getzfsvfs(fsname, &zfsvfs) == 0) {
dsl_dataset_t *ds;
ds = dmu_objset_ds(zfsvfs->z_os);
error = zfs_suspend_fs(zfsvfs);
if (error == 0) {
int resume_err;
error = dsl_dataset_rollback(fsname, zfsvfs, outnvl);
resume_err = zfs_resume_fs(zfsvfs, fsname);
resume_err = zfs_resume_fs(zfsvfs, ds);
error = error ? error : resume_err;
}
#ifdef illumos
@ -4435,8 +4438,10 @@ zfs_ioc_recv(zfs_cmd_t *zc)
if (getzfsvfs(tofs, &zfsvfs) == 0) {
/* online recv */
dsl_dataset_t *ds;
int end_err;
ds = dmu_objset_ds(zfsvfs->z_os);
error = zfs_suspend_fs(zfsvfs);
/*
* If the suspend fails, then the recv_end will
@ -4444,7 +4449,7 @@ zfs_ioc_recv(zfs_cmd_t *zc)
*/
end_err = dmu_recv_end(&drc, zfsvfs);
if (error == 0)
error = zfs_resume_fs(zfsvfs, tofs);
error = zfs_resume_fs(zfsvfs, ds);
error = error ? error : end_err;
#ifdef illumos
VFS_RELE(zfsvfs->z_vfs);
@ -4990,11 +4995,14 @@ zfs_ioc_userspace_upgrade(zfs_cmd_t *zc)
* objset needs to be closed & reopened (to grow the
* objset_phys_t). Suspend/resume the fs will do that.
*/
dsl_dataset_t *ds;
ds = dmu_objset_ds(zfsvfs->z_os);
error = zfs_suspend_fs(zfsvfs);
if (error == 0) {
dmu_objset_refresh_ownership(zfsvfs->z_os,
zfsvfs);
error = zfs_resume_fs(zfsvfs, zc->zc_name);
error = zfs_resume_fs(zfsvfs, ds);
}
}
if (error == 0)

13
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
View File

@ -2225,7 +2225,7 @@ zfs_suspend_fs(zfsvfs_t *zfsvfs)
* zfsvfs, held, and long held on entry.
*/
int
zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname)
zfs_resume_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds)
{
int err;
znode_t *zp;
@ -2234,14 +2234,13 @@ zfs_resume_fs(zfsvfs_t *zfsvfs, const char *osname)
ASSERT(RW_WRITE_HELD(&zfsvfs->z_teardown_inactive_lock));
/*
* We already own this, so just hold and rele it to update the
* objset_t, as the one we had before may have been evicted.
* We already own this, so just update the objset_t, as the one we
* had before may have been evicted.
*/
objset_t *os;
VERIFY0(dmu_objset_hold(osname, zfsvfs, &os));
VERIFY3P(os->os_dsl_dataset->ds_owner, ==, zfsvfs);
VERIFY(dsl_dataset_long_held(os->os_dsl_dataset));
dmu_objset_rele(os, zfsvfs);
VERIFY3P(ds->ds_owner, ==, zfsvfs);
VERIFY(dsl_dataset_long_held(ds));
VERIFY0(dmu_objset_from_ds(ds, &os));
err = zfsvfs_init(zfsvfs, os);
if (err != 0)