Date: Tue, 28 Jan 2020 11:29:06 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357199 - head/sys/fs/nullfs Message-ID: <202001281129.00SBT6AJ063186@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Tue Jan 28 11:29:06 2020 New Revision: 357199 URL: https://svnweb.freebsd.org/changeset/base/357199 Log: Save lower root vnode in nullfs mnt data instead of upper. Nullfs needs to know the root vnode of the lower fs during the operation. Currently it caches the upper vnode of it, which is also the root of the nullfs mount. On unmount, nullfs calls vflush() with rootrefs == 1, and aborts non-forced unmount if there are any more vnodes instantiated during vflush(). This means that the reference to the root vnode after failed non-forced unmount could be lost and nullm_rootvp points to the freed memory. Fix it by storing the reference for lower vnode instead, which is kept intact during vflush(). nullfs_root() now instantiates the upper vnode of lower root. Care about VV_ROOT flag in null_nodeget(). Reported and tested by: pho Sponsored by: The FreeBSD Foundation MFC after: 1 week Modified: head/sys/fs/nullfs/null.h head/sys/fs/nullfs/null_subr.c head/sys/fs/nullfs/null_vfsops.c Modified: head/sys/fs/nullfs/null.h ============================================================================== --- head/sys/fs/nullfs/null.h Tue Jan 28 11:22:20 2020 (r357198) +++ head/sys/fs/nullfs/null.h Tue Jan 28 11:29:06 2020 (r357199) @@ -43,7 +43,7 @@ struct null_mount { struct mount *nullm_vfs; - struct vnode *nullm_rootvp; /* Reference to root null_node */ + struct vnode *nullm_lowerrootvp; /* Ref to lower root vnode */ uint64_t nullm_flags; }; Modified: head/sys/fs/nullfs/null_subr.c ============================================================================== --- head/sys/fs/nullfs/null_subr.c Tue Jan 28 11:22:20 2020 (r357198) +++ head/sys/fs/nullfs/null_subr.c Tue Jan 28 11:29:06 2020 (r357199) @@ -252,6 +252,8 @@ null_nodeget(mp, lowervp, vpp) vp->v_type = lowervp->v_type; vp->v_data = xp; vp->v_vnlock = lowervp->v_vnlock; + if (lowervp == MOUNTTONULLMOUNT(mp)->nullm_lowerrootvp) + vp->v_vflag |= VV_ROOT; error = insmntque1(vp, mp, null_insmntque_dtr, xp); if (error != 0) return (error); Modified: head/sys/fs/nullfs/null_vfsops.c ============================================================================== --- head/sys/fs/nullfs/null_vfsops.c Tue Jan 28 11:22:20 2020 (r357198) +++ head/sys/fs/nullfs/null_vfsops.c Tue Jan 28 11:29:06 2020 (r357199) @@ -74,7 +74,7 @@ static vfs_extattrctl_t nullfs_extattrctl; static int nullfs_mount(struct mount *mp) { - struct vnode *lowerrootvp, *vp; + struct vnode *lowerrootvp; struct vnode *nullm_rootvp; struct null_mount *xmp; struct null_node *nn; @@ -158,36 +158,24 @@ nullfs_mount(struct mount *mp) M_NULLFSMNT, M_WAITOK | M_ZERO); /* - * Save reference to underlying FS + * Save pointer to underlying FS and the reference to the + * lower root vnode. */ xmp->nullm_vfs = lowerrootvp->v_mount; + vref(lowerrootvp); + xmp->nullm_lowerrootvp = lowerrootvp; + mp->mnt_data = xmp; /* - * Save reference. Each mount also holds - * a reference on the root vnode. + * Make sure the node alias worked. */ - error = null_nodeget(mp, lowerrootvp, &vp); - /* - * Make sure the node alias worked - */ - if (error) { + error = null_nodeget(mp, lowerrootvp, &nullm_rootvp); + if (error != 0) { + vrele(lowerrootvp); free(xmp, M_NULLFSMNT); return (error); } - /* - * Keep a held reference to the root vnode. - * It is vrele'd in nullfs_unmount. - */ - nullm_rootvp = vp; - nullm_rootvp->v_vflag |= VV_ROOT; - xmp->nullm_rootvp = nullm_rootvp; - - /* - * Unlock the node (either the lower or the alias) - */ - VOP_UNLOCK(vp); - if (NULLVPTOLOWERVP(nullm_rootvp)->v_mount->mnt_flag & MNT_LOCAL) { MNT_ILOCK(mp); mp->mnt_flag |= MNT_LOCAL; @@ -209,7 +197,6 @@ nullfs_mount(struct mount *mp) mp->mnt_kern_flag |= lowerrootvp->v_mount->mnt_kern_flag & (MNTK_USES_BCACHE | MNTK_NO_IOPF | MNTK_UNMAPPED_BUFS); MNT_IUNLOCK(mp); - mp->mnt_data = xmp; vfs_getnewfsid(mp); if ((xmp->nullm_flags & NULLM_CACHE) != 0) { MNT_ILOCK(xmp->nullm_vfs); @@ -219,6 +206,7 @@ nullfs_mount(struct mount *mp) } vfs_mountedfrom(mp, target); + vput(nullm_rootvp); NULLFSDEBUG("nullfs_mount: lower %s, alias at %s\n", mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntonname); @@ -235,7 +223,7 @@ nullfs_unmount(mp, mntflags) { struct null_mount *mntdata; struct mount *ump; - int error, flags, rootrefs; + int error, flags; NULLFSDEBUG("nullfs_unmount: mp = %p\n", (void *)mp); @@ -244,9 +232,9 @@ nullfs_unmount(mp, mntflags) else flags = 0; - for (rootrefs = 1;; rootrefs = 0) { + for (;;) { /* There is 1 extra root vnode reference (nullm_rootvp). */ - error = vflush(mp, rootrefs, flags, curthread); + error = vflush(mp, 0, flags, curthread); if (error) return (error); MNT_ILOCK(mp); @@ -273,6 +261,7 @@ nullfs_unmount(mp, mntflags) TAILQ_REMOVE(&ump->mnt_uppers, mp, mnt_upper_link); MNT_IUNLOCK(ump); } + vrele(mntdata->nullm_lowerrootvp); mp->mnt_data = NULL; free(mntdata, M_NULLFSMNT); return (0); @@ -285,21 +274,24 @@ nullfs_root(mp, flags, vpp) struct vnode **vpp; { struct vnode *vp; + struct null_mount *mntdata; + int error; - NULLFSDEBUG("nullfs_root(mp = %p, vp = %p->%p)\n", (void *)mp, - (void *)MOUNTTONULLMOUNT(mp)->nullm_rootvp, - (void *)NULLVPTOLOWERVP(MOUNTTONULLMOUNT(mp)->nullm_rootvp)); + mntdata = MOUNTTONULLMOUNT(mp); + NULLFSDEBUG("nullfs_root(mp = %p, vp = %p)\n", mp, + mntdata->nullm_lowerrootvp); - /* - * Return locked reference to root. - */ - vp = MOUNTTONULLMOUNT(mp)->nullm_rootvp; - VREF(vp); - - ASSERT_VOP_UNLOCKED(vp, "root vnode is locked"); - vn_lock(vp, flags | LK_RETRY); - *vpp = vp; - return 0; + error = vget(mntdata->nullm_lowerrootvp, (flags & ~LK_TYPE_MASK) | + LK_EXCLUSIVE, curthread); + if (error == 0) { + error = null_nodeget(mp, mntdata->nullm_lowerrootvp, &vp); + if (error == 0) { + if ((flags & LK_TYPE_MASK) == LK_SHARED) + vn_lock(vp, LK_DOWNGRADE | LK_RETRY); + *vpp = vp; + } + } + return (error); } static int
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202001281129.00SBT6AJ063186>