2011-04-04 13:43:41

by Anna Schumaker

[permalink] [raw]
Subject: secinfo_no_name question

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


2011-04-04 15:14:13

by J. Bruce Fields

[permalink] [raw]
Subject: Re: secinfo_no_name question

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.

2011-04-04 15:22:33

by Anna Schumaker

[permalink] [raw]
Subject: Re: secinfo_no_name question

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2011-04-10 16:29:39

by J. Bruce Fields

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

The secinfo caller actually won't want this.

Signed-off-by: J. Bruce Fields <[email protected]>
---
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


2011-04-10 16:29:39

by J. Bruce Fields

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

Signed-off-by: J. Bruce Fields <[email protected]>
---
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


2011-04-10 16:25:39

by J. Bruce Fields

[permalink] [raw]
Subject: Re: secinfo_no_name question

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.

2011-04-10 16:29:39

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 3/5] nfsd4: introduce OPDESC helper

Signed-off-by: J. Bruce Fields <[email protected]>
---
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


2011-04-10 16:29:39

by J. Bruce Fields

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

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 <[email protected]>
---
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


2011-04-10 16:29:39

by J. Bruce Fields

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

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 <[email protected]>
---
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


2011-04-05 16:12:14

by Tom Haynes

[permalink] [raw]
Subject: Re: secinfo_no_name question

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 [email protected]
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Tom Haynes
ex-cfb

2011-04-11 03:05:08

by Mi Jinlong

[permalink] [raw]
Subject: Re: [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 <[email protected]>
> ---
> 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


2011-04-11 12:42:46

by J. Bruce Fields

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

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 <[email protected]>
> > ---
> > 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.