* [PATCH 1/4] nfsd4: ignore WANT bits in open downgrade
2011-10-11 16:40 patches for 3.2 J. Bruce Fields
@ 2011-10-11 16:41 ` J. Bruce Fields
2011-10-11 16:41 ` [PATCH 2/4] nfsd4: move access/deny validity checks to xdr code J. Bruce Fields
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: J. Bruce Fields @ 2011-10-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields, stable
We don't use WANT bits yet--and sending them can probably trigger a
BUG() further down.
Cc: stable@kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 683885b..a39a542 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3574,6 +3574,8 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
if (!access_valid(od->od_share_access, cstate->minorversion)
|| !deny_valid(od->od_share_deny))
return nfserr_inval;
+ /* We don't yet support WANT bits: */
+ od->od_share_access &= NFS4_SHARE_ACCESS_MASK;
nfs4_lock_state();
status = nfs4_preprocess_confirmed_seqid_op(cstate, od->od_seqid,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/4] nfsd4: move access/deny validity checks to xdr code
2011-10-11 16:40 patches for 3.2 J. Bruce Fields
2011-10-11 16:41 ` [PATCH 1/4] nfsd4: ignore WANT bits in open downgrade J. Bruce Fields
@ 2011-10-11 16:41 ` J. Bruce Fields
2011-10-11 16:41 ` [PATCH 3/4] nfsd4: move name-length checks to xdr J. Bruce Fields
2011-10-11 16:41 ` [PATCH 4/4] nfsd4: more robust ignoring of WANT bits in OPEN J. Bruce Fields
3 siblings, 0 replies; 5+ messages in thread
From: J. Bruce Fields @ 2011-10-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
I'd rather put more of these sorts of checks into standardized xdr
decoders for the various types rather than have them cluttering up the
core logic in nfs4proc.c and nfs4state.c.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 32 --------------------
fs/nfsd/nfs4xdr.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 73 insertions(+), 39 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a39a542..6bfa293 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2418,31 +2418,6 @@ find_file(struct inode *ino)
return NULL;
}
-static inline int access_valid(u32 x, u32 minorversion)
-{
- if ((x & NFS4_SHARE_ACCESS_MASK) < NFS4_SHARE_ACCESS_READ)
- return 0;
- if ((x & NFS4_SHARE_ACCESS_MASK) > NFS4_SHARE_ACCESS_BOTH)
- return 0;
- x &= ~NFS4_SHARE_ACCESS_MASK;
- if (minorversion && x) {
- if ((x & NFS4_SHARE_WANT_MASK) > NFS4_SHARE_WANT_CANCEL)
- return 0;
- if ((x & NFS4_SHARE_WHEN_MASK) > NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED)
- return 0;
- x &= ~(NFS4_SHARE_WANT_MASK | NFS4_SHARE_WHEN_MASK);
- }
- if (x)
- return 0;
- return 1;
-}
-
-static inline int deny_valid(u32 x)
-{
- /* Note: unlike access bits, deny bits may be zero. */
- return x <= NFS4_SHARE_DENY_BOTH;
-}
-
/*
* Called to check deny when READ with all zero stateid or
* WRITE with all zero or all one stateid
@@ -2918,10 +2893,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
struct nfs4_delegation *dp = NULL;
__be32 status;
- status = nfserr_inval;
- if (!access_valid(open->op_share_access, resp->cstate.minorversion)
- || !deny_valid(open->op_share_deny))
- goto out;
/*
* Lookup file; if found, lookup stateid and check open request,
* and check for delegations in the process of being recalled.
@@ -3571,9 +3542,6 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
(int)cstate->current_fh.fh_dentry->d_name.len,
cstate->current_fh.fh_dentry->d_name.name);
- if (!access_valid(od->od_share_access, cstate->minorversion)
- || !deny_valid(od->od_share_deny))
- return nfserr_inval;
/* We don't yet support WANT bits: */
od->od_share_access &= NFS4_SHARE_ACCESS_MASK;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5779acd..94da8bb 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -639,6 +639,64 @@ nfsd4_decode_lookup(struct nfsd4_compoundargs *argp, struct nfsd4_lookup *lookup
DECODE_TAIL;
}
+static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *x)
+{
+ __be32 *p;
+ u32 w;
+
+ READ_BUF(4);
+ READ32(w);
+ *x = w;
+ switch (w & NFS4_SHARE_ACCESS_MASK) {
+ case NFS4_SHARE_ACCESS_READ:
+ case NFS4_SHARE_ACCESS_WRITE:
+ case NFS4_SHARE_ACCESS_BOTH:
+ break;
+ default:
+ return nfserr_bad_xdr;
+ }
+ w &= !NFS4_SHARE_ACCESS_MASK;
+ if (!w)
+ return nfs_ok;
+ if (!argp->minorversion)
+ return nfserr_bad_xdr;
+ switch (w & NFS4_SHARE_WANT_MASK) {
+ case NFS4_SHARE_WANT_NO_PREFERENCE:
+ case NFS4_SHARE_WANT_READ_DELEG:
+ case NFS4_SHARE_WANT_WRITE_DELEG:
+ case NFS4_SHARE_WANT_ANY_DELEG:
+ case NFS4_SHARE_WANT_NO_DELEG:
+ case NFS4_SHARE_WANT_CANCEL:
+ break;
+ default:
+ return nfserr_bad_xdr;
+ }
+ w &= !NFS4_SHARE_WANT_MASK;
+ if (!w)
+ return nfs_ok;
+ switch (w) {
+ case NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL:
+ case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
+ return nfs_ok;
+ }
+xdr_error:
+ return nfserr_bad_xdr;
+}
+
+static __be32 nfsd4_decode_share_deny(struct nfsd4_compoundargs *argp, u32 *x)
+{
+ __be32 *p;
+
+ READ_BUF(4);
+ READ32(*x);
+ /* Note: unlinke access bits, deny bits may be zero. */
+ if (*x & !NFS4_SHARE_DENY_BOTH)
+ return nfserr_bad_xdr;
+ return nfs_ok;
+xdr_error:
+ return nfserr_bad_xdr;
+}
+
static __be32
nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
{
@@ -649,10 +707,15 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
open->op_openowner = NULL;
/* seqid, share_access, share_deny, clientid, ownerlen */
- READ_BUF(16 + sizeof(clientid_t));
+ READ_BUF(4);
READ32(open->op_seqid);
- READ32(open->op_share_access);
- READ32(open->op_share_deny);
+ status = nfsd4_decode_share_access(argp, &open->op_share_access);
+ if (status)
+ goto xdr_error;
+ status = nfsd4_decode_share_deny(argp, &open->op_share_deny);
+ if (status)
+ goto xdr_error;
+ READ_BUF(sizeof(clientid_t) + 4);
COPYMEM(&open->op_clientid, sizeof(clientid_t));
READ32(open->op_owner.len);
@@ -753,11 +816,14 @@ nfsd4_decode_open_downgrade(struct nfsd4_compoundargs *argp, struct nfsd4_open_d
status = nfsd4_decode_stateid(argp, &open_down->od_stateid);
if (status)
return status;
- READ_BUF(12);
+ READ_BUF(4);
READ32(open_down->od_seqid);
- READ32(open_down->od_share_access);
- READ32(open_down->od_share_deny);
-
+ status = nfsd4_decode_share_access(argp, &open_down->od_share_access);
+ if (status)
+ return status;
+ status = nfsd4_decode_share_deny(argp, &open_down->od_share_deny);
+ if (status)
+ return status;
DECODE_TAIL;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/4] nfsd4: move name-length checks to xdr
2011-10-11 16:40 patches for 3.2 J. Bruce Fields
2011-10-11 16:41 ` [PATCH 1/4] nfsd4: ignore WANT bits in open downgrade J. Bruce Fields
2011-10-11 16:41 ` [PATCH 2/4] nfsd4: move access/deny validity checks to xdr code J. Bruce Fields
@ 2011-10-11 16:41 ` J. Bruce Fields
2011-10-11 16:41 ` [PATCH 4/4] nfsd4: more robust ignoring of WANT bits in OPEN J. Bruce Fields
3 siblings, 0 replies; 5+ messages in thread
From: J. Bruce Fields @ 2011-10-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
Again, these checks are better in the xdr code.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4state.c | 24 ++----------------------
fs/nfsd/nfs4xdr.c | 45 ++++++++++++++++++++++++++++++---------------
fs/nfsd/xdr4.h | 3 +--
3 files changed, 33 insertions(+), 39 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6bfa293..5f35f35 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1189,17 +1189,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
return clp;
}
-static int check_name(struct xdr_netobj name)
-{
- if (name.len == 0)
- return 0;
- if (name.len > NFS4_OPAQUE_LIMIT) {
- dprintk("NFSD: check_name: name too long(%d)!\n", name.len);
- return 0;
- }
- return 1;
-}
-
static void
add_to_unconfirmed(struct nfs4_client *clp, unsigned int strhashval)
{
@@ -1442,7 +1431,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
__func__, rqstp, exid, exid->clname.len, exid->clname.data,
addr_str, exid->flags, exid->spa_how);
- if (!check_name(exid->clname) || (exid->flags & ~EXCHGID4_FLAG_MASK_A))
+ if (exid->flags & ~EXCHGID4_FLAG_MASK_A)
return nfserr_inval;
/* Currently only support SP4_NONE */
@@ -1992,19 +1981,13 @@ __be32
nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_setclientid *setclid)
{
- struct xdr_netobj clname = {
- .len = setclid->se_namelen,
- .data = setclid->se_name,
- };
+ struct xdr_netobj clname = setclid->se_name;
nfs4_verifier clverifier = setclid->se_verf;
unsigned int strhashval;
struct nfs4_client *conf, *unconf, *new;
__be32 status;
char dname[HEXDIR_LEN];
- if (!check_name(clname))
- return nfserr_inval;
-
status = nfs4_make_rec_clidname(dname, &clname);
if (status)
return status;
@@ -2523,9 +2506,6 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
struct nfs4_openowner *oo = NULL;
__be32 status;
- if (!check_name(open->op_owner))
- return nfserr_inval;
-
if (STALE_CLIENTID(&open->op_clientid))
return nfserr_stale_clientid;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 94da8bb..2cab33c 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -697,6 +697,23 @@ xdr_error:
return nfserr_bad_xdr;
}
+static __be32 nfsd4_decode_opaque(struct nfsd4_compoundargs *argp, struct xdr_netobj *o)
+{
+ __be32 *p;
+
+ READ_BUF(4);
+ READ32(o->len);
+
+ if (o->len == 0 || o->len > NFS4_OPAQUE_LIMIT)
+ return nfserr_bad_xdr;
+
+ READ_BUF(o->len);
+ SAVEMEM(o->data, o->len);
+ return nfs_ok;
+xdr_error:
+ return nfserr_bad_xdr;
+}
+
static __be32
nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
{
@@ -715,13 +732,12 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
status = nfsd4_decode_share_deny(argp, &open->op_share_deny);
if (status)
goto xdr_error;
- READ_BUF(sizeof(clientid_t) + 4);
+ READ_BUF(sizeof(clientid_t));
COPYMEM(&open->op_clientid, sizeof(clientid_t));
- READ32(open->op_owner.len);
-
- /* owner, open_flag */
- READ_BUF(open->op_owner.len + 4);
- SAVEMEM(open->op_owner.data, open->op_owner.len);
+ status = nfsd4_decode_opaque(argp, &open->op_owner);
+ if (status)
+ goto xdr_error;
+ READ_BUF(4);
READ32(open->op_create);
switch (open->op_create) {
case NFS4_OPEN_NOCREATE:
@@ -964,12 +980,13 @@ nfsd4_decode_setclientid(struct nfsd4_compoundargs *argp, struct nfsd4_setclient
{
DECODE_HEAD;
- READ_BUF(12);
+ READ_BUF(8);
COPYMEM(setclientid->se_verf.data, 8);
- READ32(setclientid->se_namelen);
- READ_BUF(setclientid->se_namelen + 8);
- SAVEMEM(setclientid->se_name, setclientid->se_namelen);
+ status = nfsd4_decode_opaque(argp, &setclientid->se_name);
+ if (status)
+ return nfserr_bad_xdr;
+ READ_BUF(8);
READ32(setclientid->se_callback_prog);
READ32(setclientid->se_callback_netid_len);
@@ -1112,11 +1129,9 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
READ_BUF(NFS4_VERIFIER_SIZE);
COPYMEM(exid->verifier.data, NFS4_VERIFIER_SIZE);
- READ_BUF(4);
- READ32(exid->clname.len);
-
- READ_BUF(exid->clname.len);
- SAVEMEM(exid->clname.data, exid->clname.len);
+ status = nfsd4_decode_opaque(argp, &exid->clname);
+ if (status)
+ return nfserr_bad_xdr;
READ_BUF(4);
READ32(exid->flags);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index c901214..4c8a7ec 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -317,8 +317,7 @@ struct nfsd4_setattr {
struct nfsd4_setclientid {
nfs4_verifier se_verf; /* request */
- u32 se_namelen; /* request */
- char * se_name; /* request */
+ struct xdr_netobj se_name;
u32 se_callback_prog; /* request */
u32 se_callback_netid_len; /* request */
char * se_callback_netid_val; /* request */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4] nfsd4: more robust ignoring of WANT bits in OPEN
2011-10-11 16:40 patches for 3.2 J. Bruce Fields
` (2 preceding siblings ...)
2011-10-11 16:41 ` [PATCH 3/4] nfsd4: move name-length checks to xdr J. Bruce Fields
@ 2011-10-11 16:41 ` J. Bruce Fields
3 siblings, 0 replies; 5+ messages in thread
From: J. Bruce Fields @ 2011-10-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: J. Bruce Fields
Mask out the WANT bits right at the start instead of on each use.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/nfsd/nfs4proc.c | 3 +++
fs/nfsd/nfs4state.c | 5 ++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4e41f65..5b192a2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -315,6 +315,9 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL)
return nfserr_inval;
+ /* We don't yet support WANT bits: */
+ open->op_share_access &= NFS4_SHARE_ACCESS_MASK;
+
/*
* RFC5661 18.51.3
* Before RECLAIM_COMPLETE done, server should deny new lock
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5f35f35..2042805 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2345,8 +2345,7 @@ static inline __be32 init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_
stp->st_file = fp;
stp->st_access_bmap = 0;
stp->st_deny_bmap = 0;
- __set_bit(open->op_share_access & ~NFS4_SHARE_WANT_MASK,
- &stp->st_access_bmap);
+ __set_bit(open->op_share_access, &stp->st_access_bmap);
__set_bit(open->op_share_deny, &stp->st_deny_bmap);
stp->st_openstp = NULL;
return nfs_ok;
@@ -2690,7 +2689,7 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
static __be32
nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp, struct nfsd4_open *open)
{
- u32 op_share_access = open->op_share_access & ~NFS4_SHARE_WANT_MASK;
+ u32 op_share_access = open->op_share_access;
bool new_access;
__be32 status;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread