From 7af1242a34b8ebb993ee917a2dfb7fc7bbab60c8 Mon Sep 17 00:00:00 2001 From: Rick Macklem Date: Sat, 12 May 2012 12:02:51 +0000 Subject: [PATCH] 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. Tested by: Joel Ray Holveck (joelh at juniper.net) PR: kern/165923 Reviewed by: kib MFC after: 2 weeks --- sys/fs/nfsclient/nfs_clbio.c | 7 ++++++- sys/fs/nfsclient/nfs_clnode.c | 2 ++ sys/fs/nfsclient/nfs_clvnops.c | 16 ++++++++++++++++ sys/fs/nfsclient/nfsnode.h | 1 + sys/nfsclient/nfs_bio.c | 7 ++++++- sys/nfsclient/nfs_node.c | 2 ++ sys/nfsclient/nfs_vnops.c | 16 ++++++++++++++++ sys/nfsclient/nfsnode.h | 1 + 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c index a160d37094c9..fa9636b0db37 100644 --- a/sys/fs/nfsclient/nfs_clbio.c +++ b/sys/fs/nfsclient/nfs_clbio.c @@ -281,7 +281,11 @@ ncl_putpages(struct vop_putpages_args *ap) 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; @@ -345,6 +349,7 @@ ncl_putpages(struct vop_putpages_args *ap) iomode = NFSWRITE_FILESYNC; error = ncl_writerpc(vp, &uio, cred, &iomode, &must_commit, 0); + crfree(cred); pmap_qremove(kva, npages); relpbuf(bp, &ncl_pbuf_freecnt); diff --git a/sys/fs/nfsclient/nfs_clnode.c b/sys/fs/nfsclient/nfs_clnode.c index 21de25ef289d..db3a350d71e7 100644 --- a/sys/fs/nfsclient/nfs_clnode.c +++ b/sys/fs/nfsclient/nfs_clnode.c @@ -300,6 +300,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); diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c index 0687d6c6ab33..12e018c7ad36 100644 --- a/sys/fs/nfsclient/nfs_clvnops.c +++ b/sys/fs/nfsclient/nfs_clvnops.c @@ -513,6 +513,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); @@ -604,7 +605,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); } diff --git a/sys/fs/nfsclient/nfsnode.h b/sys/fs/nfsclient/nfsnode.h index f2992c98be50..209945a2d6a6 100644 --- a/sys/fs/nfsclient/nfsnode.h +++ b/sys/fs/nfsclient/nfsnode.h @@ -123,6 +123,7 @@ struct nfsnode { int n_directio_asyncwr; u_int64_t n_change; /* old Change attribute */ struct nfsv4node *n_v4; /* extra V4 stuff */ + struct ucred *n_writecred; /* Cred. for putpages */ }; #define n_atim n_un1.nf_atim diff --git a/sys/nfsclient/nfs_bio.c b/sys/nfsclient/nfs_bio.c index 90fe96ee0dff..2ffd82258f74 100644 --- a/sys/nfsclient/nfs_bio.c +++ b/sys/nfsclient/nfs_bio.c @@ -275,7 +275,11 @@ nfs_putpages(struct vop_putpages_args *ap) 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; @@ -339,6 +343,7 @@ nfs_putpages(struct vop_putpages_args *ap) 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); diff --git a/sys/nfsclient/nfs_node.c b/sys/nfsclient/nfs_node.c index afe3341ba6cd..3805f6c38fe1 100644 --- a/sys/nfsclient/nfs_node.c +++ b/sys/nfsclient/nfs_node.c @@ -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); } diff --git a/sys/nfsclient/nfs_vnops.c b/sys/nfsclient/nfs_vnops.c index 872317b60a21..ca9edad5f355 100644 --- a/sys/nfsclient/nfs_vnops.c +++ b/sys/nfsclient/nfs_vnops.c @@ -507,6 +507,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); @@ -563,7 +564,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); } diff --git a/sys/nfsclient/nfsnode.h b/sys/nfsclient/nfsnode.h index 60e5613127eb..c6fa73e76058 100644 --- a/sys/nfsclient/nfsnode.h +++ b/sys/nfsclient/nfsnode.h @@ -128,6 +128,7 @@ struct nfsnode { uint32_t n_namelen; int n_directio_opens; int n_directio_asyncwr; + struct ucred *n_writecred; /* Cred. for putpages */ }; #define n_atim n_un1.nf_atim