From nobody Thu Oct 14 02:19:42 2021 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 31A9217FE472; Thu, 14 Oct 2021 02:19:43 +0000 (UTC) (envelope-from git@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) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HVClg0ZSLz3rTH; Thu, 14 Oct 2021 02:19:43 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E4DA8319A; Thu, 14 Oct 2021 02:19:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19E2Jgsd026796; Thu, 14 Oct 2021 02:19:42 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19E2Jgbr026795; Thu, 14 Oct 2021 02:19:42 GMT (envelope-from git) Date: Thu, 14 Oct 2021 02:19:42 GMT Message-Id: <202110140219.19E2Jgbr026795@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Jason A. Harmening" Subject: git: 152c35ee4fda - main - unionfs: Ensure SAVENAME is set for unionfs vnode operations List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jah X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 152c35ee4fdad39d30789fd01cd2fc723ba9568b Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=152c35ee4fdad39d30789fd01cd2fc723ba9568b commit 152c35ee4fdad39d30789fd01cd2fc723ba9568b Author: Jason A. Harmening AuthorDate: 2021-09-26 02:56:04 +0000 Commit: Jason A. Harmening CommitDate: 2021-10-14 02:25:31 +0000 unionfs: Ensure SAVENAME is set for unionfs vnode operations "rm-style" system calls such as kern_frmdirat() and kern_funlinkat() don't supply SAVENAME to preserve the pathname buffer for subsequent vnode ops. For unionfs this poses an issue because the pathname may be needed for a relookup operation in unionfs_remove()/unionfs_rmdir(). Currently unionfs doesn't check for this case, leading to a panic on DIAGNOSTIC kernels and use-after-free of cn_nameptr otherwise. The unionfs node's stored buffer would suffice as a replacement for cnp->cn_nameptr in some (but not all) cases, but it's cleaner to just ensure that unionfs vnode ops always have a valid cn_nameptr by setting SAVENAME in unionfs_lookup(). While here, do some light cleanup in unionfs_lookup() and assert that HASBUF is always present in the relevant relookup calls. Reported by: pho Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D32148 --- sys/fs/unionfs/union_subr.c | 6 ++++++ sys/fs/unionfs/union_vnops.c | 35 ++++++++++++++++++----------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index eb7681df645b..380a61819f1a 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -678,6 +678,8 @@ unionfs_relookup_for_create(struct vnode *dvp, struct componentname *cnp, udvp = UNIONFSVPTOUPPERVP(dvp); vp = NULLVP; + KASSERT((cnp->cn_flags & HASBUF) != 0, + ("%s called without HASBUF", __func__)); error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr, cnp->cn_namelen, CREATE); if (error) @@ -712,6 +714,8 @@ unionfs_relookup_for_delete(struct vnode *dvp, struct componentname *cnp, udvp = UNIONFSVPTOUPPERVP(dvp); vp = NULLVP; + KASSERT((cnp->cn_flags & HASBUF) != 0, + ("%s called without HASBUF", __func__)); error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr, cnp->cn_namelen, DELETE); if (error) @@ -746,6 +750,8 @@ unionfs_relookup_for_rename(struct vnode *dvp, struct componentname *cnp, udvp = UNIONFSVPTOUPPERVP(dvp); vp = NULLVP; + KASSERT((cnp->cn_flags & HASBUF) != 0, + ("%s called without HASBUF", __func__)); error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr, cnp->cn_namelen, RENAME); if (error) diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c index eb74192fd504..b6c088fb3804 100644 --- a/sys/fs/unionfs/union_vnops.c +++ b/sys/fs/unionfs/union_vnops.c @@ -167,9 +167,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) } else if (error == ENOENT && (cnflags & MAKEENTRY) != 0) cache_enter(dvp, NULLVP, cnp); - UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n", error); - - return (error); + goto unionfs_lookup_return; } /* @@ -184,10 +182,8 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) *(ap->a_vpp) = dvp; vref(dvp); - UNIONFS_INTERNAL_DEBUG( - "unionfs_lookup: leave (%d)\n", uerror); - - return (uerror); + error = uerror; + goto unionfs_lookup_return; } if (nameiop == DELETE || nameiop == RENAME || (cnp->cn_lkflags & LK_TYPE_MASK)) @@ -246,9 +242,8 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) * check lookup result */ if (uvp == NULLVP && lvp == NULLVP) { - UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n", - (udvp != NULLVP ? uerror : lerror)); - return (udvp != NULLVP ? uerror : lerror); + error = (udvp != NULLVP ? uerror : lerror); + goto unionfs_lookup_return; } /* @@ -270,7 +265,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) error = unionfs_nodeget(dvp->v_mount, NULLVP, lvp, dvp, &vp, cnp, td); if (error != 0) - goto unionfs_lookup_out; + goto unionfs_lookup_cleanup; if (LK_SHARED == (cnp->cn_lkflags & LK_TYPE_MASK)) VOP_UNLOCK(vp); @@ -289,7 +284,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) vput(vp); else vrele(vp); - goto unionfs_lookup_out; + goto unionfs_lookup_cleanup; } if ((cnp->cn_lkflags & LK_TYPE_MASK) == LK_SHARED) vn_lock(vp, LK_SHARED | LK_RETRY); @@ -303,7 +298,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) else error = lerror; if (error != 0) - goto unionfs_lookup_out; + goto unionfs_lookup_cleanup; /* * get socket vnode. */ @@ -328,7 +323,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) if (error != 0) { UNIONFSDEBUG( "unionfs_lookup: Unable to create unionfs vnode."); - goto unionfs_lookup_out; + goto unionfs_lookup_cleanup; } if ((nameiop == DELETE || nameiop == RENAME) && (cnp->cn_lkflags & LK_TYPE_MASK) == 0) @@ -340,7 +335,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap) if ((cnflags & MAKEENTRY) && vp->v_type != VSOCK) cache_enter(dvp, vp, cnp); -unionfs_lookup_out: +unionfs_lookup_cleanup: if (uvp != NULLVP) vrele(uvp); if (lvp != NULLVP) @@ -349,6 +344,12 @@ unionfs_lookup_out: if (error == ENOENT && (cnflags & MAKEENTRY) != 0) cache_enter(dvp, NULLVP, cnp); +unionfs_lookup_return: + + /* Ensure subsequent vnops will get a valid pathname buffer. */ + if (nameiop != LOOKUP && (error == 0 || error == EJUSTRETURN)) + cnp->cn_flags |= SAVENAME; + UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n", error); return (error); @@ -1372,7 +1373,7 @@ unionfs_mkdir(struct vop_mkdir_args *ap) error = VOP_GETATTR(udvp, &va, cnp->cn_cred); if (error != 0) return (error); - if (va.va_flags & OPAQUE) + if ((va.va_flags & OPAQUE) != 0) cnp->cn_flags |= ISWHITEOUT; } @@ -1434,7 +1435,7 @@ unionfs_rmdir(struct vop_rmdir_args *ap) if (ump->um_whitemode == UNIONFS_WHITE_ALWAYS || lvp != NULLVP) cnp->cn_flags |= DOWHITEOUT; error = unionfs_relookup_for_delete(ap->a_dvp, cnp, td); - if (!error) + if (error == 0) error = VOP_RMDIR(udvp, uvp, cnp); } else if (lvp != NULLVP)