From d8a5961f887efdeef4c89041e6ec0afd5063595c Mon Sep 17 00:00:00 2001 From: Marcelo Araujo Date: Fri, 3 Oct 2014 02:24:41 +0000 Subject: [PATCH] Fix failures and warnings reported by newpynfs20090424 test tool. This fix addresses only issues with the pynfs reports, none of these issues are know to create problems for extant real clients. Submitted by: Bart Hsiao Reworked by: myself Reviewed by: rmacklem Approved by: rmacklem Sponsored by: QNAP Systems Inc. --- .../opensolaris/uts/common/fs/zfs/zfs_vnops.c | 1 + sys/fs/nfs/nfs_commonsubs.c | 29 ++++++++----- sys/fs/nfs/nfs_var.h | 4 +- sys/fs/nfs/nfsproto.h | 6 ++- sys/fs/nfsserver/nfs_nfsdport.c | 13 ++++-- sys/fs/nfsserver/nfs_nfsdserv.c | 42 +++++++++++++++---- sys/fs/nfsserver/nfs_nfsdstate.c | 29 +++++++++---- 7 files changed, 90 insertions(+), 34 deletions(-) diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c index 1f17f050a5f..109a3393d53 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c @@ -2837,6 +2837,7 @@ zfs_getattr(vnode_t *vp, vattr_t *vap, int flags, cred_t *cr, #endif vap->va_seq = zp->z_seq; vap->va_flags = 0; /* FreeBSD: Reset chflags(2) flags. */ + vap->va_filerev = zp->z_seq; /* * Add in any requested optional attributes and the create time. diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c index b94091c62e4..d9f843611b3 100644 --- a/sys/fs/nfs/nfs_commonsubs.c +++ b/sys/fs/nfs/nfs_commonsubs.c @@ -820,7 +820,6 @@ nfsv4_loadattr(struct nfsrv_descript *nd, vnode_t vp, struct dqblk dqb; uid_t savuid; #endif - if (compare) { retnotsup = 0; error = nfsrv_getattrbits(nd, &attrbits, NULL, &retnotsup); @@ -902,6 +901,12 @@ nfsv4_loadattr(struct nfsrv_descript *nd, vnode_t vp, goto nfsmout; if (compare && !(*retcmpp)) { NFSSETSUPP_ATTRBIT(&checkattrbits); + + /* Some filesystem do not support NFSv4ACL */ + if (nfsrv_useacl == 0 || nfs_supportsnfsv4acls(vp) == 0) { + NFSCLRBIT_ATTRBIT(&checkattrbits, NFSATTRBIT_ACL); + NFSCLRBIT_ATTRBIT(&checkattrbits, NFSATTRBIT_ACLSUPPORT); + } if (!NFSEQUAL_ATTRBIT(&retattrbits, &checkattrbits) || retnotsup) *retcmpp = NFSERR_NOTSAME; @@ -1052,7 +1057,7 @@ nfsv4_loadattr(struct nfsrv_descript *nd, vnode_t vp, case NFSATTRBIT_ACL: if (compare) { if (!(*retcmpp)) { - if (nfsrv_useacl) { + if (nfsrv_useacl && nfs_supportsnfsv4acls(vp)) { NFSACL_T *naclp; naclp = acl_alloc(M_WAITOK); @@ -1073,21 +1078,22 @@ nfsv4_loadattr(struct nfsrv_descript *nd, vnode_t vp, } } } else { - if (vp != NULL && aclp != NULL) - error = nfsrv_dissectacl(nd, aclp, &aceerr, - &cnt, p); - else - error = nfsrv_dissectacl(nd, NULL, &aceerr, - &cnt, p); - if (error) - goto nfsmout; + if (vp != NULL && aclp != NULL) + error = nfsrv_dissectacl(nd, aclp, &aceerr, + &cnt, p); + else + error = nfsrv_dissectacl(nd, NULL, &aceerr, + &cnt, p); + if (error) + goto nfsmout; } + attrsum += cnt; break; case NFSATTRBIT_ACLSUPPORT: NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); if (compare && !(*retcmpp)) { - if (nfsrv_useacl) { + if (nfsrv_useacl && nfs_supportsnfsv4acls(vp)) { if (fxdr_unsigned(u_int32_t, *tl) != NFSV4ACE_SUPTYPES) *retcmpp = NFSERR_NOTSAME; @@ -2090,6 +2096,7 @@ nfsv4_fillattr(struct nfsrv_descript *nd, struct mount *mp, vnode_t vp, } } } + /* * Put out the attribute bitmap for the ones being filled in * and get the field for the number of attributes returned. diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h index 2abd7e4dc88..ef15a1d686f 100644 --- a/sys/fs/nfs/nfs_var.h +++ b/sys/fs/nfs/nfs_var.h @@ -644,9 +644,9 @@ int nfsvno_updfilerev(vnode_t, struct nfsvattr *, struct ucred *, int nfsvno_fillattr(struct nfsrv_descript *, struct mount *, vnode_t, struct nfsvattr *, fhandle_t *, int, nfsattrbit_t *, struct ucred *, NFSPROC_T *, int, int, int, int, uint64_t); -int nfsrv_sattr(struct nfsrv_descript *, struct nfsvattr *, nfsattrbit_t *, +int nfsrv_sattr(struct nfsrv_descript *, vnode_t, struct nfsvattr *, nfsattrbit_t *, NFSACL_T *, NFSPROC_T *); -int nfsv4_sattr(struct nfsrv_descript *, struct nfsvattr *, nfsattrbit_t *, +int nfsv4_sattr(struct nfsrv_descript *, vnode_t, struct nfsvattr *, nfsattrbit_t *, NFSACL_T *, NFSPROC_T *); int nfsvno_checkexp(mount_t, NFSSOCKADDR_T, struct nfsexstuff *, struct ucred **); diff --git a/sys/fs/nfs/nfsproto.h b/sys/fs/nfs/nfsproto.h index 731be0777c1..768dfcbbc9a 100644 --- a/sys/fs/nfs/nfsproto.h +++ b/sys/fs/nfs/nfsproto.h @@ -996,7 +996,11 @@ struct nfsv3_sattr { NFSATTRBM_TIMEDELTA | \ NFSATTRBM_TIMEMETADATA | \ NFSATTRBM_TIMEMODIFY | \ - NFSATTRBM_MOUNTEDONFILEID) + NFSATTRBM_MOUNTEDONFILEID | \ + NFSATTRBM_QUOTAHARD | \ + NFSATTRBM_QUOTASOFT | \ + NFSATTRBM_QUOTAUSED) + #ifdef QUOTA /* diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c index 1973c14f41b..5bd4538f073 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -1008,7 +1008,7 @@ nfsvno_getsymlink(struct nfsrv_descript *nd, struct nfsvattr *nvap, *pathcpp = NULL; *lenp = 0; if ((nd->nd_flag & ND_NFSV3) && - (error = nfsrv_sattr(nd, nvap, NULL, NULL, p))) + (error = nfsrv_sattr(nd, NULL, nvap, NULL, NULL, p))) goto nfsmout; NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); len = fxdr_unsigned(int, *tl); @@ -2298,7 +2298,7 @@ nfsmout: * (Return 0 or EBADRPC) */ int -nfsrv_sattr(struct nfsrv_descript *nd, struct nfsvattr *nvap, +nfsrv_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, nfsattrbit_t *attrbitp, NFSACL_T *aclp, struct thread *p) { u_int32_t *tl; @@ -2380,7 +2380,7 @@ nfsrv_sattr(struct nfsrv_descript *nd, struct nfsvattr *nvap, }; break; case ND_NFSV4: - error = nfsv4_sattr(nd, nvap, attrbitp, aclp, p); + error = nfsv4_sattr(nd, vp, nvap, attrbitp, aclp, p); }; nfsmout: NFSEXITCODE2(error, nd); @@ -2392,7 +2392,7 @@ nfsmout: * Returns NFSERR_BADXDR if it can't be parsed, 0 otherwise. */ int -nfsv4_sattr(struct nfsrv_descript *nd, struct nfsvattr *nvap, +nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, nfsattrbit_t *attrbitp, NFSACL_T *aclp, struct thread *p) { u_int32_t *tl; @@ -2429,6 +2429,11 @@ nfsv4_sattr(struct nfsrv_descript *nd, struct nfsvattr *nvap, switch (bitpos) { case NFSATTRBIT_SIZE: NFSM_DISSECT(tl, u_int32_t *, NFSX_HYPER); + if (vp != NULL && vp->v_type != VREG) { + error = (vp->v_type == VDIR) ? NFSERR_ISDIR : + NFSERR_INVAL; + goto nfsmout; + } nvap->na_size = fxdr_hyper(tl); attrsum += NFSX_HYPER; break; diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c index 0d3ff7658ec..9bf43c37b10 100644 --- a/sys/fs/nfsserver/nfs_nfsdserv.c +++ b/sys/fs/nfsserver/nfs_nfsdserv.c @@ -210,6 +210,17 @@ nfsrvd_getattr(struct nfsrv_descript *nd, int isdgram, if (nd->nd_repstat == 0) { accmode = 0; NFSSET_ATTRBIT(&tmpbits, &attrbits); + + /* + * GETATTR with write-only attr time_access_set and time_modify_set + * should return NFS4ERR_INVAL. + */ + if (NFSISSET_ATTRBIT(&tmpbits, NFSATTRBIT_TIMEACCESSSET) || + NFSISSET_ATTRBIT(&tmpbits, NFSATTRBIT_TIMEMODIFYSET)){ + error = NFSERR_INVAL; + vput(vp); + goto out; + } if (NFSISSET_ATTRBIT(&tmpbits, NFSATTRBIT_ACL)) { NFSCLRBIT_ATTRBIT(&tmpbits, NFSATTRBIT_ACL); accmode |= VREAD_ACL; @@ -315,7 +326,7 @@ nfsrvd_setattr(struct nfsrv_descript *nd, __unused int isdgram, stateid.seqid = fxdr_unsigned(u_int32_t, *tl++); NFSBCOPY((caddr_t)tl,(caddr_t)stateid.other,NFSX_STATEIDOTHER); } - error = nfsrv_sattr(nd, &nva, &attrbits, aclp, p); + error = nfsrv_sattr(nd, vp, &nva, &attrbits, aclp, p); if (error) goto nfsmout; preat_ret = nfsvno_getattr(vp, &nva2, nd->nd_cred, p, 1); @@ -1019,7 +1030,7 @@ nfsrvd_create(struct nfsrv_descript *nd, __unused int isdgram, switch (how) { case NFSCREATE_GUARDED: case NFSCREATE_UNCHECKED: - error = nfsrv_sattr(nd, &nva, NULL, NULL, p); + error = nfsrv_sattr(nd, NULL, &nva, NULL, NULL, p); if (error) goto nfsmout; break; @@ -1204,7 +1215,7 @@ nfsrvd_mknod(struct nfsrv_descript *nd, __unused int isdgram, NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); vtyp = nfsv34tov_type(*tl); } - error = nfsrv_sattr(nd, &nva, &attrbits, aclp, p); + error = nfsrv_sattr(nd, NULL, &nva, &attrbits, aclp, p); if (error) goto nfsmout; nva.na_type = vtyp; @@ -1850,7 +1861,7 @@ nfsrvd_mkdir(struct nfsrv_descript *nd, __unused int isdgram, if (!nd->nd_repstat) { NFSVNO_ATTRINIT(&nva); if (nd->nd_flag & ND_NFSV3) { - error = nfsrv_sattr(nd, &nva, NULL, NULL, p); + error = nfsrv_sattr(nd, NULL, &nva, NULL, NULL, p); if (error) goto nfsmout; } else { @@ -1967,11 +1978,21 @@ nfsrvd_commit(struct nfsrv_descript *nd, __unused int isdgram, int error = 0, for_ret = 1, aft_ret = 1, cnt; u_int64_t off; - if (nd->nd_repstat) { + if (nd->nd_repstat) { nfsrv_wcc(nd, for_ret, &bfor, aft_ret, &aft); goto out; } + + /* Return NFSERR_ISDIR in NFSv4 when commit on a directory. */ + if (vp->v_type != VREG) { + if (nd->nd_flag & ND_NFSV3) + error = NFSERR_NOTSUPP; + else + error = (vp->v_type == VDIR) ? NFSERR_ISDIR : NFSERR_INVAL; + goto nfsmout; + } NFSM_DISSECT(tl, u_int32_t *, 3 * NFSX_UNSIGNED); + /* * XXX At this time VOP_FSYNC() does not accept offset and byte * count parameters, so these arguments are useless (someday maybe). @@ -2683,7 +2704,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram, switch (how) { case NFSCREATE_UNCHECKED: case NFSCREATE_GUARDED: - error = nfsv4_sattr(nd, &nva, &attrbits, aclp, p); + error = nfsv4_sattr(nd, NULL, &nva, &attrbits, aclp, p); if (error) goto nfsmout; /* @@ -2707,7 +2728,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram, NFSM_DISSECT(tl, u_int32_t *, NFSX_VERF); cverf[0] = *tl++; cverf[1] = *tl; - error = nfsv4_sattr(nd, &nva, &attrbits, aclp, p); + error = nfsv4_sattr(nd, vp, &nva, &attrbits, aclp, p); if (error != 0) goto nfsmout; if (NFSISSET_ATTRBIT(&attrbits, @@ -2858,7 +2879,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram, * The IETF working group decided that this is the correct * error return for all non-regular files. */ - nd->nd_repstat = NFSERR_SYMLINK; + nd->nd_repstat = (vp->v_type == VDIR) ? NFSERR_ISDIR : NFSERR_SYMLINK; } if (!nd->nd_repstat && (stp->ls_flags & NFSLCK_WRITEACCESS)) nd->nd_repstat = nfsvno_accchk(vp, VWRITE, nd->nd_cred, @@ -3197,6 +3218,11 @@ nfsrvd_opendowngrade(struct nfsrv_descript *nd, __unused int isdgram, nfsv4stateid_t stateid; nfsquad_t clientid; + /* opendowngrade can only work on a file object.*/ + if (vp->v_type != VREG) { + error = NFSERR_INVAL; + goto nfsmout; + } NFSM_DISSECT(tl, u_int32_t *, NFSX_STATEID + 3 * NFSX_UNSIGNED); stp->ls_ownerlen = 0; stp->ls_op = nd->nd_rp; diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c index 2faf064d756..97f8fffd622 100644 --- a/sys/fs/nfsserver/nfs_nfsdstate.c +++ b/sys/fs/nfsserver/nfs_nfsdstate.c @@ -1628,9 +1628,17 @@ tryagain: */ if (error == 0 && (stp->ls_flags & NFSLCK_OPEN) && ((stp->ls_openowner->ls_flags & NFSLCK_NEEDSCONFIRM) || - (getlckret == 0 && stp->ls_lfp != lfp))) - error = NFSERR_BADSTATEID; - if (error == 0 && + (getlckret == 0 && stp->ls_lfp != lfp))){ + /* + * NFSLCK_SETATTR should return OK rather than NFSERR_BADSTATEID + * The only exception is using SETATTR with SIZE. + * */ + if ((new_stp->ls_flags & + (NFSLCK_SETATTR | NFSLCK_CHECK)) != NFSLCK_SETATTR) + error = NFSERR_BADSTATEID; + } + + if (error == 0 && (stp->ls_flags & (NFSLCK_DELEGREAD | NFSLCK_DELEGWRITE)) && getlckret == 0 && stp->ls_lfp != lfp) error = NFSERR_BADSTATEID; @@ -4909,12 +4917,17 @@ tryagain: * Now, look for a conflicting open share. */ if (remove) { - LIST_FOREACH(stp, &lfp->lf_open, ls_file) { - if (stp->ls_flags & NFSLCK_WRITEDENY) { - error = NFSERR_FILEOPEN; - break; + /* + * If the entry in the directory was the last reference to the + * corresponding filesystem object, the object can be destroyed + * */ + if(lfp->lf_usecount>1) + LIST_FOREACH(stp, &lfp->lf_open, ls_file) { + if (stp->ls_flags & NFSLCK_WRITEDENY) { + error = NFSERR_FILEOPEN; + break; + } } - } } NFSUNLOCKSTATE();