From 9bd4f0a2c6b34052df6bd7002b26342960e9bc75 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Mon, 7 Nov 2016 10:55:56 +0000 Subject: [PATCH] vn_fullpath1() checked VV_ROOT and then unreferenced vp->v_mount->mnt_vnodecovered unlocked. This allowed unmount to race. Lock vnode after we noticed the VV_ROOT flag. See comments for explanation why unlocked check for the flag is considered safe. Reported and tested by: avg Sponsored by: The FreeBSD Foundation MFC after: 1 week --- sys/kern/vfs_cache.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c index 6101f186f938..bcfa50012190 100644 --- a/sys/kern/vfs_cache.c +++ b/sys/kern/vfs_cache.c @@ -2245,17 +2245,35 @@ vn_fullpath1(struct thread *td, struct vnode *vp, struct vnode *rdir, slash_prefixed = 1; } while (vp != rdir && vp != rootvnode) { - if (vp->v_vflag & VV_ROOT) { - if (vp->v_iflag & VI_DOOMED) { /* forced unmount */ - vrele(vp); + /* + * The vp vnode must be already fully constructed, + * since it is either found in namecache or obtained + * from VOP_VPTOCNP(). We may test for VV_ROOT safely + * without obtaining the vnode lock. + */ + if ((vp->v_vflag & VV_ROOT) != 0) { + vn_lock(vp, LK_RETRY | LK_SHARED); + + /* + * With the vnode locked, check for races with + * unmount, forced or not. Note that we + * already verified that vp is not equal to + * the root vnode, which means that + * mnt_vnodecovered can be NULL only for the + * case of unmount. + */ + if ((vp->v_iflag & VI_DOOMED) != 0 || + (vp1 = vp->v_mount->mnt_vnodecovered) == NULL || + vp1->v_mountedhere != vp->v_mount) { + vput(vp); error = ENOENT; SDT_PROBE3(vfs, namecache, fullpath, return, error, vp, NULL); break; } - vp1 = vp->v_mount->mnt_vnodecovered; + vref(vp1); - vrele(vp); + vput(vp); vp = vp1; continue; }