git: fcb164742b6f - main - unionfs: rework unionfs_getwritemount()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 24 Feb 2022 04:00:38 UTC
The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=fcb164742b6fb3c0f0d5995f90955acb6706d73e commit fcb164742b6fb3c0f0d5995f90955acb6706d73e Author: Jason A. Harmening <jah@FreeBSD.org> AuthorDate: 2022-02-15 03:52:21 +0000 Commit: Jason A. Harmening <jah@FreeBSD.org> CommitDate: 2022-02-24 04:10:02 +0000 unionfs: rework unionfs_getwritemount() VOP_GETWRITEMOUNT() is called on the vn_start_write() path without any vnode locks guaranteed to be held. It's therefore unsafe to blindly access per-mount and per-vnode data. Instead, follow the approach taken by nullfs and use the vnode interlock coupled with the hold count to ensure the mount and the vnode won't be recycled while they are being accessed. Reviewed by: kib (earlier version), markj, pho Tested by: pho Differential Revision: https://reviews.freebsd.org/D34282 --- sys/fs/unionfs/union_vnops.c | 49 ++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c index 8e881ffd19bb..dcea0c27abd5 100644 --- a/sys/fs/unionfs/union_vnops.c +++ b/sys/fs/unionfs/union_vnops.c @@ -1764,33 +1764,52 @@ unionfs_readlink(struct vop_readlink_args *ap) static int unionfs_getwritemount(struct vop_getwritemount_args *ap) { + struct unionfs_node *unp; struct vnode *uvp; - struct vnode *vp; + struct vnode *vp, *ovp; int error; UNIONFS_INTERNAL_DEBUG("unionfs_getwritemount: enter\n"); error = 0; vp = ap->a_vp; + uvp = NULLVP; - if (vp == NULLVP || (vp->v_mount->mnt_flag & MNT_RDONLY)) - return (EACCES); - - KASSERT_UNIONFS_VNODE(vp); - - uvp = UNIONFSVPTOUPPERVP(vp); - if (uvp == NULLVP && VREG == vp->v_type) - uvp = UNIONFSVPTOUPPERVP(VTOUNIONFS(vp)->un_dvp); + VI_LOCK(vp); + unp = VTOUNIONFS(vp); + if (unp != NULL) + uvp = unp->un_uppervp; - if (uvp != NULLVP) - error = VOP_GETWRITEMOUNT(uvp, ap->a_mpp); - else { + /* + * If our node has no upper vnode, check the parent directory. + * We may be initiating a write operation that will produce a + * new upper vnode through CoW. + */ + if (uvp == NULLVP && unp != NULL) { + ovp = vp; + vp = unp->un_dvp; + /* + * Only the root vnode should have an empty parent, but it + * should not have an empty uppervp, so we shouldn't get here. + */ + VNASSERT(vp != NULL, ovp, ("%s: NULL parent vnode", __func__)); + VI_UNLOCK(ovp); VI_LOCK(vp); - if (vp->v_holdcnt == 0) - error = EOPNOTSUPP; - else + unp = VTOUNIONFS(vp); + if (unp != NULL) + uvp = unp->un_uppervp; + if (uvp == NULLVP) error = EACCES; + } + + if (uvp != NULLVP) { + vholdnz(uvp); + VI_UNLOCK(vp); + error = VOP_GETWRITEMOUNT(uvp, ap->a_mpp); + vdrop(uvp); + } else { VI_UNLOCK(vp); + *(ap->a_mpp) = NULL; } UNIONFS_INTERNAL_DEBUG("unionfs_getwritemount: leave (%d)\n", error);