Re: 6.0 hangs (while building OOo)

On 13 Oct, Don Lewis wrote:

> The bug is that once we have unlocked pdp, another thread can do a
> lookup and overwrite dp->i_ino, so instead of getting the vnode for the
> ".." directory entry, VFS_VGET() will return the vnode for a
> subdirectory of the current directory, and when we relock the current
> directory we'll have a lock order reversal.
> 
> Even if this doesn't result in a deadlock, it looks like it has the
> potential for mucking up lookups that involve "..".  I also don't
> currently see a way for this to become a vnode lock leak.

I think the leak happens when dp->i_ino gets overwritten by the inode
number for ".".  This causes ufs_lookup() to recurse on the lock for the
current directory vnode (the lock is first acquired by VFS_VGET() and
then recursed by vn_lock()). This isn't expected by lookup(), which
compares whether the vnode returned by VOP_LOOKUP() is the same as the
directory vnode and uses this information to decide whether to call
vput() or vrele().

> The fix is to preserve a copy of dp->d_ino before unlocking pdp,
> and pass the saved value to VFS_VGET().
> 
> Index: sys/ufs/ufs/ufs_lookup.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_lookup.c,v
> retrieving revision 1.77
> diff -u -r1.77 ufs_lookup.c
> --- sys/ufs/ufs/ufs_lookup.c	13 Apr 2005 10:59:09 -0000	1.77
> +++ sys/ufs/ufs/ufs_lookup.c	13 Oct 2005 23:20:59 -0000
> _at__at_ -153,6 +153,7 _at__at_
>  	int flags = cnp->cn_flags;
>  	int nameiop = cnp->cn_nameiop;
>  	struct thread *td = cnp->cn_thread;
> +	u_int32_t saved_ino;
>  
>  	bp = NULL;
>  	slotoffset = -1;
> _at__at_ -557,8 +558,9 _at__at_
>  	 */
>  	pdp = vdp;
>  	if (flags & ISDOTDOT) {
> +		saved_ino = dp->i_ino;
>  		VOP_UNLOCK(pdp, 0, td);	/* race to get the inode */
> -		error = VFS_VGET(pdp->v_mount, dp->i_ino,
> +		error = VFS_VGET(pdp->v_mount, saved_ino,
>  		    cnp->cn_lkflags, &tdp);
>  		vn_lock(pdp, LK_EXCLUSIVE | LK_RETRY, td);
>  		if (error)