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/05/1995 20:03:53
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);
}