Skip to content

Commit

Permalink
NFSD: Replace READ* macros in nfsd4_decode_fattr()
Browse files Browse the repository at this point in the history
Let's be more careful to avoid overrunning the memory that backs
the bitmap array. This requires updating the synopsis of
nfsd4_decode_fattr().

Bruce points out that a server needs to be careful to return nfs_ok
when a client presents bitmap bits the server doesn't support. This
includes bits in bitmap words the server might not yet support.

The current READ* based implementation is good about that, but that
requirement hasn't been documented.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
  • Loading branch information
Chuck Lever committed Nov 30, 2020
1 parent 66f0476 commit d1c263a
Showing 1 changed file with 64 additions and 18 deletions.
82 changes: 64 additions & 18 deletions fs/nfsd/nfs4xdr.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,46 @@ nfsd4_decode_bitmap(struct nfsd4_compoundargs *argp, u32 *bmval)
DECODE_TAIL;
}

/**
* nfsd4_decode_bitmap4 - Decode an NFSv4 bitmap4
* @argp: NFSv4 compound argument structure
* @bmval: pointer to an array of u32's to decode into
* @bmlen: size of the @bmval array
*
* The server needs to return nfs_ok rather than nfserr_bad_xdr when
* encountering bitmaps containing bits it does not recognize. This
* includes bits in bitmap words past WORDn, where WORDn is the last
* bitmap WORD the implementation currently supports. Thus we are
* careful here to simply ignore bits in bitmap words that this
* implementation has yet to support explicitly.
*
* Return values:
* %nfs_ok: @bmval populated successfully
* %nfserr_bad_xdr: the encoded bitmap was invalid
*/
static __be32
nfsd4_decode_bitmap4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen)
{
u32 i, count;
__be32 *p;

if (xdr_stream_decode_u32(argp->xdr, &count) < 0)
return nfserr_bad_xdr;
/* request sanity */
if (count > 1000)
return nfserr_bad_xdr;
p = xdr_inline_decode(argp->xdr, count << 2);
if (!p)
return nfserr_bad_xdr;
i = 0;
while (i < count)
bmval[i++] = be32_to_cpup(p++);
while (i < bmlen)
bmval[i++] = 0;

return nfs_ok;
}

static __be32
nfsd4_decode_nfsace4(struct nfsd4_compoundargs *argp, struct nfs4_ace *ace)
{
Expand Down Expand Up @@ -352,17 +392,18 @@ nfsd4_decode_security_label(struct nfsd4_compoundargs *argp,
}

static __be32
nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
struct iattr *iattr, struct nfs4_acl **acl,
struct xdr_netobj *label, int *umask)
nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
struct iattr *iattr, struct nfs4_acl **acl,
struct xdr_netobj *label, int *umask)
{
unsigned int starting_pos;
u32 attrlist4_count;
__be32 *p, status;

DECODE_HEAD;
iattr->ia_valid = 0;
if ((status = nfsd4_decode_bitmap(argp, bmval)))
return status;
status = nfsd4_decode_bitmap4(argp, bmval, bmlen);
if (status)
return nfserr_bad_xdr;

if (bmval[0] & ~NFSD_WRITEABLE_ATTRS_WORD0
|| bmval[1] & ~NFSD_WRITEABLE_ATTRS_WORD1
Expand Down Expand Up @@ -490,7 +531,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
return nfserr_bad_xdr;

DECODE_TAIL;
return nfs_ok;
}

static __be32
Expand Down Expand Up @@ -690,9 +731,10 @@ nfsd4_decode_create(struct nfsd4_compoundargs *argp, struct nfsd4_create *create
if ((status = check_filename(create->cr_name, create->cr_namelen)))
return status;

status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
&create->cr_acl, &create->cr_label,
&create->cr_umask);
status = nfsd4_decode_fattr4(argp, create->cr_bmval,
ARRAY_SIZE(create->cr_bmval),
&create->cr_iattr, &create->cr_acl,
&create->cr_label, &create->cr_umask);
if (status)
goto out;

Expand Down Expand Up @@ -941,9 +983,10 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
switch (open->op_createmode) {
case NFS4_CREATE_UNCHECKED:
case NFS4_CREATE_GUARDED:
status = nfsd4_decode_fattr(argp, open->op_bmval,
&open->op_iattr, &open->op_acl, &open->op_label,
&open->op_umask);
status = nfsd4_decode_fattr4(argp, open->op_bmval,
ARRAY_SIZE(open->op_bmval),
&open->op_iattr, &open->op_acl,
&open->op_label, &open->op_umask);
if (status)
goto out;
break;
Expand All @@ -956,9 +999,10 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
goto xdr_error;
READ_BUF(NFS4_VERIFIER_SIZE);
COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
status = nfsd4_decode_fattr(argp, open->op_bmval,
&open->op_iattr, &open->op_acl, &open->op_label,
&open->op_umask);
status = nfsd4_decode_fattr4(argp, open->op_bmval,
ARRAY_SIZE(open->op_bmval),
&open->op_iattr, &open->op_acl,
&open->op_label, &open->op_umask);
if (status)
goto out;
break;
Expand Down Expand Up @@ -1194,8 +1238,10 @@ nfsd4_decode_setattr(struct nfsd4_compoundargs *argp, struct nfsd4_setattr *seta
status = nfsd4_decode_stateid(argp, &setattr->sa_stateid);
if (status)
return status;
return nfsd4_decode_fattr(argp, setattr->sa_bmval, &setattr->sa_iattr,
&setattr->sa_acl, &setattr->sa_label, NULL);
return nfsd4_decode_fattr4(argp, setattr->sa_bmval,
ARRAY_SIZE(setattr->sa_bmval),
&setattr->sa_iattr, &setattr->sa_acl,
&setattr->sa_label, NULL);
}

static __be32
Expand Down

0 comments on commit d1c263a

Please sign in to comment.