The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=a01ca46b9b86608bfa080a4d86d586beb9e756c9
commit a01ca46b9b86608bfa080a4d86d586beb9e756c9 Author: Jason A. Harmening <[email protected]> AuthorDate: 2022-01-17 01:03:54 +0000 Commit: Jason A. Harmening <[email protected]> CommitDate: 2022-01-30 04:38:44 +0000 unionfs: use VV_ROOT to check for root vnode in unionfs_lock() This avoids a potentially wild reference to the mount object. Additionally, simplify some of the checks around VV_ROOT in unionfs_nodeget(). Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D33914 --- sys/fs/unionfs/union_subr.c | 19 +++++++++++-------- sys/fs/unionfs/union_vfsops.c | 5 +++++ sys/fs/unionfs/union_vnops.c | 11 ++--------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index e051d42c07cc..2b5754a560c7 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -334,12 +334,6 @@ unionfs_nodeget(struct mount *mp, struct vnode *uppervp, } } - if ((uppervp == NULLVP || ump->um_uppervp != uppervp) || - (lowervp == NULLVP || ump->um_lowervp != lowervp)) { - /* dvp will be NULLVP only in case of root vnode. */ - if (dvp == NULLVP) - return (EINVAL); - } unp = malloc(sizeof(struct unionfs_node), M_UNIONFSNODE, M_WAITOK | M_ZERO); @@ -381,9 +375,18 @@ unionfs_nodeget(struct mount *mp, struct vnode *uppervp, vp->v_type = vt; vp->v_data = unp; - if ((uppervp != NULLVP && ump->um_uppervp == uppervp) && - (lowervp != NULLVP && ump->um_lowervp == lowervp)) + /* + * TODO: This is an imperfect check, as there's no guarantee that + * the underlying filesystems will always return vnode pointers + * for the root inodes that match our cached values. To reduce + * the likelihood of failure, for example in the case where either + * vnode has been forcibly doomed, we check both pointers and set + * VV_ROOT if either matches. + */ + if (ump->um_uppervp == uppervp || ump->um_lowervp == lowervp) vp->v_vflag |= VV_ROOT; + KASSERT(dvp != NULL || (vp->v_vflag & VV_ROOT) != 0, + ("%s: NULL dvp for non-root vp %p", __func__, vp)); vn_lock_pair(lowervp, false, uppervp, false); error = insmntque1(vp, mp, NULL, NULL); diff --git a/sys/fs/unionfs/union_vfsops.c b/sys/fs/unionfs/union_vfsops.c index ea49d1e189da..dd6e7084ebc1 100644 --- a/sys/fs/unionfs/union_vfsops.c +++ b/sys/fs/unionfs/union_vfsops.c @@ -241,6 +241,8 @@ unionfs_domount(struct mount *mp) /* get root vnodes */ lowerrootvp = mp->mnt_vnodecovered; upperrootvp = ndp->ni_vp; + KASSERT(lowerrootvp != NULL, ("%s: NULL lower root vp", __func__)); + KASSERT(upperrootvp != NULL, ("%s: NULL upper root vp", __func__)); /* create unionfs_mount */ ump = malloc(sizeof(struct unionfs_mount), M_UNIONFSMNT, @@ -289,6 +291,9 @@ unionfs_domount(struct mount *mp) mp->mnt_data = NULL; return (error); } + KASSERT(ump->um_rootvp != NULL, ("rootvp cannot be NULL")); + KASSERT((ump->um_rootvp->v_vflag & VV_ROOT) != 0, + ("%s: rootvp without VV_ROOT", __func__)); lowermp = vfs_register_upper_from_vp(ump->um_lowervp, mp, &ump->um_lower_link); diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c index 19779112941e..c9d8aac12362 100644 --- a/sys/fs/unionfs/union_vnops.c +++ b/sys/fs/unionfs/union_vnops.c @@ -1908,8 +1908,6 @@ unionfs_revlock(struct vnode *vp, int flags) static int unionfs_lock(struct vop_lock1_args *ap) { - struct mount *mp; - struct unionfs_mount *ump; struct unionfs_node *unp; struct vnode *vp; struct vnode *uvp; @@ -1944,13 +1942,8 @@ unionfs_lock(struct vop_lock1_args *ap) if ((flags & LK_INTERLOCK) == 0) VI_LOCK(vp); - mp = vp->v_mount; - if (mp == NULL) - goto unionfs_lock_null_vnode; - - ump = MOUNTTOUNIONFSMOUNT(mp); unp = VTOUNIONFS(vp); - if (ump == NULL || unp == NULL) + if (unp == NULL) goto unionfs_lock_null_vnode; lvp = unp->un_lowervp; uvp = unp->un_uppervp; @@ -1967,7 +1960,7 @@ unionfs_lock(struct vop_lock1_args *ap) * (ex. vfs_domount: mounted vnode is already locked.) */ if ((flags & LK_TYPE_MASK) == LK_EXCLUSIVE && - vp == ump->um_rootvp) + (vp->v_vflag & VV_ROOT) != 0) flags |= LK_CANRECURSE; if (lvp != NULLVP) {
