Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 5 Apr 2017 17:11:39 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r316529 - head/sys/fs/nfsclient
Message-ID:  <201704051711.v35HBdWF072551@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Wed Apr  5 17:11:39 2017
New Revision: 316529
URL: https://svnweb.freebsd.org/changeset/base/316529

Log:
  Handle possible vnode reclamation after ncl_vinvalbuf() call.
  
  ncl_vinvalbuf() might need to upgrade vnode lock, allowing the vnode
  to be reclaimed by other thread.  Handle the situation, indicated by
  the returned error zero and VI_DOOMED iflag set, converting it into
  EBADF.  Handle all calls, even where the vnode is exclusively locked
  right now.
  
  Reviewed by:	markj, rmacklem
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks
  X-Differential revision:	https://reviews.freebsd.org/D10241

Modified:
  head/sys/fs/nfsclient/nfs_clbio.c
  head/sys/fs/nfsclient/nfs_clvnops.c

Modified: head/sys/fs/nfsclient/nfs_clbio.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clbio.c	Wed Apr  5 16:57:53 2017	(r316528)
+++ head/sys/fs/nfsclient/nfs_clbio.c	Wed Apr  5 17:11:39 2017	(r316529)
@@ -394,8 +394,8 @@ nfs_bioread_check_cons(struct vnode *vp,
 	 */
 	old_lock = ncl_upgrade_vnlock(vp);
 	if (vp->v_iflag & VI_DOOMED) {
-		ncl_downgrade_vnlock(vp, old_lock);
-		return (EBADF);
+		error = EBADF;
+		goto out;
 	}
 
 	mtx_lock(&np->n_mtx);
@@ -406,7 +406,9 @@ nfs_bioread_check_cons(struct vnode *vp,
 				panic("nfs: bioread, not dir");
 			ncl_invaldir(vp);
 			error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
-			if (error)
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				error = EBADF;
+			if (error != 0)
 				goto out;
 		}
 		np->n_attrstamp = 0;
@@ -429,7 +431,9 @@ nfs_bioread_check_cons(struct vnode *vp,
 			if (vp->v_type == VDIR)
 				ncl_invaldir(vp);
 			error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
-			if (error)
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				error = EBADF;
+			if (error != 0)
 				goto out;
 			mtx_lock(&np->n_mtx);
 			np->n_mtime = vattr.va_mtime;
@@ -439,7 +443,7 @@ nfs_bioread_check_cons(struct vnode *vp,
 	}
 out:
 	ncl_downgrade_vnlock(vp, old_lock);
-	return error;
+	return (error);
 }
 
 /*
@@ -623,6 +627,9 @@ ncl_bioread(struct vnode *vp, struct uio
 		    while (error == NFSERR_BAD_COOKIE) {
 			ncl_invaldir(vp);
 			error = ncl_vinvalbuf(vp, 0, td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				return (EBADF);
+
 			/*
 			 * Yuck! The directory has been modified on the
 			 * server. The only way to get the block is by
@@ -943,8 +950,11 @@ ncl_write(struct vop_write_args *ap)
 #endif
 			np->n_attrstamp = 0;
 			KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp);
-			error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
-			if (error)
+			error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag &
+			    IO_VMIO) != 0 ? V_VMIO : 0), td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				error = EBADF;
+			if (error != 0)
 				return (error);
 		} else
 			mtx_unlock(&np->n_mtx);
@@ -1023,8 +1033,12 @@ ncl_write(struct vop_write_args *ap)
 			if (wouldcommit > nmp->nm_wcommitsize) {
 				np->n_attrstamp = 0;
 				KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp);
-				error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
-				if (error)
+				error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag &
+				    IO_VMIO) != 0 ? V_VMIO : 0), td, 1);
+				if (error == 0 &&
+				    (vp->v_iflag & VI_DOOMED) != 0)
+					error = EBADF;
+				if (error != 0)
 					return (error);
 				wouldcommit = biosize;
 			}

Modified: head/sys/fs/nfsclient/nfs_clvnops.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clvnops.c	Wed Apr  5 16:57:53 2017	(r316528)
+++ head/sys/fs/nfsclient/nfs_clvnops.c	Wed Apr  5 17:11:39 2017	(r316529)
@@ -520,6 +520,8 @@ nfs_open(struct vop_open_args *ap)
 	if (np->n_flag & NMODIFIED) {
 		mtx_unlock(&np->n_mtx);
 		error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
+		if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+			return (EBADF);
 		if (error == EINTR || error == EIO) {
 			if (NFS_ISV4(vp))
 				(void) nfsrpc_close(vp, 0, ap->a_td);
@@ -556,6 +558,8 @@ nfs_open(struct vop_open_args *ap)
 				np->n_direofoffset = 0;
 			mtx_unlock(&np->n_mtx);
 			error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				return (EBADF);
 			if (error == EINTR || error == EIO) {
 				if (NFS_ISV4(vp))
 					(void) nfsrpc_close(vp, 0, ap->a_td);
@@ -576,6 +580,8 @@ nfs_open(struct vop_open_args *ap)
 		if (np->n_directio_opens == 0) {
 			mtx_unlock(&np->n_mtx);
 			error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				return (EBADF);
 			if (error) {
 				if (NFS_ISV4(vp))
 					(void) nfsrpc_close(vp, 0, ap->a_td);
@@ -714,8 +720,11 @@ nfs_close(struct vop_close_args *ap)
 				 * np->n_flag &= ~NMODIFIED;
 				 */
 			}
-		} else
-		    error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
+		} else {
+			error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				return (EBADF);
+		}
 		mtx_lock(&np->n_mtx);
 	    }
  	    /* 
@@ -931,13 +940,13 @@ nfs_setattr(struct vop_setattr_args *ap)
  			if (np->n_flag & NMODIFIED) {
 			    tsize = np->n_size;
 			    mtx_unlock(&np->n_mtx);
- 			    if (vap->va_size == 0)
- 				error = ncl_vinvalbuf(vp, 0, td, 1);
- 			    else
- 				error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
- 			    if (error) {
-				vnode_pager_setsize(vp, tsize);
-				return (error);
+			    error = ncl_vinvalbuf(vp, vap->va_size == 0 ?
+			        0 : V_SAVE, td, 1);
+			    if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				    error = EBADF;
+ 			    if (error != 0) {
+				    vnode_pager_setsize(vp, tsize);
+				    return (error);
 			    }
 			    /*
 			     * Call nfscl_delegmodtime() to set the modify time
@@ -961,8 +970,10 @@ nfs_setattr(struct vop_setattr_args *ap)
 		if ((vap->va_mtime.tv_sec != VNOVAL || vap->va_atime.tv_sec != VNOVAL) && 
 		    (np->n_flag & NMODIFIED) && vp->v_type == VREG) {
 			mtx_unlock(&np->n_mtx);
-			if ((error = ncl_vinvalbuf(vp, V_SAVE, td, 1)) != 0 &&
-			    (error == EINTR || error == EIO))
+			error = ncl_vinvalbuf(vp, V_SAVE, td, 1);
+			if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+				return (EBADF);
+			if (error == EINTR || error == EIO)
 				return (error);
 		} else
 			mtx_unlock(&np->n_mtx);
@@ -1665,8 +1676,10 @@ nfs_remove(struct vop_remove_args *ap)
 		 * unnecessary delayed writes later.
 		 */
 		error = ncl_vinvalbuf(vp, 0, cnp->cn_thread, 1);
-		/* Do the rpc */
-		if (error != EINTR && error != EIO)
+		if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0)
+			error = EBADF;
+		else if (error != EINTR && error != EIO)
+			/* Do the rpc */
 			error = nfs_removerpc(dvp, vp, cnp->cn_nameptr,
 			    cnp->cn_namelen, cnp->cn_cred, cnp->cn_thread);
 		/*
@@ -3055,6 +3068,10 @@ nfs_advlock(struct vop_advlock_args *ap)
 			if ((np->n_flag & NMODIFIED) || ret ||
 			    np->n_change != va.va_filerev) {
 				(void) ncl_vinvalbuf(vp, V_SAVE, td, 1);
+				if ((vp->v_iflag & VI_DOOMED) != 0) {
+					NFSVOPUNLOCK(vp, 0);
+					return (EBADF);
+				}
 				np->n_attrstamp = 0;
 				KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp);
 				ret = VOP_GETATTR(vp, &va, cred);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201704051711.v35HBdWF072551>