In ufs_checkpath(), recheck that '..' still points to the inode with

the same inode number after VFS_VGET() and relock of the vp. If '..'
changed, redo the lookup. To reduce code duplication, move the code to
read '..' dirent into the static helper function ufs_dir_dd_ino().

Supply the source inode number as an argument to ufs_checkpath() instead
of the source inode itself. The inode is unlocked, thus it might be
reclaimed, causing accesses to the freed memory.

Use vn_vget_ino() to get the '..' vnode by its inode number, instead of
directly code VFS_VGET() and relock, to properly busy the mount point
while vp lock is dropped.

Noted and reviewed by:	tegge
Tested by:	pho
MFC after:	1 month

3 files changed, 55 insertions, 41 deletions

diff --git a/sys/ufs/ufs/ufs_extern.h b/sys/ufs/ufs/ufs_extern.h
index 40a659d..2cb9801 100644
--- a/sys/ufs/ufs/ufs_extern.h
+++ b/sys/ufs/ufs/ufs_extern.h
@@ -58,7 +58,7 @@ int	 ufs_bmap(struct vop_bmap_args *);
 int	 ufs_bmaparray(struct vnode *, ufs2_daddr_t, ufs2_daddr_t *,
 	    struct buf *, int *, int *);
 int	 ufs_fhtovp(struct mount *, struct ufid *, struct vnode **);
-int	 ufs_checkpath(struct inode *, struct inode *, struct ucred *);
+int	 ufs_checkpath(ino_t, struct inode *, struct ucred *);
 void	 ufs_dirbad(struct inode *, doff_t, char *);
 int	 ufs_dirbadentry(struct vnode *, struct direct *, int);
 int	 ufs_dirempty(struct inode *, ino_t, struct ucred *);
diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c
index 791d28a..fddefc5 100644
--- a/sys/ufs/ufs/ufs_lookup.c
+++ b/sys/ufs/ufs/ufs_lookup.c
@@ -1237,69 +1237,81 @@ ufs_dirempty(ip, parentino, cred)
 	return (1);
 }
 
+static int
+ufs_dir_dd_ino(struct vnode *vp, struct ucred *cred, ino_t *dd_ino)
+{
+	struct dirtemplate dirbuf;
+	int error, namlen;
+
+	if (vp->v_type != VDIR)
+		return (ENOTDIR);
+	error = vn_rdwr(UIO_READ, vp, (caddr_t)&dirbuf,
+	    sizeof (struct dirtemplate), (off_t)0, UIO_SYSSPACE,
+	    IO_NODELOCKED | IO_NOMACCHECK, cred, NOCRED, (int *)0, NULL);
+	if (error != 0)
+		return (error);
+#if (BYTE_ORDER == LITTLE_ENDIAN)
+	if (OFSFMT(vp))
+		namlen = dirbuf.dotdot_type;
+	else
+		namlen = dirbuf.dotdot_namlen;
+#else
+	namlen = dirbuf.dotdot_namlen;
+#endif
+	if (namlen != 2 || dirbuf.dotdot_name[0] != '.' ||
+	    dirbuf.dotdot_name[1] != '.')
+		return (ENOTDIR);
+	*dd_ino = dirbuf.dotdot_ino;
+	return (0);
+}
+
 /*
  * Check if source directory is in the path of the target directory.
  * Target is supplied locked, source is unlocked.
  * The target is always vput before returning.
  */
 int
-ufs_checkpath(source, target, cred)
-	struct inode *source, *target;
-	struct ucred *cred;
+ufs_checkpath(ino_t source_ino, struct inode *target, struct ucred *cred)
 {
-	struct vnode *vp;
-	int error, namlen;
-	ino_t rootino;
-	struct dirtemplate dirbuf;
+	struct vnode *vp, *vp1;
+	int error;
+	ino_t dd_ino;
 
 	vp = ITOV(target);
-	if (target->i_number == source->i_number) {
+	if (target->i_number == source_ino) {
 		error = EEXIST;
 		goto out;
 	}
-	rootino = ROOTINO;
 	error = 0;
-	if (target->i_number == rootino)
+	if (target->i_number == ROOTINO)
 		goto out;
 
 	for (;;) {
-		if (vp->v_type != VDIR) {
-			error = ENOTDIR;
-			break;
-		}
-		error = vn_rdwr(UIO_READ, vp, (caddr_t)&dirbuf,
-			sizeof (struct dirtemplate), (off_t)0, UIO_SYSSPACE,
-			IO_NODELOCKED | IO_NOMACCHECK, cred, NOCRED, (int *)0,
-			(struct thread *)0);
+		error = ufs_dir_dd_ino(vp, cred, &dd_ino);
 		if (error != 0)
 			break;
-#		if (BYTE_ORDER == LITTLE_ENDIAN)
-			if (OFSFMT(vp))
-				namlen = dirbuf.dotdot_type;
-			else
-				namlen = dirbuf.dotdot_namlen;
-#		else
-			namlen = dirbuf.dotdot_namlen;
-#		endif
-		if (namlen != 2 ||
-		    dirbuf.dotdot_name[0] != '.' ||
-		    dirbuf.dotdot_name[1] != '.') {
-			error = ENOTDIR;
-			break;
-		}
-		if (dirbuf.dotdot_ino == source->i_number) {
+		if (dd_ino == source_ino) {
 			error = EINVAL;
 			break;
 		}
-		if (dirbuf.dotdot_ino == rootino)
+		if (dd_ino == ROOTINO)
 			break;
-		vput(vp);
-		error = VFS_VGET(vp->v_mount, dirbuf.dotdot_ino,
-		    LK_EXCLUSIVE, &vp);
-		if (error) {
-			vp = NULL;
+		error = vn_vget_ino(vp, dd_ino, LK_EXCLUSIVE, &vp1);
+		if (error != 0)
+			break;
+		/* Recheck that ".." still points to vp1 after relock of vp */
+		error = ufs_dir_dd_ino(vp, cred, &dd_ino);
+		if (error != 0) {
+			vput(vp1);
 			break;
 		}
+		/* Redo the check of ".." if directory was reparented */
+		if (dd_ino != VTOI(vp1)->i_number) {
+			vput(vp1);
+			continue;
+		}
+		vput(vp);
+		vp = vp1;
 	}
 
 out:
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index 7646b05..570a987c 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1036,6 +1036,7 @@ ufs_rename(ap)
 	struct direct newdir;
 	int doingdirectory = 0, oldparent = 0, newparent = 0;
 	int error = 0, ioflag;
+	ino_t fvp_ino;
 
 #ifdef INVARIANTS
 	if ((tcnp->cn_flags & HASBUF) == 0 ||
@@ -1146,6 +1147,7 @@ abortit:
 	 * call to checkpath().
 	 */
 	error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred, tcnp->cn_thread);
+	fvp_ino = ip->i_number;
 	VOP_UNLOCK(fvp, 0);
 	if (oldparent != dp->i_number)
 		newparent = dp->i_number;
@@ -1154,7 +1156,7 @@ abortit:
 			goto bad;
 		if (xp != NULL)
 			vput(tvp);
-		error = ufs_checkpath(ip, dp, tcnp->cn_cred);
+		error = ufs_checkpath(fvp_ino, dp, tcnp->cn_cred);
 		if (error)
 			goto out;
 		if ((tcnp->cn_flags & SAVESTART) == 0)