From owner-svn-src-all@freebsd.org Tue Jan 28 11:29:07 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 7FF6A1FF8C8; Tue, 28 Jan 2020 11:29:07 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 486PX32v8zz3JMD; Tue, 28 Jan 2020 11:29:07 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5E86E2235C; Tue, 28 Jan 2020 11:29:07 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 00SBT7TU063191; Tue, 28 Jan 2020 11:29:07 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 00SBT6AJ063186; Tue, 28 Jan 2020 11:29:06 GMT (envelope-from kib@FreeBSD.org) Message-Id: <202001281129.00SBT6AJ063186@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Tue, 28 Jan 2020 11:29:06 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357199 - head/sys/fs/nullfs X-SVN-Group: head X-SVN-Commit-Author: kib X-SVN-Commit-Paths: head/sys/fs/nullfs X-SVN-Commit-Revision: 357199 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jan 2020 11:29:07 -0000 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