The branch main has been updated by jah:
URL:
https://cgit.FreeBSD.org/src/commit/?id=fcb164742b6fb3c0f0d5995f90955acb6706d73e
commit fcb164742b6fb3c0f0d5995f90955acb6706d73e
Author: Jason A. Harmening <j...@freebsd.org>
AuthorDate: 2022-02-15 03:52:21 +0000
Commit: Jason A. Harmening <j...@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);
