[PATCH 17/24] NFS: take a delegation reference in nfs4_get_valid_delegation

[Christoph Hellwig <hch@lst.de>]

Currently most work on struct nfs_delegation happens directly under RCU
protection.  This is generally fine, despite that long RCU sections are
not good for performance.  But for operations later taking a reference
to the delegation to perform blocking work, refcount_inc is used, which
can be racy against dropping the last reference and thus lead to use
after frees in extremely rare cases.

Fix this by taking a reference in nfs4_get_valid_delegation using
refcount_inc_not_zero so that the callers have a stabilized reference
they can work with and can be moved outside the RCU critical section.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/callback_proc.c | 13 ++++++---
 fs/nfs/delegation.c    | 62 ++++++++++++++++++++++--------------------
 fs/nfs/delegation.h    |  1 +
 fs/nfs/nfs4proc.c      | 26 +++++++++---------
 4 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 8397c43358bd..57550020c819 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -51,12 +51,18 @@ __be32 nfs4_callback_getattr(void *argp, void *resp,
 				-ntohl(res->status));
 		goto out;
 	}
-	rcu_read_lock();
+
 	delegation = nfs4_get_valid_delegation(inode);
-	if (delegation == NULL || (delegation->type & FMODE_WRITE) == 0)
+	if (!delegation)
 		goto out_iput;
-	res->size = i_size_read(inode);
+	if ((delegation->type & FMODE_WRITE) == 0) {
+		nfs_put_delegation(delegation);
+		goto out_iput;
+	}
 	res->change_attr = delegation->change_attr;
+	nfs_put_delegation(delegation);
+
+	res->size = i_size_read(inode);
 	if (nfs_have_writebacks(inode))
 		res->change_attr++;
 	res->atime = inode_get_atime(inode);
@@ -71,7 +77,6 @@ __be32 nfs4_callback_getattr(void *argp, void *resp,
 			  FATTR4_WORD2_TIME_DELEG_MODIFY) & args->bitmap[2];
 	res->status = 0;
 out_iput:
-	rcu_read_unlock();
 	trace_nfs4_cb_getattr(cps->clp, &args->fh, inode, -ntohl(res->status));
 	nfs_iput_and_deactive(inode);
 out:
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index f7d5622c625a..811e84b559ee 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -66,7 +66,7 @@ static struct nfs_delegation *nfs_get_delegation(struct nfs_delegation *delegati
 	return delegation;
 }
 
-static void nfs_put_delegation(struct nfs_delegation *delegation)
+void nfs_put_delegation(struct nfs_delegation *delegation)
 {
 	if (refcount_dec_and_test(&delegation->refcount))
 		__nfs_free_delegation(delegation);
@@ -104,10 +104,14 @@ struct nfs_delegation *nfs4_get_valid_delegation(const struct inode *inode)
 {
 	struct nfs_delegation *delegation;
 
+	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
-	if (nfs4_is_valid_delegation(delegation, 0))
-		return delegation;
-	return NULL;
+	if (!nfs4_is_valid_delegation(delegation, 0) ||
+	    !refcount_inc_not_zero(&delegation->refcount))
+		delegation = NULL;
+	rcu_read_unlock();
+
+	return delegation;
 }
 
 static int nfs4_do_check_delegation(struct inode *inode, fmode_t type,
@@ -789,10 +793,11 @@ void nfs4_inode_set_return_delegation_on_close(struct inode *inode)
 
 	if (!inode)
 		return;
-	rcu_read_lock();
+
 	delegation = nfs4_get_valid_delegation(inode);
 	if (!delegation)
-		goto out;
+		return;
+
 	spin_lock(&delegation->lock);
 	if (!delegation->inode)
 		goto out_unlock;
@@ -806,8 +811,7 @@ void nfs4_inode_set_return_delegation_on_close(struct inode *inode)
 	spin_unlock(&delegation->lock);
 	if (ret)
 		nfs_clear_verifier_delegated(inode);
-out:
-	rcu_read_unlock();
+	nfs_put_delegation(delegation);
 	nfs_end_delegation_return(inode, ret, 0);
 }
 
@@ -823,10 +827,10 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
 	struct nfs_delegation *delegation;
 	struct nfs_delegation *ret = NULL;
 
-	rcu_read_lock();
 	delegation = nfs4_get_valid_delegation(inode);
 	if (!delegation)
-		goto out;
+		return;
+
 	if (test_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags) ||
 	    atomic_long_read(&NFS_SERVER(inode)->nr_active_delegations) >=
 	    nfs_delegation_watermark) {
@@ -842,8 +846,8 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
 		if (ret)
 			nfs_clear_verifier_delegated(inode);
 	}
-out:
-	rcu_read_unlock();
+
+	nfs_put_delegation(delegation);
 	nfs_end_delegation_return(inode, ret, 0);
 }
 
@@ -858,17 +862,17 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
 int nfs4_inode_make_writeable(struct inode *inode)
 {
 	struct nfs_delegation *delegation;
+	int error = 0;
 
-	rcu_read_lock();
 	delegation = nfs4_get_valid_delegation(inode);
-	if (delegation == NULL ||
-	    (nfs4_has_session(NFS_SERVER(inode)->nfs_client) &&
-	     (delegation->type & FMODE_WRITE))) {
-		rcu_read_unlock();
+	if (!delegation)
 		return 0;
-	}
-	rcu_read_unlock();
-	return nfs4_inode_return_delegation(inode);
+
+	if (!nfs4_has_session(NFS_SERVER(inode)->nfs_client) ||
+	    !(delegation->type & FMODE_WRITE))
+		error = nfs4_inode_return_delegation(inode);
+	nfs_put_delegation(delegation);
+	return error;
 }
 
 static void
@@ -1111,24 +1115,24 @@ int nfs_async_inode_return_delegation(struct inode *inode,
 	struct nfs_client *clp = server->nfs_client;
 	struct nfs_delegation *delegation;
 
-	rcu_read_lock();
 	delegation = nfs4_get_valid_delegation(inode);
-	if (delegation == NULL)
-		goto out_enoent;
+	if (!delegation)
+		return -ENOENT;
+
 	if (stateid != NULL &&
-	    !clp->cl_mvops->match_stateid(&delegation->stateid, stateid))
-		goto out_enoent;
+	    !clp->cl_mvops->match_stateid(&delegation->stateid, stateid)) {
+		nfs_put_delegation(delegation);
+		return -ENOENT;
+	}
+
 	nfs_mark_return_delegation(server, delegation);
-	rcu_read_unlock();
+	nfs_put_delegation(delegation);
 
 	/* If there are any application leases or delegations, recall them */
 	break_lease(inode, O_WRONLY | O_RDWR | O_NONBLOCK);
 
 	nfs_delegation_run_state_manager(clp);
 	return 0;
-out_enoent:
-	rcu_read_unlock();
-	return -ENOENT;
 }
 
 static struct inode *
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index fef1f4126e8f..d1c5da3e66ea 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -80,6 +80,7 @@ bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_state
 bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode);
 
 struct nfs_delegation *nfs4_get_valid_delegation(const struct inode *inode);
+void nfs_put_delegation(struct nfs_delegation *delegation);
 void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
 int nfs4_have_delegation(struct inode *inode, fmode_t type, int flags);
 int nfs4_check_delegation(struct inode *inode, fmode_t type);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 03b1f98eb989..2b28f56d8544 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1615,10 +1615,11 @@ static bool can_open_delegated(const struct inode *inode, fmode_t fmode,
 	struct nfs_delegation *delegation;
 	bool ret = false;
 
-	rcu_read_lock();
 	delegation = nfs4_get_valid_delegation(inode);
-	if (!delegation || (delegation->type & fmode) != fmode)
-		goto out_unlock;
+	if (!delegation)
+		return false;
+	if ((delegation->type & fmode) != fmode)
+		goto out_put_delegation;
 
 	switch (claim) {
 	case NFS4_OPEN_CLAIM_PREVIOUS:
@@ -1637,8 +1638,8 @@ static bool can_open_delegated(const struct inode *inode, fmode_t fmode,
 		break;
 	}
 
-out_unlock:
-	rcu_read_unlock();
+out_put_delegation:
+	nfs_put_delegation(delegation);
 	return ret;
 }
 
@@ -1913,10 +1914,11 @@ int update_open_stateid(struct nfs4_state *state,
 
 	fmode &= (FMODE_READ|FMODE_WRITE);
 
-	rcu_read_lock();
 	spin_lock(&state->owner->so_lock);
 	if (open_stateid != NULL) {
+		rcu_read_lock();
 		nfs_state_set_open_stateid(state, open_stateid, fmode, &freeme);
+		rcu_read_unlock();
 		ret = 1;
 	}
 
@@ -1940,11 +1942,11 @@ int update_open_stateid(struct nfs4_state *state,
 	ret = 1;
 no_delegation_unlock:
 	spin_unlock(&deleg_cur->lock);
+	nfs_put_delegation(deleg_cur);
 no_delegation:
 	if (ret)
 		update_open_stateflags(state, fmode);
 	spin_unlock(&state->owner->so_lock);
-	rcu_read_unlock();
 
 	if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags))
 		nfs4_schedule_state_manager(clp);
@@ -1978,14 +1980,12 @@ static void nfs4_return_incompatible_delegation(struct inode *inode, fmode_t fmo
 	struct nfs_delegation *delegation;
 
 	fmode &= FMODE_READ|FMODE_WRITE;
-	rcu_read_lock();
 	delegation = nfs4_get_valid_delegation(inode);
-	if (delegation == NULL || (delegation->type & fmode) == fmode) {
-		rcu_read_unlock();
+	if (!delegation)
 		return;
-	}
-	rcu_read_unlock();
-	nfs4_inode_return_delegation(inode);
+	if ((delegation->type & fmode) != fmode)
+		nfs4_inode_return_delegation(inode);
+	nfs_put_delegation(delegation);
 }
 
 static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata)
-- 
2.47.3