Date: Tue, 27 Jul 2021 16:58:57 GMT From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: d5b078163e0d - main - null_bypass(): prevent loosing the only reference to the lower vnode Message-ID: <202107271658.16RGwvF2083492@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=d5b078163e0d6bb2fe36f8e49a44853908d5e2db commit d5b078163e0d6bb2fe36f8e49a44853908d5e2db Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-07-20 00:53:08 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2021-07-27 16:58:47 +0000 null_bypass(): prevent loosing the only reference to the lower vnode The upper vnode reference to the lower vnode is the only reference that keeps our pointer to the lower vnode alive. If lower vnode is relocked during the VOP call, upper vnode might become unlocked and reclaimed, which invalidates our reference. Add a transient vhold around VOP call. Reported and tested by: pho Reviewed by: markj Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D31310 --- sys/fs/nullfs/null_vnops.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c index 0d5092f5e33d..47b0bda32b6b 100644 --- a/sys/fs/nullfs/null_vnops.c +++ b/sys/fs/nullfs/null_vnops.c @@ -266,6 +266,17 @@ null_bypass(struct vop_generic_args *ap) old_vps[i] = *this_vp_p; *(vps_p[i]) = NULLVPTOLOWERVP(*this_vp_p); + /* + * The upper vnode reference to the lower + * vnode is the only reference that keeps our + * pointer to the lower vnode alive. If lower + * vnode is relocked during the VOP call, + * upper vnode might become unlocked and + * reclaimed, which invalidates our reference. + * Add a transient hold around VOP call. + */ + vhold(*this_vp_p); + /* * XXX - Several operations have the side effect * of vrele'ing their vp's. We must account for @@ -300,6 +311,7 @@ null_bypass(struct vop_generic_args *ap) lvp = *(vps_p[i]); /* + * Get rid of the transient hold on lvp. * If lowervp was unlocked during VOP * operation, nullfs upper vnode could have * been reclaimed, which changes its v_vnlock @@ -307,11 +319,14 @@ null_bypass(struct vop_generic_args *ap) * must move lock ownership from lower to * upper (reclaimed) vnode. */ - if (lvp != NULLVP && - VOP_ISLOCKED(lvp) == LK_EXCLUSIVE && - old_vps[i]->v_vnlock != lvp->v_vnlock) { - VOP_UNLOCK(lvp); - VOP_LOCK(old_vps[i], LK_EXCLUSIVE | LK_RETRY); + if (lvp != NULLVP) { + if (VOP_ISLOCKED(lvp) == LK_EXCLUSIVE && + old_vps[i]->v_vnlock != lvp->v_vnlock) { + VOP_UNLOCK(lvp); + VOP_LOCK(old_vps[i], LK_EXCLUSIVE | + LK_RETRY); + } + vdrop(lvp); } *(vps_p[i]) = old_vps[i];
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107271658.16RGwvF2083492>