secinfo_no_name question

secinfo_no_name question

[Bryan Schumaker]

Hi Bruce,

I'm looking at secinfo_no_name on the client.  RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH.  My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME.  The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.  

Am I doing something wrong?  Is this a server problem?

Thanks,
Bryan

Re: secinfo_no_name question

[J. Bruce Fields]

On Mon, Apr 04, 2011 at 09:43:24AM -0400, Bryan Schumaker wrote:
> Hi Bruce,
> 
> I'm looking at secinfo_no_name on the client.  RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH.  My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME.  The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.  
> 
> Am I doing something wrong?  Is this a server problem?

Could be; is the compound is being sent with a security flavor that
*isn't* permitted on the root export?

If so I believe the compound should have succeeded--the server needs
some special exception there that we may have left out....

--b.

Re: secinfo_no_name question

[Bryan Schumaker]

On 04/04/2011 11:14 AM, J. Bruce Fields wrote:
> On Mon, Apr 04, 2011 at 09:43:24AM -0400, Bryan Schumaker wrote:
>> Hi Bruce,
>>
>> I'm looking at secinfo_no_name on the client.  RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH.  My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME.  The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.  
>>
>> Am I doing something wrong?  Is this a server problem?
> 
> Could be; is the compound is being sent with a security flavor that
> *isn't* permitted on the root export?
> 
> If so I believe the compound should have succeeded--the server needs
> some special exception there that we may have left out....

The root export is set up with sec=null and my compound is using auth_unix.  

> 
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: secinfo_no_name question

[Tom Haynes]

On Mon, Apr 04, 2011 at 11:22:31AM -0400, Bryan Schumaker wrote:
> On 04/04/2011 11:14 AM, J. Bruce Fields wrote:
> > On Mon, Apr 04, 2011 at 09:43:24AM -0400, Bryan Schumaker wrote:
> >> Hi Bruce,
> >>
> >> I'm looking at secinfo_no_name on the client.  RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH.  My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME.  The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.  
> >>
> >> Am I doing something wrong?  Is this a server problem?
> > 
> > Could be; is the compound is being sent with a security flavor that
> > *isn't* permitted on the root export?

It should never fail:

2.6.3.1.1.5.  Put Filehandle Operation + SECINFO/SECINFO_NO_NAME

...

   The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC to a put
   filehandle operation that is immediately followed by SECINFO or
   SECINFO_NO_NAME.  The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC
   from SECINFO or SECINFO_NO_NAME.


> > 
> > If so I believe the compound should have succeeded--the server needs
> > some special exception there that we may have left out....
> 
> The root export is set up with sec=null and my compound is using auth_unix.  
> 
> > 
> > --b.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Tom Haynes
ex-cfb

Re: secinfo_no_name question

[J. Bruce Fields]

On Tue, Apr 05, 2011 at 11:09:33AM -0500, Tom Haynes wrote:
> On Mon, Apr 04, 2011 at 11:22:31AM -0400, Bryan Schumaker wrote:
> > On 04/04/2011 11:14 AM, J. Bruce Fields wrote:
> > > On Mon, Apr 04, 2011 at 09:43:24AM -0400, Bryan Schumaker wrote:
> > >> Hi Bruce,
> > >>
> > >> I'm looking at secinfo_no_name on the client.  RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH.  My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME.  The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.  
> > >>
> > >> Am I doing something wrong?  Is this a server problem?
> > > 
> > > Could be; is the compound is being sent with a security flavor that
> > > *isn't* permitted on the root export?
> 
> It should never fail:
> 
> 2.6.3.1.1.5.  Put Filehandle Operation + SECINFO/SECINFO_NO_NAME
> 
> ...
> 
>    The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC to a put
>    filehandle operation that is immediately followed by SECINFO or
>    SECINFO_NO_NAME.  The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC
>    from SECINFO or SECINFO_NO_NAME.

And I thought I'd implemented those recommendations at some point--I
guess not!  My attempt follows.

--b.

[PATCH 1/5] nfsd: distinguish functions of NFSD_MAY_* flags

[J. Bruce Fields]

Most of the NFSD_MAY_* flags actually request permissions, but over the
years we've accreted a few that modify the behavior of the permission or
open code in other ways.

Distinguish the two cases a little more.  In particular, allow the
shortcut at the start of nfsd_permission to ignore the
non-permission-requesting bits.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/vfs.c |    2 +-
 fs/nfsd/vfs.h |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 2e1cebd..ac4f0b4 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
 	struct inode	*inode = dentry->d_inode;
 	int		err;
 
-	if (acc == NFSD_MAY_NOP)
+	if (acc & NFSD_MAY_MASK == NFSD_MAY_NOP)
 		return 0;
 #if 0
 	dprintk("nfsd: permission 0x%x%s%s%s%s%s%s%s mode 0%o%s%s%s\n",
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 9a370a5..1036913 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -17,6 +17,9 @@
 #define NFSD_MAY_SATTR		8
 #define NFSD_MAY_TRUNC		16
 #define NFSD_MAY_LOCK		32
+#define NFSD_MAY_MASK		63
+
+/* extra hints to permission and open routines: */
 #define NFSD_MAY_OWNER_OVERRIDE	64
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
-- 
1.7.1

[PATCH 2/5] nfsd4: allow fh_verify caller to skip pseudoflavor checks

[J. Bruce Fields]

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfsfh.c |    2 +-
 fs/nfsd/vfs.h   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 55c8e63..90c6aa6 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -344,7 +344,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, int access)
 	 * which clients virtually always use auth_sys for,
 	 * even while using RPCSEC_GSS for NFS.
 	 */
-	if (access & NFSD_MAY_LOCK)
+	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
 		goto skip_pseudoflavor_check;
 	/*
 	 * Clients may expect to be able to use auth_sys during mount,
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 1036913..4d2509f 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -24,6 +24,7 @@
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
 #define NFSD_MAY_NOT_BREAK_LEASE 512
+#define NFSD_MAY_BYPASS_GSS	1024
 
 #define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
 #define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
-- 
1.7.1

[PATCH 3/5] nfsd4: introduce OPDESC helper

[J. Bruce Fields]

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5fcb139..126b8f7 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1031,6 +1031,11 @@ static __be32 nfs41_check_op_ordering(struct nfsd4_compoundargs *args)
 	return nfs_ok;
 }
 
+static inline struct nfsd4_operation *OPDESC(struct nfsd4_op *op)
+{
+	return &nfsd4_ops[op->opnum];
+}
+
 /*
  * COMPOUND call.
  */
@@ -1108,7 +1113,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
 			goto encode_op;
 		}
 
-		opdesc = &nfsd4_ops[op->opnum];
+		opdesc = OPDESC(op);
 
 		if (!cstate->current_fh.fh_dentry) {
 			if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
-- 
1.7.1

[PATCH 4/5] nfsd4: make fh_verify responsibility of nfsd_lookup_dentry caller

[J. Bruce Fields]

The secinfo caller actually won't want this.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c |    3 +++
 fs/nfsd/vfs.c      |    9 +++------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 126b8f7..8059ada 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -762,6 +762,9 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 err;
 
 	fh_init(&resfh, NFS4_FHSIZE);
+	err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
+	if (err)
+		return err;
 	err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
 				    secinfo->si_name, secinfo->si_namelen,
 				    &exp, &dentry);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index ac4f0b4..7d75890 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -181,16 +181,10 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	struct svc_export	*exp;
 	struct dentry		*dparent;
 	struct dentry		*dentry;
-	__be32			err;
 	int			host_err;
 
 	dprintk("nfsd: nfsd_lookup(fh %s, %.*s)\n", SVCFH_fmt(fhp), len,name);
 
-	/* Obtain dentry and export. */
-	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
-	if (err)
-		return err;
-
 	dparent = fhp->fh_dentry;
 	exp  = fhp->fh_export;
 	exp_get(exp);
@@ -254,6 +248,9 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	struct dentry		*dentry;
 	__be32 err;
 
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
+	if (err)
+		return err;
 	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
 	if (err)
 		return err;
-- 
1.7.1

[PATCH 5/5] nfsd4: fix wrongsec handling for PUTFH + op cases

[J. Bruce Fields]

When PUTFH is followed by an operation that uses the filehandle, and
when the current client is using a security flavor that is inconsistent
with the given filehandle, we have a choice: we can return WRONGSEC
either when the current filehandle is set using the PUTFH, or when the
filehandle is first used by the following operation.

Follow the recommendations of RFC 5661 in making this choice.

(Our current behavior prevented the client from doing security
negotiation by returning WRONGSEC on SECINFO.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/export.c   |    6 -----
 fs/nfsd/nfs4proc.c |   57 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index ad000ae..b9566e4 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1354,12 +1354,6 @@ exp_pseudoroot(struct svc_rqst *rqstp, struct svc_fh *fhp)
 	if (IS_ERR(exp))
 		return nfserrno(PTR_ERR(exp));
 	rv = fh_compose(fhp, exp, exp->ex_path.dentry, NULL);
-	if (rv)
-		goto out;
-	rv = check_nfsd_access(exp, rqstp);
-	if (rv)
-		fh_put(fhp);
-out:
 	exp_put(exp);
 	return rv;
 }
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 8059ada..c7d0b1e 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -403,7 +403,7 @@ nfsd4_putfh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	cstate->current_fh.fh_handle.fh_size = putfh->pf_fhlen;
 	memcpy(&cstate->current_fh.fh_handle.fh_base, putfh->pf_fhval,
 	       putfh->pf_fhlen);
-	return fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_NOP);
+	return fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_BYPASS_GSS);
 }
 
 static __be32
@@ -762,9 +762,6 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 err;
 
 	fh_init(&resfh, NFS4_FHSIZE);
-	err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
-	if (err)
-		return err;
 	err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
 				    secinfo->si_name, secinfo->si_namelen,
 				    &exp, &dentry);
@@ -989,6 +986,9 @@ enum nfsd4_op_flags {
 	ALLOWED_WITHOUT_FH = 1 << 0,	/* No current filehandle required */
 	ALLOWED_ON_ABSENT_FS = 1 << 1,	/* ops processed on absent fs */
 	ALLOWED_AS_FIRST_OP = 1 << 2,	/* ops reqired first in compound */
+	/* For rfc 5661 section 2.6.3.1.1: */
+	OP_HANDLES_WRONGSEC = 1 << 3,
+	OP_IS_PUTFH_LIKE = 1 << 4,
 };
 
 struct nfsd4_operation {
@@ -1039,6 +1039,34 @@ static inline struct nfsd4_operation *OPDESC(struct nfsd4_op *op)
 	return &nfsd4_ops[op->opnum];
 }
 
+static bool need_wrongsec_check(struct svc_rqst *rqstp)
+{
+	struct nfsd4_compoundres *resp = rqstp->rq_resp;
+	struct nfsd4_compoundargs *argp = rqstp->rq_argp;
+	struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
+	struct nfsd4_op *next = &argp->ops[resp->opcnt];
+	struct nfsd4_operation *thisd;
+	struct nfsd4_operation *nextd;
+
+	/*
+	 * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
+	 * put-filehandle operation if we're not going to use the
+	 * result:
+	 */
+	if (argp->opcnt == resp->opcnt)
+		return false;
+
+	thisd = OPDESC(this);
+	nextd = OPDESC(next);
+	/*
+	 * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
+	 * errors themselves as necessary; others should check for them
+	 * now:
+	 */
+	return thisd->op_flags & OP_IS_PUTFH_LIKE
+		&& !(nextd->op_flags & OP_HANDLES_WRONGSEC);
+}
+
 /*
  * COMPOUND call.
  */
@@ -1134,6 +1162,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
 		else
 			BUG_ON(op->status == nfs_ok);
 
+		if (!op->status && need_wrongsec_check(rqstp))
+			op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+
 encode_op:
 		/* Only from SEQUENCE */
 		if (resp->cstate.status == nfserr_replay_cache) {
@@ -1225,10 +1256,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_LOOKUP] = {
 		.op_func = (nfsd4op_func)nfsd4_lookup,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_LOOKUP",
 	},
 	[OP_LOOKUPP] = {
 		.op_func = (nfsd4op_func)nfsd4_lookupp,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_LOOKUPP",
 	},
 	[OP_NVERIFY] = {
@@ -1237,6 +1270,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_OPEN] = {
 		.op_func = (nfsd4op_func)nfsd4_open,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_OPEN",
 	},
 	[OP_OPEN_CONFIRM] = {
@@ -1249,17 +1283,20 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_PUTFH] = {
 		.op_func = (nfsd4op_func)nfsd4_putfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTFH",
 	},
 	[OP_PUTPUBFH] = {
 		.op_func = (nfsd4op_func)nfsd4_putrootfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTPUBFH",
 	},
 	[OP_PUTROOTFH] = {
 		.op_func = (nfsd4op_func)nfsd4_putrootfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_PUTROOTFH",
 	},
 	[OP_READ] = {
@@ -1289,15 +1326,18 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_RESTOREFH] = {
 		.op_func = (nfsd4op_func)nfsd4_restorefh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+				| OP_IS_PUTFH_LIKE,
 		.op_name = "OP_RESTOREFH",
 	},
 	[OP_SAVEFH] = {
 		.op_func = (nfsd4op_func)nfsd4_savefh,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SAVEFH",
 	},
 	[OP_SECINFO] = {
 		.op_func = (nfsd4op_func)nfsd4_secinfo,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO",
 	},
 	[OP_SETATTR] = {
@@ -1361,6 +1401,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SECINFO_NO_NAME] = {
 		.op_func = (nfsd4op_func)nfsd4_secinfo_no_name,
+		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO_NO_NAME",
 	},
 };
-- 
1.7.1

Re: [PATCH 1/5] nfsd: distinguish functions of NFSD_MAY_* flags

[Mi Jinlong]

J. Bruce Fields 写道:
> Most of the NFSD_MAY_* flags actually request permissions, but over the
> years we've accreted a few that modify the behavior of the permission or
> open code in other ways.
> 
> Distinguish the two cases a little more.  In particular, allow the
> shortcut at the start of nfsd_permission to ignore the
> non-permission-requesting bits.
> 
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
>  fs/nfsd/vfs.c |    2 +-
>  fs/nfsd/vfs.h |    3 +++
>  2 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 2e1cebd..ac4f0b4 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
>  	struct inode	*inode = dentry->d_inode;
>  	int		err;
>  
> -	if (acc == NFSD_MAY_NOP)
> +	if (acc & NFSD_MAY_MASK == NFSD_MAY_NOP)
>  		return 0;

  Maybe there is a problem, the priority of '==' is higher than '&', 
  this line equal to "if (acc & (NFSD_MAY_MASK == NFSD_MAY_NOP))",
  "return 0" will appears every time, I think it's not we really want.

  "if ((acc & NFSD_MAY_MASK) == NFSD_MAY_NOP)" is we need,
  do you think so?

-- 
----
thanks
Mi Jinlong

>  #if 0
>  	dprintk("nfsd: permission 0x%x%s%s%s%s%s%s%s mode 0%o%s%s%s\n",
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 9a370a5..1036913 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -17,6 +17,9 @@
>  #define NFSD_MAY_SATTR		8
>  #define NFSD_MAY_TRUNC		16
>  #define NFSD_MAY_LOCK		32
> +#define NFSD_MAY_MASK		63
> +
> +/* extra hints to permission and open routines: */
>  #define NFSD_MAY_OWNER_OVERRIDE	64
>  #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
>  #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256

-- 
----
thanks
Mi Jinlong

Re: [PATCH 1/5] nfsd: distinguish functions of NFSD_MAY_* flags

[J. Bruce Fields]

On Mon, Apr 11, 2011 at 11:06:35AM +0800, Mi Jinlong wrote:
>=20
>=20
> J. Bruce Fields =E5=86=99=E9=81=93:
> > Most of the NFSD_MAY_* flags actually request permissions, but over=
 the
> > years we've accreted a few that modify the behavior of the permissi=
on or
> > open code in other ways.
> >=20
> > Distinguish the two cases a little more.  In particular, allow the
> > shortcut at the start of nfsd_permission to ignore the
> > non-permission-requesting bits.
> >=20
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  fs/nfsd/vfs.c |    2 +-
> >  fs/nfsd/vfs.h |    3 +++
> >  2 files changed, 4 insertions(+), 1 deletions(-)
> >=20
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 2e1cebd..ac4f0b4 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struc=
t svc_export *exp,
> >  	struct inode	*inode =3D dentry->d_inode;
> >  	int		err;
> > =20
> > -	if (acc =3D=3D NFSD_MAY_NOP)
> > +	if (acc & NFSD_MAY_MASK =3D=3D NFSD_MAY_NOP)
> >  		return 0;
>=20
>   Maybe there is a problem, the priority of '=3D=3D' is higher than '=
&',=20
>   this line equal to "if (acc & (NFSD_MAY_MASK =3D=3D NFSD_MAY_NOP))"=
,
>   "return 0" will appears every time, I think it's not we really want=
=2E
>=20
>   "if ((acc & NFSD_MAY_MASK) =3D=3D NFSD_MAY_NOP)" is we need,
>   do you think so?

Yes, thanks--fixed.--b.