Close a race in the ufs_lookup() code that handles the ISDOTDOT

case by saving the value of dp->i_ino before unlocking the vnode
for the current directory and passing the saved value to VFS_VGET().

Without this change, another thread can overwrite dp->i_ino after
the current directory is unlocked, causing  ufs_lookup() to lock
and return the wrong vnode in place of the vnode for its parent
directory.  A deadlock can occur if dp->i_ino was changed to a
subdirectory of the current directory because the root to leaf vnode
lock ordering will be violated.  A vnode lock can be leaked if
dp->i_ino was changed to point to the current directory, which
causes the current vnode lock for the current directory to be
recursed, which confuses lookup() into calling vrele() when it
should be calling vput().

The probability of this bug being triggered seems to be quite low
unless the sysctl variable debug.vfscache is set to 0.

Reviewed by:	jhb
MFC after:	2 weeks
This commit is contained in:
Don Lewis 2005-10-14 22:13:33 +00:00
parent 4fdd2813ff
commit 12d360453c

View File

@ -153,6 +153,7 @@ ufs_lookup(ap)
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;
@ -557,8 +558,9 @@ ufs_lookup(ap)
*/
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)