Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 27 May 2012 14:20:47 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r236150 - in stable/8/sys: fs/nfsclient nfsclient
Message-ID:  <201205271420.q4REKl88054037@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rmacklem
Date: Sun May 27 14:20:46 2012
New Revision: 236150
URL: http://svn.freebsd.org/changeset/base/236150

Log:
  MFC: r235332
  PR# 165923 reported intermittent write failures for dirty
  memory mapped pages being written back on an NFS mount.
  Since any thread can call VOP_PUTPAGES() to write back a
  dirty page, the credentials of that thread may not have
  write access to the file on an NFS server. (Often the uid
  is 0, which may be mapped to "nobody" in the NFS server.)
  Although there is no completely correct fix for this
  (NFS servers check access on every write RPC instead of at
  open/mmap time), this patch avoids the common cases by
  holding onto a credential that recently opened the file
  for writing and uses that credential for the write RPCs
  being done by VOP_PUTPAGES() for both NFS clients.

Modified:
  stable/8/sys/fs/nfsclient/nfs_clbio.c
  stable/8/sys/fs/nfsclient/nfs_clnode.c
  stable/8/sys/fs/nfsclient/nfs_clvnops.c
  stable/8/sys/fs/nfsclient/nfsnode.h
  stable/8/sys/nfsclient/nfs_bio.c
  stable/8/sys/nfsclient/nfs_node.c
  stable/8/sys/nfsclient/nfs_vnops.c
  stable/8/sys/nfsclient/nfsnode.h
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/boot/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/e1000/   (props changed)

Modified: stable/8/sys/fs/nfsclient/nfs_clbio.c
==============================================================================
--- stable/8/sys/fs/nfsclient/nfs_clbio.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/fs/nfsclient/nfs_clbio.c	Sun May 27 14:20:46 2012	(r236150)
@@ -271,7 +271,11 @@ ncl_putpages(struct vop_putpages_args *a
 	vp = ap->a_vp;
 	np = VTONFS(vp);
 	td = curthread;				/* XXX */
-	cred = curthread->td_ucred;		/* XXX */
+	/* Set the cred to n_writecred for the write rpcs. */
+	if (np->n_writecred != NULL)
+		cred = crhold(np->n_writecred);
+	else
+		cred = crhold(curthread->td_ucred);	/* XXX */
 	nmp = VFSTONFS(vp->v_mount);
 	pages = ap->a_m;
 	count = ap->a_count;
@@ -335,6 +339,7 @@ ncl_putpages(struct vop_putpages_args *a
 	    iomode = NFSWRITE_FILESYNC;
 
 	error = ncl_writerpc(vp, &uio, cred, &iomode, &must_commit, 0);
+	crfree(cred);
 
 	pmap_qremove(kva, npages);
 	relpbuf(bp, &ncl_pbuf_freecnt);

Modified: stable/8/sys/fs/nfsclient/nfs_clnode.c
==============================================================================
--- stable/8/sys/fs/nfsclient/nfs_clnode.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/fs/nfsclient/nfs_clnode.c	Sun May 27 14:20:46 2012	(r236150)
@@ -297,6 +297,8 @@ ncl_reclaim(struct vop_reclaim_args *ap)
 			FREE((caddr_t)dp2, M_NFSDIROFF);
 		}
 	}
+	if (np->n_writecred != NULL)
+		crfree(np->n_writecred);
 	FREE((caddr_t)np->n_fhp, M_NFSFH);
 	if (np->n_v4 != NULL)
 		FREE((caddr_t)np->n_v4, M_NFSV4NODE);

Modified: stable/8/sys/fs/nfsclient/nfs_clvnops.c
==============================================================================
--- stable/8/sys/fs/nfsclient/nfs_clvnops.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/fs/nfsclient/nfs_clvnops.c	Sun May 27 14:20:46 2012	(r236150)
@@ -480,6 +480,7 @@ nfs_open(struct vop_open_args *ap)
 	struct vattr vattr;
 	int error;
 	int fmode = ap->a_mode;
+	struct ucred *cred;
 
 	if (vp->v_type != VREG && vp->v_type != VDIR && vp->v_type != VLNK)
 		return (EOPNOTSUPP);
@@ -570,7 +571,22 @@ nfs_open(struct vop_open_args *ap)
 		}
 		np->n_directio_opens++;
 	}
+
+	/*
+	 * If this is an open for writing, capture a reference to the
+	 * credentials, so they can be used by ncl_putpages(). Using
+	 * these write credentials is preferable to the credentials of
+	 * whatever thread happens to be doing the VOP_PUTPAGES() since
+	 * the write RPCs are less likely to fail with EACCES.
+	 */
+	if ((fmode & FWRITE) != 0) {
+		cred = np->n_writecred;
+		np->n_writecred = crhold(ap->a_cred);
+	} else
+		cred = NULL;
 	mtx_unlock(&np->n_mtx);
+	if (cred != NULL)
+		crfree(cred);
 	vnode_create_vobject(vp, vattr.va_size, ap->a_td);
 	return (0);
 }

Modified: stable/8/sys/fs/nfsclient/nfsnode.h
==============================================================================
--- stable/8/sys/fs/nfsclient/nfsnode.h	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/fs/nfsclient/nfsnode.h	Sun May 27 14:20:46 2012	(r236150)
@@ -136,6 +136,7 @@ struct nfsnode {
 	struct nfsv4node	*n_v4;		/* extra V4 stuff */
 	struct timespec		n_unused4;
 	struct timespec		n_unused5;
+	struct ucred		*n_writecred;	/* Cred. for putpages */
 };
 
 #define	n_atim		n_un1.nf_atim

Modified: stable/8/sys/nfsclient/nfs_bio.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_bio.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/nfsclient/nfs_bio.c	Sun May 27 14:20:46 2012	(r236150)
@@ -268,7 +268,11 @@ nfs_putpages(struct vop_putpages_args *a
 	vp = ap->a_vp;
 	np = VTONFS(vp);
 	td = curthread;				/* XXX */
-	cred = curthread->td_ucred;		/* XXX */
+	/* Set the cred to n_writecred for the write rpcs. */
+	if (np->n_writecred != NULL)
+		cred = crhold(np->n_writecred);
+	else
+		cred = crhold(curthread->td_ucred);	/* XXX */
 	nmp = VFSTONFS(vp->v_mount);
 	pages = ap->a_m;
 	count = ap->a_count;
@@ -332,6 +336,7 @@ nfs_putpages(struct vop_putpages_args *a
 	    iomode = NFSV3WRITE_FILESYNC;
 
 	error = (nmp->nm_rpcops->nr_writerpc)(vp, &uio, cred, &iomode, &must_commit);
+	crfree(cred);
 
 	pmap_qremove(kva, npages);
 	relpbuf(bp, &nfs_pbuf_freecnt);

Modified: stable/8/sys/nfsclient/nfs_node.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_node.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/nfsclient/nfs_node.c	Sun May 27 14:20:46 2012	(r236150)
@@ -270,6 +270,8 @@ nfs_reclaim(struct vop_reclaim_args *ap)
 			free((caddr_t)dp2, M_NFSDIROFF);
 		}
 	}
+	if (np->n_writecred != NULL)
+		crfree(np->n_writecred);
 	if (np->n_fhsize > NFS_SMALLFH) {
 		free((caddr_t)np->n_fhp, M_NFSBIGFH);
 	}

Modified: stable/8/sys/nfsclient/nfs_vnops.c
==============================================================================
--- stable/8/sys/nfsclient/nfs_vnops.c	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/nfsclient/nfs_vnops.c	Sun May 27 14:20:46 2012	(r236150)
@@ -510,6 +510,7 @@ nfs_open(struct vop_open_args *ap)
 	struct vattr vattr;
 	int error;
 	int fmode = ap->a_mode;
+	struct ucred *cred;
 
 	if (vp->v_type != VREG && vp->v_type != VDIR && vp->v_type != VLNK)
 		return (EOPNOTSUPP);
@@ -566,7 +567,22 @@ nfs_open(struct vop_open_args *ap)
 		}
 		np->n_directio_opens++;
 	}
+
+	/*
+	 * If this is an open for writing, capture a reference to the
+	 * credentials, so they can be used by nfs_putpages(). Using
+	 * these write credentials is preferable to the credentials of
+	 * whatever thread happens to be doing the VOP_PUTPAGES() since
+	 * the write RPCs are less likely to fail with EACCES.
+	 */
+	if ((fmode & FWRITE) != 0) {
+		cred = np->n_writecred;
+		np->n_writecred = crhold(ap->a_cred);
+	} else
+		cred = NULL;
 	mtx_unlock(&np->n_mtx);
+	if (cred != NULL)
+		crfree(cred);
 	vnode_create_vobject(vp, vattr.va_size, ap->a_td);
 	return (0);
 }

Modified: stable/8/sys/nfsclient/nfsnode.h
==============================================================================
--- stable/8/sys/nfsclient/nfsnode.h	Sun May 27 13:33:54 2012	(r236149)
+++ stable/8/sys/nfsclient/nfsnode.h	Sun May 27 14:20:46 2012	(r236150)
@@ -141,6 +141,7 @@ struct nfsnode {
 	struct nfs_attrcache_timestamp n_unused;
 	struct timespec		n_unused4;
 	struct timespec		n_unused5;
+	struct ucred		*n_writecred;	/* Cred. for putpages */
 };
 
 #define n_atim		n_un1.nf_atim



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