linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@primarydata.com>
To: bfields@fieldses.org
Cc: linux-nfs@vger.kernel.org, hch@infradead.org
Subject: [PATCH 11/40] nfsd: ensure atomicity in nfsd4_free_stateid and nfsd4_validate_stateid
Date: Mon, 21 Jul 2014 11:02:23 -0400	[thread overview]
Message-ID: <1405954972-28904-12-git-send-email-jlayton@primarydata.com> (raw)
In-Reply-To: <1405954972-28904-1-git-send-email-jlayton@primarydata.com>

Hold the cl_lock over the bulk of these functions. In addition to
ensuring that they aren't freed prematurely, this will also help prevent
a potential race that could be introduced later. Once we remove the
client_mutex, it'll be possible for FREE_STATEID and CLOSE to race and
for both to try to put the "persistent" reference to the stateid.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/nfsd/nfs4state.c | 67 +++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 33 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d83f1a27aded..70119d2a69fd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1703,16 +1703,6 @@ find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
 	return ret;
 }
 
-static struct nfs4_stid *find_stateid(struct nfs4_client *cl, stateid_t *t)
-{
-	struct nfs4_stid *ret;
-
-	spin_lock(&cl->cl_lock);
-	ret = find_stateid_locked(cl, t);
-	spin_unlock(&cl->cl_lock);
-	return ret;
-}
-
 static struct nfs4_stid *find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
 {
 	struct nfs4_stid *s;
@@ -4115,10 +4105,10 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 {
 	struct nfs4_stid *s;
 	struct nfs4_ol_stateid *ols;
-	__be32 status;
+	__be32 status = nfserr_bad_stateid;
 
 	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid))
-		return nfserr_bad_stateid;
+		return status;
 	/* Client debugging aid. */
 	if (!same_clid(&stateid->si_opaque.so_clid, &cl->cl_clientid)) {
 		char addr_str[INET6_ADDRSTRLEN];
@@ -4126,34 +4116,42 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 				 sizeof(addr_str));
 		pr_warn_ratelimited("NFSD: client %s testing state ID "
 					"with incorrect client ID\n", addr_str);
-		return nfserr_bad_stateid;
+		return status;
 	}
-	s = find_stateid(cl, stateid);
+	spin_lock(&cl->cl_lock);
+	s = find_stateid_locked(cl, stateid);
 	if (!s)
-		return nfserr_bad_stateid;
+		goto out_unlock;
 	status = check_stateid_generation(stateid, &s->sc_stateid, 1);
 	if (status)
-		return status;
+		goto out_unlock;
 	switch (s->sc_type) {
 	case NFS4_DELEG_STID:
-		return nfs_ok;
+		status = nfs_ok;
+		break;
 	case NFS4_REVOKED_DELEG_STID:
-		return nfserr_deleg_revoked;
+		status = nfserr_deleg_revoked;
+		break;
 	case NFS4_OPEN_STID:
 	case NFS4_LOCK_STID:
 		ols = openlockstateid(s);
 		if (ols->st_stateowner->so_is_open_owner
 	    			&& !(openowner(ols->st_stateowner)->oo_flags
 						& NFS4_OO_CONFIRMED))
-			return nfserr_bad_stateid;
-		return nfs_ok;
+			status = nfserr_bad_stateid;
+		else
+			status = nfs_ok;
+		break;
 	default:
 		printk("unknown stateid type %x\n", s->sc_type);
 		/* Fallthrough */
 	case NFS4_CLOSED_STID:
 	case NFS4_CLOSED_DELEG_STID:
-		return nfserr_bad_stateid;
+		status = nfserr_bad_stateid;
 	}
+out_unlock:
+	spin_unlock(&cl->cl_lock);
+	return status;
 }
 
 static __be32
@@ -4304,34 +4302,37 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 ret = nfserr_bad_stateid;
 
 	nfs4_lock_state();
-	s = find_stateid(cl, stateid);
+	spin_lock(&cl->cl_lock);
+	s = find_stateid_locked(cl, stateid);
 	if (!s)
-		goto out;
+		goto out_unlock;
 	switch (s->sc_type) {
 	case NFS4_DELEG_STID:
 		ret = nfserr_locks_held;
-		goto out;
+		break;
 	case NFS4_OPEN_STID:
 	case NFS4_LOCK_STID:
 		ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
 		if (ret)
-			goto out;
-		if (s->sc_type == NFS4_LOCK_STID)
-			ret = nfsd4_free_lock_stateid(openlockstateid(s));
-		else
+			break;
+		if (s->sc_type != NFS4_LOCK_STID) {
 			ret = nfserr_locks_held;
-		break;
+			break;
+		}
+		spin_unlock(&cl->cl_lock);
+		ret = nfsd4_free_lock_stateid(openlockstateid(s));
+		goto out;
 	case NFS4_REVOKED_DELEG_STID:
 		dp = delegstateid(s);
-		spin_lock(&cl->cl_lock);
 		list_del_init(&dp->dl_recall_lru);
 		spin_unlock(&cl->cl_lock);
 		nfs4_put_delegation(dp);
 		ret = nfs_ok;
-		break;
-	default:
-		ret = nfserr_bad_stateid;
+		goto out;
+	/* Default falls through and returns nfserr_bad_stateid */
 	}
+out_unlock:
+	spin_unlock(&cl->cl_lock);
 out:
 	nfs4_unlock_state();
 	return ret;
-- 
1.9.3


  parent reply	other threads:[~2014-07-21 15:03 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-21 15:02 [PATCH 00/40] nfsd: stateid and stateowner refcounting overhaul Jeff Layton
2014-07-21 15:02 ` [PATCH 01/40] nfsd4: use cl_lock to synchronize all stateid idr calls Jeff Layton
2014-07-27 13:21   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 02/40] nfsd: Add reference counting to the lock and open stateids Jeff Layton
2014-07-27 13:23   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 03/40] nfsd: Add a struct nfs4_file field to struct nfs4_stid Jeff Layton
2014-07-27 13:24   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 04/40] nfsd: Replace nfs4_ol_stateid->st_file with the st_stid.sc_file Jeff Layton
2014-07-27 13:25   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 05/40] nfsd: Ensure atomicity of stateid destruction and idr tree removal Jeff Layton
2014-07-27 13:28   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 06/40] nfsd: Cleanup the freeing of stateids Jeff Layton
2014-07-27 13:35   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 07/40] nfsd: do filp_close in sc_free callback for lock stateids Jeff Layton
2014-07-27 13:37   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 08/40] nfsd: Add locking to protect the state owner lists Jeff Layton
2014-07-27 13:42   ` Christoph Hellwig
2014-07-29 11:42     ` Jeff Layton
2014-07-21 15:02 ` [PATCH 09/40] nfsd: clean up races in lock stateid searching and creation Jeff Layton
2014-07-27 13:44   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 10/40] nfsd: Slight cleanup of find_stateid() Jeff Layton
2014-07-27 13:38   ` Christoph Hellwig
2014-07-21 15:02 ` Jeff Layton [this message]
2014-07-27 13:46   ` [PATCH 11/40] nfsd: ensure atomicity in nfsd4_free_stateid and nfsd4_validate_stateid Christoph Hellwig
2014-07-21 15:02 ` [PATCH 12/40] nfsd: Add reference counting to lock stateids Jeff Layton
2014-07-21 15:02 ` [PATCH 13/40] nfsd: nfsd4_locku() must reference the lock stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 14/40] nfsd: Ensure that nfs4_open_delegation() references the delegation stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 15/40] nfsd: nfsd4_process_open2() must reference " Jeff Layton
2014-07-21 15:02 ` [PATCH 16/40] nfsd: nfsd4_process_open2() must reference the open stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 17/40] nfsd: Prepare nfsd4_close() for open stateid referencing Jeff Layton
2014-07-21 15:02 ` [PATCH 18/40] nfsd: nfsd4_open_confirm() must reference the open stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 19/40] nfsd: Add reference counting to nfs4_preprocess_confirmed_seqid_op Jeff Layton
2014-07-21 15:02 ` [PATCH 20/40] nfsd: Migrate the stateid reference into nfs4_preprocess_seqid_op Jeff Layton
2014-07-21 15:02 ` [PATCH 21/40] nfsd: Migrate the stateid reference into nfs4_lookup_stateid() Jeff Layton
2014-07-21 15:02 ` [PATCH 22/40] nfsd: Migrate the stateid reference into nfs4_find_stateid_by_type() Jeff Layton
2014-07-21 15:02 ` [PATCH 23/40] nfsd: Add reference counting to state owners Jeff Layton
2014-07-27 13:58   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 24/40] nfsd: Add a mutex to protect the NFSv4.0 open owner replay cache Jeff Layton
2014-07-21 15:02 ` [PATCH 25/40] nfsd: Keep a reference to the open stateid for the NFSv4.0 " Jeff Layton
2014-07-27 13:59   ` Christoph Hellwig
2014-07-21 15:02 ` [PATCH 26/40] nfsd: clean up lockowner refcounting when finding them Jeff Layton
2014-07-21 15:02 ` [PATCH 27/40] nfsd: add an operation for unhashing a stateowner Jeff Layton
2014-07-21 15:02 ` [PATCH 28/40] nfsd: Make lock stateid take a reference to the lockowner Jeff Layton
2014-07-21 15:02 ` [PATCH 29/40] nfsd: clean up refcounting for lockowners Jeff Layton
2014-07-21 15:02 ` [PATCH 30/40] nfsd: make openstateids hold references to their openowners Jeff Layton
2014-07-21 15:02 ` [PATCH 31/40] nfsd: don't allow CLOSE to proceed until refcount on stateid drops Jeff Layton
2014-07-21 15:02 ` [PATCH 32/40] nfsd: Protect adding/removing open state owners using client_lock Jeff Layton
2014-07-21 15:02 ` [PATCH 33/40] nfsd: Protect adding/removing lock " Jeff Layton
2014-07-21 15:02 ` [PATCH 34/40] nfsd: Move the open owner hash table into struct nfs4_client Jeff Layton
2014-07-21 15:02 ` [PATCH 35/40] nfsd: clean up and reorganize release_lockowner Jeff Layton
2014-07-21 15:02 ` [PATCH 36/40] nfsd: add locking to stateowner release Jeff Layton
2014-07-21 15:02 ` [PATCH 37/40] nfsd: optimize destroy_lockowner cl_lock thrashing Jeff Layton
2014-07-21 15:02 ` [PATCH 38/40] nfsd: close potential race in nfsd4_free_stateid Jeff Layton
2014-07-21 15:02 ` [PATCH 39/40] nfsd: reduce cl_lock thrashing in release_openowner Jeff Layton
2014-07-21 15:02 ` [PATCH 40/40] nfsd: don't thrash the cl_lock while freeing an open stateid Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405954972-28904-12-git-send-email-jlayton@primarydata.com \
    --to=jlayton@primarydata.com \
    --cc=bfields@fieldses.org \
    --cc=hch@infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).