linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@gmail.com>
To: linux-nfs@vger.kernel.org
Subject: [PATCH 14/14] NFSv4: Fix races between open and delegreturn
Date: Wed, 23 Oct 2019 19:56:00 -0400	[thread overview]
Message-ID: <20191023235600.10880-15-trond.myklebust@hammerspace.com> (raw)
In-Reply-To: <20191023235600.10880-14-trond.myklebust@hammerspace.com>

If the server returns the same delegation in an open that we just used
in a delegreturn, we need to ensure we don't apply that stateid if
the delegreturn has freed it on the server.
To do so, we ensure that we do not free the storage for the delegation
until either it is replaced by a new one, or we throw the inode out of
cache.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/delegation.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 00c6c343dced..db7cf480c108 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -218,7 +218,6 @@ static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *
 				delegation->cred,
 				&delegation->stateid,
 				issync);
-	nfs_free_delegation(delegation);
 	return res;
 }
 
@@ -291,7 +290,6 @@ nfs_detach_delegation_locked(struct nfs_inode *nfsi,
 		spin_unlock(&delegation->lock);
 		return NULL;
 	}
-	set_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
 	list_del_rcu(&delegation->super_list);
 	delegation->inode = NULL;
 	rcu_assign_pointer(nfsi->delegation, NULL);
@@ -318,10 +316,12 @@ nfs_inode_detach_delegation(struct inode *inode)
 	struct nfs_server *server = NFS_SERVER(inode);
 	struct nfs_delegation *delegation;
 
-	delegation = nfs_start_delegation_return(nfsi);
-	if (delegation == NULL)
-		return NULL;
-	return nfs_detach_delegation(nfsi, delegation, server);
+	rcu_read_lock();
+	delegation = rcu_dereference(nfsi->delegation);
+	if (delegation != NULL)
+		delegation = nfs_detach_delegation(nfsi, delegation, server);
+	rcu_read_unlock();
+	return delegation;
 }
 
 static void
@@ -420,8 +420,10 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
 	spin_unlock(&clp->cl_lock);
 	if (delegation != NULL)
 		nfs_free_delegation(delegation);
-	if (freeme != NULL)
+	if (freeme != NULL) {
 		nfs_do_return_delegation(inode, freeme, 0);
+		nfs_free_delegation(freeme);
+	}
 	return status;
 }
 
@@ -431,7 +433,6 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
 static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation *delegation, int issync)
 {
 	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
-	struct nfs_inode *nfsi = NFS_I(inode);
 	int err = 0;
 
 	if (delegation == NULL)
@@ -453,8 +454,6 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
 		nfs_abort_delegation_return(delegation, clp);
 		goto out;
 	}
-	if (!nfs_detach_delegation(nfsi, delegation, NFS_SERVER(inode)))
-		goto out;
 
 	err = nfs_do_return_delegation(inode, delegation, issync);
 out:
@@ -597,6 +596,7 @@ void nfs_inode_evict_delegation(struct inode *inode)
 	if (delegation != NULL) {
 		set_bit(NFS_DELEGATION_INODE_FREEING, &delegation->flags);
 		nfs_do_return_delegation(inode, delegation, 1);
+		nfs_free_delegation(delegation);
 	}
 }
 
@@ -744,10 +744,9 @@ static void nfs_mark_delegation_revoked(struct nfs_server *server,
 {
 	set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
 	delegation->stateid.type = NFS4_INVALID_STATEID_TYPE;
-	nfs_mark_return_delegation(server, delegation);
 }
 
-static bool nfs_revoke_delegation(struct inode *inode,
+static void nfs_revoke_delegation(struct inode *inode,
 		const nfs4_stateid *stateid)
 {
 	struct nfs_delegation *delegation;
@@ -780,19 +779,12 @@ static bool nfs_revoke_delegation(struct inode *inode,
 	rcu_read_unlock();
 	if (ret)
 		nfs_inode_find_state_and_recover(inode, stateid);
-	return ret;
 }
 
 void nfs_remove_bad_delegation(struct inode *inode,
 		const nfs4_stateid *stateid)
 {
-	struct nfs_delegation *delegation;
-
-	if (!nfs_revoke_delegation(inode, stateid))
-		return;
-	delegation = nfs_inode_detach_delegation(inode);
-	if (delegation)
-		nfs_free_delegation(delegation);
+	nfs_revoke_delegation(inode, stateid);
 }
 EXPORT_SYMBOL_GPL(nfs_remove_bad_delegation);
 
@@ -816,7 +808,7 @@ void nfs_delegation_mark_returned(struct inode *inode,
 	    nfs4_stateid_is_newer(&delegation->stateid, stateid))
 		goto out_clear_returning;
 
-	set_bit(NFS_DELEGATION_REVOKED, &delegation->flags);
+	nfs_mark_delegation_revoked(NFS_SERVER(inode), delegation);
 
 out_clear_returning:
 	clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
-- 
2.21.0


  reply	other threads:[~2019-10-23 23:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-23 23:55 [PATCH 00/14] Delegation bugfixes Trond Myklebust
2019-10-23 23:55 ` [PATCH 01/14] NFSv4: Don't allow a cached open with a revoked delegation Trond Myklebust
2019-10-23 23:55   ` [PATCH 02/14] NFSv4: Fix delegation handling in update_open_stateid() Trond Myklebust
2019-10-23 23:55     ` [PATCH 03/14] NFSv4: nfs4_callback_getattr() should ignore revoked delegations Trond Myklebust
2019-10-23 23:55       ` [PATCH 04/14] NFSv4: Delegation recalls should not find " Trond Myklebust
2019-10-23 23:55         ` [PATCH 05/14] NFSv4: fail nfs4_refresh_delegation_stateid() when the delegation was revoked Trond Myklebust
2019-10-23 23:55           ` [PATCH 06/14] NFS: Rename nfs_inode_return_delegation_noreclaim() Trond Myklebust
2019-10-23 23:55             ` [PATCH 07/14] NFSv4: Don't remove the delegation from the super_list more than once Trond Myklebust
2019-10-23 23:55               ` [PATCH 08/14] NFSv4: Hold the delegation spinlock when updating the seqid Trond Myklebust
2019-10-23 23:55                 ` [PATCH 09/14] NFSv4: Clear the NFS_DELEGATION_REVOKED flag in nfs_update_inplace_delegation() Trond Myklebust
2019-10-23 23:55                   ` [PATCH 10/14] NFSv4: Update the stateid seqid in nfs_revoke_delegation() Trond Myklebust
2019-10-23 23:55                     ` [PATCH 11/14] NFSv4: Revoke the delegation on success in nfs4_delegreturn_done() Trond Myklebust
2019-10-23 23:55                       ` [PATCH 12/14] NFSv4: Ignore requests to return the delegation if it was revoked Trond Myklebust
2019-10-23 23:55                         ` [PATCH 13/14] NFSv4: Don't reclaim delegations that have been returned or revoked Trond Myklebust
2019-10-23 23:56                           ` Trond Myklebust [this message]
2019-10-31 15:27 ` [PATCH 00/14] Delegation bugfixes Olga Kornievskaia
2019-10-31 15:49   ` Olga Kornievskaia
2019-10-31 16:15     ` Olga Kornievskaia
2019-10-31 22:54       ` Trond Myklebust

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=20191023235600.10880-15-trond.myklebust@hammerspace.com \
    --to=trondmy@gmail.com \
    --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).