Re: [PATCH 3/3] NFS: SETCLIENTID truncates client ID and netid

[Peter Staubach <staubach@redhat.com>]

Chuck Lever wrote:
> The sc_name field is currently 56 bytes long.  This is not large enough
> to hold a pair of IPv6 addresses, the authentication type, the protocol
> name, and a uniquifier number.  The maximum possible size of the name
> string using IPv6 addresses is just under 110 bytes, so I increased the
> size of the sc_name field to accomodate this maximum.
>
> In addition, the strings in the nfs4_setclientid structure are
> constructed with scnprintf(), which wants to terminate its output with
> '\0'.  The sc_netid field was large enough only for a three byte netid
> string and a '\0' so inet6 netids were being truncated.  Perhaps we
> don't need the overhead of scnprintf() to do a simple string copy, but
> I fixed this by increasing the size of the buffer by one byte.
>
> Since all three of the string buffers in nfs4_setclientid are
> constructed with scnprintf(), I increased the size of all three by one
> byte to document the requirement, although I don't think either the
> universal address field or the name field will be so small that these
> strings get truncated in this way.
>
> The size of the Linux client's client ID on the wire will be larger
> than before.  RFC 3530 suggests the size limit for client IDs is 1024,
> and we are still well below that.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>
>  include/linux/nfs_xdr.h |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 8c77c11..dc34977 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -672,16 +672,16 @@ struct nfs4_rename_res {
>  	struct nfs_fattr *		new_fattr;
>  };
>  
> -#define NFS4_SETCLIENTID_NAMELEN	(56)
> +#define NFS4_SETCLIENTID_NAMELEN	(128)
>   

Perhaps (127) might have been a better choice here?  In the
struct below, the arrays end up being allocated to (128) + 1
plus whatever alignment bytes are valid for the platform.
This wastes a fair amount of space.

       ps

>  struct nfs4_setclientid {
>  	const nfs4_verifier *		sc_verifier;
>  	unsigned int			sc_name_len;
> -	char				sc_name[NFS4_SETCLIENTID_NAMELEN];
> +	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
>  	u32				sc_prog;
>  	unsigned int			sc_netid_len;
> -	char				sc_netid[RPCBIND_MAXNETIDLEN];
> +	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
>  	unsigned int			sc_uaddr_len;
> -	char				sc_uaddr[RPCBIND_MAXUADDRLEN];
> +	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
>  	u32				sc_cb_ident;
>  };
>  
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>