git: d5b078163e0d - main - null_bypass(): prevent loosing the only reference to the lower vnode

[Konstantin Belousov]

The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=d5b078163e0d6bb2fe36f8e49a44853908d5e2db

commit d5b078163e0d6bb2fe36f8e49a44853908d5e2db
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2021-07-20 00:53:08 +0000
Commit:     Konstantin Belousov <kib at 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];