Subject: Re: kern/1540: union FS botches namei/link locking protocols
To: None <gnats-bugs@NetBSD.ORG, jtk@kolvir.arlington.ma.us>
From: John Kohl <jtk@kolvir.arlington.ma.us>
List: netbsd-bugs
Date: 10/02/1995 19:37:44
Here is a patch which seems to fix what I believe is the underlying
problem reported in kern/1540:

===================================================================
RCS file: RCS/union_vnops.c,v
retrieving revision 1.12
diff -c -r1.12 union_vnops.c
*** union_vnops.c	1995/07/24 22:51:01	1.12
--- union_vnops.c	1995/10/02 23:37:11
***************
*** 983,988 ****
--- 983,992 ----
  	return (error);
  }
  
+ /* a_vp: directory in which to link
+    a_tdvp: new target of the link
+    a_cnp: name for the link
+    */
  int
  union_link(ap)
  	struct vop_link_args /* {
***************
*** 998,1018 ****
  
  	un = VTOUNION(ap->a_vp);
  
  	if (ap->a_vp->v_op != ap->a_tdvp->v_op) {
  		tdvp = ap->a_tdvp;
  	} else {
  		struct union_node *tdun = VTOUNION(ap->a_tdvp);
  		if (tdun->un_uppervp == NULLVP) {
  			VOP_LOCK(ap->a_tdvp);
  			if (un->un_uppervp == tdun->un_dirvp) {
! 				un->un_flags &= ~UN_ULOCK;
! 				VOP_UNLOCK(un->un_uppervp);
  			}
  			error = union_copyup(tdun, 1, ap->a_cnp->cn_cred,
  						ap->a_cnp->cn_proc);
  			if (un->un_uppervp == tdun->un_dirvp) {
! 				VOP_LOCK(un->un_uppervp);
! 				un->un_flags |= UN_ULOCK;
  			}
  			VOP_UNLOCK(ap->a_tdvp);
  		}
--- 1002,1057 ----
  
  	un = VTOUNION(ap->a_vp);
  
+ #ifdef DIAGNOSTIC
+ 	if (!(ap->a_cnp->cn_flags & LOCKPARENT)) {
+ 	    printf("union_link called without LOCKPARENT set!\n");
+ 	    error = EIO; /* need some error code for "caller is a bozo" */
+ 	} else
+ #endif
  	if (ap->a_vp->v_op != ap->a_tdvp->v_op) {
  		tdvp = ap->a_tdvp;
  	} else {
  		struct union_node *tdun = VTOUNION(ap->a_tdvp);
  		if (tdun->un_uppervp == NULLVP) {
+ 			/*
+ 			 * needs to be copied up before we can link it.
+ 			 */
  			VOP_LOCK(ap->a_tdvp);
  			if (un->un_uppervp == tdun->un_dirvp) {
! 				VOP_UNLOCK(ap->a_vp);
  			}
  			error = union_copyup(tdun, 1, ap->a_cnp->cn_cred,
  						ap->a_cnp->cn_proc);
  			if (un->un_uppervp == tdun->un_dirvp) {
! 				/* During copyup, we dropped the lock on the
! 				 * dir and invalidated any saved namei lookup
! 				 * state for the directory we'll be entering
! 				 * the link in.  We need to re-run the lookup
! 				 * in that directory to reset any state needed
! 				 * for VOP_LINK.
! 				 * Call relookup on the union-layer to
! 				 * reset the state.
! 				 */
! 				vp  = NULLVP;
! 				if (un->un_uppervp == NULLVP ||
! 				    /*
! 				     * relookup starts with an unlocked node,
! 				     * and since LOCKPARENT is set returns
! 				     * the starting directory locked.
! 				     */
! 				    (error = relookup(ap->a_vp,
! 						      &vp, ap->a_cnp))) {
! 					vrele(ap->a_vp);
! 					VOP_UNLOCK(ap->a_tdvp);
! 					return EROFS;
! 				}
! 				if (vp != NULLVP) {
! 					/* The name we want to create has
! 					   mysteriously appeared (a race?) */
! 					error = EEXIST;
! 					VOP_UNLOCK(ap->a_tdvp);
! 					goto croak;
! 				}
  			}
  			VOP_UNLOCK(ap->a_tdvp);
  		}
***************
*** 1024,1029 ****
--- 1063,1069 ----
  		error = EROFS;
  
  	if (error) {
+ croak:
  		vput(ap->a_vp);
  		return (error);
  	}