[PATCH 0/4] nfs: fix NFSv4.x client name generation problems

[PATCH 0/4] nfs: fix NFSv4.x client name generation problems

[Jeff Layton]

Michael Skralivetsky reported some problems recently that turned out to
be client name string collisions. He has a test environment where the
hostnames are quite long, and only differ at the very end.

The v4.1 name string buffer is currently limited to 48 bytes, which is
very small. When we go to build the name string, the end can be
truncated and if the hostnames are long enough, this can allow the
client identifiers to be duplicates of one another.

This patchset should fix the problem by allowing the client name string
to be as long as the spec allows (1k). It also gets rid of the large-ish
on-stack buffers that are currently used to build these strings, and
fixes some potential memory reclaim recursion problems that could occur
when these strings are generated.

Jeff Layton (4):
  nfs: convert setclientid and exchange_id encoders to use
    clp->cl_owner_id
  nfs: update maxsz values for SETCLIENTID and EXCHANGE_ID
  nfs: make nfs4_init_nonuniform_client_string use a dynamically
    allocated buffer
  nfs: make nfs4_init_uniform_client_string use a dynamically allocated
    buffer

 fs/nfs/nfs4proc.c       | 159 ++++++++++++++++++++++++++++++++++--------------
 fs/nfs/nfs4xdr.c        |  11 ++--
 include/linux/nfs_xdr.h |   8 +--
 3 files changed, 121 insertions(+), 57 deletions(-)

-- 
2.4.2

[PATCH 1/4] nfs: convert setclientid and exchange_id encoders to use clp->cl_owner_id

[Jeff Layton]

...instead of buffers that are part of their arg structs. We already
hold a reference to the client, so we might as well use the allocated
buffer. In the event that we can't allocate the clp->cl_owner_id, then
just return -ENOMEM.

Note too that we switch from a GFP_KERNEL allocation here to GFP_NOFS.
It's possible we could end up trying to do a SETCLIENTID or EXCHANGE_ID
in order to reclaim some memory, and the GFP_KERNEL allocations in the
existing code could cause recursion back into NFS reclaim.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/nfs4proc.c       | 25 ++++++++++++++++++-------
 fs/nfs/nfs4xdr.c        |  7 ++++---
 include/linux/nfs_xdr.h |  2 +-
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 55e1e3af23a3..24256bf389e5 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4972,7 +4972,7 @@ nfs4_init_nonuniform_client_string(struct nfs_client *clp,
 				rpc_peeraddr2str(clp->cl_rpcclient,
 							RPC_DISPLAY_PROTO));
 	rcu_read_unlock();
-	clp->cl_owner_id = kstrdup(buf, GFP_KERNEL);
+	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
 	return result;
 }
 
@@ -4996,7 +4996,7 @@ nfs4_init_uniform_client_string(struct nfs_client *clp,
 		result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
 				clp->rpc_ops->version, clp->cl_minorversion,
 				nodename);
-	clp->cl_owner_id = kstrdup(buf, GFP_KERNEL);
+	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
 	return result;
 }
 
@@ -5044,7 +5044,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 	struct nfs4_setclientid setclientid = {
 		.sc_verifier = &sc_verifier,
 		.sc_prog = program,
-		.sc_cb_ident = clp->cl_cb_ident,
+		.sc_clnt = clp,
 	};
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
@@ -5074,6 +5074,12 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 				nfs4_init_nonuniform_client_string(clp,
 						setclientid.sc_name,
 						sizeof(setclientid.sc_name));
+
+	if (!clp->cl_owner_id) {
+		status = -ENOMEM;
+		goto out;
+	}
+
 	/* cb_client4 */
 	setclientid.sc_netid_len =
 				nfs4_init_callback_netid(clp,
@@ -5083,9 +5089,9 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 				sizeof(setclientid.sc_uaddr), "%s.%u.%u",
 				clp->cl_ipaddr, port >> 8, port & 255);
 
-	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
+	dprintk("NFS call  setclientid auth=%s, '%s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
-		setclientid.sc_name_len, setclientid.sc_name);
+		clp->cl_owner_id);
 	task = rpc_run_task(&task_setup_data);
 	if (IS_ERR(task)) {
 		status = PTR_ERR(task);
@@ -6848,9 +6854,14 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	nfs4_init_boot_verifier(clp, &verifier);
 	args.id_len = nfs4_init_uniform_client_string(clp, args.id,
 							sizeof(args.id));
-	dprintk("NFS call  exchange_id auth=%s, '%.*s'\n",
+	if (!clp->cl_owner_id) {
+		status = -ENOMEM;
+		goto out;
+	}
+
+	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
-		args.id_len, args.id);
+		clp->cl_owner_id);
 
 	res.server_owner = kzalloc(sizeof(struct nfs41_server_owner),
 					GFP_NOFS);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 0aea97841d30..2826e117e3df 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1667,13 +1667,13 @@ static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclie
 	encode_op_hdr(xdr, OP_SETCLIENTID, decode_setclientid_maxsz, hdr);
 	encode_nfs4_verifier(xdr, setclientid->sc_verifier);
 
-	encode_string(xdr, setclientid->sc_name_len, setclientid->sc_name);
+	encode_string(xdr, strlen(setclientid->sc_clnt->cl_owner_id), setclientid->sc_clnt->cl_owner_id);
 	p = reserve_space(xdr, 4);
 	*p = cpu_to_be32(setclientid->sc_prog);
 	encode_string(xdr, setclientid->sc_netid_len, setclientid->sc_netid);
 	encode_string(xdr, setclientid->sc_uaddr_len, setclientid->sc_uaddr);
 	p = reserve_space(xdr, 4);
-	*p = cpu_to_be32(setclientid->sc_cb_ident);
+	*p = cpu_to_be32(setclientid->sc_clnt->cl_cb_ident);
 }
 
 static void encode_setclientid_confirm(struct xdr_stream *xdr, const struct nfs4_setclientid_res *arg, struct compound_hdr *hdr)
@@ -1747,7 +1747,8 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 	encode_op_hdr(xdr, OP_EXCHANGE_ID, decode_exchange_id_maxsz, hdr);
 	encode_nfs4_verifier(xdr, args->verifier);
 
-	encode_string(xdr, args->id_len, args->id);
+	encode_string(xdr, strlen(args->client->cl_owner_id),
+			args->client->cl_owner_id);
 
 	encode_uint32(xdr, args->flags);
 	encode_uint32(xdr, args->state_protect.how);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 93ab6071bbe9..16991f5e274e 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -994,7 +994,7 @@ struct nfs4_setclientid {
 	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
 	unsigned int			sc_uaddr_len;
 	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
-	u32				sc_cb_ident;
+	struct nfs_client		*sc_clnt;
 	struct rpc_cred			*sc_cred;
 };
 
-- 
2.4.2

[PATCH 2/4] nfs: update maxsz values for SETCLIENTID and EXCHANGE_ID

[Jeff Layton]

The spec allows for up to NFS4_OPAQUE_LIMIT (1k). While we'll almost
certainly never use that much, these ops are generally the only ones
in the compound so we might as well allow for them to be that large.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/nfs4xdr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 2826e117e3df..e105b9363f0a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -139,7 +139,7 @@ static int nfs4_stat_to_errno(int);
 #define encode_setclientid_maxsz \
 				(op_encode_hdr_maxsz + \
 				XDR_QUADLEN(NFS4_VERIFIER_SIZE) + \
-				XDR_QUADLEN(NFS4_SETCLIENTID_NAMELEN) + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
 				1 /* sc_prog */ + \
 				1 + XDR_QUADLEN(RPCBIND_MAXNETIDLEN) + \
 				1 + XDR_QUADLEN(RPCBIND_MAXUADDRLEN) + \
@@ -288,7 +288,7 @@ static int nfs4_stat_to_errno(int);
 #define encode_exchange_id_maxsz (op_encode_hdr_maxsz + \
 				encode_verifier_maxsz + \
 				1 /* co_ownerid.len */ + \
-				XDR_QUADLEN(NFS4_EXCHANGE_ID_LEN) + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
 				1 /* flags */ + \
 				1 /* spa_how */ + \
 				/* max is SP4_MACH_CRED (for now) */ + \
-- 
2.4.2

[PATCH 3/4] nfs: make nfs4_init_nonuniform_client_string use a dynamically allocated buffer

[Jeff Layton]

The way the *_client_string functions work is a little goofy. They build
the string in an on-stack buffer and then use kstrdup to copy it. This
is not only stack-heavy but artificially limits the size of the client
name string. Change it so that we determine the length of the string,
allocate it and then scnprintf into it.

Since the contents of the nonuniform string depend on rcu-managed data
structures, it's possible that they'll change between when we allocate
the string and when we go to fill it. If that happens, free the string,
recalculate the length and try again. If it the mismatch isn't resolved
on the second try then just give up and return -EINVAL.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/nfs4proc.c | 69 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 24256bf389e5..49e3b3e55887 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4955,25 +4955,49 @@ static void nfs4_init_boot_verifier(const struct nfs_client *clp,
 	memcpy(bootverf->data, verf, sizeof(bootverf->data));
 }
 
-static unsigned int
-nfs4_init_nonuniform_client_string(struct nfs_client *clp,
-				   char *buf, size_t len)
+static int
+nfs4_init_nonuniform_client_string(struct nfs_client *clp)
 {
-	unsigned int result;
+	int result;
+	size_t len;
+	char *str;
+	bool retried = false;
 
 	if (clp->cl_owner_id != NULL)
-		return strlcpy(buf, clp->cl_owner_id, len);
+		return 0;
+retry:
+	rcu_read_lock();
+	len = 10 + strlen(clp->cl_ipaddr) + 1 +
+		strlen(rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR)) +
+		1 +
+		strlen(rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_PROTO)) +
+		1;
+	rcu_read_unlock();
+
+	if (len > NFS4_OPAQUE_LIMIT + 1)
+		return -EINVAL;
+
+	str = kmalloc(len, GFP_NOFS);
+	if (!str)
+		return -ENOMEM;
 
 	rcu_read_lock();
-	result = scnprintf(buf, len, "Linux NFSv4.0 %s/%s %s",
-				clp->cl_ipaddr,
-				rpc_peeraddr2str(clp->cl_rpcclient,
-							RPC_DISPLAY_ADDR),
-				rpc_peeraddr2str(clp->cl_rpcclient,
-							RPC_DISPLAY_PROTO));
+	result = scnprintf(str, len, "Linux NFSv4.0 %s/%s %s",
+			clp->cl_ipaddr,
+			rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR),
+			rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_PROTO));
 	rcu_read_unlock();
-	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
-	return result;
+
+	/* Did something change? */
+	if (result >= len) {
+		kfree(str);
+		if (retried)
+			return -EINVAL;
+		retried = true;
+		goto retry;
+	}
+	clp->cl_owner_id = str;
+	return 0;
 }
 
 static unsigned int
@@ -5064,20 +5088,19 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 
 	/* nfs_client_id4 */
 	nfs4_init_boot_verifier(clp, &sc_verifier);
-	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags))
+	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags)) {
 		setclientid.sc_name_len =
 				nfs4_init_uniform_client_string(clp,
 						setclientid.sc_name,
 						sizeof(setclientid.sc_name));
-	else
-		setclientid.sc_name_len =
-				nfs4_init_nonuniform_client_string(clp,
-						setclientid.sc_name,
-						sizeof(setclientid.sc_name));
-
-	if (!clp->cl_owner_id) {
-		status = -ENOMEM;
-		goto out;
+		if (!clp->cl_owner_id) {
+			status = -ENOMEM;
+			goto out;
+		}
+	} else {
+		status = nfs4_init_nonuniform_client_string(clp);
+		if (status)
+			goto out;
 	}
 
 	/* cb_client4 */
-- 
2.4.2

[PATCH 4/4] nfs: make nfs4_init_uniform_client_string use a dynamically allocated buffer

[Jeff Layton]

Change the uniform client string generator to use the same scheme as
the nonuniform one. This one is a little simpler since we don't need to
deal with RCU, but we do have two different cases, depending on whether
there is a uniquifier or not.

Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
 fs/nfs/nfs4proc.c       | 105 ++++++++++++++++++++++++++++++++----------------
 include/linux/nfs_xdr.h |   6 ---
 2 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 49e3b3e55887..479b0f583f33 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5000,28 +5000,71 @@ retry:
 	return 0;
 }
 
-static unsigned int
-nfs4_init_uniform_client_string(struct nfs_client *clp,
-				char *buf, size_t len)
+static int
+nfs4_init_uniquifier_client_string(struct nfs_client *clp)
 {
-	const char *nodename = clp->cl_rpcclient->cl_nodename;
-	unsigned int result;
+	int result;
+	size_t len;
+	char *str;
 
 	if (clp->cl_owner_id != NULL)
-		return strlcpy(buf, clp->cl_owner_id, len);
+		return 0;
+
+	len = 10 + 10 + 1 + 10 + 1 +
+		strlen(nfs4_client_id_uniquifier) + 1 +
+		strlen(clp->cl_rpcclient->cl_nodename) + 1;
+
+	if (len > NFS4_OPAQUE_LIMIT + 1)
+		return -EINVAL;
+
+	str = kmalloc(len, GFP_NOFS);
+	if (!str)
+		return -ENOMEM;
+
+	result = scnprintf(str, len, "Linux NFSv%u.%u %s/%s",
+			clp->rpc_ops->version, clp->cl_minorversion,
+			nfs4_client_id_uniquifier,
+			clp->cl_rpcclient->cl_nodename);
+	if (result >= len) {
+		kfree(str);
+		return -EINVAL;
+	}
+	clp->cl_owner_id = str;
+	return 0;
+}
+
+static int
+nfs4_init_uniform_client_string(struct nfs_client *clp)
+{
+	int result;
+	size_t len;
+	char *str;
+
+	if (clp->cl_owner_id != NULL)
+		return 0;
 
 	if (nfs4_client_id_uniquifier[0] != '\0')
-		result = scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
-				clp->rpc_ops->version,
-				clp->cl_minorversion,
-				nfs4_client_id_uniquifier,
-				nodename);
-	else
-		result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
-				clp->rpc_ops->version, clp->cl_minorversion,
-				nodename);
-	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
-	return result;
+		return nfs4_init_uniquifier_client_string(clp);
+
+	len = 10 + 10 + 1 + 10 + 1 +
+		strlen(clp->cl_rpcclient->cl_nodename) + 1;
+
+	str = kmalloc(len, GFP_NOFS);
+	if (!str)
+		return -ENOMEM;
+
+	if (len > NFS4_OPAQUE_LIMIT + 1)
+		return -EINVAL;
+
+	result = scnprintf(str, len, "Linux NFSv%u.%u %s",
+			clp->rpc_ops->version, clp->cl_minorversion,
+			clp->cl_rpcclient->cl_nodename);
+	if (result >= len) {
+		kfree(str);
+		return -EINVAL;
+	}
+	clp->cl_owner_id = str;
+	return 0;
 }
 
 /*
@@ -5088,20 +5131,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 
 	/* nfs_client_id4 */
 	nfs4_init_boot_verifier(clp, &sc_verifier);
-	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags)) {
-		setclientid.sc_name_len =
-				nfs4_init_uniform_client_string(clp,
-						setclientid.sc_name,
-						sizeof(setclientid.sc_name));
-		if (!clp->cl_owner_id) {
-			status = -ENOMEM;
-			goto out;
-		}
-	} else {
+
+	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags))
+		status = nfs4_init_uniform_client_string(clp);
+	else
 		status = nfs4_init_nonuniform_client_string(clp);
-		if (status)
-			goto out;
-	}
+
+	if (status)
+		goto out;
 
 	/* cb_client4 */
 	setclientid.sc_netid_len =
@@ -6875,12 +6912,10 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	};
 
 	nfs4_init_boot_verifier(clp, &verifier);
-	args.id_len = nfs4_init_uniform_client_string(clp, args.id,
-							sizeof(args.id));
-	if (!clp->cl_owner_id) {
-		status = -ENOMEM;
+
+	status = nfs4_init_uniform_client_string(clp);
+	if (status)
 		goto out;
-	}
 
 	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 16991f5e274e..c959e7eb5bd4 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -984,11 +984,8 @@ struct nfs4_readlink_res {
 	struct nfs4_sequence_res	seq_res;
 };
 
-#define NFS4_SETCLIENTID_NAMELEN	(127)
 struct nfs4_setclientid {
 	const nfs4_verifier *		sc_verifier;
-	unsigned int			sc_name_len;
-	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
 	u32				sc_prog;
 	unsigned int			sc_netid_len;
 	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
@@ -1142,12 +1139,9 @@ struct nfs41_state_protection {
 	struct nfs4_op_map allow;
 };
 
-#define NFS4_EXCHANGE_ID_LEN	(48)
 struct nfs41_exchange_id_args {
 	struct nfs_client		*client;
 	nfs4_verifier			*verifier;
-	unsigned int 			id_len;
-	char 				id[NFS4_EXCHANGE_ID_LEN];
 	u32				flags;
 	struct nfs41_state_protection	state_protect;
 };
-- 
2.4.2

Re: [PATCH 4/4] nfs: make nfs4_init_uniform_client_string use a dynamically allocated buffer

[Jeff Layton]

On Thu,  4 Jun 2015 12:44:29 -0400
Jeff Layton <jlayton@poochiereds.net> wrote:

> Change the uniform client string generator to use the same scheme as
> the nonuniform one. This one is a little simpler since we don't need to
> deal with RCU, but we do have two different cases, depending on whether
> there is a uniquifier or not.
> 
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> ---
>  fs/nfs/nfs4proc.c       | 105 ++++++++++++++++++++++++++++++++----------------
>  include/linux/nfs_xdr.h |   6 ---
>  2 files changed, 70 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 49e3b3e55887..479b0f583f33 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5000,28 +5000,71 @@ retry:
>  	return 0;
>  }
>  
> -static unsigned int
> -nfs4_init_uniform_client_string(struct nfs_client *clp,
> -				char *buf, size_t len)
> +static int
> +nfs4_init_uniquifier_client_string(struct nfs_client *clp)
>  {
> -	const char *nodename = clp->cl_rpcclient->cl_nodename;
> -	unsigned int result;
> +	int result;
> +	size_t len;
> +	char *str;
>  
>  	if (clp->cl_owner_id != NULL)
> -		return strlcpy(buf, clp->cl_owner_id, len);
> +		return 0;
> +

Self review...

The above isn't necessary since we check that in the caller.

> +	len = 10 + 10 + 1 + 10 + 1 +
> +		strlen(nfs4_client_id_uniquifier) + 1 +
> +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> +
> +	if (len > NFS4_OPAQUE_LIMIT + 1)
> +		return -EINVAL;
> +
> +	str = kmalloc(len, GFP_NOFS);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	result = scnprintf(str, len, "Linux NFSv%u.%u %s/%s",
> +			clp->rpc_ops->version, clp->cl_minorversion,
> +			nfs4_client_id_uniquifier,
> +			clp->cl_rpcclient->cl_nodename);
> +	if (result >= len) {
> +		kfree(str);
> +		return -EINVAL;
> +	}
> +	clp->cl_owner_id = str;
> +	return 0;
> +}
> +
> +static int
> +nfs4_init_uniform_client_string(struct nfs_client *clp)
> +{
> +	int result;
> +	size_t len;
> +	char *str;
> +
> +	if (clp->cl_owner_id != NULL)
> +		return 0;
>  
>  	if (nfs4_client_id_uniquifier[0] != '\0')
> -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
> -				clp->rpc_ops->version,
> -				clp->cl_minorversion,
> -				nfs4_client_id_uniquifier,
> -				nodename);
> -	else
> -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
> -				clp->rpc_ops->version, clp->cl_minorversion,
> -				nodename);
> -	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
> -	return result;
> +		return nfs4_init_uniquifier_client_string(clp);
> +
> +	len = 10 + 10 + 1 + 10 + 1 +
> +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> +
> +	str = kmalloc(len, GFP_NOFS);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	if (len > NFS4_OPAQUE_LIMIT + 1)
> +		return -EINVAL;
> +

The above two if statements should be reordered. We want to check the
length before the allocation or we leak memory here when it's too long.
I'll fix that in v2 once everyone has had a chance to comment on the
rest.

> +	result = scnprintf(str, len, "Linux NFSv%u.%u %s",
> +			clp->rpc_ops->version, clp->cl_minorversion,
> +			clp->cl_rpcclient->cl_nodename);
> +	if (result >= len) {
> +		kfree(str);
> +		return -EINVAL;
> +	}
> +	clp->cl_owner_id = str;
> +	return 0;
>  }
>  
>  /*
> @@ -5088,20 +5131,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  
>  	/* nfs_client_id4 */
>  	nfs4_init_boot_verifier(clp, &sc_verifier);
> -	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags)) {
> -		setclientid.sc_name_len =
> -				nfs4_init_uniform_client_string(clp,
> -						setclientid.sc_name,
> -						sizeof(setclientid.sc_name));
> -		if (!clp->cl_owner_id) {
> -			status = -ENOMEM;
> -			goto out;
> -		}
> -	} else {
> +
> +	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags))
> +		status = nfs4_init_uniform_client_string(clp);
> +	else
>  		status = nfs4_init_nonuniform_client_string(clp);
> -		if (status)
> -			goto out;
> -	}
> +
> +	if (status)
> +		goto out;
>  
>  	/* cb_client4 */
>  	setclientid.sc_netid_len =
> @@ -6875,12 +6912,10 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>  	};
>  
>  	nfs4_init_boot_verifier(clp, &verifier);
> -	args.id_len = nfs4_init_uniform_client_string(clp, args.id,
> -							sizeof(args.id));
> -	if (!clp->cl_owner_id) {
> -		status = -ENOMEM;
> +
> +	status = nfs4_init_uniform_client_string(clp);
> +	if (status)
>  		goto out;
> -	}
>  
>  	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
>  		clp->cl_rpcclient->cl_auth->au_ops->au_name,
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 16991f5e274e..c959e7eb5bd4 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -984,11 +984,8 @@ struct nfs4_readlink_res {
>  	struct nfs4_sequence_res	seq_res;
>  };
>  
> -#define NFS4_SETCLIENTID_NAMELEN	(127)
>  struct nfs4_setclientid {
>  	const nfs4_verifier *		sc_verifier;
> -	unsigned int			sc_name_len;
> -	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
>  	u32				sc_prog;
>  	unsigned int			sc_netid_len;
>  	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
> @@ -1142,12 +1139,9 @@ struct nfs41_state_protection {
>  	struct nfs4_op_map allow;
>  };
>  
> -#define NFS4_EXCHANGE_ID_LEN	(48)
>  struct nfs41_exchange_id_args {
>  	struct nfs_client		*client;
>  	nfs4_verifier			*verifier;
> -	unsigned int 			id_len;
> -	char 				id[NFS4_EXCHANGE_ID_LEN];
>  	u32				flags;
>  	struct nfs41_state_protection	state_protect;
>  };


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH 4/4] nfs: make nfs4_init_uniform_client_string use a dynamically allocated buffer

[Jeff Layton]

On Thu, 4 Jun 2015 13:16:53 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Jun 4, 2015, at 12:44 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> 
> > Change the uniform client string generator to use the same scheme as
> > the nonuniform one.
> 
> This is a scary sentence. What do you mean by “same scheme” ? Is your new
> code following the same requirements for uniform client strings?
> 
> Are you changing the client to use only uniform strings? That will break
> some servers that cleave to the recommendations in RFC 7530.
> 

Sorry, I meant "same scheme as in patch #3". The actual string contents
are unchanged in this series, the only thing that is changing is how
the buffers for them are allocated.

I'll revise the patch description accordingly...

Thanks!

> 
> > This one is a little simpler since we don't need to
> > deal with RCU, but we do have two different cases, depending on whether
> > there is a uniquifier or not.
> > 
> > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> > ---
> > fs/nfs/nfs4proc.c       | 105 ++++++++++++++++++++++++++++++++----------------
> > include/linux/nfs_xdr.h |   6 ---
> > 2 files changed, 70 insertions(+), 41 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 49e3b3e55887..479b0f583f33 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5000,28 +5000,71 @@ retry:
> > 	return 0;
> > }
> > 
> > -static unsigned int
> > -nfs4_init_uniform_client_string(struct nfs_client *clp,
> > -				char *buf, size_t len)
> > +static int
> > +nfs4_init_uniquifier_client_string(struct nfs_client *clp)
> > {
> > -	const char *nodename = clp->cl_rpcclient->cl_nodename;
> > -	unsigned int result;
> > +	int result;
> > +	size_t len;
> > +	char *str;
> > 
> > 	if (clp->cl_owner_id != NULL)
> > -		return strlcpy(buf, clp->cl_owner_id, len);
> > +		return 0;
> > +
> > +	len = 10 + 10 + 1 + 10 + 1 +
> > +		strlen(nfs4_client_id_uniquifier) + 1 +
> > +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> > +
> > +	if (len > NFS4_OPAQUE_LIMIT + 1)
> > +		return -EINVAL;
> > +
> > +	str = kmalloc(len, GFP_NOFS);
> > +	if (!str)
> > +		return -ENOMEM;
> > +
> > +	result = scnprintf(str, len, "Linux NFSv%u.%u %s/%s",
> > +			clp->rpc_ops->version, clp->cl_minorversion,
> > +			nfs4_client_id_uniquifier,
> > +			clp->cl_rpcclient->cl_nodename);
> > +	if (result >= len) {
> > +		kfree(str);
> > +		return -EINVAL;
> > +	}
> > +	clp->cl_owner_id = str;
> > +	return 0;
> > +}
> > +
> > +static int
> > +nfs4_init_uniform_client_string(struct nfs_client *clp)
> > +{
> > +	int result;
> > +	size_t len;
> > +	char *str;
> > +
> > +	if (clp->cl_owner_id != NULL)
> > +		return 0;
> > 
> > 	if (nfs4_client_id_uniquifier[0] != '\0')
> > -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
> > -				clp->rpc_ops->version,
> > -				clp->cl_minorversion,
> > -				nfs4_client_id_uniquifier,
> > -				nodename);
> > -	else
> > -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
> > -				clp->rpc_ops->version, clp->cl_minorversion,
> > -				nodename);
> > -	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
> > -	return result;
> > +		return nfs4_init_uniquifier_client_string(clp);
> > +
> > +	len = 10 + 10 + 1 + 10 + 1 +
> > +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> > +
> > +	str = kmalloc(len, GFP_NOFS);
> > +	if (!str)
> > +		return -ENOMEM;
> > +
> > +	if (len > NFS4_OPAQUE_LIMIT + 1)
> > +		return -EINVAL;
> > +
> > +	result = scnprintf(str, len, "Linux NFSv%u.%u %s",
> > +			clp->rpc_ops->version, clp->cl_minorversion,
> > +			clp->cl_rpcclient->cl_nodename);
> > +	if (result >= len) {
> > +		kfree(str);
> > +		return -EINVAL;
> > +	}
> > +	clp->cl_owner_id = str;
> > +	return 0;
> > }
> > 
> > /*
> > @@ -5088,20 +5131,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> > 
> > 	/* nfs_client_id4 */
> > 	nfs4_init_boot_verifier(clp, &sc_verifier);
> > -	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags)) {
> > -		setclientid.sc_name_len =
> > -				nfs4_init_uniform_client_string(clp,
> > -						setclientid.sc_name,
> > -						sizeof(setclientid.sc_name));
> > -		if (!clp->cl_owner_id) {
> > -			status = -ENOMEM;
> > -			goto out;
> > -		}
> > -	} else {
> > +
> > +	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags))
> > +		status = nfs4_init_uniform_client_string(clp);
> > +	else
> > 		status = nfs4_init_nonuniform_client_string(clp);
> > -		if (status)
> > -			goto out;
> > -	}
> > +
> > +	if (status)
> > +		goto out;
> > 
> > 	/* cb_client4 */
> > 	setclientid.sc_netid_len =
> > @@ -6875,12 +6912,10 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> > 	};
> > 
> > 	nfs4_init_boot_verifier(clp, &verifier);
> > -	args.id_len = nfs4_init_uniform_client_string(clp, args.id,
> > -							sizeof(args.id));
> > -	if (!clp->cl_owner_id) {
> > -		status = -ENOMEM;
> > +
> > +	status = nfs4_init_uniform_client_string(clp);
> > +	if (status)
> > 		goto out;
> > -	}
> > 
> > 	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
> > 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 16991f5e274e..c959e7eb5bd4 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -984,11 +984,8 @@ struct nfs4_readlink_res {
> > 	struct nfs4_sequence_res	seq_res;
> > };
> > 
> > -#define NFS4_SETCLIENTID_NAMELEN	(127)
> > struct nfs4_setclientid {
> > 	const nfs4_verifier *		sc_verifier;
> > -	unsigned int			sc_name_len;
> > -	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
> > 	u32				sc_prog;
> > 	unsigned int			sc_netid_len;
> > 	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
> > @@ -1142,12 +1139,9 @@ struct nfs41_state_protection {
> > 	struct nfs4_op_map allow;
> > };
> > 
> > -#define NFS4_EXCHANGE_ID_LEN	(48)
> > struct nfs41_exchange_id_args {
> > 	struct nfs_client		*client;
> > 	nfs4_verifier			*verifier;
> > -	unsigned int 			id_len;
> > -	char 				id[NFS4_EXCHANGE_ID_LEN];
> > 	u32				flags;
> > 	struct nfs41_state_protection	state_protect;
> > };
> > -- 
> > 2.4.2
> > 
> > --
> > 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
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH 4/4] nfs: make nfs4_init_uniform_client_string use a dynamically allocated buffer

[Chuck Lever]

On Jun 4, 2015, at 12:44 PM, Jeff Layton <jlayton@poochiereds.net> wrote:

> Change the uniform client string generator to use the same scheme as
> the nonuniform one.

This is a scary sentence. What do you mean by “same scheme” ? Is your new
code following the same requirements for uniform client strings?

Are you changing the client to use only uniform strings? That will break
some servers that cleave to the recommendations in RFC 7530.


> This one is a little simpler since we don't need to
> deal with RCU, but we do have two different cases, depending on whether
> there is a uniquifier or not.
> 
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> ---
> fs/nfs/nfs4proc.c       | 105 ++++++++++++++++++++++++++++++++----------------
> include/linux/nfs_xdr.h |   6 ---
> 2 files changed, 70 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 49e3b3e55887..479b0f583f33 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5000,28 +5000,71 @@ retry:
> 	return 0;
> }
> 
> -static unsigned int
> -nfs4_init_uniform_client_string(struct nfs_client *clp,
> -				char *buf, size_t len)
> +static int
> +nfs4_init_uniquifier_client_string(struct nfs_client *clp)
> {
> -	const char *nodename = clp->cl_rpcclient->cl_nodename;
> -	unsigned int result;
> +	int result;
> +	size_t len;
> +	char *str;
> 
> 	if (clp->cl_owner_id != NULL)
> -		return strlcpy(buf, clp->cl_owner_id, len);
> +		return 0;
> +
> +	len = 10 + 10 + 1 + 10 + 1 +
> +		strlen(nfs4_client_id_uniquifier) + 1 +
> +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> +
> +	if (len > NFS4_OPAQUE_LIMIT + 1)
> +		return -EINVAL;
> +
> +	str = kmalloc(len, GFP_NOFS);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	result = scnprintf(str, len, "Linux NFSv%u.%u %s/%s",
> +			clp->rpc_ops->version, clp->cl_minorversion,
> +			nfs4_client_id_uniquifier,
> +			clp->cl_rpcclient->cl_nodename);
> +	if (result >= len) {
> +		kfree(str);
> +		return -EINVAL;
> +	}
> +	clp->cl_owner_id = str;
> +	return 0;
> +}
> +
> +static int
> +nfs4_init_uniform_client_string(struct nfs_client *clp)
> +{
> +	int result;
> +	size_t len;
> +	char *str;
> +
> +	if (clp->cl_owner_id != NULL)
> +		return 0;
> 
> 	if (nfs4_client_id_uniquifier[0] != '\0')
> -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
> -				clp->rpc_ops->version,
> -				clp->cl_minorversion,
> -				nfs4_client_id_uniquifier,
> -				nodename);
> -	else
> -		result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
> -				clp->rpc_ops->version, clp->cl_minorversion,
> -				nodename);
> -	clp->cl_owner_id = kstrdup(buf, GFP_NOFS);
> -	return result;
> +		return nfs4_init_uniquifier_client_string(clp);
> +
> +	len = 10 + 10 + 1 + 10 + 1 +
> +		strlen(clp->cl_rpcclient->cl_nodename) + 1;
> +
> +	str = kmalloc(len, GFP_NOFS);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	if (len > NFS4_OPAQUE_LIMIT + 1)
> +		return -EINVAL;
> +
> +	result = scnprintf(str, len, "Linux NFSv%u.%u %s",
> +			clp->rpc_ops->version, clp->cl_minorversion,
> +			clp->cl_rpcclient->cl_nodename);
> +	if (result >= len) {
> +		kfree(str);
> +		return -EINVAL;
> +	}
> +	clp->cl_owner_id = str;
> +	return 0;
> }
> 
> /*
> @@ -5088,20 +5131,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> 
> 	/* nfs_client_id4 */
> 	nfs4_init_boot_verifier(clp, &sc_verifier);
> -	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags)) {
> -		setclientid.sc_name_len =
> -				nfs4_init_uniform_client_string(clp,
> -						setclientid.sc_name,
> -						sizeof(setclientid.sc_name));
> -		if (!clp->cl_owner_id) {
> -			status = -ENOMEM;
> -			goto out;
> -		}
> -	} else {
> +
> +	if (test_bit(NFS_CS_MIGRATION, &clp->cl_flags))
> +		status = nfs4_init_uniform_client_string(clp);
> +	else
> 		status = nfs4_init_nonuniform_client_string(clp);
> -		if (status)
> -			goto out;
> -	}
> +
> +	if (status)
> +		goto out;
> 
> 	/* cb_client4 */
> 	setclientid.sc_netid_len =
> @@ -6875,12 +6912,10 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
> 	};
> 
> 	nfs4_init_boot_verifier(clp, &verifier);
> -	args.id_len = nfs4_init_uniform_client_string(clp, args.id,
> -							sizeof(args.id));
> -	if (!clp->cl_owner_id) {
> -		status = -ENOMEM;
> +
> +	status = nfs4_init_uniform_client_string(clp);
> +	if (status)
> 		goto out;
> -	}
> 
> 	dprintk("NFS call  exchange_id auth=%s, '%s'\n",
> 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 16991f5e274e..c959e7eb5bd4 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -984,11 +984,8 @@ struct nfs4_readlink_res {
> 	struct nfs4_sequence_res	seq_res;
> };
> 
> -#define NFS4_SETCLIENTID_NAMELEN	(127)
> struct nfs4_setclientid {
> 	const nfs4_verifier *		sc_verifier;
> -	unsigned int			sc_name_len;
> -	char				sc_name[NFS4_SETCLIENTID_NAMELEN + 1];
> 	u32				sc_prog;
> 	unsigned int			sc_netid_len;
> 	char				sc_netid[RPCBIND_MAXNETIDLEN + 1];
> @@ -1142,12 +1139,9 @@ struct nfs41_state_protection {
> 	struct nfs4_op_map allow;
> };
> 
> -#define NFS4_EXCHANGE_ID_LEN	(48)
> struct nfs41_exchange_id_args {
> 	struct nfs_client		*client;
> 	nfs4_verifier			*verifier;
> -	unsigned int 			id_len;
> -	char 				id[NFS4_EXCHANGE_ID_LEN];
> 	u32				flags;
> 	struct nfs41_state_protection	state_protect;
> };
> -- 
> 2.4.2
> 
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 2/4] nfs: update maxsz values for SETCLIENTID and EXCHANGE_ID

[Jeff Layton]

On Thu, 4 Jun 2015 13:26:57 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> Hi Jeff-
> 
> On Jun 4, 2015, at 12:44 PM, Jeff Layton <jlayton@poochiereds.net> wrote:
> 
> > The spec allows for up to NFS4_OPAQUE_LIMIT (1k). While we'll almost
> > certainly never use that much, these ops are generally the only ones
> > in the compound so we might as well allow for them to be that large.
> > 
> > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> > ---
> > fs/nfs/nfs4xdr.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > index 2826e117e3df..e105b9363f0a 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -139,7 +139,7 @@ static int nfs4_stat_to_errno(int);
> > #define encode_setclientid_maxsz \
> > 				(op_encode_hdr_maxsz + \
> > 				XDR_QUADLEN(NFS4_VERIFIER_SIZE) + \
> > -				XDR_QUADLEN(NFS4_SETCLIENTID_NAMELEN) + \
> > +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
> 
> Should you add another word here for the length of the opaque?
> 

Hmmm...good point. It looks like the old code didn't do that. Sure,
I'll roll that in on the next iteration.

> > 				1 /* sc_prog */ + \
> > 				1 + XDR_QUADLEN(RPCBIND_MAXNETIDLEN) + \
> > 				1 + XDR_QUADLEN(RPCBIND_MAXUADDRLEN) + \
> > @@ -288,7 +288,7 @@ static int nfs4_stat_to_errno(int);
> > #define encode_exchange_id_maxsz (op_encode_hdr_maxsz + \
> > 				encode_verifier_maxsz + \
> > 				1 /* co_ownerid.len */ + \
> > -				XDR_QUADLEN(NFS4_EXCHANGE_ID_LEN) + \
> > +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
> > 				1 /* flags */ + \
> > 				1 /* spa_how */ + \
> > 				/* max is SP4_MACH_CRED (for now) */ + \
> > -- 
> > 2.4.2
> > 
> > --
> > 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
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH 2/4] nfs: update maxsz values for SETCLIENTID and EXCHANGE_ID

[Chuck Lever]

Hi Jeff-

On Jun 4, 2015, at 12:44 PM, Jeff Layton <jlayton@poochiereds.net> wrote:

> The spec allows for up to NFS4_OPAQUE_LIMIT (1k). While we'll almost
> certainly never use that much, these ops are generally the only ones
> in the compound so we might as well allow for them to be that large.
> 
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
> ---
> fs/nfs/nfs4xdr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 2826e117e3df..e105b9363f0a 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -139,7 +139,7 @@ static int nfs4_stat_to_errno(int);
> #define encode_setclientid_maxsz \
> 				(op_encode_hdr_maxsz + \
> 				XDR_QUADLEN(NFS4_VERIFIER_SIZE) + \
> -				XDR_QUADLEN(NFS4_SETCLIENTID_NAMELEN) + \
> +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \

Should you add another word here for the length of the opaque?

> 				1 /* sc_prog */ + \
> 				1 + XDR_QUADLEN(RPCBIND_MAXNETIDLEN) + \
> 				1 + XDR_QUADLEN(RPCBIND_MAXUADDRLEN) + \
> @@ -288,7 +288,7 @@ static int nfs4_stat_to_errno(int);
> #define encode_exchange_id_maxsz (op_encode_hdr_maxsz + \
> 				encode_verifier_maxsz + \
> 				1 /* co_ownerid.len */ + \
> -				XDR_QUADLEN(NFS4_EXCHANGE_ID_LEN) + \
> +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
> 				1 /* flags */ + \
> 				1 /* spa_how */ + \
> 				/* max is SP4_MACH_CRED (for now) */ + \
> -- 
> 2.4.2
> 
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com