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
This commit is contained in:
Jason A. Harmening 2022-01-16 17:03:54 -08:00
parent 67c58cd729
commit a01ca46b9b
3 changed files with 18 additions and 17 deletions

View File

@ -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);

View File

@ -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);

View File

@ -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) {