NFS: take a delegation reference in nfs4_get_valid_delegation

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>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 8397c43358bd13cd964f30600f208d434a74be0f..57550020c8195f8eb31b55b7a0c83e911aad8108 100644 (file)
--- 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 a64c30efa1a3e55bfbb72681ee54f39161753b13..0e91ee9aaa1b61e4f5c398cef871bbabcb7f5ac7 100644 (file)
--- 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,
@@ -794,10 +798,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;
@@ -811,8 +816,7 @@ out_unlock:
        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);
 }
 
@@ -828,10 +832,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) {
@@ -847,8 +851,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);
 }
 
@@ -863,17 +867,17 @@ out:
 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
@@ -1116,24 +1120,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 fef1f4126e8f37b0464ab80b3e9cacdd2c543ad4..d1c5da3e66ea1591e15bd34673ce58229687fa33 100644 (file)
--- 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 935ec446e52e6014a0f4657a36297a2626a791fd..9d43016d9aa14427029c46cb12bcb21d9729b6d5 100644 (file)
--- 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)