Skip to content

Commit

Permalink
nfsd: catch errors in decode_fattr earlier
Browse files Browse the repository at this point in the history
3c8e031 "NFSv4: do exact check about attribute specified" fixed
some handling of unsupported-attribute errors, but it also delayed
checking for unwriteable attributes till after we decode them.  This
could lead to odd behavior in the case a client attemps to set an
attribute we don't know about followed by one we try to parse.  In that
case the parser for the known attribute will attempt to parse the
unknown attribute.  It should fail in some safe way, but the error might
at least be incorrect (probably bad_xdr instead of inval).  So, it's
better to do that check at the start.

As far as I know this doesn't cause any problems with current clients
but it might be a minor issue e.g. if we encounter a future client that
supports a new attribute that we currently don't.

Cc: Yu Zhiguo <yuzg@cn.fujitsu.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
  • Loading branch information
J. Bruce Fields committed Nov 1, 2016
1 parent 916d2d8 commit e864c18
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
15 changes: 9 additions & 6 deletions fs/nfsd/nfs4xdr.c
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,14 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
if ((status = nfsd4_decode_bitmap(argp, bmval)))
return status;

if (bmval[0] & ~NFSD_WRITEABLE_ATTRS_WORD0
|| bmval[1] & ~NFSD_WRITEABLE_ATTRS_WORD1
|| bmval[2] & ~NFSD_WRITEABLE_ATTRS_WORD2) {
if (nfsd_attrs_supported(argp->minorversion, bmval))
return nfserr_inval;
return nfserr_attrnotsupp;
}

READ_BUF(4);
expected_len = be32_to_cpup(p++);

Expand Down Expand Up @@ -449,12 +457,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
return nfserr_jukebox;
}
#endif

if (bmval[0] & ~NFSD_WRITEABLE_ATTRS_WORD0
|| bmval[1] & ~NFSD_WRITEABLE_ATTRS_WORD1
|| bmval[2] & ~NFSD_WRITEABLE_ATTRS_WORD2)
READ_BUF(expected_len - len);
else if (len != expected_len)
if (len != expected_len)
goto xdr_error;

DECODE_TAIL;
Expand Down
6 changes: 5 additions & 1 deletion fs/nfsd/nfsd.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,11 @@ static inline bool nfsd_attrs_supported(u32 minorversion, u32 *bmval)
#define NFSD_WRITEONLY_ATTRS_WORD1 \
(FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)

/* These are the only attrs allowed in CREATE/OPEN/SETATTR. */
/*
* These are the only attrs allowed in CREATE/OPEN/SETATTR. Don't add
* a writeable attribute here without also adding code to parse it to
* nfsd4_decode_fattr().
*/
#define NFSD_WRITEABLE_ATTRS_WORD0 \
(FATTR4_WORD0_SIZE | FATTR4_WORD0_ACL)
#define NFSD_WRITEABLE_ATTRS_WORD1 \
Expand Down

0 comments on commit e864c18

Please sign in to comment.