Re: [PATCH] nfsd41: error out when client sets maxreq_sz or maxresp_sz too small

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, May 18, 2011 at 10:12:43AM +0800, Mi Jinlong wrote:
> According to RFC5661, 18.36.3, 
> 
>  "if the client selects a value for ca_maxresponsesize such that
>   a replier on a channel could never send a response,the server
>   SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."
> 
> This patch let server error out when client sets maxreq_sz less than
> SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.
> 
> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com>
> ---
>  fs/nfsd/nfs4xdr.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index c6766af..8983d03 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -131,6 +131,14 @@ xdr_error:					\
>  	}					\
>  } while (0)
>  
> +#define op_decode_hdr_size	(1)
> +#define op_encode_hdr_size	(2)
> +
> +#define decode_sequence_size	(op_decode_hdr_size + \
> +				XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + 4)
> +#define encode_sequence_size	(op_encode_hdr_size + \
> +				XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + 5)
> +

>From the description of ca_maxrequestsize on p. 515:

	"This size represents the XDR encoded size of the request,
	including the RPC headers (including security flavor credentials
	and verifiers) but excludes any RPC transport framing headers."

There's no way to know how big the verifier and credential will be, but
for the purposes of this function I guess we could assume they're both 2
u32's (flavor + zero length).

Looks fine to me otherwise.

I assume you checked these are the ops that give the shortest possible
request and response.

--b.

>  static __be32 *read_buf(struct nfsd4_compoundargs *argp, u32 nbytes)
>  {
>  	/* We want more bytes than seem to be available.
> @@ -1154,7 +1162,17 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
>  	READ_BUF(28);
>  	READ32(dummy); /* headerpadsz is always 0 */
>  	READ32(sess->fore_channel.maxreq_sz);
> +	if (sess->fore_channel.maxreq_sz < decode_sequence_size) {
> +		status = nfserr_toosmall;
> +		goto out;
> +	}
> +
>  	READ32(sess->fore_channel.maxresp_sz);
> +	if (sess->fore_channel.maxresp_sz < encode_sequence_size) {
> +		status = nfserr_toosmall;
> +		goto out;
> +	}
> +
>  	READ32(sess->fore_channel.maxresp_cached);
>  	READ32(sess->fore_channel.maxops);
>  	READ32(sess->fore_channel.maxreqs);
> -- 
> 1.7.4.5
> 
> 
>