Skip to contentSkip to conversation

Patch "nfsd: ensure that delegation stateid hash references are only put once" has been added to the 4.2-stable tree

PATCH [email protected] 1 participant 1 message Sep 26, 2015

Subsystems (2)
Stable branch NFS server
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

nfsd: ensure that delegation stateid hash references are only put once

to the 4.2-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
nfsd-ensure-that-delegation-stateid-hash-references-are-only-put-once.patch
and it can be found in the queue-4.2 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 3fcbbd244ed1d20dc0eb7d48d729503992fa9b7d Mon Sep 17 00:00:00 2001

From: Jeff Layton <[email protected]>
Date: Mon, 24 Aug 2015 12:41:48 -0400
Subject: nfsd: ensure that delegation stateid hash references are only put once

From: Jeff Layton <[email protected]>

commit 3fcbbd244ed1d20dc0eb7d48d729503992fa9b7d upstream.

It's possible that a DELEGRETURN could race with (e.g.) client expiry, in which case we could end up putting the delegation hash reference more than once.

Have unhash_delegation_locked return a bool that indicates whether it was already unhashed. In the case of destroy_delegation we only conditionally put the hash reference if that returns true.

The other callers of unhash_delegation_locked call it while walking list_heads that shouldn't yet be detached. If we find that it doesn't return true in those cases, then throw a WARN_ON as that indicates that we have a partially hashed delegation, and that something is likely very wrong.

Tested-by Andrew W Elble <[email protected]>
Tested-by Anna Schumaker <[email protected]>
Signed-off-by Jeff Layton <[email protected]>
Signed-off-by J. Bruce Fields <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 fs/nfsd/nfs4state.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)
fs/nfsd/nfs4state.c +17 -9
@@ -777,13 +777,16 @@ hash_delegation_locked(struct nfs4_deleg
 	list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
 }
 
-static void
+static bool
 unhash_delegation_locked(struct nfs4_delegation *dp)
 {
 	struct nfs4_file *fp = dp->dl_stid.sc_file;
 
 	lockdep_assert_held(&state_lock);
 
+	if (list_empty(&dp->dl_perfile))
+		return false;
+
 	dp->dl_stid.sc_type = NFS4_CLOSED_DELEG_STID;
 	/* Ensure that deleg break won't try to requeue it */
 	++dp->dl_time;

@@ -792,16 +795,21 @@ unhash_delegation_locked(struct nfs4_del
 	list_del_init(&dp->dl_recall_lru);
 	list_del_init(&dp->dl_perfile);
 	spin_unlock(&fp->fi_lock);
+	return true;
 }
 
 static void destroy_delegation(struct nfs4_delegation *dp)
 {
+	bool unhashed;
+
 	spin_lock(&state_lock);
-	unhash_delegation_locked(dp);
+	unhashed = unhash_delegation_locked(dp);
 	spin_unlock(&state_lock);
-	put_clnt_odstate(dp->dl_clnt_odstate);
-	nfs4_put_deleg_lease(dp->dl_stid.sc_file);
-	nfs4_put_stid(&dp->dl_stid);
+	if (unhashed) {
+		put_clnt_odstate(dp->dl_clnt_odstate);
+		nfs4_put_deleg_lease(dp->dl_stid.sc_file);
+		nfs4_put_stid(&dp->dl_stid);
+	}
 }
 
 static void revoke_delegation(struct nfs4_delegation *dp)

@@ -1725,7 +1733,7 @@ __destroy_client(struct nfs4_client *clp
 	spin_lock(&state_lock);
 	while (!list_empty(&clp->cl_delegations)) {
 		dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
-		unhash_delegation_locked(dp);
+		WARN_ON(!unhash_delegation_locked(dp));
 		list_add(&dp->dl_recall_lru, &reaplist);
 	}
 	spin_unlock(&state_lock);

@@ -4356,7 +4364,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 			new_timeo = min(new_timeo, t);
 			break;
 		}
-		unhash_delegation_locked(dp);
+		WARN_ON(!unhash_delegation_locked(dp));
 		list_add(&dp->dl_recall_lru, &reaplist);
 	}
 	spin_unlock(&state_lock);

@@ -6319,7 +6327,7 @@ static u64 nfsd_find_all_delegations(str
 				continue;
 
 			atomic_inc(&clp->cl_refcount);
-			unhash_delegation_locked(dp);
+			WARN_ON(!unhash_delegation_locked(dp));
 			list_add(&dp->dl_recall_lru, victims);
 		}
 		++count;

@@ -6649,7 +6657,7 @@ nfs4_state_shutdown_net(struct net *net)
 	spin_lock(&state_lock);
 	list_for_each_safe(pos, next, &nn->del_recall_lru) {
 		dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
-		unhash_delegation_locked(dp);
+		WARN_ON(!unhash_delegation_locked(dp));
 		list_add(&dp->dl_recall_lru, &reaplist);
 	}
 	spin_unlock(&state_lock);

Patches currently in stable-queue which might be from [email protected] are

queue-4.2/nfsd-ensure-that-delegation-stateid-hash-references-are-only-put-once.patch queue-4.2/nfsd-ensure-that-the-ol-stateid-hash-reference-is-only-put-once.patch