[PATCH 0/3] nfs: fix v4.0 callback channel auth failures

[PATCH 0/3] nfs: fix v4.0 callback channel auth failures

[Jeff Layton]

Earlier this week, we had a lively discussion about how to fix the
bogus way that the callback channel tries to authenticate requests
coming in. The consensus was that the right approach is to save off
the acceptor name in a GSSAPI SETCLIENTID call, and then to compare
that to the initiator name in the callback requests.

This patchset is the kernel portion of that change. There is also
a companion patchset for gssd to make it pass the acceptor name
to the kernel in the downcall.

Jeff Layton (3):
  auth_gss: fetch the acceptor name out of the downcall
  sunrpc: add a new "stringify_acceptor" rpc_credop
  nfs4: copy acceptor name from context to nfs_client

 fs/nfs/callback.c               | 12 ++++++
 fs/nfs/client.c                 |  1 +
 fs/nfs/nfs4proc.c               | 30 ++++++++++++++-
 include/linux/nfs_fs_sb.h       |  1 +
 include/linux/nfs_xdr.h         |  1 +
 include/linux/sunrpc/auth.h     |  2 +
 include/linux/sunrpc/auth_gss.h |  1 +
 net/sunrpc/auth.c               |  9 +++++
 net/sunrpc/auth_gss/auth_gss.c  | 82 +++++++++++++++++++++++++++++------------
 9 files changed, 115 insertions(+), 24 deletions(-)

-- 
1.9.0

[PATCH 1/3] auth_gss: fetch the acceptor name out of the downcall

[Jeff Layton]

If rpc.gssd sends us an acceptor name string trailing the context token,
stash it as part of the context.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/sunrpc/auth_gss.h |  1 +
 net/sunrpc/auth_gss/auth_gss.c  | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/auth_gss.h b/include/linux/sunrpc/auth_gss.h
index f1cfd4c85cd0..cbc6875fb9cf 100644
--- a/include/linux/sunrpc/auth_gss.h
+++ b/include/linux/sunrpc/auth_gss.h
@@ -71,6 +71,7 @@ struct gss_cl_ctx {
 	spinlock_t		gc_seq_lock;
 	struct gss_ctx __rcu	*gc_gss_ctx;
 	struct xdr_netobj	gc_wire_ctx;
+	struct xdr_netobj	gc_acceptor;
 	u32			gc_win;
 	unsigned long		gc_expiry;
 	struct rcu_head		gc_rcu;
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 36e431ee1c90..f341aabb92db 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -262,9 +262,22 @@ gss_fill_context(const void *p, const void *end, struct gss_cl_ctx *ctx, struct
 		p = ERR_PTR(ret);
 		goto err;
 	}
-	dprintk("RPC:       %s Success. gc_expiry %lu now %lu timeout %u\n",
-		__func__, ctx->gc_expiry, now, timeout);
-	return q;
+
+	/* is there any trailing data? */
+	if (q == end) {
+		p = q;
+		goto done;
+	}
+
+	/* pull in acceptor name (if there is one) */
+	p = simple_get_netobj(q, end, &ctx->gc_acceptor);
+	if (IS_ERR(p))
+		goto err;
+done:
+	dprintk("RPC:       %s Success. gc_expiry %lu now %lu timeout %u acceptor %.*s\n",
+		__func__, ctx->gc_expiry, now, timeout, ctx->gc_acceptor.len,
+		ctx->gc_acceptor.data);
+	return p;
 err:
 	dprintk("RPC:       %s returns error %ld\n", __func__, -PTR_ERR(p));
 	return p;
@@ -1225,6 +1238,7 @@ gss_do_free_ctx(struct gss_cl_ctx *ctx)
 
 	gss_delete_sec_context(&ctx->gc_gss_ctx);
 	kfree(ctx->gc_wire_ctx.data);
+	kfree(ctx->gc_acceptor.data);
 	kfree(ctx);
 }
 
-- 
1.9.0

[PATCH 2/3] sunrpc: add a new "stringify_acceptor" rpc_credop

[Jeff Layton]

...and add an new rpc_auth function to call it when it exists. This
is only applicable for AUTH_GSS mechanisms, so we only specify this
for those sorts of credentials.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/sunrpc/auth.h    |  2 ++
 net/sunrpc/auth.c              |  9 ++++++
 net/sunrpc/auth_gss/auth_gss.c | 62 ++++++++++++++++++++++++++++--------------
 3 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 790be1472792..c683b9a06913 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -140,6 +140,7 @@ struct rpc_credops {
 						void *, __be32 *, void *);
 	int			(*crkey_timeout)(struct rpc_cred *);
 	bool			(*crkey_to_expire)(struct rpc_cred *);
+	char *			(*crstringify_acceptor)(struct rpc_cred *);
 };
 
 extern const struct rpc_authops	authunix_ops;
@@ -182,6 +183,7 @@ void			rpcauth_clear_credcache(struct rpc_cred_cache *);
 int			rpcauth_key_timeout_notify(struct rpc_auth *,
 						struct rpc_cred *);
 bool			rpcauth_cred_key_to_expire(struct rpc_cred *);
+char *			rpcauth_stringify_acceptor(struct rpc_cred *);
 
 static inline
 struct rpc_cred *	get_rpccred(struct rpc_cred *cred)
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 5285ead196c0..ef51bb77c465 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -363,6 +363,15 @@ rpcauth_cred_key_to_expire(struct rpc_cred *cred)
 }
 EXPORT_SYMBOL_GPL(rpcauth_cred_key_to_expire);
 
+char *
+rpcauth_stringify_acceptor(struct rpc_cred *cred)
+{
+	if (!cred->cr_ops->crstringify_acceptor)
+		return NULL;
+	return cred->cr_ops->crstringify_acceptor(cred);
+}
+EXPORT_SYMBOL_GPL(rpcauth_stringify_acceptor);
+
 /*
  * Destroy a list of credentials
  */
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index f341aabb92db..2f5b860f3491 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1346,6 +1346,26 @@ gss_cred_init(struct rpc_auth *auth, struct rpc_cred *cred)
 	return err;
 }
 
+static char *
+gss_stringify_acceptor(struct rpc_cred *cred)
+{
+	char *string;
+	struct gss_cred *gss_cred = container_of(cred, struct gss_cred, gc_base);
+	struct xdr_netobj *acceptor = &gss_cred->gc_ctx->gc_acceptor;
+
+	/* no point if there's no string */
+	if (!acceptor->len)
+		return NULL;
+
+	string = kmalloc(acceptor->len + 1, GFP_KERNEL);
+	if (!string)
+		return string;
+
+	memcpy(string, acceptor->data, acceptor->len);
+	string[acceptor->len] = '\0';
+	return string;
+}
+
 /*
  * Returns -EACCES if GSS context is NULL or will expire within the
  * timeout (miliseconds)
@@ -1923,29 +1943,31 @@ static const struct rpc_authops authgss_ops = {
 };
 
 static const struct rpc_credops gss_credops = {
-	.cr_name	= "AUTH_GSS",
-	.crdestroy	= gss_destroy_cred,
-	.cr_init	= gss_cred_init,
-	.crbind		= rpcauth_generic_bind_cred,
-	.crmatch	= gss_match,
-	.crmarshal	= gss_marshal,
-	.crrefresh	= gss_refresh,
-	.crvalidate	= gss_validate,
-	.crwrap_req	= gss_wrap_req,
-	.crunwrap_resp	= gss_unwrap_resp,
-	.crkey_timeout	= gss_key_timeout,
+	.cr_name		= "AUTH_GSS",
+	.crdestroy		= gss_destroy_cred,
+	.cr_init		= gss_cred_init,
+	.crbind			= rpcauth_generic_bind_cred,
+	.crmatch		= gss_match,
+	.crmarshal		= gss_marshal,
+	.crrefresh		= gss_refresh,
+	.crvalidate		= gss_validate,
+	.crwrap_req		= gss_wrap_req,
+	.crunwrap_resp		= gss_unwrap_resp,
+	.crkey_timeout		= gss_key_timeout,
+	.crstringify_acceptor	= gss_stringify_acceptor,
 };
 
 static const struct rpc_credops gss_nullops = {
-	.cr_name	= "AUTH_GSS",
-	.crdestroy	= gss_destroy_nullcred,
-	.crbind		= rpcauth_generic_bind_cred,
-	.crmatch	= gss_match,
-	.crmarshal	= gss_marshal,
-	.crrefresh	= gss_refresh_null,
-	.crvalidate	= gss_validate,
-	.crwrap_req	= gss_wrap_req,
-	.crunwrap_resp	= gss_unwrap_resp,
+	.cr_name		= "AUTH_GSS",
+	.crdestroy		= gss_destroy_nullcred,
+	.crbind			= rpcauth_generic_bind_cred,
+	.crmatch		= gss_match,
+	.crmarshal		= gss_marshal,
+	.crrefresh		= gss_refresh_null,
+	.crvalidate		= gss_validate,
+	.crwrap_req		= gss_wrap_req,
+	.crunwrap_resp		= gss_unwrap_resp,
+	.crstringify_acceptor	= gss_stringify_acceptor,
 };
 
 static const struct rpc_pipe_ops gss_upcall_ops_v0 = {
-- 
1.9.0

[PATCH 3/3] nfs4: copy acceptor name from context to nfs_client

[Jeff Layton]

The current CB_COMPOUND handling code tries to compare the principal
name of the request with the cl_hostname in the client. This is not
guaranteed to ever work, particularly if the client happened to mount
a CNAME of the server or a non-fqdn.

Fix this by instead comparing the cr_principal string with the acceptor
name that we get from gssd. In the event that gssd didn't send one
down (i.e. it was too old), then we fall back to trying to use the
cl_hostname as we do today.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/callback.c         | 12 ++++++++++++
 fs/nfs/client.c           |  1 +
 fs/nfs/nfs4proc.c         | 30 +++++++++++++++++++++++++++++-
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  1 +
 5 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 073b4cf67ed9..54de482143cc 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
 	if (p == NULL)
 		return 0;
 
+	/*
+	 * Did we get the acceptor from userland during the SETCLIENID
+	 * negotiation?
+	 */
+	if (clp->cl_acceptor)
+		return !strcmp(p, clp->cl_acceptor);
+
+	/*
+	 * Otherwise try to verify it using the cl_hostname. Note that this
+	 * doesn't work if a non-canonical hostname was used in the devname.
+	 */
+
 	/* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
 
 	if (memcmp(p, "nfs@", 4) != 0)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1d09289c8f0e..6a461378fcf2 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
 	put_net(clp->cl_net);
 	put_nfs_version(clp->cl_nfs_mod);
 	kfree(clp->cl_hostname);
+	kfree(clp->cl_acceptor);
 	kfree(clp);
 
 	dprintk("<-- nfs_free_client()\n");
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 397be39c6dc8..cc6c3fe3e060 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
 		return scnprintf(buf, len, "tcp");
 }
 
+static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
+{
+	struct nfs4_setclientid *sc = calldata;
+
+	if (task->tk_status == 0)
+		*sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);
+}
+
+static const struct rpc_call_ops nfs4_setclientid_ops = {
+	.rpc_call_done = nfs4_setclientid_done,
+};
+
 /**
  * nfs4_proc_setclientid - Negotiate client ID
  * @clp: state data structure
@@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 		.sc_verifier = &sc_verifier,
 		.sc_prog = program,
 		.sc_cb_ident = clp->cl_cb_ident,
+		.sc_acceptor = &clp->cl_acceptor,
 	};
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
@@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 		.rpc_resp = res,
 		.rpc_cred = cred,
 	};
+	struct rpc_task *task;
+	struct rpc_task_setup task_setup_data = {
+		.rpc_client = clp->cl_rpcclient,
+		.rpc_message = &msg,
+		.callback_ops = &nfs4_setclientid_ops,
+		.callback_data = &setclientid,
+		.flags = RPC_TASK_TIMEOUT,
+	};
 	int status;
 
 	/* nfs_client_id4 */
@@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
 		clp->cl_rpcclient->cl_auth->au_ops->au_name,
 		setclientid.sc_name_len, setclientid.sc_name);
-	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
+	task = rpc_run_task(&task_setup_data);
+	if (IS_ERR(task)) {
+		status = PTR_ERR(task);
+		goto out;
+	}
+	status = task->tk_status;
+	rpc_put_task(task);
+out:
 	trace_nfs4_setclientid(clp, status);
 	dprintk("NFS reply setclientid: %d\n", status);
 	return status;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 1150ea41b626..922be2e050f5 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -45,6 +45,7 @@ struct nfs_client {
 	struct sockaddr_storage	cl_addr;	/* server identifier */
 	size_t			cl_addrlen;
 	char *			cl_hostname;	/* hostname of server */
+	char *			cl_acceptor;	/* GSSAPI acceptor name */
 	struct list_head	cl_share_link;	/* link in global client list */
 	struct list_head	cl_superblocks;	/* List of nfs_server structs */
 
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 6fb5b2335b59..5945d88a26e2 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
 	unsigned int			sc_uaddr_len;
 	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
 	u32				sc_cb_ident;
+	char				**sc_acceptor;
 };
 
 struct nfs4_setclientid_res {
-- 
1.9.0

Re: [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client

[Trond Myklebust]

On Thu, 2014-04-10 at 16:29 -0400, Jeff Layton wrote:
> The current CB_COMPOUND handling code tries to compare the principal
> name of the request with the cl_hostname in the client. This is not
> guaranteed to ever work, particularly if the client happened to mount
> a CNAME of the server or a non-fqdn.
> 
> Fix this by instead comparing the cr_principal string with the acceptor
> name that we get from gssd. In the event that gssd didn't send one
> down (i.e. it was too old), then we fall back to trying to use the
> cl_hostname as we do today.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/nfs/callback.c         | 12 ++++++++++++
>  fs/nfs/client.c           |  1 +
>  fs/nfs/nfs4proc.c         | 30 +++++++++++++++++++++++++++++-
>  include/linux/nfs_fs_sb.h |  1 +
>  include/linux/nfs_xdr.h   |  1 +
>  5 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 073b4cf67ed9..54de482143cc 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
>  	if (p == NULL)
>  		return 0;
>  
> +	/*
> +	 * Did we get the acceptor from userland during the SETCLIENID
> +	 * negotiation?
> +	 */
> +	if (clp->cl_acceptor)
> +		return !strcmp(p, clp->cl_acceptor);
> +
> +	/*
> +	 * Otherwise try to verify it using the cl_hostname. Note that this
> +	 * doesn't work if a non-canonical hostname was used in the devname.
> +	 */
> +
>  	/* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
>  
>  	if (memcmp(p, "nfs@", 4) != 0)
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 1d09289c8f0e..6a461378fcf2 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
>  	put_net(clp->cl_net);
>  	put_nfs_version(clp->cl_nfs_mod);
>  	kfree(clp->cl_hostname);
> +	kfree(clp->cl_acceptor);
>  	kfree(clp);
>  
>  	dprintk("<-- nfs_free_client()\n");
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 397be39c6dc8..cc6c3fe3e060 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
>  		return scnprintf(buf, len, "tcp");
>  }
>  
> +static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
> +{
> +	struct nfs4_setclientid *sc = calldata;
> +
> +	if (task->tk_status == 0)
> +		*sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);

We can't call a GFP_KERNEL memory allocator from within a RPC callback
function. This needs to be done outside the RPC call.

> +}
> +
> +static const struct rpc_call_ops nfs4_setclientid_ops = {
> +	.rpc_call_done = nfs4_setclientid_done,
> +};
> +
>  /**
>   * nfs4_proc_setclientid - Negotiate client ID
>   * @clp: state data structure
> @@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  		.sc_verifier = &sc_verifier,
>  		.sc_prog = program,
>  		.sc_cb_ident = clp->cl_cb_ident,
> +		.sc_acceptor = &clp->cl_acceptor,
>  	};
>  	struct rpc_message msg = {
>  		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
> @@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  		.rpc_resp = res,
>  		.rpc_cred = cred,
>  	};
> +	struct rpc_task *task;
> +	struct rpc_task_setup task_setup_data = {
> +		.rpc_client = clp->cl_rpcclient,
> +		.rpc_message = &msg,
> +		.callback_ops = &nfs4_setclientid_ops,
> +		.callback_data = &setclientid,
> +		.flags = RPC_TASK_TIMEOUT,
> +	};
>  	int status;
>  
>  	/* nfs_client_id4 */
> @@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
>  	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
>  		clp->cl_rpcclient->cl_auth->au_ops->au_name,
>  		setclientid.sc_name_len, setclientid.sc_name);
> -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> +	task = rpc_run_task(&task_setup_data);
> +	if (IS_ERR(task)) {
> +		status = PTR_ERR(task);
> +		goto out;
> +	}
> +	status = task->tk_status;

Why not just do it here?

> +	rpc_put_task(task);
> +out:
>  	trace_nfs4_setclientid(clp, status);
>  	dprintk("NFS reply setclientid: %d\n", status);
>  	return status;
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 1150ea41b626..922be2e050f5 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -45,6 +45,7 @@ struct nfs_client {
>  	struct sockaddr_storage	cl_addr;	/* server identifier */
>  	size_t			cl_addrlen;
>  	char *			cl_hostname;	/* hostname of server */
> +	char *			cl_acceptor;	/* GSSAPI acceptor name */
>  	struct list_head	cl_share_link;	/* link in global client list */
>  	struct list_head	cl_superblocks;	/* List of nfs_server structs */
>  
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 6fb5b2335b59..5945d88a26e2 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
>  	unsigned int			sc_uaddr_len;
>  	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
>  	u32				sc_cb_ident;
> +	char				**sc_acceptor;
>  };
>  
>  struct nfs4_setclientid_res {

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: [PATCH 3/3] nfs4: copy acceptor name from context to nfs_client

[Jeff Layton]

On Wed, 28 May 2014 17:39:32 -0400
Trond Myklebust <trond.myklebust@primarydata.com> wrote:

> On Thu, 2014-04-10 at 16:29 -0400, Jeff Layton wrote:
> > The current CB_COMPOUND handling code tries to compare the principal
> > name of the request with the cl_hostname in the client. This is not
> > guaranteed to ever work, particularly if the client happened to mount
> > a CNAME of the server or a non-fqdn.
> > 
> > Fix this by instead comparing the cr_principal string with the acceptor
> > name that we get from gssd. In the event that gssd didn't send one
> > down (i.e. it was too old), then we fall back to trying to use the
> > cl_hostname as we do today.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/nfs/callback.c         | 12 ++++++++++++
> >  fs/nfs/client.c           |  1 +
> >  fs/nfs/nfs4proc.c         | 30 +++++++++++++++++++++++++++++-
> >  include/linux/nfs_fs_sb.h |  1 +
> >  include/linux/nfs_xdr.h   |  1 +
> >  5 files changed, 44 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index 073b4cf67ed9..54de482143cc 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -428,6 +428,18 @@ check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
> >  	if (p == NULL)
> >  		return 0;
> >  
> > +	/*
> > +	 * Did we get the acceptor from userland during the SETCLIENID
> > +	 * negotiation?
> > +	 */
> > +	if (clp->cl_acceptor)
> > +		return !strcmp(p, clp->cl_acceptor);
> > +
> > +	/*
> > +	 * Otherwise try to verify it using the cl_hostname. Note that this
> > +	 * doesn't work if a non-canonical hostname was used in the devname.
> > +	 */
> > +
> >  	/* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
> >  
> >  	if (memcmp(p, "nfs@", 4) != 0)
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 1d09289c8f0e..6a461378fcf2 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -252,6 +252,7 @@ void nfs_free_client(struct nfs_client *clp)
> >  	put_net(clp->cl_net);
> >  	put_nfs_version(clp->cl_nfs_mod);
> >  	kfree(clp->cl_hostname);
> > +	kfree(clp->cl_acceptor);
> >  	kfree(clp);
> >  
> >  	dprintk("<-- nfs_free_client()\n");
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 397be39c6dc8..cc6c3fe3e060 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -4896,6 +4896,18 @@ nfs4_init_callback_netid(const struct nfs_client *clp, char *buf, size_t len)
> >  		return scnprintf(buf, len, "tcp");
> >  }
> >  
> > +static void nfs4_setclientid_done(struct rpc_task *task, void *calldata)
> > +{
> > +	struct nfs4_setclientid *sc = calldata;
> > +
> > +	if (task->tk_status == 0)
> > +		*sc->sc_acceptor = rpcauth_stringify_acceptor(task->tk_rqstp->rq_cred);
> 
> We can't call a GFP_KERNEL memory allocator from within a RPC callback
> function. This needs to be done outside the RPC call.
> 

Ahh right...I should know that by now. Sorry...

> > +}
> > +
> > +static const struct rpc_call_ops nfs4_setclientid_ops = {
> > +	.rpc_call_done = nfs4_setclientid_done,
> > +};
> > +
> >  /**
> >   * nfs4_proc_setclientid - Negotiate client ID
> >   * @clp: state data structure
> > @@ -4915,6 +4927,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> >  		.sc_verifier = &sc_verifier,
> >  		.sc_prog = program,
> >  		.sc_cb_ident = clp->cl_cb_ident,
> > +		.sc_acceptor = &clp->cl_acceptor,
> >  	};
> >  	struct rpc_message msg = {
> >  		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID],
> > @@ -4922,6 +4935,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> >  		.rpc_resp = res,
> >  		.rpc_cred = cred,
> >  	};
> > +	struct rpc_task *task;
> > +	struct rpc_task_setup task_setup_data = {
> > +		.rpc_client = clp->cl_rpcclient,
> > +		.rpc_message = &msg,
> > +		.callback_ops = &nfs4_setclientid_ops,
> > +		.callback_data = &setclientid,
> > +		.flags = RPC_TASK_TIMEOUT,
> > +	};
> >  	int status;
> >  
> >  	/* nfs_client_id4 */
> > @@ -4948,7 +4969,14 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> >  	dprintk("NFS call  setclientid auth=%s, '%.*s'\n",
> >  		clp->cl_rpcclient->cl_auth->au_ops->au_name,
> >  		setclientid.sc_name_len, setclientid.sc_name);
> > -	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
> > +	task = rpc_run_task(&task_setup_data);
> > +	if (IS_ERR(task)) {
> > +		status = PTR_ERR(task);
> > +		goto out;
> > +	}
> > +	status = task->tk_status;
> 
> Why not just do it here?
> 

It's been a while since I've looked at this set, but I think I tried
that initially. At this point, the rq_cred has been destroyed, so it's
too late to call rpcauth_stringify_acceptor on it.

I guess what I can do is turn the sc_acceptor field into an rpc_cred
pointer, and have nfs4_setclientid_done bump the refcount on it and
set the pointer. Then I can call rpcauth_stringify_acceptor on it here,
and put the cred.

I'll do that once I have a chance to respin and retest. It may be a bit
as I have some other priorities at the moment. ;)

> > +	rpc_put_task(task);
> > +out:
> >  	trace_nfs4_setclientid(clp, status);
> >  	dprintk("NFS reply setclientid: %d\n", status);
> >  	return status;
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index 1150ea41b626..922be2e050f5 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -45,6 +45,7 @@ struct nfs_client {
> >  	struct sockaddr_storage	cl_addr;	/* server identifier */
> >  	size_t			cl_addrlen;
> >  	char *			cl_hostname;	/* hostname of server */
> > +	char *			cl_acceptor;	/* GSSAPI acceptor name */
> >  	struct list_head	cl_share_link;	/* link in global client list */
> >  	struct list_head	cl_superblocks;	/* List of nfs_server structs */
> >  
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 6fb5b2335b59..5945d88a26e2 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -1010,6 +1010,7 @@ struct nfs4_setclientid {
> >  	unsigned int			sc_uaddr_len;
> >  	char				sc_uaddr[RPCBIND_MAXUADDRLEN + 1];
> >  	u32				sc_cb_ident;
> > +	char				**sc_acceptor;
> >  };
> >  
> >  struct nfs4_setclientid_res {
> 


-- 
Jeff Layton <jlayton@primarydata.com>