fix unsafe modification of zfs_vnodeops when DIAGNOSTIC is enabled

The idea was to avoid a false assertion in zfs_lock, but it was
implemented very dangerously and incorrectly.

Reported by:	pho
Tested by:	pho
MFC after:	1 week
This commit is contained in:
Andriy Gapon 2016-11-20 14:00:50 +00:00
parent 4e0bab8df7
commit 17055fcda7
2 changed files with 9 additions and 13 deletions

View File

@ -5963,6 +5963,10 @@ zfs_vptocnp(struct vop_vptocnp_args *ap)
} }
#ifdef DIAGNOSTIC #ifdef DIAGNOSTIC
#define CHECK_LOR ((flags & LK_NOWAIT) == 0 && vp->v_mount != NULL && \
(vp->v_iflag & VI_DOOMED) == 0 && vp->v_data != NULL && \
(zp->z_pflags & ZFS_XATTR) == 0)
static int static int
zfs_lock(ap) zfs_lock(ap)
struct vop_lock1_args /* { struct vop_lock1_args /* {
@ -5979,22 +5983,21 @@ zfs_lock(ap)
int err; int err;
vp = ap->a_vp; vp = ap->a_vp;
zp = vp->v_data;
flags = ap->a_flags; flags = ap->a_flags;
if ((flags & LK_INTERLOCK) == 0 && (flags & LK_NOWAIT) == 0 && if ((flags & LK_INTERLOCK) == 0 && CHECK_LOR) {
(vp->v_iflag & VI_DOOMED) == 0 && (zp = vp->v_data) != NULL &&
(zp->z_pflags & ZFS_XATTR) == 0) {
zfsvfs = zp->z_zfsvfs; zfsvfs = zp->z_zfsvfs;
VERIFY(!RRM_LOCK_HELD(&zfsvfs->z_teardown_lock)); VERIFY(!RRM_LOCK_HELD(&zfsvfs->z_teardown_lock));
} }
err = vop_stdlock(ap); err = vop_stdlock(ap);
if ((flags & LK_INTERLOCK) != 0 && (flags & LK_NOWAIT) == 0 && if ((flags & LK_INTERLOCK) != 0 && CHECK_LOR) {
(vp->v_iflag & VI_DOOMED) == 0 && (zp = vp->v_data) != NULL &&
(zp->z_pflags & ZFS_XATTR) == 0) {
zfsvfs = zp->z_zfsvfs; zfsvfs = zp->z_zfsvfs;
VERIFY(!RRM_LOCK_HELD(&zfsvfs->z_teardown_lock)); VERIFY(!RRM_LOCK_HELD(&zfsvfs->z_teardown_lock));
} }
return (err); return (err);
} }
#undef CHECK_LOR
#endif #endif
struct vop_vector zfs_vnodeops; struct vop_vector zfs_vnodeops;

View File

@ -727,14 +727,7 @@ zfs_znode_alloc(zfsvfs_t *zfsvfs, dmu_buf_t *db, int blksz,
/* /*
* Acquire vnode lock before making it available to the world. * Acquire vnode lock before making it available to the world.
*/ */
#ifdef DIAGNOSTIC
vop_lock1_t *orig_lock = vp->v_op->vop_lock1;
vp->v_op->vop_lock1 = vop_stdlock;
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
vp->v_op->vop_lock1 = orig_lock;
#else
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
#endif
VN_LOCK_AREC(vp); VN_LOCK_AREC(vp);
if (vp->v_type != VFIFO) if (vp->v_type != VFIFO)
VN_LOCK_ASHARE(vp); VN_LOCK_ASHARE(vp);