Return-Path: linux-nfs-owner@vger.kernel.org Received: from mx1.redhat.com ([209.132.183.28]:50790 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752950Ab3EKPUf (ORCPT ); Sat, 11 May 2013 11:20:35 -0400 Message-ID: <518E6174.7010602@RedHat.com> Date: Sat, 11 May 2013 11:19:16 -0400 From: Steve Dickson MIME-Version: 1.0 To: "J. Bruce Fields" CC: Trond Myklebust , "J. Bruce Fields" , "David P. Quigley" , Linux NFS list , Linux FS devel list , Linux Security List , SELinux List Subject: Re: [PATCH 16/17] NFSD: Server implementation of MAC Labeling References: <1367515151-31015-1-git-send-email-SteveD@redhat.com> <1367515151-31015-17-git-send-email-SteveD@redhat.com> <20130509015033.GI23747@fieldses.org> In-Reply-To: <20130509015033.GI23747@fieldses.org> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: On 08/05/13 21:50, J. Bruce Fields wrote: > This also doesn't handle the unsupported case correctly. > > Applying the following fix to my tree. > > --b. > > commit 4438e5f801c43cc0f383fdc85f3ec294a4155df1 > Author: J. Bruce Fields > Date: Wed May 8 21:38:00 2013 -0400 > > nfsd4: fix handling of case when labels are unsupported > > Copy the logic in the ACL case to ensure that we just turn off the > bitmap in the getattr case instead of erroring out, and to ensure that > we get the supported_attributes attribute right. > > Signed-off-by: J. Bruce Fields > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index 6e96c0d..3b4e59c 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -2010,29 +2010,21 @@ nfsd4_encode_aclname(struct svc_rqst *rqstp, struct nfs4_ace *ace, > > #ifdef CONFIG_NFSD_V4_SECURITY_LABEL > static inline __be32 > -nfsd4_encode_security_label(struct svc_rqst *rqstp, struct dentry *dentry, __be32 **pp, int *buflen) > +nfsd4_encode_security_label(struct svc_rqst *rqstp, void *context, int len, __be32 **pp, int *buflen) > { > - void *context; > - int host_err; > - __be32 err = nfserr_resource; > - u32 len; > uint32_t pi = 0; > uint32_t lfs = 0; > __be32 *p = *pp; > > - host_err = security_inode_getsecctx(dentry->d_inode, &context, &len); > - if (host_err) > - return nfserrno(host_err); > - > if (*buflen < ((XDR_QUADLEN(len) << 2) + 4 + 4 + 4)) > - goto out; > + return nfserr_resource; > > /* > * For now we use a 0 here to indicate the null translation; in > * the future we may place a call to translation code here. > */ > if ((*buflen -= 8) < 0) > - goto out; > + return nfserr_resource; > > WRITE32(lfs); > WRITE32(pi); > @@ -2040,10 +2032,7 @@ nfsd4_encode_security_label(struct svc_rqst *rqstp, struct dentry *dentry, __be3 > *buflen -= (XDR_QUADLEN(len) << 2) + 4; > > *pp = p; > - err = 0; > -out: > - security_release_secctx(context, len); > - return err; > + return 0; > } > #else > static inline __be32 > @@ -2110,6 +2099,9 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp, > int err; > int aclsupport = 0; > struct nfs4_acl *acl = NULL; > + void *context = NULL; > + int contextlen; > + bool contextsupport = false; > struct nfsd4_compoundres *resp = rqstp->rq_resp; > u32 minorversion = resp->cstate.minorversion; > struct path path = { > @@ -2163,6 +2155,21 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp, > } > } > > +#ifdef CONFIG_NFSD_V4_SECURITY_LABEL > + if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) || > + bmval[0] & FATTR4_WORD0_SUPPORTED_ATTRS) { > + err = security_inode_getsecctx(dentry->d_inode, > + &context, &contextlen); > + contextsupport = (err = 0); Don't you really mean "contextsupport = (err == 0);" ? Otherwise contextsupport will never be set to 1. steved. > + if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) { > + if (err == -EOPNOTSUPP) > + bmval2 &= ~FATTR4_WORD2_SECURITY_LABEL; > + else if (err) > + goto out_nfserr; > + } > + } > +#endif /* CONFIG_NFSD_V4_SECURITY_LABEL */ > + > if (bmval2) { > if ((buflen -= 16) < 0) > goto out_resource; > @@ -2191,6 +2198,8 @@ nfsd4_encode_fattr(struct svc_fh *fhp, struct svc_export *exp, > > if (!aclsupport) > word0 &= ~FATTR4_WORD0_ACL; > + if (!contextsupport) > + word2 &= ~FATTR4_WORD2_SECURITY_LABEL; > if (!word2) { > if ((buflen -= 12) < 0) > goto out_resource; > @@ -2499,8 +2508,8 @@ out_acl: > WRITE64(stat.ino); > } > if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) { > - status = nfsd4_encode_security_label(rqstp, dentry, > - &p, &buflen); > + status = nfsd4_encode_security_label(rqstp, context, > + contextlen, &p, &buflen); > if (status) > goto out; > } > @@ -2516,6 +2525,8 @@ out_acl: > status = nfs_ok; > > out: > + if (context) > + security_release_secctx(context, contextlen); > kfree(acl); > if (fhp == &tempfh) > fh_put(&tempfh); >