File system locking fixups, mostly from NetBSD:

- cache_lookup
        move common code from various fs's here
        always return with vnode and parent locked
        adjust return codes
- PDIRUNLOCK - new flag set if lookup couldn't lock parent vnode
- kernfs and procfs
        lock vnode in get_root
        don't unlock (again) in kernfs_freevp
        fix memory leak in procfs

From tedu@stanford.edu
deraadt@ and various other ok

7 files changed, 147 insertions, 61 deletions

diff --git a/sys/miscfs/fdesc/fdesc_vnops.c b/sys/miscfs/fdesc/fdesc_vnops.c
index c54cbe0a4e3..1e96dc206d3 100644
--- a/sys/miscfs/fdesc/fdesc_vnops.c
+++ b/sys/miscfs/fdesc/fdesc_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: fdesc_vnops.c,v 1.32 2002/08/23 15:39:31 art Exp $	*/
+/*	$OpenBSD: fdesc_vnops.c,v 1.33 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: fdesc_vnops.c,v 1.32 1996/04/11 11:24:29 mrg Exp $	*/
 
 /*
@@ -254,11 +254,9 @@ fdesc_lookup(v)
 	struct vnode *fvp;
 	char *ln;
 
-	VOP_UNLOCK(dvp, 0, p);
 	if (cnp->cn_namelen == 1 && *pname == '.') {
 		*vpp = dvp;
 		VREF(dvp);
-		vn_lock(dvp, LK_SHARED | LK_RETRY, p);
 		return (0);
 	}
 
@@ -277,8 +275,7 @@ fdesc_lookup(v)
 				goto bad;
 			*vpp = fvp;
 			fvp->v_type = VDIR;
-			vn_lock(fvp, LK_SHARED | LK_RETRY, p);
-			return (0);
+			goto good;
 		}
 
 		if (cnp->cn_namelen == 3 && bcmp(pname, "tty", 3) == 0) {
@@ -292,8 +289,7 @@ fdesc_lookup(v)
 				goto bad;
 			*vpp = fvp;
 			fvp->v_type = VCHR;
-			vn_lock(fvp, LK_SHARED | LK_RETRY, p);
-			return (0);
+			goto good;
 		}
 
 		ln = 0;
@@ -323,8 +319,7 @@ fdesc_lookup(v)
 			VTOFDESC(fvp)->fd_link = ln;
 			*vpp = fvp;
 			fvp->v_type = VLNK;
-			vn_lock(fvp, LK_SHARED | LK_RETRY, p);
-			return (0);
+			goto good;
 		} else {
 			error = ENOENT;
 			goto bad;
@@ -334,10 +329,18 @@ fdesc_lookup(v)
 
 	case Fdevfd:
 		if (cnp->cn_namelen == 2 && bcmp(pname, "..", 2) == 0) {
-			if ((error = fdesc_root(dvp->v_mount, vpp))) 
+			VOP_UNLOCK(dvp, 0, p);
+			cnp->cn_flags |= PDIRUNLOCK;
+			error = fdesc_root(dvp->v_mount, vpp);
+			if (error)
 				goto bad;
-
-			return (0);
+			/* If we're at the last component and need the
+			 * parent locked, undo the unlock above.
+			 */
+			if (((~cnp->cn_flags & (ISLASTCN | LOCKPARENT)) == 0) &&
+			    ((error = vn_lock(dvp, LK_EXCLUSIVE, p)) == 0))
+				cnp->cn_flags &= ~PDIRUNLOCK;
+			return (error);
 		}
 
 		fd = 0;
@@ -361,15 +364,26 @@ fdesc_lookup(v)
 		if (error)
 			goto bad;
 		VTOFDESC(fvp)->fd_fd = fd;
-		vn_lock(fvp, LK_SHARED | LK_RETRY, p);
 		*vpp = fvp;
-		return (0);
+		goto good;
 	}
 
-bad:;
-	vn_lock(dvp, LK_SHARED | LK_RETRY, p);
+bad:
 	*vpp = NULL;
 	return (error);
+
+good:
+	/*
+	 * As "." was special cased above, we now unlock the parent if we're
+	 * supposed to. We're only supposed to not unlock if this is the
+	 * last component, and the caller requested LOCKPARENT. So if either
+	 * condition is false, unlock.
+	 */
+	if (((~cnp->cn_flags) & (ISLASTCN | LOCKPARENT)) != 0) {
+		VOP_UNLOCK(dvp, 0, p);
+		cnp->cn_flags |= PDIRUNLOCK;
+	}
+	return (0);
 }
 
 int
diff --git a/sys/miscfs/kernfs/kernfs_vfsops.c b/sys/miscfs/kernfs/kernfs_vfsops.c
index 68ded75d74b..561c2c4e904 100644
--- a/sys/miscfs/kernfs/kernfs_vfsops.c
+++ b/sys/miscfs/kernfs/kernfs_vfsops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kernfs_vfsops.c,v 1.19 2002/10/12 02:03:46 krw Exp $	*/
+/*	$OpenBSD: kernfs_vfsops.c,v 1.20 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: kernfs_vfsops.c,v 1.26 1996/04/22 01:42:27 christos Exp $	*/
 
 /*
@@ -102,7 +102,10 @@ kernfs_mount(mp, path, data, ndp, p)
 	struct nameidata *ndp;
 	struct proc *p;
 {
+	int error = 0;
 	size_t size;
+	struct vnode *rvp;
+	struct kern_target *kt;
 
 #ifdef KERNFS_DIAGNOSTIC
 	printf("kernfs_mount(mp = %p)\n", mp);
@@ -114,7 +117,17 @@ kernfs_mount(mp, path, data, ndp, p)
 	if (mp->mnt_flag & MNT_UPDATE)
 		return (EOPNOTSUPP);
 
+	kt = kernfs_findtarget(".", 1);
+	error = kernfs_allocvp(kt, mp, &rvp);
+	if (error)
+		return (error);
+
+	rvp->v_type = VDIR;
+	rvp->v_flag |= VROOT;
+
 	mp->mnt_flag |= MNT_LOCAL;
+	mp->mnt_data = (qaddr_t)rvp;
+	vrele(rvp);
 	vfs_getnewfsid(mp);
 
 	(void) copyinstr(path, mp->mnt_stat.f_mntonname, MNAMELEN - 1, &size);
@@ -158,8 +171,12 @@ kernfs_unmount(mp, mntflags, p)
 #ifdef KERNFS_DIAGNOSTIC
 	printf("kernfs_unmount: calling vflush\n");
 #endif
-	if ((error = vflush(mp, 0, flags)) != 0)
+
+	if ((error = vflush(mp, 0, flags)) != 0) {
 		return (error);
+	}
+
+	mp->mnt_data = 0;
 
 	return (0);
 }
@@ -169,24 +186,16 @@ kernfs_root(mp, vpp)
 	struct mount *mp;
 	struct vnode **vpp;
 {
-	struct kern_target *kt;
-	int error;
+	struct vnode *vp = (struct vnode *)mp->mnt_data;
 
 #ifdef KERNFS_DIAGNOSTIC
 	printf("kernfs_root(mp = %p)\n", mp);
 #endif
-	kt = kernfs_findtarget(".", 1);
-	/* this should never happen */
-	if (kt == NULL) 
-		panic("kernfs_root: findtarget returned NULL");
-	
-	error = kernfs_allocvp(kt, mp, vpp);
-	/* this should never happen */
-	if (error) 
-		panic("kernfs_root: couldn't find root");
-
-	return(0);
-	
+
+	vget(vp, LK_EXCLUSIVE | LK_RETRY, curproc);
+	*vpp = vp;
+
+	return (0);
 }
 
 int
diff --git a/sys/miscfs/kernfs/kernfs_vnops.c b/sys/miscfs/kernfs/kernfs_vnops.c
index 269c0842653..3db0fadc84e 100644
--- a/sys/miscfs/kernfs/kernfs_vnops.c
+++ b/sys/miscfs/kernfs/kernfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kernfs_vnops.c,v 1.28 2002/10/12 02:03:46 krw Exp $	*/
+/*	$OpenBSD: kernfs_vnops.c,v 1.29 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: kernfs_vnops.c,v 1.43 1996/03/16 23:52:47 christos Exp $	*/
 
 /*
@@ -287,7 +287,6 @@ kernfs_freevp(vp, p)
 	struct kernfs_node *kf = VTOKERN(vp);
 
 	TAILQ_REMOVE(&kfshead, kf, list);
-	VOP_UNLOCK(vp, 0, p);
 	FREE(vp->v_data, M_TEMP);
 	vp->v_data = 0;
 	return(0);
@@ -487,15 +486,16 @@ kernfs_lookup(v)
 	struct proc *p = cnp->cn_proc;
 	struct kern_target *kt;
 	struct vnode *vp;
-	int error;
+	int error, wantpunlock;
 
 #ifdef KERNFS_DIAGNOSTIC
 	printf("kernfs_lookup(%p)\n", ap);
 	printf("kernfs_lookup(dp = %p, vpp = %p, cnp = %p)\n", dvp, vpp, ap->a_cnp);
 	printf("kernfs_lookup(%s)\n", pname);
 #endif
-	VOP_UNLOCK(dvp, 0, p);
+
 	*vpp = NULLVP;
+	cnp->cn_flags &= ~PDIRUNLOCK;
 
 	if (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)
 		return (EROFS);
@@ -503,14 +503,14 @@ kernfs_lookup(v)
 	if (cnp->cn_namelen == 1 && *pname == '.') { 
 		*vpp = dvp;
 		VREF(dvp);
-		vn_lock(dvp, LK_SHARED | LK_RETRY, p);
 		return (0);
 	}
 
+	wantpunlock = (~cnp->cn_flags & (LOCKPARENT | ISLASTCN));
+
 	kt = kernfs_findtarget(pname, cnp->cn_namelen);
 	if (kt == NULL) {
 		/* not found */
-		vn_lock(dvp, LK_SHARED | LK_RETRY, p);
 		return(cnp->cn_nameiop == LOOKUP ? ENOENT : EROFS);
 	}
 
@@ -524,17 +524,25 @@ kernfs_lookup(v)
 		*vpp = vp;
 		if (vget(vp, LK_EXCLUSIVE, p)) 
 			goto loop;
+		if (wantpunlock) {
+			VOP_UNLOCK(dvp, 0, p);
+			cnp->cn_flags |= PDIRUNLOCK;
+		}
 		return(0);
 	}
 
 
 	if ((error = kernfs_allocvp(kt, dvp->v_mount, vpp)) != 0) {
-		vn_lock(dvp, LK_SHARED | LK_RETRY, p);
 		return(error);
 	}
 
 	vn_lock(*vpp, LK_SHARED | LK_RETRY, p);
-	return(error);
+
+	if (wantpunlock) {
+		VOP_UNLOCK(dvp, 0, p);
+		cnp->cn_flags |= PDIRUNLOCK;
+	}
+	return (0);
 }
 		
 /*ARGSUSED*/
diff --git a/sys/miscfs/portal/portal_vnops.c b/sys/miscfs/portal/portal_vnops.c
index 5eb8a967ac9..c0b05be010a 100644
--- a/sys/miscfs/portal/portal_vnops.c
+++ b/sys/miscfs/portal/portal_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: portal_vnops.c,v 1.12 2002/03/14 01:27:08 millert Exp $	*/
+/*	$OpenBSD: portal_vnops.c,v 1.13 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: portal_vnops.c,v 1.17 1996/02/13 13:12:57 mycroft Exp $	*/
 
 /*
@@ -187,6 +187,7 @@ portal_lookup(v)
 	struct vnode **vpp = ap->a_vpp;
 	struct vnode *dvp = ap->a_dvp;
 	char *pname = cnp->cn_nameptr;
+	struct proc *p = cnp->cn_proc;
 	struct portalnode *pt;
 	int error;
 	struct vnode *fvp = 0;
@@ -201,7 +202,6 @@ portal_lookup(v)
 	if (cnp->cn_namelen == 1 && *pname == '.') {
 		*vpp = dvp;
 		VREF(dvp);
-		/*VOP_LOCK(dvp);*/
 		return (0);
 	}
 
@@ -228,7 +228,15 @@ portal_lookup(v)
 	pt->pt_fileid = portal_fileid++;
 
 	*vpp = fvp;
-	/*VOP_LOCK(fvp);*/
+	VOP_LOCK(fvp, LK_EXCLUSIVE, p);
+	/*
+	 * As we are the last component of the path name, fix up
+	 * the locking on the directory node.
+	 */
+	if ((cnp->cn_flags & LOCKPARENT) == 0) {
+		VOP_UNLOCK(dvp, 0, p);
+		cnp->cn_flags |= PDIRUNLOCK;
+	}
 	return (0);
 
 bad:;
diff --git a/sys/miscfs/procfs/procfs.h b/sys/miscfs/procfs/procfs.h
index 9ee10e559fa..83577da1934 100644
--- a/sys/miscfs/procfs/procfs.h
+++ b/sys/miscfs/procfs/procfs.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs.h,v 1.15 2002/03/14 01:27:08 millert Exp $	*/
+/*	$OpenBSD: procfs.h,v 1.16 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: procfs.h,v 1.17 1996/02/12 15:01:41 christos Exp $	*/
 
 /*
@@ -104,6 +104,7 @@ struct procfs_args {
 struct procfsmount {
 	void *pmnt_exechook;
 	int pmnt_flags;
+	struct vnode *rvp;
 };
 
 #define VFSTOPROC(mp)	((struct procfsmount *)(mp)->mnt_data)
diff --git a/sys/miscfs/procfs/procfs_vfsops.c b/sys/miscfs/procfs/procfs_vfsops.c
index 899369f0ffa..166de670487 100644
--- a/sys/miscfs/procfs/procfs_vfsops.c
+++ b/sys/miscfs/procfs/procfs_vfsops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_vfsops.c,v 1.16 2002/03/14 01:27:08 millert Exp $	*/
+/*	$OpenBSD: procfs_vfsops.c,v 1.17 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: procfs_vfsops.c,v 1.25 1996/02/09 22:40:53 christos Exp $	*/
 
 /*
@@ -82,6 +82,7 @@ procfs_mount(mp, path, data, ndp, p)
 	size_t size;
 	struct procfsmount *pmnt;
 	struct procfs_args args;
+	struct vnode *rvp;
 	int error;
 
 	if (UIO_MX & (UIO_MX-1)) {
@@ -102,10 +103,18 @@ procfs_mount(mp, path, data, ndp, p)
 	} else
 		args.flags = 0;
 
+	error = procfs_allocvp(mp, &rvp, 0, Proot);
+	if (error)
+		return (error);
+	rvp->v_type = VDIR;
+	rvp->v_flag |= VROOT;
+
 	mp->mnt_flag |= MNT_LOCAL;
 	pmnt = (struct procfsmount *) malloc(sizeof(struct procfsmount),
 	    M_UFSMNT, M_WAITOK);	/* XXX need new malloc type */
 
+	pmnt->rvp = rvp;
+
 	mp->mnt_data = (qaddr_t)pmnt;
 	vfs_getnewfsid(mp);
 
@@ -134,6 +143,7 @@ procfs_unmount(mp, mntflags, p)
 	int error;
 	extern int doforce;
 	int flags = 0;
+	struct vnode *rvp = VFSTOPROC(mp)->rvp;
 
 	if (mntflags & MNT_FORCE) {
 		/* procfs can never be rootfs so don't check for it */
@@ -142,8 +152,15 @@ procfs_unmount(mp, mntflags, p)
 		flags |= FORCECLOSE;
 	}
 
-	if ((error = vflush(mp, 0, flags)) != 0)
+	vrele(rvp);
+
+	if ((error = vflush(mp, 0, flags)) != 0) {
+		vget(rvp, 0, curproc);
 		return (error);
+	}
+
+	free(VFSTOPROC(mp), M_UFSMNT);
+	mp->mnt_data = 0;
 
 	return (0);
 }
@@ -153,8 +170,13 @@ procfs_root(mp, vpp)
 	struct mount *mp;
 	struct vnode **vpp;
 {
+	struct vnode *vp = VFSTOPROC(mp)->rvp;
 
-	return (procfs_allocvp(mp, vpp, 0, Proot));
+	VREF(vp);
+	vn_lock(vp, LK_EXCLUSIVE, curproc);
+	*vpp = vp;
+
+	return (0);
 }
 
 /* ARGSUSED */
diff --git a/sys/miscfs/procfs/procfs_vnops.c b/sys/miscfs/procfs/procfs_vnops.c
index 9848edcadd5..4b0ec1630bd 100644
--- a/sys/miscfs/procfs/procfs_vnops.c
+++ b/sys/miscfs/procfs/procfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: procfs_vnops.c,v 1.26 2002/04/06 23:40:13 miod Exp $	*/
+/*	$OpenBSD: procfs_vnops.c,v 1.27 2003/01/31 17:37:50 art Exp $	*/
 /*	$NetBSD: procfs_vnops.c,v 1.40 1996/03/16 23:52:55 christos Exp $	*/
 
 /*
@@ -751,9 +751,10 @@ procfs_lookup(v)
 	pid_t pid;
 	struct pfsnode *pfs;
 	struct proc *p = NULL;
-	int i, error, iscurproc = 0, isself = 0;
+	int i, error, wantpunlock, iscurproc = 0, isself = 0;
 
 	*vpp = NULL;
+	cnp->cn_flags &= ~PDIRUNLOCK;
 
 	if (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)
 		return (EROFS);
@@ -761,10 +762,10 @@ procfs_lookup(v)
 	if (cnp->cn_namelen == 1 && *pname == '.') {
 		*vpp = dvp;
 		VREF(dvp);
-		/*VOP_LOCK(dvp);*/
 		return (0);
 	}
 
+	wantpunlock = (~cnp->cn_flags & (LOCKPARENT | ISLASTCN));
 	pfs = VTOPFS(dvp);
 	switch (pfs->pfs_type) {
 	case Proot:
@@ -777,12 +778,10 @@ procfs_lookup(v)
 		if (iscurproc || isself) {
 			error = procfs_allocvp(dvp->v_mount, vpp, 0,
 			    iscurproc ? Pcurproc : Pself);
-#if 0
 			if ((error == 0) && (wantpunlock)) {
-				VOP_UNLOCK(dvp, 0);
+				VOP_UNLOCK(dvp, 0, curp);
 				cnp->cn_flags |= PDIRUNLOCK;
 			}
-#endif
 			return (error);
 		}
 
@@ -798,12 +797,10 @@ procfs_lookup(v)
 		if (i != nproc_root_targets) {
 			error = procfs_allocvp(dvp->v_mount, vpp, 0,
 			    pt->pt_pfstype);
-#if 0
 			if ((error == 0) && (wantpunlock)) {
-				VOP_UNLOCK(dvp, 0);
+				VOP_UNLOCK(dvp, 0, curp);
 				cnp->cn_flags |= PDIRUNLOCK;
 			}
-#endif
 			return (error);
 		}
 
@@ -815,11 +812,29 @@ procfs_lookup(v)
 		if (p == 0)
 			break;
 
-		return (procfs_allocvp(dvp->v_mount, vpp, pid, Pproc));
+		error = procfs_allocvp(dvp->v_mount, vpp, pid, Pproc);
+		if ((error == 0) && wantpunlock) {
+			VOP_UNLOCK(dvp, 0, curp);
+			cnp->cn_flags |= PDIRUNLOCK;
+		}
+		return (error);
 
 	case Pproc:
-		if (cnp->cn_flags & ISDOTDOT)
-			return (procfs_root(dvp->v_mount, vpp));
+		/*
+		 * do the .. dance. We unlock the directory, and then
+		 * get the root dir. That will automatically return ..
+		 * locked. Then if the caller wanted dvp locked, we
+		 * re-lock.
+		 */
+		if (cnp->cn_flags & ISDOTDOT) {
+			VOP_UNLOCK(dvp, 0, p);
+			cnp->cn_flags |= PDIRUNLOCK;
+			error = procfs_root(dvp->v_mount, vpp);
+			if ((error == 0) && (wantpunlock == 0) &&
+			    ((error = vn_lock(dvp, LK_EXCLUSIVE, curp)) == 0))
+				cnp->cn_flags &= ~PDIRUNLOCK;
+			return (error);
+		}
 
 		p = pfind(pfs->pfs_pid);
 		if (p == 0)
@@ -840,12 +855,21 @@ procfs_lookup(v)
 			/* We already checked that it exists. */
 			VREF(fvp);
 			vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, curp);
+			if (wantpunlock) {
+				VOP_UNLOCK(dvp, 0, curp);
+				cnp->cn_flags |= PDIRUNLOCK;
+			}
 			*vpp = fvp;
 			return (0);
 		}
 
-		return (procfs_allocvp(dvp->v_mount, vpp, pfs->pfs_pid,
-		    pt->pt_pfstype));
+		error =  procfs_allocvp(dvp->v_mount, vpp, pfs->pfs_pid,
+		    pt->pt_pfstype);
+		if ((error == 0) && (wantpunlock)) {
+			VOP_UNLOCK(dvp, 0, curp);
+			cnp->cn_flags |= PDIRUNLOCK;
+		}
+		return (error);
 
 	default:
 		return (ENOTDIR);