Skip to contentSkip to conversation

exchange_id changes for 3.5

PATCH New J. Bruce Fields 1 participant 12 messages May 16, 2012

Subsystems (3)
Networking NFS client NFS server
lore
Share & tools
lore

J. Bruce Fields

author
#0 lore raw

Clean up exchange_id and fix a few bugs along the way, noticed while I was working on something else.

Also fix the longstanding setclientid/setclientid_confirm bug that we compare only mapped uid's when determining the principal associated with a client, though those mapped uid's are often the same.

--b.

author
#1 lore raw

PATCH 1/11 nfsd4: exchange_id has a pointless copy

From: "J. Bruce Fields" <[email protected]>

We just verified above that these two verifiers are already the same.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |    1 -
 1 file changed, 1 deletion(-)
fs/nfsd/nfs4state.c +0 -1
index c33f052..b699c27f 100644
@@ -1550,7 +1550,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 		 * Falling into 18.35.4 case 2, possible router replay.
 		 * Leave confirmed record intact and return same result.
 		 */
-		copy_verf(conf, &verf);
 		new = conf;
 		goto out_copy;
 	}
Signature · 1 line
1.7.9.5
author
#2 lore raw

PATCH 2/11 nfsd4: exchange_id error cleanup

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

There's no point to the dprintk here as the main proc_compound loop already does this.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
fs/nfsd/nfs4state.c +1 -5
index b699c27f..6b7b884 100644
@@ -1508,13 +1508,11 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	status = nfs4_make_rec_clidname(dname, &exid->clname);
 
 	if (status)
-		goto error;
+		return status;
 
 	strhashval = clientstr_hashval(dname);
 
 	nfs4_lock_state();
-	status = nfs_ok;
-
 	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {
 		if (!clp_used_exchangeid(conf)) {

@@ -1593,8 +1591,6 @@ out_copy:
 
 out:
 	nfs4_unlock_state();
-error:
-	dprintk("nfsd4_exchange_id returns %d\n", ntohl(status));
 	return status;
 }
Signature · 1 line
1.7.9.5
author
#3 lore raw

PATCH 3/11 nfsd4: exchange_id: check creds before killing confirmed client

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

We mustn't allow a client to destroy another client with established state unless it has the right credential.

And some minor cleanup.

(Note: our comparison of credentials is actually pretty bogus currently; that will need to be fixed in another patch.)

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
fs/nfsd/nfs4state.c +8 -9
index 6b7b884..9d77181 100644
@@ -1519,6 +1519,14 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			status = nfserr_clid_inuse; /* XXX: ? */
 			goto out;
 		}
+		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
+			/* 18.35.4 case 9 */
+			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A)
+				status = nfserr_perm;
+			else /* case 3 */
+				status = nfserr_clid_inuse;
+			goto out;
+		}
 		if (!same_verf(&verf, &conf->cl_verifier)) {
 			/* 18.35.4 case 8 */
 			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {

@@ -1529,15 +1537,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			expire_client(conf);
 			goto out_new;
 		}
-		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
-			/* 18.35.4 case 9 */
-			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
-				status = nfserr_perm;
-				goto out;
-			}
-			expire_client(conf);
-			goto out_new;
-		}
 		/*
 		 * Set bit when the owner id and verifier map to an already
 		 * confirmed client id (18.35.3).
Signature · 1 line
1.7.9.5
author
#4 lore raw

PATCH 4/11 nfsd4: allow an EXCHANGE_ID to kill a 4.0 client

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Following rfc 5661 section 2.4.1, we can permit a 4.1 client to remove an established 4.0 client's state.

(But we don't allow updates.)

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +8 -2
index 9d77181..76459a7 100644
@@ -1516,8 +1516,10 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {
 		if (!clp_used_exchangeid(conf)) {
-			status = nfserr_clid_inuse; /* XXX: ? */
-			goto out;
+			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
+				status = nfserr_inval; /* buggy client */
+				goto out;
+			}
 		}
 		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
 			/* 18.35.4 case 9 */

@@ -1537,6 +1539,10 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			expire_client(conf);
 			goto out_new;
 		}
+		if (!clp_used_exchangeid(conf)) {
+			status = nfserr_inval;
+			goto out;
+		}
 		/*
 		 * Set bit when the owner id and verifier map to an already
 		 * confirmed client id (18.35.3).
Signature · 1 line
1.7.9.5
author
#5 lore raw

PATCH 5/11 nfsd4: exchange_id cleanup: local shorthands for repeated tests

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
fs/nfsd/nfs4state.c +10 -6
index 76459a7..e7c4b04 100644
@@ -1483,6 +1483,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	char			addr_str[INET6_ADDRSTRLEN];
 	nfs4_verifier		verf = exid->verifier;
 	struct sockaddr		*sa = svc_addr(rqstp);
+	bool	update = exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A;
 
 	rpc_ntop(sa, addr_str, sizeof(addr_str));
 	dprintk("%s rqstp=%p exid=%p clname.len=%u clname.data=%p "

@@ -1515,23 +1516,26 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	nfs4_lock_state();
 	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {
+		bool creds_match = same_creds(&conf->cl_cred, &rqstp->rq_cred);
+		bool verfs_match = same_verf(&verf, &conf->cl_verifier);
+
 		if (!clp_used_exchangeid(conf)) {
-			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
+			if (update) {
 				status = nfserr_inval; /* buggy client */
 				goto out;
 			}
 		}
-		if (!same_creds(&conf->cl_cred, &rqstp->rq_cred)) {
+		if (!creds_match) {
 			/* 18.35.4 case 9 */
-			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A)
+			if (update)
 				status = nfserr_perm;
 			else /* case 3 */
 				status = nfserr_clid_inuse;
 			goto out;
 		}
-		if (!same_verf(&verf, &conf->cl_verifier)) {
+		if (!verfs_match) {
 			/* 18.35.4 case 8 */
-			if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
+			if (update) {
 				status = nfserr_not_same;
 				goto out;
 			}

@@ -1558,7 +1562,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 	}
 
 	/* 18.35.4 case 7 */
-	if (exid->flags & EXCHGID4_FLAG_UPD_CONFIRMED_REC_A) {
+	if (update) {
 		status = nfserr_noent;
 		goto out;
 	}
Signature · 1 line
1.7.9.5
author
#6 lore raw

PATCH 6/11 nfsd4: exchange_id cleanup: comments

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Make these comments a bit more concise and uniform.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)
fs/nfsd/nfs4state.c +10 -26
index e7c4b04..1239ff4 100644
@@ -1513,6 +1513,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 
 	strhashval = clientstr_hashval(dname);
 
+	/* Cases below refer to rfc 5661 section 18.35.4: */
 	nfs4_lock_state();
 	conf = find_confirmed_client_by_str(dname, strhashval);
 	if (conf) {

@@ -1520,26 +1521,24 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 		bool verfs_match = same_verf(&verf, &conf->cl_verifier);
 
 		if (!clp_used_exchangeid(conf)) {
-			if (update) {
-				status = nfserr_inval; /* buggy client */
+			if (update) { /* buggy client */
+				status = nfserr_inval;
 				goto out;
 			}
 		}
-		if (!creds_match) {
-			/* 18.35.4 case 9 */
+		if (!creds_match) { /* case 9 */
 			if (update)
 				status = nfserr_perm;
 			else /* case 3 */
 				status = nfserr_clid_inuse;
 			goto out;
 		}
-		if (!verfs_match) {
-			/* 18.35.4 case 8 */
+		if (!verfs_match) { /* case 8 */
 			if (update) {
 				status = nfserr_not_same;
 				goto out;
 			}
-			/* Client reboot: destroy old state */
+			/* case 5, client reboot */
 			expire_client(conf);
 			goto out_new;
 		}

@@ -1547,38 +1546,23 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			status = nfserr_inval;
 			goto out;
 		}
-		/*
-		 * Set bit when the owner id and verifier map to an already
-		 * confirmed client id (18.35.3).
-		 */
+		/* case 2 */
 		exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
-
-		/*
-		 * Falling into 18.35.4 case 2, possible router replay.
-		 * Leave confirmed record intact and return same result.
-		 */
 		new = conf;
 		goto out_copy;
 	}
 
-	/* 18.35.4 case 7 */
-	if (update) {
+	if (update) { /* case 7 */
 		status = nfserr_noent;
 		goto out;
 	}
 
 	unconf  = find_unconfirmed_client_by_str(dname, strhashval);
-	if (unconf) {
-		/*
-		 * Possible retry or client restart.  Per 18.35.4 case 4,
-		 * a new unconfirmed record should be generated regardless
-		 * of whether any properties have changed.
-		 */
+	if (unconf) /* case 4, possible retry or client restart */
 		expire_client(unconf);
-	}
 
+	/* case 1 (normal case) */
 out_new:
-	/* Normal case */
 	new = create_client(exid->clname, dname, rqstp, &verf);
 	if (new == NULL) {
 		status = nfserr_jukebox;
Signature · 1 line
1.7.9.5
author
#7 lore raw

PATCH 7/11 nfsd4: rearrange exchange_id logic to simplify

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Minor cleanup: it's simpler to have separate code paths for the update and non-update cases.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
fs/nfsd/nfs4state.c +20 -20
index 1239ff4..0c553af 100644
@@ -1520,36 +1520,36 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 		bool creds_match = same_creds(&conf->cl_cred, &rqstp->rq_cred);
 		bool verfs_match = same_verf(&verf, &conf->cl_verifier);
 
-		if (!clp_used_exchangeid(conf)) {
-			if (update) { /* buggy client */
+		if (update) {
+			if (!clp_used_exchangeid(conf)) { /* buggy client */
 				status = nfserr_inval;
 				goto out;
 			}
-		}
-		if (!creds_match) { /* case 9 */
-			if (update)
+			if (!creds_match) { /* case 9 */
 				status = nfserr_perm;
-			else /* case 3 */
-				status = nfserr_clid_inuse;
-			goto out;
-		}
-		if (!verfs_match) { /* case 8 */
-			if (update) {
+				goto out;
+			}
+			if (!verfs_match) { /* case 8 */
 				status = nfserr_not_same;
 				goto out;
 			}
-			/* case 5, client reboot */
-			expire_client(conf);
-			goto out_new;
+			/* case 6 */
+			exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
+			new = conf;
+			goto out_copy;
 		}
-		if (!clp_used_exchangeid(conf)) {
-			status = nfserr_inval;
+		if (!creds_match) { /* case 3 */
+			status = nfserr_clid_inuse;
 			goto out;
 		}
-		/* case 2 */
-		exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
-		new = conf;
-		goto out_copy;
+		if (verfs_match) { /* case 2 */
+			exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
+			new = conf;
+			goto out_copy;
+		}
+		/* case 5, client reboot */
+		expire_client(conf);
+		goto out_new;
 	}
 
 	if (update) { /* case 7 */
Signature · 1 line
1.7.9.5
author
#8 lore raw

PATCH 8/11 nfsd4: allow removing clients not holding state

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

RFC 5661 actually says we should allow an exchange_id to remove a matching client, even if the exchange_id comes from a different principal, *if* the victim client lacks any state.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +19 -2
index 0c553af..e7f61d3 100644
@@ -1471,6 +1471,19 @@ nfsd4_set_ex_flags(struct nfs4_client *new, struct nfsd4_exchange_id *clid)
 	clid->flags = new->cl_exchange_flags;
 }
 
+static bool client_has_state(struct nfs4_client *clp)
+{
+	/*
+	 * Note clp->cl_openowners check isn't quite right: there's no
+	 * need to count owners without stateid's.
+	 *
+	 * Also note we should probably be using this in 4.0 case too.
+	 */
+	return list_empty(&clp->cl_openowners)
+		&& list_empty(&clp->cl_delegations)
+		&& list_empty(&clp->cl_sessions);
+}
+
 __be32
 nfsd4_exchange_id(struct svc_rqst *rqstp,
 		  struct nfsd4_compound_state *cstate,

@@ -1539,8 +1552,11 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			goto out_copy;
 		}
 		if (!creds_match) { /* case 3 */
-			status = nfserr_clid_inuse;
-			goto out;
+			if (client_has_state(conf)) {
+				status = nfserr_clid_inuse;
+				goto out;
+			}
+			goto expire_client;
 		}
 		if (verfs_match) { /* case 2 */
 			exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;

@@ -1548,6 +1564,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
 			goto out_copy;
 		}
 		/* case 5, client reboot */
+expire_client:
 		expire_client(conf);
 		goto out_new;
 	}
Signature · 1 line
1.7.9.5
author
#9 lore raw

PATCH 9/11 nfsd4: move principal name into svc_cred

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Instead of keeping the principal name associated with a request in a structure that's private to auth_gss and using an accessor function, move it to svc_cred.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfs/callback.c                  |    2 +-
 fs/nfsd/nfs4callback.c             |    5 +++--
 fs/nfsd/nfs4state.c                |   34 +++++++++++++++++-----------------
 fs/nfsd/state.h                    |    1 -
 include/linux/sunrpc/svcauth.h     |    9 +++++++++
 include/linux/sunrpc/svcauth_gss.h |    1 -
 net/sunrpc/auth_gss/svcauth_gss.c  |   25 ++++++-------------------
 net/sunrpc/svcauth_unix.c          |    2 ++
 8 files changed, 38 insertions(+), 41 deletions(-)
fs/nfs/callback.c +1 -1
index cff3940..970659d 100644
@@ -343,7 +343,7 @@ void nfs_callback_down(int minorversion)
 int
 check_gss_callback_principal(struct nfs_client *clp, struct svc_rqst *rqstp)
 {
-	char *p = svc_gss_principal(rqstp);
+	char *p = rqstp->rq_cred.cr_principal;
 
 	if (rqstp->rq_authop->flavour != RPC_AUTH_GSS)
 		return 1;
fs/nfsd/nfs4callback.c +3 -2
index c8e9f63..a5fd6b9 100644
@@ -650,9 +650,10 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
 	struct rpc_clnt *client;
 
 	if (clp->cl_minorversion == 0) {
-		if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
+		if (!clp->cl_cred.cr_principal &&
+				(clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
 			return -EINVAL;
-		args.client_name = clp->cl_principal;
+		args.client_name = clp->cl_cred.cr_principal;
 		args.prognumber	= conn->cb_prog,
 		args.protocol = XPRT_TRANSPORT_TCP;
 		args.authflavor = clp->cl_flavor;
fs/nfsd/nfs4state.c +17 -17
index e7f61d3..625ec80 100644
@@ -1050,9 +1050,7 @@ free_client(struct nfs4_client *clp)
 		list_del(&ses->se_perclnt);
 		nfsd4_put_session_locked(ses);
 	}
-	if (clp->cl_cred.cr_group_info)
-		put_group_info(clp->cl_cred.cr_group_info);
-	kfree(clp->cl_principal);
+	free_svc_cred(&clp->cl_cred);
 	kfree(clp->cl_name.data);
 	kfree(clp);
 }

@@ -1133,12 +1131,20 @@ static void copy_clid(struct nfs4_client *target, struct nfs4_client *source)
 	target->cl_clientid.cl_id = source->cl_clientid.cl_id; 
 }
 
-static void copy_cred(struct svc_cred *target, struct svc_cred *source)
+static int copy_cred(struct svc_cred *target, struct svc_cred *source)
 {
+	if (source->cr_principal) {
+		target->cr_principal =
+				kstrdup(source->cr_principal, GFP_KERNEL);
+		if (target->cr_principal == NULL)
+			return -ENOMEM;
+	} else
+		target->cr_principal = NULL;
 	target->cr_uid = source->cr_uid;
 	target->cr_gid = source->cr_gid;
 	target->cr_group_info = source->cr_group_info;
 	get_group_info(target->cr_group_info);
+	return 0;
 }
 
 static int same_name(const char *n1, const char *n2)

@@ -1205,25 +1211,20 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
 {
 	struct nfs4_client *clp;
 	struct sockaddr *sa = svc_addr(rqstp);
-	char *princ;
+	int ret;
 
 	clp = alloc_client(name);
 	if (clp == NULL)
 		return NULL;
 
 	INIT_LIST_HEAD(&clp->cl_sessions);
-
-	princ = svc_gss_principal(rqstp);
-	if (princ) {
-		clp->cl_principal = kstrdup(princ, GFP_KERNEL);
-		if (clp->cl_principal == NULL) {
-			spin_lock(&client_lock);
-			free_client(clp);
-			spin_unlock(&client_lock);
-			return NULL;
-		}
+	ret = copy_cred(&clp->cl_cred, &rqstp->rq_cred);
+	if (ret) {
+		spin_lock(&client_lock);
+		free_client(clp);
+		spin_unlock(&client_lock);
+		return NULL;
 	}
-
 	idr_init(&clp->cl_stateids);
 	memcpy(clp->cl_recdir, recdir, HEXDIR_LEN);
 	atomic_set(&clp->cl_refcount, 0);

@@ -1242,7 +1243,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
 	copy_verf(clp, verf);
 	rpc_copy_addr((struct sockaddr *) &clp->cl_addr, sa);
 	clp->cl_flavor = rqstp->rq_flavor;
-	copy_cred(&clp->cl_cred, &rqstp->rq_cred);
 	gen_confirm(clp);
 	clp->cl_cb_session = NULL;
 	return clp;
fs/nfsd/state.h +0 -1
index 89ab137..849091e 100644
@@ -232,7 +232,6 @@ struct nfs4_client {
 	time_t                  cl_time;        /* time of last lease renewal */
 	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
 	u32			cl_flavor;	/* setclientid pseudoflavor */
-	char			*cl_principal;	/* setclientid principal name */
 	struct svc_cred		cl_cred; 	/* setclientid principal */
 	clientid_t		cl_clientid;	/* generated by server */
 	nfs4_verifier		cl_confirm;	/* generated by server */
include/linux/sunrpc/svcauth.h +9 -0
index 2c54683..16fe477 100644
@@ -15,13 +15,22 @@
 #include <linux/sunrpc/msg_prot.h>
 #include <linux/sunrpc/cache.h>
 #include <linux/hash.h>
+#include <linux/cred.h>
 
 struct svc_cred {
 	uid_t			cr_uid;
 	gid_t			cr_gid;
 	struct group_info	*cr_group_info;
+	char			*cr_principal; /* for gss */
 };
 
+static inline void free_svc_cred(struct svc_cred *cred)
+{
+	if (cred->cr_group_info)
+		put_group_info(cred->cr_group_info);
+	kfree(cred->cr_principal);
+}
+
 struct svc_rqst;		/* forward decl */
 struct in6_addr;
include/linux/sunrpc/svcauth_gss.h +0 -1
index 7c32daa..726aff1 100644
@@ -22,7 +22,6 @@ int gss_svc_init_net(struct net *net);
 void gss_svc_shutdown_net(struct net *net);
 int svcauth_gss_register_pseudoflavor(u32 pseudoflavor, char * name);
 u32 svcauth_gss_flavor(struct auth_domain *dom);
-char *svc_gss_principal(struct svc_rqst *);
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SUNRPC_SVCAUTH_GSS_H */
net/sunrpc/auth_gss/svcauth_gss.c +6 -19
index f0a0cd4..d091d7d 100644
@@ -335,7 +335,6 @@ struct rsc {
 	struct svc_cred		cred;
 	struct gss_svc_seq_data	seqdata;
 	struct gss_ctx		*mechctx;
-	char			*client_name;
 };
 
 static struct rsc *rsc_update(struct cache_detail *cd, struct rsc *new, struct rsc *old);

@@ -346,9 +345,7 @@ static void rsc_free(struct rsc *rsci)
 	kfree(rsci->handle.data);
 	if (rsci->mechctx)
 		gss_delete_sec_context(&rsci->mechctx);
-	if (rsci->cred.cr_group_info)
-		put_group_info(rsci->cred.cr_group_info);
-	kfree(rsci->client_name);
+	free_svc_cred(&rsci->cred);
 }
 
 static void rsc_put(struct kref *ref)

@@ -386,7 +383,7 @@ rsc_init(struct cache_head *cnew, struct cache_head *ctmp)
 	tmp->handle.data = NULL;
 	new->mechctx = NULL;
 	new->cred.cr_group_info = NULL;
-	new->client_name = NULL;
+	new->cred.cr_principal = NULL;
 }
 
 static void

@@ -401,8 +398,8 @@ update_rsc(struct cache_head *cnew, struct cache_head *ctmp)
 	spin_lock_init(&new->seqdata.sd_lock);
 	new->cred = tmp->cred;
 	tmp->cred.cr_group_info = NULL;
-	new->client_name = tmp->client_name;
-	tmp->client_name = NULL;
+	new->cred.cr_principal = tmp->cred.cr_principal;
+	tmp->cred.cr_principal = NULL;
 }
 
 static struct cache_head *

@@ -496,8 +493,8 @@ static int rsc_parse(struct cache_detail *cd,
 		/* get client name */
 		len = qword_get(&mesg, buf, mlen);
 		if (len > 0) {
-			rsci.client_name = kstrdup(buf, GFP_KERNEL);
-			if (!rsci.client_name)
+			rsci.cred.cr_principal = kstrdup(buf, GFP_KERNEL);
+			if (!rsci.cred.cr_principal)
 				goto out;
 		}
 

@@ -927,16 +924,6 @@ struct gss_svc_data {
 	struct rsc			*rsci;
 };
 
-char *svc_gss_principal(struct svc_rqst *rqstp)
-{
-	struct gss_svc_data *gd = (struct gss_svc_data *)rqstp->rq_auth_data;
-
-	if (gd && gd->rsci)
-		return gd->rsci->client_name;
-	return NULL;
-}
-EXPORT_SYMBOL_GPL(svc_gss_principal);
-
 static int
 svcauth_gss_set_client(struct svc_rqst *rqstp)
 {
net/sunrpc/svcauth_unix.c +2 -0
index 9c3b9f0..12e4897 100644
@@ -740,6 +740,7 @@ svcauth_null_accept(struct svc_rqst *rqstp, __be32 *authp)
 	struct svc_cred	*cred = &rqstp->rq_cred;
 
 	cred->cr_group_info = NULL;
+	cred->cr_principal = NULL;
 	rqstp->rq_client = NULL;
 
 	if (argv->iov_len < 3*4)

@@ -805,6 +806,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 	int		len   = argv->iov_len;
 
 	cred->cr_group_info = NULL;
+	cred->cr_principal = NULL;
 	rqstp->rq_client = NULL;
 
 	if ((len -= 3*4) < 0)
Signature · 1 line
1.7.9.5
author
#10 lore raw

PATCH 10/11 nfsd4: stricter cred comparison for setclientid/exchange_id

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

The typical setclientid or exchange_id will probably be performed with a credential that maps to either root or nobody, so comparing just uid's is unlikely to be useful. So, use everything else we can get our hands on.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/nfs4state.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
fs/nfsd/nfs4state.c +21 -1
index 625ec80..f6d2a55 100644
@@ -1164,11 +1164,31 @@ same_clid(clientid_t *cl1, clientid_t *cl2)
 	return (cl1->cl_boot == cl2->cl_boot) && (cl1->cl_id == cl2->cl_id);
 }
 
+static bool groups_equal(struct group_info *g1, struct group_info *g2)
+{
+	int i;
+
+	if (g1->ngroups != g2->ngroups)
+		return false;
+	for (i=0; i<g1->ngroups; i++)
+		if (GROUP_AT(g1, i) != GROUP_AT(g2, i))
+			return false;
+	return true;
+}
+
 /* XXX what about NGROUP */
 static int
 same_creds(struct svc_cred *cr1, struct svc_cred *cr2)
 {
-	return cr1->cr_uid == cr2->cr_uid;
+	if ((cr1->cr_uid != cr2->cr_uid)
+		|| (cr1->cr_gid != cr2->cr_gid)
+		|| !groups_equal(cr1->cr_group_info, cr2->cr_group_info))
+		return false;
+	if (cr1->cr_principal == cr2->cr_principal)
+		return true;
+	if (!cr1->cr_principal || !cr2->cr_principal)
+		return false;
+	return 0 == strcmp(cr1->cr_principal, cr1->cr_principal);
 }
 
 static void gen_clid(struct nfs4_client *clp)
Signature · 1 line
1.7.9.5
author
#11 lore raw

PATCH 11/11 nfsd4: move rq_flavor into svc_cred

In reply to J. Bruce Fields

From: "J. Bruce Fields" <[email protected]>

Move the rq_flavor into struct svc_cred, and use it in setclientid and exchange_id comparisons as well.

Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/auth.c                    |    2 +-
 fs/nfsd/export.c                  |    6 +++---
 fs/nfsd/nfs4idmap.c               |    4 ++--
 fs/nfsd/nfs4state.c               |    6 +++---
 include/linux/sunrpc/svc.h        |    1 -
 include/linux/sunrpc/svcauth.h    |    1 +
 net/sunrpc/auth_gss/svcauth_gss.c |    2 +-
 net/sunrpc/svcauth_unix.c         |    4 ++--
 8 files changed, 13 insertions(+), 13 deletions(-)
fs/nfsd/auth.c +1 -1
index 79717a4..b42eaf3 100644
@@ -10,7 +10,7 @@ int nfsexp_flags(struct svc_rqst *rqstp, struct svc_export *exp)
 	struct exp_flavor_info *end = exp->ex_flavors + exp->ex_nflavors;
 
 	for (f = exp->ex_flavors; f < end; f++) {
-		if (f->pseudoflavor == rqstp->rq_flavor)
+		if (f->pseudoflavor == rqstp->rq_cred.cr_flavor)
 			return f->flags;
 	}
 	return exp->ex_flags;
fs/nfsd/export.c +3 -3
index ec16364..ba23349 100644
@@ -904,13 +904,13 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
 		return 0;
 	/* ip-address based client; check sec= export option: */
 	for (f = exp->ex_flavors; f < end; f++) {
-		if (f->pseudoflavor == rqstp->rq_flavor)
+		if (f->pseudoflavor == rqstp->rq_cred.cr_flavor)
 			return 0;
 	}
 	/* defaults in absence of sec= options: */
 	if (exp->ex_nflavors == 0) {
-		if (rqstp->rq_flavor == RPC_AUTH_NULL ||
-		    rqstp->rq_flavor == RPC_AUTH_UNIX)
+		if (rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
+		    rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)
 			return 0;
 	}
 	return nfserr_wrongsec;
fs/nfsd/nfs4idmap.c +2 -2
index 286a7f8..dae36f1 100644
@@ -605,7 +605,7 @@ numeric_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namel
 static __be32
 do_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namelen, uid_t *id)
 {
-	if (nfs4_disable_idmapping && rqstp->rq_flavor < RPC_AUTH_GSS)
+	if (nfs4_disable_idmapping && rqstp->rq_cred.cr_flavor < RPC_AUTH_GSS)
 		if (numeric_name_to_id(rqstp, type, name, namelen, id))
 			return 0;
 		/*

@@ -618,7 +618,7 @@ do_name_to_id(struct svc_rqst *rqstp, int type, const char *name, u32 namelen, u
 static int
 do_id_to_name(struct svc_rqst *rqstp, int type, uid_t id, char *name)
 {
-	if (nfs4_disable_idmapping && rqstp->rq_flavor < RPC_AUTH_GSS)
+	if (nfs4_disable_idmapping && rqstp->rq_cred.cr_flavor < RPC_AUTH_GSS)
 		return sprintf(name, "%u", id);
 	return idmap_id_to_name(rqstp, type, id, name);
 }
fs/nfsd/nfs4state.c +3 -3
index f6d2a55..3a8c2df 100644
@@ -1140,6 +1140,7 @@ static int copy_cred(struct svc_cred *target, struct svc_cred *source)
 			return -ENOMEM;
 	} else
 		target->cr_principal = NULL;
+	target->cr_flavor = source->cr_flavor;
 	target->cr_uid = source->cr_uid;
 	target->cr_gid = source->cr_gid;
 	target->cr_group_info = source->cr_group_info;

@@ -1176,11 +1177,11 @@ static bool groups_equal(struct group_info *g1, struct group_info *g2)
 	return true;
 }
 
-/* XXX what about NGROUP */
 static int
 same_creds(struct svc_cred *cr1, struct svc_cred *cr2)
 {
-	if ((cr1->cr_uid != cr2->cr_uid)
+	if ((cr1->cr_flavor != cr2->cr_flavor)
+		|| (cr1->cr_uid != cr2->cr_uid)
 		|| (cr1->cr_gid != cr2->cr_gid)
 		|| !groups_equal(cr1->cr_group_info, cr2->cr_group_info))
 		return false;

@@ -1262,7 +1263,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
 	rpc_init_wait_queue(&clp->cl_cb_waitq, "Backchannel slot table");
 	copy_verf(clp, verf);
 	rpc_copy_addr((struct sockaddr *) &clp->cl_addr, sa);
-	clp->cl_flavor = rqstp->rq_flavor;
 	gen_confirm(clp);
 	clp->cl_cb_session = NULL;
 	return clp;
include/linux/sunrpc/svc.h +0 -1
index 2b43e02..40e0a27 100644
@@ -232,7 +232,6 @@ struct svc_rqst {
 	struct svc_pool *	rq_pool;	/* thread pool */
 	struct svc_procedure *	rq_procinfo;	/* procedure info */
 	struct auth_ops *	rq_authop;	/* authentication flavour */
-	u32			rq_flavor;	/* pseudoflavor */
 	struct svc_cred		rq_cred;	/* auth info */
 	void *			rq_xprt_ctxt;	/* transport specific context ptr */
 	struct svc_deferred_req*rq_deferred;	/* deferred request we are replaying */
include/linux/sunrpc/svcauth.h +1 -0
index 16fe477..dd74084 100644
@@ -21,6 +21,7 @@ struct svc_cred {
 	uid_t			cr_uid;
 	gid_t			cr_gid;
 	struct group_info	*cr_group_info;
+	u32			cr_flavor; /* pseudoflavor */
 	char			*cr_principal; /* for gss */
 };
net/sunrpc/auth_gss/svcauth_gss.c +1 -1
index d091d7d..bcb7737 100644
@@ -1202,7 +1202,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp, __be32 *authp)
 		}
 		svcdata->rsci = rsci;
 		cache_get(&rsci->h);
-		rqstp->rq_flavor = gss_svc_to_pseudoflavor(
+		rqstp->rq_cred.cr_flavor = gss_svc_to_pseudoflavor(
 					rsci->mechctx->mech_type, gc->gc_svc);
 		ret = SVC_OK;
 		goto out;
net/sunrpc/svcauth_unix.c +2 -2
index 12e4897..88962cf 100644
@@ -768,7 +768,7 @@ svcauth_null_accept(struct svc_rqst *rqstp, __be32 *authp)
 	svc_putnl(resv, RPC_AUTH_NULL);
 	svc_putnl(resv, 0);
 
-	rqstp->rq_flavor = RPC_AUTH_NULL;
+	rqstp->rq_cred.cr_flavor = RPC_AUTH_NULL;
 	return SVC_OK;
 }
 

@@ -839,7 +839,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp, __be32 *authp)
 	svc_putnl(resv, RPC_AUTH_NULL);
 	svc_putnl(resv, 0);
 
-	rqstp->rq_flavor = RPC_AUTH_UNIX;
+	rqstp->rq_cred.cr_flavor = RPC_AUTH_UNIX;
 	return SVC_OK;
 
 badcred:
Signature · 1 line
1.7.9.5