The current implementation of vn_lock() is racy. Modification of

the vnode operations vector for active vnodes is unsafe because it
is not known whether deadfs or the original file system will be
called.

- Pass down LK_RETRY to the lock operation (hint for deadfs only).

- Change deadfs lock operation to return ENOENT if LK_RETRY is unset.

- Change all other lock operations to check for dead vnode once
  the vnode is locked and unlock and return ENOENT in this case.

With these changes in place vnode lock operations will never succeed
after vclean() has marked the vnode as VI_XLOCK and before vclean()
has changed the operations vector.

Adresses PR kern/37706 (Forced unmount of file systems is unsafe)

Discussed on tech-kern.

Welcome to 6.99.33

share/man/man9/vnodeops.9

@@ -1,4 +1,4 @@
-.\"     $NetBSD: vnodeops.9,v 1.92 2014/02/07 15:29:21 hannken Exp $
+.\"     $NetBSD: vnodeops.9,v 1.93 2014/02/27 16:51:37 hannken Exp $
 .\"
 .\" Copyright (c) 2001, 2005, 2006 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd February 7, 2014
+.Dd February 27, 2014
 .Dt VNODEOPS 9
 .Os
 .Sh NAME
@@ -1071,6 +1071,11 @@ If
 contains
 .Dv LK_NOWAIT
 and the lock is busy, the operation will return immediately with an error code.
+If
+.Fa flags
+contains
+.Dv LK_RETRY
+this is a hint the caller wants the lock on dead vnodes too.
 If the operation is successful zero is returned, otherwise an
 appropriate error code is returned.
 .Fn VOP_LOCK

share/man/man9/vnsubr.9

@@ -1,4 +1,4 @@
-.\"     $NetBSD: vnsubr.9,v 1.41 2011/01/30 07:04:48 rmind Exp $
+.\"     $NetBSD: vnsubr.9,v 1.42 2014/02/27 16:51:37 hannken Exp $
 .\"
 .\" Copyright (c) 2001, 2005, 2006 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd January 30, 2010
+.Dd February 27, 2014
 .Dt VNSUBR 9
 .Os
 .Sh NAME
@@ -138,14 +138,11 @@ exclusive lock
 .It LK_NOWAIT
 do not sleep to await lock
 .It LK_RETRY
-retry lock operation until locked
+allow lock operation on dead vnode
 .El
 .Pp
 If the operation is successful zero is returned, otherwise an
 appropriate error code is returned.
-The vnode interlock
-.Em v_interlock
-is released on return.
 .Pp
 .Fn vn_lock
 must not be called when the vnode's reference count is zero.

sys/coda/coda_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: coda_vnops.c,v 1.94 2014/02/07 15:29:21 hannken Exp $	*/
+/*	$NetBSD: coda_vnops.c,v 1.95 2014/02/27 16:51:37 hannken Exp $	*/
 
 /*
  *
@@ -46,7 +46,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: coda_vnops.c,v 1.94 2014/02/07 15:29:21 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: coda_vnops.c,v 1.95 2014/02/27 16:51:37 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -229,8 +229,7 @@ coda_open(void *v)
 
     MARK_ENTRY(CODA_OPEN_STATS);
 
-    if (!VOP_ISLOCKED(vp))
-	VOP_LOCK(vp, LK_EXCLUSIVE);
+    KASSERT(VOP_ISLOCKED(vp));
     /* Check for open of control file. */
     if (IS_CTL_VP(vp)) {
 	/* if (WRITABLE(flag)) */
@@ -1745,8 +1744,6 @@ coda_grab_vnode(vnode_t *uvp, dev_t dev, ino_t ino, vnode_t **vpp)
 
     /*
      * Obtain vnode from mount point and inode.
-     * XXX VFS_VGET does not clearly define locked/referenced state of
-     * returned vnode.
      */
     error = VFS_VGET(mp, ino, vpp);
     if (error) {
@@ -1757,8 +1754,7 @@ coda_grab_vnode(vnode_t *uvp, dev_t dev, ino_t ino, vnode_t **vpp)
     /* share the underlying vnode lock with the coda vnode */
     mutex_obj_hold((*vpp)->v_interlock);
     uvm_obj_setlock(&uvp->v_uobj, (*vpp)->v_interlock);
-    if (!VOP_ISLOCKED(*vpp))
-	VOP_LOCK(*vpp, LK_EXCLUSIVE);
+    KASSERT(VOP_ISLOCKED(*vpp));
     return(0);
 }
 

sys/fs/adosfs/adutil.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: adutil.c,v 1.15 2011/06/12 03:35:52 rmind Exp $	*/
+/*	$NetBSD: adutil.c,v 1.16 2014/02/27 16:51:37 hannken Exp $	*/
 
 /*
  * Copyright (c) 1994 Christian E. Hopps
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: adutil.c,v 1.15 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: adutil.c,v 1.16 2014/02/27 16:51:37 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/vnode.h>
@@ -85,7 +85,10 @@ adosfs_ahashget(struct mount *mp, ino_t an)
 void
 adosfs_ainshash(struct adosfsmount *amp, struct anode *ap)
 {
-	VOP_LOCK(ATOV(ap), LK_EXCLUSIVE);
+	int error __diagused;
+
+	error = VOP_LOCK(ATOV(ap), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 
 	mutex_enter(&adosfs_hashlock);
 	LIST_INSERT_HEAD(&amp->anodetab[AHASH(ap->block)], ap, link);

sys/fs/cd9660/cd9660_node.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: cd9660_node.c,v 1.29 2011/06/12 03:35:52 rmind Exp $	*/
+/*	$NetBSD: cd9660_node.c,v 1.30 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*-
  * Copyright (c) 1982, 1986, 1989, 1994
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cd9660_node.c,v 1.29 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cd9660_node.c,v 1.30 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -170,6 +170,7 @@ void
 cd9660_ihashins(struct iso_node *ip)
 {
 	struct ihashhead *ipp;
+	int error __diagused;
 
 	KASSERT(mutex_owned(&cd9660_hashlock));
 
@@ -178,7 +179,8 @@ cd9660_ihashins(struct iso_node *ip)
 	LIST_INSERT_HEAD(ipp, ip, i_hash);
 	mutex_exit(&cd9660_ihash_lock);
 
-	VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+	error = VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 }
 
 /*

sys/fs/efs/efs_ihash.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: efs_ihash.c,v 1.9 2012/04/29 20:27:31 dsl Exp $	*/
+/*	$NetBSD: efs_ihash.c,v 1.10 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1991, 1993
@@ -36,7 +36,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: efs_ihash.c,v 1.9 2012/04/29 20:27:31 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: efs_ihash.c,v 1.10 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -162,11 +162,13 @@ void
 efs_ihashins(struct efs_inode *eip)
 {
 	struct ihashhead *ipp;
+	int error __diagused;
 
 	KASSERT(mutex_owned(&efs_hashlock));
 
 	/* lock the inode, then put it on the appropriate hash list */
-	VOP_LOCK(EFS_ITOV(eip), LK_EXCLUSIVE);
+	error = VOP_LOCK(EFS_ITOV(eip), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 
 	mutex_enter(&efs_ihash_lock);
 	ipp = &ihashtbl[INOHASH(eip->ei_dev, eip->ei_number)];

sys/fs/filecorefs/filecore_node.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: filecore_node.c,v 1.25 2011/06/12 03:35:52 rmind Exp $	*/
+/*	$NetBSD: filecore_node.c,v 1.26 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*-
  * Copyright (c) 1982, 1986, 1989, 1994
@@ -67,7 +67,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: filecore_node.c,v 1.25 2011/06/12 03:35:52 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: filecore_node.c,v 1.26 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -188,13 +188,15 @@ void
 filecore_ihashins(struct filecore_node *ip)
 {
 	struct ihashhead *ipp;
+	int error __diagused;
 
 	mutex_enter(&filecore_ihash_lock);
 	ipp = &filecorehashtbl[INOHASH(ip->i_dev, ip->i_number)];
 	LIST_INSERT_HEAD(ipp, ip, i_hash);
 	mutex_exit(&filecore_ihash_lock);
 
-	VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+	error = VOP_LOCK(ITOV(ip), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 }
 
 /*

sys/fs/hfs/hfs_nhash.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: hfs_nhash.c,v 1.12 2011/06/12 03:35:53 rmind Exp $	*/
+/*	$NetBSD: hfs_nhash.c,v 1.13 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2005, 2007 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: hfs_nhash.c,v 1.12 2011/06/12 03:35:53 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: hfs_nhash.c,v 1.13 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -148,9 +148,11 @@ void
 hfs_nhashinsert(struct hfsnode *hp)
 {
 	struct nhashhead *hpp;
+	int error __diagused;
 
 	/* lock the inode, then put it on the appropriate hash list */
-	VOP_LOCK(HTOV(hp), LK_EXCLUSIVE);
+	error = VOP_LOCK(HTOV(hp), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 
 	mutex_enter(&hfs_nhash_lock);
 	hpp = &nhashtbl[HNOHASH(hp->h_dev, hp->h_rec.u.cnid, hp->h_fork)];

sys/fs/ptyfs/ptyfs_subr.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: ptyfs_subr.c,v 1.25 2012/10/24 23:36:15 christos Exp $	*/
+/*	$NetBSD: ptyfs_subr.c,v 1.26 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*
  * Copyright (c) 1993
@@ -73,7 +73,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.25 2012/10/24 23:36:15 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.26 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -397,9 +397,11 @@ static void
 ptyfs_hashins(struct ptyfsnode *pp)
 {
 	struct ptyfs_hashhead *ppp;
+	int error __diagused;
 
 	/* lock the ptyfsnode, then put it on the appropriate hash list */
-	VOP_LOCK(PTYFSTOV(pp), LK_EXCLUSIVE);
+	error = VOP_LOCK(PTYFSTOV(pp), LK_EXCLUSIVE);
+	KASSERT(error == 0);
 
 	mutex_enter(&ptyfs_used_slock);
 	ppp = &ptyfs_used_tbl[PTYHASH(pp->ptyfs_type, pp->ptyfs_pty,

sys/fs/tmpfs/tmpfs_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: tmpfs_vnops.c,v 1.117 2014/02/17 20:16:52 maxv Exp $	*/
+/*	$NetBSD: tmpfs_vnops.c,v 1.118 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*
  * Copyright (c) 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.117 2014/02/17 20:16:52 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.118 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/dirent.h>
@@ -1082,7 +1082,6 @@ tmpfs_reclaim(void *v)
 	bool recycle;
 
 	mutex_enter(&node->tn_vlock);
-	VOP_LOCK(vp, LK_EXCLUSIVE);
 
 	/* Disassociate inode from vnode. */
 	node->tn_vnode = NULL;
@@ -1091,7 +1090,6 @@ tmpfs_reclaim(void *v)
 	/* If inode is not referenced, i.e. no links, then destroy it. */
 	recycle = node->tn_links == 0 && TMPFS_NODE_RECLAIMING(node) == 0;
 
-	VOP_UNLOCK(vp);
 	mutex_exit(&node->tn_vlock);
 
 	if (recycle) {

sys/fs/union/union_vnops.c

@@ -1,4 +1,4 @@
-/*	$NetBSD: union_vnops.c,v 1.56 2014/02/16 09:50:25 hannken Exp $	*/
+/*	$NetBSD: union_vnops.c,v 1.57 2014/02/27 16:51:38 hannken Exp $	*/
 
 /*
  * Copyright (c) 1992, 1993, 1994, 1995
@@ -72,7 +72,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.56 2014/02/16 09:50:25 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: union_vnops.c,v 1.57 2014/02/27 16:51:38 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1598,10 +1598,32 @@ union_lock(void *v)
 		int a_flags;
 	} */ *ap = v;
 	struct vnode *vp;
-	struct union_node *un;
+	struct union_node *un = VTOUNION(ap->a_vp);
+	int flags = ap->a_flags;
 	int error;
 
-	un = VTOUNION(ap->a_vp);
+	if ((flags & LK_NOWAIT) != 0) {
+		if (!mutex_tryenter(&un->un_lock))
+			return EBUSY;
+		vp = LOCKVP(ap->a_vp);
+		if (!mutex_tryenter(vp->v_interlock)) {
+			mutex_exit(&un->un_lock);
+			return EBUSY;
+		}
+		if ((vp->v_iflag & (VI_XLOCK | VI_CLEAN)) != 0) {
+			mutex_exit(vp->v_interlock);
+			mutex_exit(&un->un_lock);
+			return EBUSY;
+		}
+		mutex_exit(vp->v_interlock);
+		if (vp == ap->a_vp)
+			error = genfs_lock(ap);
+		else
+			error = VOP_LOCK(vp, flags);
+		mutex_exit(&un->un_lock);
+		return error;
+	}
+
 	mutex_enter(&un->un_lock);
 	for (;;) {
 		vp = LOCKVP(ap->a_vp);
@@ -1620,9 +1642,22 @@ union_lock(void *v)
 		else
 			VOP_UNLOCK(vp);
 	}
+	mutex_enter(vp->v_interlock);
+	if ((vp->v_iflag & (VI_XLOCK | VI_CLEAN)) != 0) {
+		if (vp == ap->a_vp)
+			genfs_unlock(ap);
+		else
+			VOP_UNLOCK(vp);
+		if ((vp->v_iflag & VI_XLOCK))
+			vwait(vp, VI_XLOCK);
+		mutex_exit(vp->v_interlock);
+		mutex_exit(&un->un_lock);
+		return ENOENT;
+	}
+	mutex_exit(vp->v_interlock);
 	mutex_exit(&un->un_lock);
 
-	return error;
+	return 0;
 }
 
 int