[PATCH 1/6] SUNRPC: Fix a bug in rpcauth_lookup_credcache()

[PATCH 1/6] SUNRPC: Fix a bug in rpcauth_lookup_credcache()

[Trond Myklebust]

The hash bucket is for some reason always being set to zero.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 include/linux/sunrpc/auth.h |    4 ++--
 net/sunrpc/auth.c           |    5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 7a69ca3..84d5f3a 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -59,8 +59,8 @@ struct rpc_cred {
 /*
  * Client authentication handle
  */
-#define RPC_CREDCACHE_NR	8
-#define RPC_CREDCACHE_MASK	(RPC_CREDCACHE_NR - 1)
+#define RPC_CREDCACHE_HASHBITS	4
+#define RPC_CREDCACHE_NR	(1 << RPC_CREDCACHE_HASHBITS)
 struct rpc_cred_cache {
 	struct hlist_head	hashtable[RPC_CREDCACHE_NR];
 	spinlock_t		lock;
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index eca941c..b38f6ee 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/errno.h>
+#include <linux/hash.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/spinlock.h>
 
@@ -280,7 +281,9 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
 	struct hlist_node *pos;
 	struct rpc_cred	*cred = NULL,
 			*entry, *new;
-	int		nr = 0;
+	unsigned int nr;
+
+	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
 
 	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
 		nr = acred->uid & RPC_CREDCACHE_MASK;

Re: [PATCH 1/6] SUNRPC: Fix a bug in rpcauth_lookup_credcache()

[J. Bruce Fields]

Sorry for the delayed response--I read over these offline and them
forgot about them....:

On Thu, Mar 13, 2008 at 01:48:07PM -0400, Trond Myklebust wrote:
> The hash bucket is for some reason always being set to zero.

The changelog seems out of sync with the code; nr is normally set a
little lower to the low bits of acred->uid:

...
> @@ -280,7 +281,9 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
>  	struct hlist_node *pos;
>  	struct rpc_cred	*cred = NULL,
>  			*entry, *new;
> -	int		nr = 0;
> +	unsigned int nr;
> +
> +	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
>  
>  	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
>  		nr = acred->uid & RPC_CREDCACHE_MASK;

So what you're really doing is switching to a better hash fn.  (Except
that the result is being overwritten immediately afterwards in the
!RPCAUTH_LOOKUP_ROOTRCREDS case.)

Wouldn't it have made more sense to replace the existing assignment,
then remove just make it unconditional in next patch?

--b.

Re: [PATCH 1/6] SUNRPC: Fix a bug in rpcauth_lookup_credcache()

[Trond Myklebust]

On Fri, 2008-03-28 at 11:12 -0400, J. Bruce Fields wrote:
> Sorry for the delayed response--I read over these offline and them
> forgot about them....:
> 
> On Thu, Mar 13, 2008 at 01:48:07PM -0400, Trond Myklebust wrote:
> > The hash bucket is for some reason always being set to zero.
> 
> The changelog seems out of sync with the code; nr is normally set a
> little lower to the low bits of acred->uid:
> 
> ...
> > @@ -280,7 +281,9 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
> >  	struct hlist_node *pos;
> >  	struct rpc_cred	*cred = NULL,
> >  			*entry, *new;
> > -	int		nr = 0;
> > +	unsigned int nr;
> > +
> > +	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
> >  
> >  	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
> >  		nr = acred->uid & RPC_CREDCACHE_MASK;
> 
> So what you're really doing is switching to a better hash fn.  (Except
> that the result is being overwritten immediately afterwards in the
> !RPCAUTH_LOOKUP_ROOTRCREDS case.)
> 
> Wouldn't it have made more sense to replace the existing assignment,
> then remove just make it unconditional in next patch?

Originally I developed the second patch first, then added this one.
After a spot of re-ordering, I got confused.

The end result is correct, though, and git bisection will not be broken,
so I figure we might as well keep it rather than rebuilding the git
trees...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
authentication, and then only as a special case in the code. This patch
removes the auth_sys special casing, and replaces it with generic code.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 include/linux/sunrpc/auth.h |    4 ++-
 net/sunrpc/auth.c           |   35 +++++++++++++++------------
 net/sunrpc/auth_unix.c      |   56 ++++++++++++++++++-------------------------
 net/sunrpc/sched.c          |    4 ++-
 4 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 84d5f3a..012566a 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -89,7 +89,6 @@ struct rpc_auth {
 
 /* Flags for rpcauth_lookupcred() */
 #define RPCAUTH_LOOKUP_NEW		0x01	/* Accept an uninitialised cred */
-#define RPCAUTH_LOOKUP_ROOTCREDS	0x02	/* This really ought to go! */
 
 /*
  * Client authentication ops
@@ -136,7 +135,8 @@ void			rpcauth_release(struct rpc_auth *);
 struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
 void			rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
 struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
-struct rpc_cred *	rpcauth_bindcred(struct rpc_task *);
+void			rpcauth_bindcred(struct rpc_task *);
+void			rpcauth_bind_root_cred(struct rpc_task *);
 void			rpcauth_holdcred(struct rpc_task *);
 void			put_rpccred(struct rpc_cred *);
 void			rpcauth_unbindcred(struct rpc_task *);
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index b38f6ee..b0f2b2e 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
 
 	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
 
-	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
-		nr = acred->uid & RPC_CREDCACHE_MASK;
-
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
 		if (!entry->cr_ops->crmatch(acred, entry, flags))
@@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
 }
 EXPORT_SYMBOL_GPL(rpcauth_init_cred);
 
-struct rpc_cred *
-rpcauth_bindcred(struct rpc_task *task)
+void
+rpcauth_bind_root_cred(struct rpc_task *task)
 {
 	struct rpc_auth *auth = task->tk_client->cl_auth;
 	struct auth_cred acred = {
-		.uid = current->fsuid,
-		.gid = current->fsgid,
-		.group_info = current->group_info,
+		.uid = 0,
+		.gid = 0,
 	};
 	struct rpc_cred *ret;
-	int flags = 0;
 
 	dprintk("RPC: %5u looking up %s cred\n",
 		task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
-	get_group_info(acred.group_info);
-	if (task->tk_flags & RPC_TASK_ROOTCREDS)
-		flags |= RPCAUTH_LOOKUP_ROOTCREDS;
-	ret = auth->au_ops->lookup_cred(auth, &acred, flags);
+	ret = auth->au_ops->lookup_cred(auth, &acred, 0);
+	if (!IS_ERR(ret))
+		task->tk_msg.rpc_cred = ret;
+	else
+		task->tk_status = PTR_ERR(ret);
+}
+
+void
+rpcauth_bindcred(struct rpc_task *task)
+{
+	struct rpc_auth *auth = task->tk_client->cl_auth;
+	struct rpc_cred *ret;
+
+	dprintk("RPC: %5u looking up %s cred\n",
+		task->tk_pid, auth->au_ops->au_name);
+	ret = rpcauth_lookupcred(auth, 0);
 	if (!IS_ERR(ret))
 		task->tk_msg.rpc_cred = ret;
 	else
 		task->tk_status = PTR_ERR(ret);
-	put_group_info(acred.group_info);
-	return ret;
 }
 
 void
diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
index 5ed91e5..b763710 100644
--- a/net/sunrpc/auth_unix.c
+++ b/net/sunrpc/auth_unix.c
@@ -60,7 +60,8 @@ static struct rpc_cred *
 unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
 {
 	struct unx_cred	*cred;
-	int		i;
+	unsigned int groups = 0;
+	unsigned int i;
 
 	dprintk("RPC:       allocating UNIX cred for uid %d gid %d\n",
 			acred->uid, acred->gid);
@@ -70,21 +71,17 @@ unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
 
 	rpcauth_init_cred(&cred->uc_base, acred, auth, &unix_credops);
 	cred->uc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
-	if (flags & RPCAUTH_LOOKUP_ROOTCREDS) {
-		cred->uc_uid = 0;
-		cred->uc_gid = 0;
-		cred->uc_gids[0] = NOGROUP;
-	} else {
-		int groups = acred->group_info->ngroups;
-		if (groups > NFS_NGROUPS)
-			groups = NFS_NGROUPS;
-
-		cred->uc_gid = acred->gid;
-		for (i = 0; i < groups; i++)
-			cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
-		if (i < NFS_NGROUPS)
-		  cred->uc_gids[i] = NOGROUP;
-	}
+
+	if (acred->group_info != NULL)
+		groups = acred->group_info->ngroups;
+	if (groups > NFS_NGROUPS)
+		groups = NFS_NGROUPS;
+
+	cred->uc_gid = acred->gid;
+	for (i = 0; i < groups; i++)
+		cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
+	if (i < NFS_NGROUPS)
+		cred->uc_gids[i] = NOGROUP;
 
 	return &cred->uc_base;
 }
@@ -118,26 +115,21 @@ static int
 unx_match(struct auth_cred *acred, struct rpc_cred *rcred, int flags)
 {
 	struct unx_cred	*cred = container_of(rcred, struct unx_cred, uc_base);
-	int		i;
+	unsigned int groups = 0;
+	unsigned int i;
 
-	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS)) {
-		int groups;
 
-		if (cred->uc_uid != acred->uid
-		 || cred->uc_gid != acred->gid)
-			return 0;
+	if (cred->uc_uid != acred->uid || cred->uc_gid != acred->gid)
+		return 0;
 
+	if (acred->group_info != NULL)
 		groups = acred->group_info->ngroups;
-		if (groups > NFS_NGROUPS)
-			groups = NFS_NGROUPS;
-		for (i = 0; i < groups ; i++)
-			if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
-				return 0;
-		return 1;
-	}
-	return (cred->uc_uid == 0
-	     && cred->uc_gid == 0
-	     && cred->uc_gids[0] == (gid_t) NOGROUP);
+	if (groups > NFS_NGROUPS)
+		groups = NFS_NGROUPS;
+	for (i = 0; i < groups ; i++)
+		if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
+			return 0;
+	return 1;
 }
 
 /*
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index cae219c..7db956f 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -821,8 +821,10 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
 		/* Bind the user cred */
 		if (task->tk_msg.rpc_cred != NULL)
 			rpcauth_holdcred(task);
-		else
+		else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
 			rpcauth_bindcred(task);
+		else
+			rpcauth_bind_root_cred(task);
 		if (task->tk_action == NULL)
 			rpc_call_start(task);
 	}

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Olga Kornievskaia]

Trond,

    We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
machine creds for authenticated callback.

Trond Myklebust wrote:
> The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
> authentication, and then only as a special case in the code. This patch
> removes the auth_sys special casing, and replaces it with generic code.
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>
>  include/linux/sunrpc/auth.h |    4 ++-
>  net/sunrpc/auth.c           |   35 +++++++++++++++------------
>  net/sunrpc/auth_unix.c      |   56 ++++++++++++++++++-------------------------
>  net/sunrpc/sched.c          |    4 ++-
>  4 files changed, 49 insertions(+), 50 deletions(-)
>
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 84d5f3a..012566a 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -89,7 +89,6 @@ struct rpc_auth {
>  
>  /* Flags for rpcauth_lookupcred() */
>  #define RPCAUTH_LOOKUP_NEW		0x01	/* Accept an uninitialised cred */
> -#define RPCAUTH_LOOKUP_ROOTCREDS	0x02	/* This really ought to go! */
>  
>  /*
>   * Client authentication ops
> @@ -136,7 +135,8 @@ void			rpcauth_release(struct rpc_auth *);
>  struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
>  void			rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
>  struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
> -struct rpc_cred *	rpcauth_bindcred(struct rpc_task *);
> +void			rpcauth_bindcred(struct rpc_task *);
> +void			rpcauth_bind_root_cred(struct rpc_task *);
>  void			rpcauth_holdcred(struct rpc_task *);
>  void			put_rpccred(struct rpc_cred *);
>  void			rpcauth_unbindcred(struct rpc_task *);
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index b38f6ee..b0f2b2e 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
>  
>  	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
>  
> -	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
> -		nr = acred->uid & RPC_CREDCACHE_MASK;
> -
>  	rcu_read_lock();
>  	hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
>  		if (!entry->cr_ops->crmatch(acred, entry, flags))
> @@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
>  }
>  EXPORT_SYMBOL_GPL(rpcauth_init_cred);
>  
> -struct rpc_cred *
> -rpcauth_bindcred(struct rpc_task *task)
> +void
> +rpcauth_bind_root_cred(struct rpc_task *task)
>  {
>  	struct rpc_auth *auth = task->tk_client->cl_auth;
>  	struct auth_cred acred = {
> -		.uid = current->fsuid,
> -		.gid = current->fsgid,
> -		.group_info = current->group_info,
> +		.uid = 0,
> +		.gid = 0,
>  	};
>  	struct rpc_cred *ret;
> -	int flags = 0;
>  
>  	dprintk("RPC: %5u looking up %s cred\n",
>  		task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
> -	get_group_info(acred.group_info);
> -	if (task->tk_flags & RPC_TASK_ROOTCREDS)
> -		flags |= RPCAUTH_LOOKUP_ROOTCREDS;
> -	ret = auth->au_ops->lookup_cred(auth, &acred, flags);
> +	ret = auth->au_ops->lookup_cred(auth, &acred, 0);
> +	if (!IS_ERR(ret))
> +		task->tk_msg.rpc_cred = ret;
> +	else
> +		task->tk_status = PTR_ERR(ret);
> +}
> +
> +void
> +rpcauth_bindcred(struct rpc_task *task)
> +{
> +	struct rpc_auth *auth = task->tk_client->cl_auth;
> +	struct rpc_cred *ret;
> +
> +	dprintk("RPC: %5u looking up %s cred\n",
> +		task->tk_pid, auth->au_ops->au_name);
> +	ret = rpcauth_lookupcred(auth, 0);
>  	if (!IS_ERR(ret))
>  		task->tk_msg.rpc_cred = ret;
>  	else
>  		task->tk_status = PTR_ERR(ret);
> -	put_group_info(acred.group_info);
> -	return ret;
>  }
>  
>  void
> diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
> index 5ed91e5..b763710 100644
> --- a/net/sunrpc/auth_unix.c
> +++ b/net/sunrpc/auth_unix.c
> @@ -60,7 +60,8 @@ static struct rpc_cred *
>  unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
>  {
>  	struct unx_cred	*cred;
> -	int		i;
> +	unsigned int groups = 0;
> +	unsigned int i;
>  
>  	dprintk("RPC:       allocating UNIX cred for uid %d gid %d\n",
>  			acred->uid, acred->gid);
> @@ -70,21 +71,17 @@ unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
>  
>  	rpcauth_init_cred(&cred->uc_base, acred, auth, &unix_credops);
>  	cred->uc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
> -	if (flags & RPCAUTH_LOOKUP_ROOTCREDS) {
> -		cred->uc_uid = 0;
> -		cred->uc_gid = 0;
> -		cred->uc_gids[0] = NOGROUP;
> -	} else {
> -		int groups = acred->group_info->ngroups;
> -		if (groups > NFS_NGROUPS)
> -			groups = NFS_NGROUPS;
> -
> -		cred->uc_gid = acred->gid;
> -		for (i = 0; i < groups; i++)
> -			cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> -		if (i < NFS_NGROUPS)
> -		  cred->uc_gids[i] = NOGROUP;
> -	}
> +
> +	if (acred->group_info != NULL)
> +		groups = acred->group_info->ngroups;
> +	if (groups > NFS_NGROUPS)
> +		groups = NFS_NGROUPS;
> +
> +	cred->uc_gid = acred->gid;
> +	for (i = 0; i < groups; i++)
> +		cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> +	if (i < NFS_NGROUPS)
> +		cred->uc_gids[i] = NOGROUP;
>  
>  	return &cred->uc_base;
>  }
> @@ -118,26 +115,21 @@ static int
>  unx_match(struct auth_cred *acred, struct rpc_cred *rcred, int flags)
>  {
>  	struct unx_cred	*cred = container_of(rcred, struct unx_cred, uc_base);
> -	int		i;
> +	unsigned int groups = 0;
> +	unsigned int i;
>  
> -	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS)) {
> -		int groups;
>  
> -		if (cred->uc_uid != acred->uid
> -		 || cred->uc_gid != acred->gid)
> -			return 0;
> +	if (cred->uc_uid != acred->uid || cred->uc_gid != acred->gid)
> +		return 0;
>  
> +	if (acred->group_info != NULL)
>  		groups = acred->group_info->ngroups;
> -		if (groups > NFS_NGROUPS)
> -			groups = NFS_NGROUPS;
> -		for (i = 0; i < groups ; i++)
> -			if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> -				return 0;
> -		return 1;
> -	}
> -	return (cred->uc_uid == 0
> -	     && cred->uc_gid == 0
> -	     && cred->uc_gids[0] == (gid_t) NOGROUP);
> +	if (groups > NFS_NGROUPS)
> +		groups = NFS_NGROUPS;
> +	for (i = 0; i < groups ; i++)
> +		if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> +			return 0;
> +	return 1;
>  }
>  
>  /*
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index cae219c..7db956f 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -821,8 +821,10 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
>  		/* Bind the user cred */
>  		if (task->tk_msg.rpc_cred != NULL)
>  			rpcauth_holdcred(task);
> -		else
> +		else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
>  			rpcauth_bindcred(task);
> +		else
> +			rpcauth_bind_root_cred(task);
>  		if (task->tk_action == NULL)
>  			rpc_call_start(task);
>  	}
>
> --
> 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
>
>   

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> Trond,
> 
>     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> machine creds for authenticated callback.

I'd strongly suggest using a different flag for that purpose. The
function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
swap-over-nfs to use root credentials when paging out memory.

That is not the same as machine creds...

Cheers
  Trond
 
-- 
Trond Myklebust
NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > Trond,
> > 
> >     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> > machine creds for authenticated callback.
> 
> I'd strongly suggest using a different flag for that purpose. The
> function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> swap-over-nfs to use root credentials when paging out memory.
> 
> That is not the same as machine creds...


In fact, I'd strongly urge you to add the information for machine creds
to the 'struct auth_cred' instead. The latter is the lookup key for
'rpc_authops->lookup_cred()'.

Cheers
  Trond

-- 
Trond Myklebust
NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[J. Bruce Fields]

On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
> 
> On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > Trond,
> > > 
> > >     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> > > machine creds for authenticated callback.
> > 
> > I'd strongly suggest using a different flag for that purpose. The
> > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > swap-over-nfs to use root credentials when paging out memory.
> > 
> > That is not the same as machine creds...
> 
> 
> In fact, I'd strongly urge you to add the information for machine creds
> to the 'struct auth_cred' instead. The latter is the lookup key for
> 'rpc_authops->lookup_cred()'.

So something like 

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 7a69ca3..d624169 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -26,6 +26,7 @@ struct auth_cred {
 	uid_t	uid;
 	gid_t	gid;
 	struct group_info *group_info;
+	int	is_machine_cred;
 };
 
?  No objection, but it seems like mild overkill for a single bit when
the necessary functions already take a flag parameter.  Or do you want
this information in that structure for future uses of the auth_cred as a
key into a more general cred cache?

--b.

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:
> On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
> > 
> > On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > > Trond,
> > > > 
> > > >     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> > > > machine creds for authenticated callback.
> > > 
> > > I'd strongly suggest using a different flag for that purpose. The
> > > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > > swap-over-nfs to use root credentials when paging out memory.
> > > 
> > > That is not the same as machine creds...
> > 
> > 
> > In fact, I'd strongly urge you to add the information for machine creds
> > to the 'struct auth_cred' instead. The latter is the lookup key for
> > 'rpc_authops->lookup_cred()'.
> 
> So something like 
> 
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 7a69ca3..d624169 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -26,6 +26,7 @@ struct auth_cred {
>  	uid_t	uid;
>  	gid_t	gid;
>  	struct group_info *group_info;
> +	int	is_machine_cred;
>  };

Yes. Unless we need more information. Will we ever be in a situation
where we want to specify a full 

> ?  No objection, but it seems like mild overkill for a single bit when
> the necessary functions already take a flag parameter.

The flag parameter should be for internal use by the RPC auth code to
pass around lookup intent information (see gss_match(), which is the
only remaining callback that actually uses that parameter).

The lookup key, OTOH, should be a single structure.

>   Or do you want
> this information in that structure for future uses of the auth_cred as a
> key into a more general cred cache?

See the rest of the patch series...

Basically, if the administrator changes our security landscape on the
fly, either by migrating our filesystem, or by changing the supported
security flavours, then we're going to have to renegotiate security on
the fly. We really do _not_ want the NFS structures to cache RPC
credentials which are tied to a particular auth mechanism or rpcsec_gss
session.
The "generic" credential I'm proposing is the tool for carrying all the
information that is needed in order to lookup/create an on-the-wire
credential at the instant when we issue the RPC call.

-- 
Trond Myklebust
NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

On Thu, 2008-03-13 at 16:36 -0400, Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:

> > So something like 
> > 
> > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> > index 7a69ca3..d624169 100644
> > --- a/include/linux/sunrpc/auth.h
> > +++ b/include/linux/sunrpc/auth.h
> > @@ -26,6 +26,7 @@ struct auth_cred {
> >  	uid_t	uid;
> >  	gid_t	gid;
> >  	struct group_info *group_info;
> > +	int	is_machine_cred;
> >  };
> 
> Yes. Unless we need more information. Will we ever be in a situation
> where we want to specify a full 

Sorry, I meant to say 'a full principal'. IOW: will we ever need to
specify principals beyond the simple 'uid/keyring' and 'machine cred'
distinction.

-- 
Trond Myklebust
NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[J. Bruce Fields]

On Thu, Mar 13, 2008 at 04:36:06PM -0400, Trond Myklebust wrote:
> 
> On Thu, 2008-03-13 at 15:43 -0400, J. Bruce Fields wrote:
> > On Thu, Mar 13, 2008 at 03:25:38PM -0400, Trond Myklebust wrote:
> > > 
> > > On Thu, 2008-03-13 at 15:19 -0400, Trond Myklebust wrote:
> > > > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> > > > > Trond,
> > > > > 
> > > > >     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> > > > > machine creds for authenticated callback.
> > > > 
> > > > I'd strongly suggest using a different flag for that purpose. The
> > > > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > > > swap-over-nfs to use root credentials when paging out memory.
> > > > 
> > > > That is not the same as machine creds...
> > > 
> > > 
> > > In fact, I'd strongly urge you to add the information for machine creds
> > > to the 'struct auth_cred' instead. The latter is the lookup key for
> > > 'rpc_authops->lookup_cred()'.
> > 
> > So something like 
> > 
> > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> > index 7a69ca3..d624169 100644
> > --- a/include/linux/sunrpc/auth.h
> > +++ b/include/linux/sunrpc/auth.h
> > @@ -26,6 +26,7 @@ struct auth_cred {
> >  	uid_t	uid;
> >  	gid_t	gid;
> >  	struct group_info *group_info;
> > +	int	is_machine_cred;
> >  };
> 
> Yes. Unless we need more information. Will we ever be in a situation
> where we want to specify a full 

A full principal name, or something?  No, I can't think of a reason for
now.

> > ?  No objection, but it seems like mild overkill for a single bit when
> > the necessary functions already take a flag parameter.
> 
> The flag parameter should be for internal use by the RPC auth code to
> pass around lookup intent information (see gss_match(), which is the
> only remaining callback that actually uses that parameter).
> 
> The lookup key, OTOH, should be a single structure.
> 
> >   Or do you want
> > this information in that structure for future uses of the auth_cred as a
> > key into a more general cred cache?
> 
> See the rest of the patch series...

Whoops, OK, will do.

> Basically, if the administrator changes our security landscape on the
> fly, either by migrating our filesystem, or by changing the supported
> security flavours, then we're going to have to renegotiate security on
> the fly. We really do _not_ want the NFS structures to cache RPC
> credentials which are tied to a particular auth mechanism or rpcsec_gss
> session.
> The "generic" credential I'm proposing is the tool for carrying all the
> information that is needed in order to lookup/create an on-the-wire
> credential at the instant when we issue the RPC call.

Got it; putting the information that we want a host credential into the
auth_cred sounds right. Thanks!

--b.

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Olga Kornievskaia]

Trond Myklebust wrote:
> On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
>   
>> Trond,
>>
>>     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
>> machine creds for authenticated callback..
>>     
>
> I'd strongly suggest using a different flag for that purpose. The
> function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> swap-over-nfs to use root credentials when paging out memory.
>
> That is not the same as machine creds...
>   
I apologize I forgot that I introduced a new flag instead, 
RPCAUTH_USE_MACH_CRED. It should fit with the other flags such as 
RPCAUTH_CRED_NEW in auth.h.

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Trond Myklebust]

On Thu, 2008-03-13 at 15:30 -0400, Olga Kornievskaia wrote:
> 
> Trond Myklebust wrote:
> > On Thu, 2008-03-13 at 15:11 -0400, Olga Kornievskaia wrote:
> >   
> >> Trond,
> >>
> >>     We were thinking of using RPCAUTH_LOOKUP_ROOTCREDS flag to acquire 
> >> machine creds for authenticated callback..
> >>     
> >
> > I'd strongly suggest using a different flag for that purpose. The
> > function of RPCAUTH_LOOKUP_ROOTCREDS _today_ is to allow a future
> > swap-over-nfs to use root credentials when paging out memory.
> >
> > That is not the same as machine creds...
> >   
> I apologize I forgot that I introduced a new flag instead, 
> RPCAUTH_USE_MACH_CRED. It should fit with the other flags such as 
> RPCAUTH_CRED_NEW in auth.h.

As I said, I'd rather have that flag in 'struct auth_cred' so that it
works with the "generic" cred mechanism.

Cheers
  Trond

-- 
Trond Myklebust
NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[J. Bruce Fields]

On Thu, Mar 13, 2008 at 01:48:08PM -0400, Trond Myklebust wrote:
> The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
> authentication, and then only as a special case in the code. This patch
> removes the auth_sys special casing, and replaces it with generic code.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  include/linux/sunrpc/auth.h |    4 ++-
>  net/sunrpc/auth.c           |   35 +++++++++++++++------------
>  net/sunrpc/auth_unix.c      |   56 ++++++++++++++++++-------------------------
>  net/sunrpc/sched.c          |    4 ++-
>  4 files changed, 49 insertions(+), 50 deletions(-)
> 
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 84d5f3a..012566a 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -89,7 +89,6 @@ struct rpc_auth {
>  
>  /* Flags for rpcauth_lookupcred() */
>  #define RPCAUTH_LOOKUP_NEW		0x01	/* Accept an uninitialised cred */
> -#define RPCAUTH_LOOKUP_ROOTCREDS	0x02	/* This really ought to go! */
>  
>  /*
>   * Client authentication ops
> @@ -136,7 +135,8 @@ void			rpcauth_release(struct rpc_auth *);
>  struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
>  void			rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
>  struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
> -struct rpc_cred *	rpcauth_bindcred(struct rpc_task *);
> +void			rpcauth_bindcred(struct rpc_task *);
> +void			rpcauth_bind_root_cred(struct rpc_task *);
>  void			rpcauth_holdcred(struct rpc_task *);
>  void			put_rpccred(struct rpc_cred *);
>  void			rpcauth_unbindcred(struct rpc_task *);
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index b38f6ee..b0f2b2e 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
>  
>  	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
>  
> -	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
> -		nr = acred->uid & RPC_CREDCACHE_MASK;
> -
>  	rcu_read_lock();
>  	hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr], cr_hash) {
>  		if (!entry->cr_ops->crmatch(acred, entry, flags))
> @@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
>  }
>  EXPORT_SYMBOL_GPL(rpcauth_init_cred);
>  
> -struct rpc_cred *
> -rpcauth_bindcred(struct rpc_task *task)
> +void
> +rpcauth_bind_root_cred(struct rpc_task *task)
>  {
>  	struct rpc_auth *auth = task->tk_client->cl_auth;
>  	struct auth_cred acred = {
> -		.uid = current->fsuid,
> -		.gid = current->fsgid,
> -		.group_info = current->group_info,
> +		.uid = 0,
> +		.gid = 0,
>  	};
>  	struct rpc_cred *ret;
> -	int flags = 0;
>  
>  	dprintk("RPC: %5u looking up %s cred\n",
>  		task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
> -	get_group_info(acred.group_info);
> -	if (task->tk_flags & RPC_TASK_ROOTCREDS)
> -		flags |= RPCAUTH_LOOKUP_ROOTCREDS;
> -	ret = auth->au_ops->lookup_cred(auth, &acred, flags);
> +	ret = auth->au_ops->lookup_cred(auth, &acred, 0);
> +	if (!IS_ERR(ret))
> +		task->tk_msg.rpc_cred = ret;
> +	else
> +		task->tk_status = PTR_ERR(ret);
> +}
> +
> +void
> +rpcauth_bindcred(struct rpc_task *task)
> +{
> +	struct rpc_auth *auth = task->tk_client->cl_auth;
> +	struct rpc_cred *ret;
> +
> +	dprintk("RPC: %5u looking up %s cred\n",
> +		task->tk_pid, auth->au_ops->au_name);
> +	ret = rpcauth_lookupcred(auth, 0);
>  	if (!IS_ERR(ret))
>  		task->tk_msg.rpc_cred = ret;
>  	else
>  		task->tk_status = PTR_ERR(ret);
> -	put_group_info(acred.group_info);
> -	return ret;
>  }
>  
>  void
> diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
> index 5ed91e5..b763710 100644
> --- a/net/sunrpc/auth_unix.c
> +++ b/net/sunrpc/auth_unix.c
> @@ -60,7 +60,8 @@ static struct rpc_cred *
>  unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
>  {
>  	struct unx_cred	*cred;
> -	int		i;
> +	unsigned int groups = 0;
> +	unsigned int i;

I don't really care, I'm just curious: why bother to make small counter
variables unsigned?

>  
>  	dprintk("RPC:       allocating UNIX cred for uid %d gid %d\n",
>  			acred->uid, acred->gid);
> @@ -70,21 +71,17 @@ unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
>  
>  	rpcauth_init_cred(&cred->uc_base, acred, auth, &unix_credops);
>  	cred->uc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
> -	if (flags & RPCAUTH_LOOKUP_ROOTCREDS) {
> -		cred->uc_uid = 0;
> -		cred->uc_gid = 0;
> -		cred->uc_gids[0] = NOGROUP;
> -	} else {
> -		int groups = acred->group_info->ngroups;
> -		if (groups > NFS_NGROUPS)
> -			groups = NFS_NGROUPS;
> -
> -		cred->uc_gid = acred->gid;
> -		for (i = 0; i < groups; i++)
> -			cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> -		if (i < NFS_NGROUPS)
> -		  cred->uc_gids[i] = NOGROUP;
> -	}
> +
> +	if (acred->group_info != NULL)
> +		groups = acred->group_info->ngroups;
> +	if (groups > NFS_NGROUPS)
> +		groups = NFS_NGROUPS;
> +
> +	cred->uc_gid = acred->gid;
> +	for (i = 0; i < groups; i++)
> +		cred->uc_gids[i] = GROUP_AT(acred->group_info, i);
> +	if (i < NFS_NGROUPS)
> +		cred->uc_gids[i] = NOGROUP;
>  
>  	return &cred->uc_base;
>  }
> @@ -118,26 +115,21 @@ static int
>  unx_match(struct auth_cred *acred, struct rpc_cred *rcred, int flags)
>  {
>  	struct unx_cred	*cred = container_of(rcred, struct unx_cred, uc_base);
> -	int		i;
> +	unsigned int groups = 0;
> +	unsigned int i;
>  
> -	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS)) {
> -		int groups;
>  
> -		if (cred->uc_uid != acred->uid
> -		 || cred->uc_gid != acred->gid)
> -			return 0;
> +	if (cred->uc_uid != acred->uid || cred->uc_gid != acred->gid)
> +		return 0;
>  
> +	if (acred->group_info != NULL)
>  		groups = acred->group_info->ngroups;
> -		if (groups > NFS_NGROUPS)
> -			groups = NFS_NGROUPS;
> -		for (i = 0; i < groups ; i++)
> -			if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> -				return 0;
> -		return 1;
> -	}
> -	return (cred->uc_uid == 0
> -	     && cred->uc_gid == 0
> -	     && cred->uc_gids[0] == (gid_t) NOGROUP);
> +	if (groups > NFS_NGROUPS)
> +		groups = NFS_NGROUPS;
> +	for (i = 0; i < groups ; i++)
> +		if (cred->uc_gids[i] != GROUP_AT(acred->group_info, i))
> +			return 0;
> +	return 1;

This changes the behavior slightly in the ROOTCREDS case so it no longer
expects uc_gids[0] to be NOGROUP.  I assume that doesn't matter?

(Also, this is unchanged by the patch, but I'm curious whether there's
any particular reason it matches the supplementary groups in the way it
does: it expects them to agree up to the number of groups supplied in
acred, but ignores any others.  Is this just arbitrary?)

--b.

>  }
>  
>  /*
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index cae219c..7db956f 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -821,8 +821,10 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
>  		/* Bind the user cred */
>  		if (task->tk_msg.rpc_cred != NULL)
>  			rpcauth_holdcred(task);
> -		else
> +		else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
>  			rpcauth_bindcred(task);
> +		else
> +			rpcauth_bind_root_cred(task);
>  		if (task->tk_action == NULL)
>  			rpc_call_start(task);
>  	}
> 
> --
> 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

Re: [PATCH 2/6] SUNRPC: Fix RPCAUTH_LOOKUP_ROOTCREDS

[Chuck Lever]

On Mar 28, 2008, at 11:32 AM, J. Bruce Fields wrote:
> On Thu, Mar 13, 2008 at 01:48:08PM -0400, Trond Myklebust wrote:
>> The current RPCAUTH_LOOKUP_ROOTCREDS flag only works for AUTH_SYS
>> authentication, and then only as a special case in the code. This  
>> patch
>> removes the auth_sys special casing, and replaces it with generic  
>> code.
>>
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>> ---
>>
>>  include/linux/sunrpc/auth.h |    4 ++-
>>  net/sunrpc/auth.c           |   35 +++++++++++++++------------
>>  net/sunrpc/auth_unix.c      |   56 +++++++++++++++++ 
>> +-------------------------
>>  net/sunrpc/sched.c          |    4 ++-
>>  4 files changed, 49 insertions(+), 50 deletions(-)
>>
>> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/ 
>> auth.h
>> index 84d5f3a..012566a 100644
>> --- a/include/linux/sunrpc/auth.h
>> +++ b/include/linux/sunrpc/auth.h
>> @@ -89,7 +89,6 @@ struct rpc_auth {
>>
>>  /* Flags for rpcauth_lookupcred() */
>>  #define RPCAUTH_LOOKUP_NEW		0x01	/* Accept an uninitialised cred */
>> -#define RPCAUTH_LOOKUP_ROOTCREDS	0x02	/* This really ought to go! */
>>
>>  /*
>>   * Client authentication ops
>> @@ -136,7 +135,8 @@ void			rpcauth_release(struct rpc_auth *);
>>  struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *,  
>> struct auth_cred *, int);
>>  void			rpcauth_init_cred(struct rpc_cred *, const struct  
>> auth_cred *, struct rpc_auth *, const struct rpc_credops *);
>>  struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
>> -struct rpc_cred *	rpcauth_bindcred(struct rpc_task *);
>> +void			rpcauth_bindcred(struct rpc_task *);
>> +void			rpcauth_bind_root_cred(struct rpc_task *);
>>  void			rpcauth_holdcred(struct rpc_task *);
>>  void			put_rpccred(struct rpc_cred *);
>>  void			rpcauth_unbindcred(struct rpc_task *);
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index b38f6ee..b0f2b2e 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -285,9 +285,6 @@ rpcauth_lookup_credcache(struct rpc_auth  
>> *auth, struct auth_cred * acred,
>>
>>  	nr = hash_long(acred->uid, RPC_CREDCACHE_HASHBITS);
>>
>> -	if (!(flags & RPCAUTH_LOOKUP_ROOTCREDS))
>> -		nr = acred->uid & RPC_CREDCACHE_MASK;
>> -
>>  	rcu_read_lock();
>>  	hlist_for_each_entry_rcu(entry, pos, &cache->hashtable[nr],  
>> cr_hash) {
>>  		if (!entry->cr_ops->crmatch(acred, entry, flags))
>> @@ -378,30 +375,38 @@ rpcauth_init_cred(struct rpc_cred *cred,  
>> const struct auth_cred *acred,
>>  }
>>  EXPORT_SYMBOL_GPL(rpcauth_init_cred);
>>
>> -struct rpc_cred *
>> -rpcauth_bindcred(struct rpc_task *task)
>> +void
>> +rpcauth_bind_root_cred(struct rpc_task *task)
>>  {
>>  	struct rpc_auth *auth = task->tk_client->cl_auth;
>>  	struct auth_cred acred = {
>> -		.uid = current->fsuid,
>> -		.gid = current->fsgid,
>> -		.group_info = current->group_info,
>> +		.uid = 0,
>> +		.gid = 0,
>>  	};
>>  	struct rpc_cred *ret;
>> -	int flags = 0;
>>
>>  	dprintk("RPC: %5u looking up %s cred\n",
>>  		task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
>> -	get_group_info(acred.group_info);
>> -	if (task->tk_flags & RPC_TASK_ROOTCREDS)
>> -		flags |= RPCAUTH_LOOKUP_ROOTCREDS;
>> -	ret = auth->au_ops->lookup_cred(auth, &acred, flags);
>> +	ret = auth->au_ops->lookup_cred(auth, &acred, 0);
>> +	if (!IS_ERR(ret))
>> +		task->tk_msg.rpc_cred = ret;
>> +	else
>> +		task->tk_status = PTR_ERR(ret);
>> +}
>> +
>> +void
>> +rpcauth_bindcred(struct rpc_task *task)
>> +{
>> +	struct rpc_auth *auth = task->tk_client->cl_auth;
>> +	struct rpc_cred *ret;
>> +
>> +	dprintk("RPC: %5u looking up %s cred\n",
>> +		task->tk_pid, auth->au_ops->au_name);
>> +	ret = rpcauth_lookupcred(auth, 0);
>>  	if (!IS_ERR(ret))
>>  		task->tk_msg.rpc_cred = ret;
>>  	else
>>  		task->tk_status = PTR_ERR(ret);
>> -	put_group_info(acred.group_info);
>> -	return ret;
>>  }
>>
>>  void
>> diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
>> index 5ed91e5..b763710 100644
>> --- a/net/sunrpc/auth_unix.c
>> +++ b/net/sunrpc/auth_unix.c
>> @@ -60,7 +60,8 @@ static struct rpc_cred *
>>  unx_create_cred(struct rpc_auth *auth, struct auth_cred *acred,  
>> int flags)
>>  {
>>  	struct unx_cred	*cred;
>> -	int		i;
>> +	unsigned int groups = 0;
>> +	unsigned int i;
>
> I don't really care, I'm just curious: why bother to make small  
> counter
> variables unsigned?

If the counter is being compared to an unsigned somewhere, it's  
cleaner to make the counter unsigned as well.

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

[PATCH 3/6] SUNRPC: Clean up rpcauth_bindcred()

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 include/linux/sunrpc/auth.h |    4 +---
 net/sunrpc/auth.c           |   28 ++++++++++++++++++----------
 net/sunrpc/sched.c          |   11 ++++-------
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 012566a..348546c 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -135,9 +135,7 @@ void			rpcauth_release(struct rpc_auth *);
 struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int);
 void			rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
 struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
-void			rpcauth_bindcred(struct rpc_task *);
-void			rpcauth_bind_root_cred(struct rpc_task *);
-void			rpcauth_holdcred(struct rpc_task *);
+void			rpcauth_bindcred(struct rpc_task *, struct rpc_cred *, int);
 void			put_rpccred(struct rpc_cred *);
 void			rpcauth_unbindcred(struct rpc_task *);
 __be32 *		rpcauth_marshcred(struct rpc_task *, __be32 *);
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index b0f2b2e..1cdb163 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -375,7 +375,15 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
 }
 EXPORT_SYMBOL_GPL(rpcauth_init_cred);
 
-void
+static void
+rpcauth_generic_bind_cred(struct rpc_task *task, struct rpc_cred *cred)
+{
+	task->tk_msg.rpc_cred = get_rpccred(cred);
+	dprintk("RPC: %5u holding %s cred %p\n", task->tk_pid,
+			cred->cr_auth->au_ops->au_name, cred);
+}
+
+static void
 rpcauth_bind_root_cred(struct rpc_task *task)
 {
 	struct rpc_auth *auth = task->tk_client->cl_auth;
@@ -394,8 +402,8 @@ rpcauth_bind_root_cred(struct rpc_task *task)
 		task->tk_status = PTR_ERR(ret);
 }
 
-void
-rpcauth_bindcred(struct rpc_task *task)
+static void
+rpcauth_bind_new_cred(struct rpc_task *task)
 {
 	struct rpc_auth *auth = task->tk_client->cl_auth;
 	struct rpc_cred *ret;
@@ -410,14 +418,14 @@ rpcauth_bindcred(struct rpc_task *task)
 }
 
 void
-rpcauth_holdcred(struct rpc_task *task)
+rpcauth_bindcred(struct rpc_task *task, struct rpc_cred *cred, int flags)
 {
-	struct rpc_cred *cred = task->tk_msg.rpc_cred;
-	if (cred != NULL) {
-		get_rpccred(cred);
-		dprintk("RPC: %5u holding %s cred %p\n", task->tk_pid,
-				cred->cr_auth->au_ops->au_name, cred);
-	}
+	if (cred != NULL)
+		rpcauth_generic_bind_cred(task, cred);
+	else if (flags & RPC_TASK_ROOTCREDS)
+		rpcauth_bind_root_cred(task);
+	else
+		rpcauth_bind_new_cred(task);
 }
 
 void
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 7db956f..6eab9bf 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -817,14 +817,11 @@ static void rpc_init_task(struct rpc_task *task, const struct rpc_task_setup *ta
 		task->tk_action = rpc_prepare_task;
 
 	if (task_setup_data->rpc_message != NULL) {
-		memcpy(&task->tk_msg, task_setup_data->rpc_message, sizeof(task->tk_msg));
+		task->tk_msg.rpc_proc = task_setup_data->rpc_message->rpc_proc;
+		task->tk_msg.rpc_argp = task_setup_data->rpc_message->rpc_argp;
+		task->tk_msg.rpc_resp = task_setup_data->rpc_message->rpc_resp;
 		/* Bind the user cred */
-		if (task->tk_msg.rpc_cred != NULL)
-			rpcauth_holdcred(task);
-		else if (!(task_setup_data->flags & RPC_TASK_ROOTCREDS))
-			rpcauth_bindcred(task);
-		else
-			rpcauth_bind_root_cred(task);
+		rpcauth_bindcred(task, task_setup_data->rpc_message->rpc_cred, task_setup_data->flags);
 		if (task->tk_action == NULL)
 			rpc_call_start(task);
 	}

[PATCH 4/6] SUNRPC: Add a generic RPC credential

[Trond Myklebust]

Add an rpc credential that is not tied to any particular auth mechanism,
but that can be cached by NFS, and later used to look up a cred for
whichever auth mechanism that turns out to be valid when the RPC call is
being made.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 include/linux/sunrpc/auth.h |    3 +
 net/sunrpc/Makefile         |    2 -
 net/sunrpc/auth.c           |    1 
 net/sunrpc/auth_generic.c   |  142 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 1 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 348546c..70644ed 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -125,9 +125,12 @@ extern const struct rpc_authops	authunix_ops;
 extern const struct rpc_authops	authnull_ops;
 
 void __init		rpc_init_authunix(void);
+void __init		rpc_init_generic_auth(void);
 void __init		rpcauth_init_module(void);
 void __exit		rpcauth_remove_module(void);
+void __exit		rpc_destroy_generic_auth(void);
 
+struct rpc_cred *	rpc_lookup_cred(void);
 int			rpcauth_register(const struct rpc_authops *);
 int			rpcauth_unregister(const struct rpc_authops *);
 struct rpc_auth *	rpcauth_create(rpc_authflavor_t, struct rpc_clnt *);
diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile
index 92e1dbe..5369aa3 100644
--- a/net/sunrpc/Makefile
+++ b/net/sunrpc/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_SUNRPC_GSS) += auth_gss/
 obj-$(CONFIG_SUNRPC_XPRT_RDMA) += xprtrdma/
 
 sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \
-	    auth.o auth_null.o auth_unix.o \
+	    auth.o auth_null.o auth_unix.o auth_generic.o \
 	    svc.o svcsock.o svcauth.o svcauth_unix.o \
 	    rpcb_clnt.o timer.o xdr.o \
 	    sunrpc_syms.o cache.o rpc_pipe.o \
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 1cdb163..012f2a3 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -566,6 +566,7 @@ static struct shrinker rpc_cred_shrinker = {
 void __init rpcauth_init_module(void)
 {
 	rpc_init_authunix();
+	rpc_init_generic_auth();
 	register_shrinker(&rpc_cred_shrinker);
 }
 
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
new file mode 100644
index 0000000..6f129b1
--- /dev/null
+++ b/net/sunrpc/auth_generic.c
@@ -0,0 +1,142 @@
+/*
+ * Generic RPC credential
+ *
+ * Copyright (C) 2008, Trond Myklebust <Trond.Myklebust@netapp.com>
+ */
+
+#include <linux/err.h>
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/sunrpc/auth.h>
+#include <linux/sunrpc/clnt.h>
+#include <linux/sunrpc/debug.h>
+#include <linux/sunrpc/sched.h>
+
+#ifdef RPC_DEBUG
+# define RPCDBG_FACILITY	RPCDBG_AUTH
+#endif
+
+struct generic_cred {
+	struct rpc_cred gc_base;
+	struct auth_cred acred;
+};
+
+static struct rpc_auth generic_auth;
+static struct rpc_cred_cache generic_cred_cache;
+static const struct rpc_credops generic_credops;
+
+/*
+ * Public call interface
+ */
+struct rpc_cred *rpc_lookup_cred(void)
+{
+	return rpcauth_lookupcred(&generic_auth, 0);
+}
+EXPORT_SYMBOL_GPL(rpc_lookup_cred);
+
+/*
+ * Lookup generic creds for current process
+ */
+static struct rpc_cred *
+generic_lookup_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
+{
+	return rpcauth_lookup_credcache(&generic_auth, acred, flags);
+}
+
+static struct rpc_cred *
+generic_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
+{
+	struct generic_cred *gcred;
+
+	gcred = kmalloc(sizeof(*gcred), GFP_KERNEL);
+	if (gcred == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	rpcauth_init_cred(&gcred->gc_base, acred, &generic_auth, &generic_credops);
+	gcred->gc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
+
+	gcred->acred.uid = acred->uid;
+	gcred->acred.gid = acred->gid;
+	gcred->acred.group_info = acred->group_info;
+	if (gcred->acred.group_info != NULL)
+		get_group_info(gcred->acred.group_info);
+
+	dprintk("RPC:       allocated generic cred %p for uid %d gid %d\n",
+			gcred, acred->uid, acred->gid);
+	return &gcred->gc_base;
+}
+
+static void
+generic_free_cred(struct rpc_cred *cred)
+{
+	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
+
+	dprintk("RPC:       generic_free_cred %p\n", gcred);
+	if (gcred->acred.group_info != NULL)
+		put_group_info(gcred->acred.group_info);
+	kfree(gcred);
+}
+
+static void
+generic_free_cred_callback(struct rcu_head *head)
+{
+	struct rpc_cred *cred = container_of(head, struct rpc_cred, cr_rcu);
+	generic_free_cred(cred);
+}
+
+static void
+generic_destroy_cred(struct rpc_cred *cred)
+{
+	call_rcu(&cred->cr_rcu, generic_free_cred_callback);
+}
+
+/*
+ * Match credentials against current process creds.
+ */
+static int
+generic_match(struct auth_cred *acred, struct rpc_cred *cred, int flags)
+{
+	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
+
+	if (gcred->acred.uid != acred->uid ||
+	    gcred->acred.gid != acred->gid ||
+	    gcred->acred.group_info != acred->group_info)
+		return 0;
+	return 1;
+}
+
+void __init rpc_init_generic_auth(void)
+{
+	spin_lock_init(&generic_cred_cache.lock);
+}
+
+void __exit rpc_destroy_generic_auth(void)
+{
+	rpcauth_clear_credcache(&generic_cred_cache);
+}
+
+static struct rpc_cred_cache generic_cred_cache = {
+	{{ NULL, },},
+};
+
+static const struct rpc_authops generic_auth_ops = {
+	.owner = THIS_MODULE,
+#ifdef RPC_DEBUG
+	.au_name = "Generic",
+#endif
+	.lookup_cred = generic_lookup_cred,
+	.crcreate = generic_create_cred,
+};
+
+static struct rpc_auth generic_auth = {
+	.au_ops = &generic_auth_ops,
+	.au_count = ATOMIC_INIT(0),
+	.au_credcache = &generic_cred_cache,
+};
+
+static const struct rpc_credops generic_credops = {
+	.cr_name = "Generic cred",
+	.crdestroy = generic_destroy_cred,
+	.crmatch = generic_match,
+};

Re: [PATCH 4/6] SUNRPC: Add a generic RPC credential

[J. Bruce Fields]

On Thu, Mar 13, 2008 at 01:48:09PM -0400, Trond Myklebust wrote:
> Add an rpc credential that is not tied to any particular auth mechanism,
> but that can be cached by NFS, and later used to look up a cred for
> whichever auth mechanism that turns out to be valid when the RPC call is
> being made.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  include/linux/sunrpc/auth.h |    3 +
>  net/sunrpc/Makefile         |    2 -
>  net/sunrpc/auth.c           |    1 
>  net/sunrpc/auth_generic.c   |  142 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 147 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> index 348546c..70644ed 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -125,9 +125,12 @@ extern const struct rpc_authops	authunix_ops;
>  extern const struct rpc_authops	authnull_ops;
>  
>  void __init		rpc_init_authunix(void);
> +void __init		rpc_init_generic_auth(void);
>  void __init		rpcauth_init_module(void);
>  void __exit		rpcauth_remove_module(void);
> +void __exit		rpc_destroy_generic_auth(void);
>  
> +struct rpc_cred *	rpc_lookup_cred(void);
>  int			rpcauth_register(const struct rpc_authops *);
>  int			rpcauth_unregister(const struct rpc_authops *);
>  struct rpc_auth *	rpcauth_create(rpc_authflavor_t, struct rpc_clnt *);
> diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile
> index 92e1dbe..5369aa3 100644
> --- a/net/sunrpc/Makefile
> +++ b/net/sunrpc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_SUNRPC_GSS) += auth_gss/
>  obj-$(CONFIG_SUNRPC_XPRT_RDMA) += xprtrdma/
>  
>  sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \
> -	    auth.o auth_null.o auth_unix.o \
> +	    auth.o auth_null.o auth_unix.o auth_generic.o \
>  	    svc.o svcsock.o svcauth.o svcauth_unix.o \
>  	    rpcb_clnt.o timer.o xdr.o \
>  	    sunrpc_syms.o cache.o rpc_pipe.o \
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index 1cdb163..012f2a3 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -566,6 +566,7 @@ static struct shrinker rpc_cred_shrinker = {
>  void __init rpcauth_init_module(void)
>  {
>  	rpc_init_authunix();
> +	rpc_init_generic_auth();
>  	register_shrinker(&rpc_cred_shrinker);
>  }
>  
> diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
> new file mode 100644
> index 0000000..6f129b1
> --- /dev/null
> +++ b/net/sunrpc/auth_generic.c
> @@ -0,0 +1,142 @@
> +/*
> + * Generic RPC credential
> + *
> + * Copyright (C) 2008, Trond Myklebust <Trond.Myklebust@netapp.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/sunrpc/auth.h>
> +#include <linux/sunrpc/clnt.h>
> +#include <linux/sunrpc/debug.h>
> +#include <linux/sunrpc/sched.h>
> +
> +#ifdef RPC_DEBUG
> +# define RPCDBG_FACILITY	RPCDBG_AUTH
> +#endif
> +
> +struct generic_cred {
> +	struct rpc_cred gc_base;
> +	struct auth_cred acred;
> +};

Does generic_cred need both gc_base.cr_uid and acred.uid?  Will those
ever disagree?

--b.

> +
> +static struct rpc_auth generic_auth;
> +static struct rpc_cred_cache generic_cred_cache;
> +static const struct rpc_credops generic_credops;
> +
> +/*
> + * Public call interface
> + */
> +struct rpc_cred *rpc_lookup_cred(void)
> +{
> +	return rpcauth_lookupcred(&generic_auth, 0);
> +}
> +EXPORT_SYMBOL_GPL(rpc_lookup_cred);
> +
> +/*
> + * Lookup generic creds for current process
> + */
> +static struct rpc_cred *
> +generic_lookup_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
> +{
> +	return rpcauth_lookup_credcache(&generic_auth, acred, flags);
> +}
> +
> +static struct rpc_cred *
> +generic_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
> +{
> +	struct generic_cred *gcred;
> +
> +	gcred = kmalloc(sizeof(*gcred), GFP_KERNEL);
> +	if (gcred == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rpcauth_init_cred(&gcred->gc_base, acred, &generic_auth, &generic_credops);
> +	gcred->gc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
> +
> +	gcred->acred.uid = acred->uid;
> +	gcred->acred.gid = acred->gid;
> +	gcred->acred.group_info = acred->group_info;
> +	if (gcred->acred.group_info != NULL)
> +		get_group_info(gcred->acred.group_info);
> +
> +	dprintk("RPC:       allocated generic cred %p for uid %d gid %d\n",
> +			gcred, acred->uid, acred->gid);
> +	return &gcred->gc_base;
> +}
> +
> +static void
> +generic_free_cred(struct rpc_cred *cred)
> +{
> +	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
> +
> +	dprintk("RPC:       generic_free_cred %p\n", gcred);
> +	if (gcred->acred.group_info != NULL)
> +		put_group_info(gcred->acred.group_info);
> +	kfree(gcred);
> +}
> +
> +static void
> +generic_free_cred_callback(struct rcu_head *head)
> +{
> +	struct rpc_cred *cred = container_of(head, struct rpc_cred, cr_rcu);
> +	generic_free_cred(cred);
> +}
> +
> +static void
> +generic_destroy_cred(struct rpc_cred *cred)
> +{
> +	call_rcu(&cred->cr_rcu, generic_free_cred_callback);
> +}
> +
> +/*
> + * Match credentials against current process creds.
> + */
> +static int
> +generic_match(struct auth_cred *acred, struct rpc_cred *cred, int flags)
> +{
> +	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
> +
> +	if (gcred->acred.uid != acred->uid ||
> +	    gcred->acred.gid != acred->gid ||
> +	    gcred->acred.group_info != acred->group_info)
> +		return 0;
> +	return 1;
> +}
> +
> +void __init rpc_init_generic_auth(void)
> +{
> +	spin_lock_init(&generic_cred_cache.lock);
> +}
> +
> +void __exit rpc_destroy_generic_auth(void)
> +{
> +	rpcauth_clear_credcache(&generic_cred_cache);
> +}
> +
> +static struct rpc_cred_cache generic_cred_cache = {
> +	{{ NULL, },},
> +};
> +
> +static const struct rpc_authops generic_auth_ops = {
> +	.owner = THIS_MODULE,
> +#ifdef RPC_DEBUG
> +	.au_name = "Generic",
> +#endif
> +	.lookup_cred = generic_lookup_cred,
> +	.crcreate = generic_create_cred,
> +};
> +
> +static struct rpc_auth generic_auth = {
> +	.au_ops = &generic_auth_ops,
> +	.au_count = ATOMIC_INIT(0),
> +	.au_credcache = &generic_cred_cache,
> +};
> +
> +static const struct rpc_credops generic_credops = {
> +	.cr_name = "Generic cred",
> +	.crdestroy = generic_destroy_cred,
> +	.crmatch = generic_match,
> +};
> 
> --
> 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

Re: [PATCH 4/6] SUNRPC: Add a generic RPC credential

[Trond Myklebust]

On Fri, 2008-03-28 at 11:35 -0400, J. Bruce Fields wrote:
> On Thu, Mar 13, 2008 at 01:48:09PM -0400, Trond Myklebust wrote:
> > Add an rpc credential that is not tied to any particular auth mechanism,
> > but that can be cached by NFS, and later used to look up a cred for
> > whichever auth mechanism that turns out to be valid when the RPC call is
> > being made.
> > 
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> > 
> >  include/linux/sunrpc/auth.h |    3 +
> >  net/sunrpc/Makefile         |    2 -
> >  net/sunrpc/auth.c           |    1 
> >  net/sunrpc/auth_generic.c   |  142 +++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 147 insertions(+), 1 deletions(-)
> > 
> > diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
> > index 348546c..70644ed 100644
> > --- a/include/linux/sunrpc/auth.h
> > +++ b/include/linux/sunrpc/auth.h
> > @@ -125,9 +125,12 @@ extern const struct rpc_authops	authunix_ops;
> >  extern const struct rpc_authops	authnull_ops;
> >  
> >  void __init		rpc_init_authunix(void);
> > +void __init		rpc_init_generic_auth(void);
> >  void __init		rpcauth_init_module(void);
> >  void __exit		rpcauth_remove_module(void);
> > +void __exit		rpc_destroy_generic_auth(void);
> >  
> > +struct rpc_cred *	rpc_lookup_cred(void);
> >  int			rpcauth_register(const struct rpc_authops *);
> >  int			rpcauth_unregister(const struct rpc_authops *);
> >  struct rpc_auth *	rpcauth_create(rpc_authflavor_t, struct rpc_clnt *);
> > diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile
> > index 92e1dbe..5369aa3 100644
> > --- a/net/sunrpc/Makefile
> > +++ b/net/sunrpc/Makefile
> > @@ -8,7 +8,7 @@ obj-$(CONFIG_SUNRPC_GSS) += auth_gss/
> >  obj-$(CONFIG_SUNRPC_XPRT_RDMA) += xprtrdma/
> >  
> >  sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \
> > -	    auth.o auth_null.o auth_unix.o \
> > +	    auth.o auth_null.o auth_unix.o auth_generic.o \
> >  	    svc.o svcsock.o svcauth.o svcauth_unix.o \
> >  	    rpcb_clnt.o timer.o xdr.o \
> >  	    sunrpc_syms.o cache.o rpc_pipe.o \
> > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> > index 1cdb163..012f2a3 100644
> > --- a/net/sunrpc/auth.c
> > +++ b/net/sunrpc/auth.c
> > @@ -566,6 +566,7 @@ static struct shrinker rpc_cred_shrinker = {
> >  void __init rpcauth_init_module(void)
> >  {
> >  	rpc_init_authunix();
> > +	rpc_init_generic_auth();
> >  	register_shrinker(&rpc_cred_shrinker);
> >  }
> >  
> > diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
> > new file mode 100644
> > index 0000000..6f129b1
> > --- /dev/null
> > +++ b/net/sunrpc/auth_generic.c
> > @@ -0,0 +1,142 @@
> > +/*
> > + * Generic RPC credential
> > + *
> > + * Copyright (C) 2008, Trond Myklebust <Trond.Myklebust@netapp.com>
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/types.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/sunrpc/auth.h>
> > +#include <linux/sunrpc/clnt.h>
> > +#include <linux/sunrpc/debug.h>
> > +#include <linux/sunrpc/sched.h>
> > +
> > +#ifdef RPC_DEBUG
> > +# define RPCDBG_FACILITY	RPCDBG_AUTH
> > +#endif
> > +
> > +struct generic_cred {
> > +	struct rpc_cred gc_base;
> > +	struct auth_cred acred;
> > +};
> 
> Does generic_cred need both gc_base.cr_uid and acred.uid?  Will those
> ever disagree?

cr_uid is still used by some of the other creds, so I can't remove it
from the rpc_cred yet. It will take a couple more cleanups before we can
do that.

> --b.
> 
> > +
> > +static struct rpc_auth generic_auth;
> > +static struct rpc_cred_cache generic_cred_cache;
> > +static const struct rpc_credops generic_credops;
> > +
> > +/*
> > + * Public call interface
> > + */
> > +struct rpc_cred *rpc_lookup_cred(void)
> > +{
> > +	return rpcauth_lookupcred(&generic_auth, 0);
> > +}
> > +EXPORT_SYMBOL_GPL(rpc_lookup_cred);
> > +
> > +/*
> > + * Lookup generic creds for current process
> > + */
> > +static struct rpc_cred *
> > +generic_lookup_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
> > +{
> > +	return rpcauth_lookup_credcache(&generic_auth, acred, flags);
> > +}
> > +
> > +static struct rpc_cred *
> > +generic_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags)
> > +{
> > +	struct generic_cred *gcred;
> > +
> > +	gcred = kmalloc(sizeof(*gcred), GFP_KERNEL);
> > +	if (gcred == NULL)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	rpcauth_init_cred(&gcred->gc_base, acred, &generic_auth, &generic_credops);
> > +	gcred->gc_base.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE;
> > +
> > +	gcred->acred.uid = acred->uid;
> > +	gcred->acred.gid = acred->gid;
> > +	gcred->acred.group_info = acred->group_info;
> > +	if (gcred->acred.group_info != NULL)
> > +		get_group_info(gcred->acred.group_info);
> > +
> > +	dprintk("RPC:       allocated generic cred %p for uid %d gid %d\n",
> > +			gcred, acred->uid, acred->gid);
> > +	return &gcred->gc_base;
> > +}
> > +
> > +static void
> > +generic_free_cred(struct rpc_cred *cred)
> > +{
> > +	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
> > +
> > +	dprintk("RPC:       generic_free_cred %p\n", gcred);
> > +	if (gcred->acred.group_info != NULL)
> > +		put_group_info(gcred->acred.group_info);
> > +	kfree(gcred);
> > +}
> > +
> > +static void
> > +generic_free_cred_callback(struct rcu_head *head)
> > +{
> > +	struct rpc_cred *cred = container_of(head, struct rpc_cred, cr_rcu);
> > +	generic_free_cred(cred);
> > +}
> > +
> > +static void
> > +generic_destroy_cred(struct rpc_cred *cred)
> > +{
> > +	call_rcu(&cred->cr_rcu, generic_free_cred_callback);
> > +}
> > +
> > +/*
> > + * Match credentials against current process creds.
> > + */
> > +static int
> > +generic_match(struct auth_cred *acred, struct rpc_cred *cred, int flags)
> > +{
> > +	struct generic_cred *gcred = container_of(cred, struct generic_cred, gc_base);
> > +
> > +	if (gcred->acred.uid != acred->uid ||
> > +	    gcred->acred.gid != acred->gid ||
> > +	    gcred->acred.group_info != acred->group_info)
> > +		return 0;
> > +	return 1;
> > +}
> > +
> > +void __init rpc_init_generic_auth(void)
> > +{
> > +	spin_lock_init(&generic_cred_cache.lock);
> > +}
> > +
> > +void __exit rpc_destroy_generic_auth(void)
> > +{
> > +	rpcauth_clear_credcache(&generic_cred_cache);
> > +}
> > +
> > +static struct rpc_cred_cache generic_cred_cache = {
> > +	{{ NULL, },},
> > +};
> > +
> > +static const struct rpc_authops generic_auth_ops = {
> > +	.owner = THIS_MODULE,
> > +#ifdef RPC_DEBUG
> > +	.au_name = "Generic",
> > +#endif
> > +	.lookup_cred = generic_lookup_cred,
> > +	.crcreate = generic_create_cred,
> > +};
> > +
> > +static struct rpc_auth generic_auth = {
> > +	.au_ops = &generic_auth_ops,
> > +	.au_count = ATOMIC_INIT(0),
> > +	.au_credcache = &generic_cred_cache,
> > +};
> > +
> > +static const struct rpc_credops generic_credops = {
> > +	.cr_name = "Generic cred",
> > +	.crdestroy = generic_destroy_cred,
> > +	.crmatch = generic_match,
> > +};
> > 
> > --
> > 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
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

[PATCH 5/6] SUNRPC: Add an rpc_credop callback for binding a credential to an rpc_task

[Trond Myklebust]

We need the ability to treat 'generic' creds specially, since they want to
bind instances of the auth cred instead of binding themselves.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 include/linux/sunrpc/auth.h    |    2 ++
 net/sunrpc/auth.c              |    5 +++--
 net/sunrpc/auth_generic.c      |   15 +++++++++++++++
 net/sunrpc/auth_gss/auth_gss.c |    2 ++
 net/sunrpc/auth_null.c         |    1 +
 net/sunrpc/auth_unix.c         |    1 +
 6 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 70644ed..e93cd8a 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -112,6 +112,7 @@ struct rpc_credops {
 	void			(*crdestroy)(struct rpc_cred *);
 
 	int			(*crmatch)(struct auth_cred *, struct rpc_cred *, int);
+	void			(*crbind)(struct rpc_task *, struct rpc_cred *);
 	__be32 *		(*crmarshal)(struct rpc_task *, __be32 *);
 	int			(*crrefresh)(struct rpc_task *);
 	__be32 *		(*crvalidate)(struct rpc_task *, __be32 *);
@@ -139,6 +140,7 @@ struct rpc_cred *	rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *
 void			rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *);
 struct rpc_cred *	rpcauth_lookupcred(struct rpc_auth *, int);
 void			rpcauth_bindcred(struct rpc_task *, struct rpc_cred *, int);
+void			rpcauth_generic_bind_cred(struct rpc_task *, struct rpc_cred *);
 void			put_rpccred(struct rpc_cred *);
 void			rpcauth_unbindcred(struct rpc_task *);
 __be32 *		rpcauth_marshcred(struct rpc_task *, __be32 *);
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 012f2a3..d65dd79 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -375,13 +375,14 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
 }
 EXPORT_SYMBOL_GPL(rpcauth_init_cred);
 
-static void
+void
 rpcauth_generic_bind_cred(struct rpc_task *task, struct rpc_cred *cred)
 {
 	task->tk_msg.rpc_cred = get_rpccred(cred);
 	dprintk("RPC: %5u holding %s cred %p\n", task->tk_pid,
 			cred->cr_auth->au_ops->au_name, cred);
 }
+EXPORT_SYMBOL_GPL(rpcauth_generic_bind_cred);
 
 static void
 rpcauth_bind_root_cred(struct rpc_task *task)
@@ -421,7 +422,7 @@ void
 rpcauth_bindcred(struct rpc_task *task, struct rpc_cred *cred, int flags)
 {
 	if (cred != NULL)
-		rpcauth_generic_bind_cred(task, cred);
+		cred->cr_ops->crbind(task, cred);
 	else if (flags & RPC_TASK_ROOTCREDS)
 		rpcauth_bind_root_cred(task);
 	else
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index 6f129b1..6a3f77c 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -35,6 +35,20 @@ struct rpc_cred *rpc_lookup_cred(void)
 }
 EXPORT_SYMBOL_GPL(rpc_lookup_cred);
 
+static void
+generic_bind_cred(struct rpc_task *task, struct rpc_cred *cred)
+{
+	struct rpc_auth *auth = task->tk_client->cl_auth;
+	struct auth_cred *acred = &container_of(cred, struct generic_cred, gc_base)->acred;
+	struct rpc_cred *ret;
+
+	ret = auth->au_ops->lookup_cred(auth, acred, 0);
+	if (!IS_ERR(ret))
+		task->tk_msg.rpc_cred = ret;
+	else
+		task->tk_status = PTR_ERR(ret);
+}
+
 /*
  * Lookup generic creds for current process
  */
@@ -138,5 +152,6 @@ static struct rpc_auth generic_auth = {
 static const struct rpc_credops generic_credops = {
 	.cr_name = "Generic cred",
 	.crdestroy = generic_destroy_cred,
+	.crbind = generic_bind_cred,
 	.crmatch = generic_match,
 };
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index ef63849..d34f6df 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1300,6 +1300,7 @@ 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,
@@ -1311,6 +1312,7 @@ static const struct rpc_credops gss_credops = {
 static const struct rpc_credops gss_nullops = {
 	.cr_name	= "AUTH_GSS",
 	.crdestroy	= gss_destroy_cred,
+	.crbind		= rpcauth_generic_bind_cred,
 	.crmatch	= gss_match,
 	.crmarshal	= gss_marshal,
 	.crrefresh	= gss_refresh_null,
diff --git a/net/sunrpc/auth_null.c b/net/sunrpc/auth_null.c
index 537d0e8..3c26c18 100644
--- a/net/sunrpc/auth_null.c
+++ b/net/sunrpc/auth_null.c
@@ -125,6 +125,7 @@ static
 const struct rpc_credops null_credops = {
 	.cr_name	= "AUTH_NULL",
 	.crdestroy	= nul_destroy_cred,
+	.crbind		= rpcauth_generic_bind_cred,
 	.crmatch	= nul_match,
 	.crmarshal	= nul_marshal,
 	.crrefresh	= nul_refresh,
diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
index b763710..04e936a 100644
--- a/net/sunrpc/auth_unix.c
+++ b/net/sunrpc/auth_unix.c
@@ -237,6 +237,7 @@ static
 const struct rpc_credops unix_credops = {
 	.cr_name	= "AUTH_UNIX",
 	.crdestroy	= unx_destroy_cred,
+	.crbind		= rpcauth_generic_bind_cred,
 	.crmatch	= unx_match,
 	.crmarshal	= unx_marshal,
 	.crrefresh	= unx_refresh,

[PATCH 6/6] SUNRPC: Add a helper rpcauth_lookup_generic_cred()

[Trond Myklebust]

The NFSv4 protocol allows clients to negotiate security protocols on the
fly in the case where an administrator on the server changes the export
settings and/or in the case where we may have a filesystem migration event.

Instead of having the NFS client code cache credentials that are tied to a
particular AUTH method it is therefore preferable to have a generic credential
that can be converted into whatever AUTH is in use by the RPC client when
the read/write/sillyrename/... is put on the wire.

We do this by means of the new "generic" credential, which basically just
caches the minimal information that is needed to look up an RPCSEC_GSS,
AUTH_SYS, or AUTH_NULL credential.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c      |    2 +-
 fs/nfs/inode.c    |    2 +-
 fs/nfs/nfs4proc.c |    8 ++++----
 fs/nfs/unlink.c   |    2 +-
 net/sunrpc/auth.c |    1 -
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 6cea747..d583654 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1966,7 +1966,7 @@ force_lookup:
 	if (!NFS_PROTO(inode)->access)
 		goto out_notsup;
 
-	cred = rpcauth_lookupcred(NFS_CLIENT(inode)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (!IS_ERR(cred)) {
 		res = nfs_do_access(inode, cred, mask);
 		put_rpccred(cred);
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index c49f6d8..15f7873 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -613,7 +613,7 @@ int nfs_open(struct inode *inode, struct file *filp)
 	struct nfs_open_context *ctx;
 	struct rpc_cred *cred;
 
-	cred = rpcauth_lookupcred(NFS_CLIENT(inode)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (IS_ERR(cred))
 		return PTR_ERR(cred);
 	ctx = alloc_nfs_open_context(filp->f_path.mnt, filp->f_path.dentry, cred);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index bbb0d58..f38d057 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1408,7 +1408,7 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 		BUG_ON(nd->intent.open.flags & O_CREAT);
 	}
 
-	cred = rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (IS_ERR(cred))
 		return (struct dentry *)cred;
 	parent = dentry->d_parent;
@@ -1443,7 +1443,7 @@ nfs4_open_revalidate(struct inode *dir, struct dentry *dentry, int openflags, st
 	struct rpc_cred *cred;
 	struct nfs4_state *state;
 
-	cred = rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (IS_ERR(cred))
 		return PTR_ERR(cred);
 	state = nfs4_do_open(dir, &path, openflags, NULL, cred);
@@ -1660,7 +1660,7 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 
 	nfs_fattr_init(fattr);
 	
-	cred = rpcauth_lookupcred(NFS_CLIENT(inode)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (IS_ERR(cred))
 		return PTR_ERR(cred);
 
@@ -1896,7 +1896,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 	struct rpc_cred *cred;
 	int status = 0;
 
-	cred = rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
+	cred = rpc_lookup_cred();
 	if (IS_ERR(cred)) {
 		status = PTR_ERR(cred);
 		goto out;
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 7574153..3adf8b2 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -234,7 +234,7 @@ nfs_async_unlink(struct inode *dir, struct dentry *dentry)
 	if (data == NULL)
 		goto out;
 
-	data->cred = rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
+	data->cred = rpc_lookup_cred();
 	if (IS_ERR(data->cred)) {
 		status = PTR_ERR(data->cred);
 		goto out_free;
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index d65dd79..0632cd0 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -356,7 +356,6 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags)
 	put_group_info(acred.group_info);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(rpcauth_lookupcred);
 
 void
 rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,