From: Trond Myklebust <trond.mykleb...@hammerspace.com>

[ Upstream commit 27a30cf64a5cbe2105e4ff9613246b32d584766a ]

The logic for checking in nfs41_check_open_stateid() whether the state
is supported by a delegation is inverted. In addition, it makes more
sense to perform that check before we check for expired locks.

Fixes: 8a64c4ef106d1 ("NFSv4.1: Even if the stateid is OK,...")
Signed-off-by: Trond Myklebust <trond.mykleb...@hammerspace.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
 fs/nfs/nfs4proc.c | 65 +++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 11a5aa46e64c3..b3fd75c8629a4 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1654,6 +1654,14 @@ static void nfs_state_set_open_stateid(struct nfs4_state 
*state,
        write_sequnlock(&state->seqlock);
 }
 
+static void nfs_state_clear_open_state_flags(struct nfs4_state *state)
+{
+       clear_bit(NFS_O_RDWR_STATE, &state->flags);
+       clear_bit(NFS_O_WRONLY_STATE, &state->flags);
+       clear_bit(NFS_O_RDONLY_STATE, &state->flags);
+       clear_bit(NFS_OPEN_STATE, &state->flags);
+}
+
 static void nfs_state_set_delegation(struct nfs4_state *state,
                const nfs4_stateid *deleg_stateid,
                fmode_t fmode)
@@ -2045,13 +2053,7 @@ static int nfs4_open_recover(struct nfs4_opendata 
*opendata, struct nfs4_state *
 {
        int ret;
 
-       /* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
-       clear_bit(NFS_O_RDWR_STATE, &state->flags);
-       clear_bit(NFS_O_WRONLY_STATE, &state->flags);
-       clear_bit(NFS_O_RDONLY_STATE, &state->flags);
        /* memory barrier prior to reading state->n_* */
-       clear_bit(NFS_DELEGATED_STATE, &state->flags);
-       clear_bit(NFS_OPEN_STATE, &state->flags);
        smp_rmb();
        ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
        if (ret != 0)
@@ -2127,6 +2129,8 @@ static int nfs4_open_reclaim(struct nfs4_state_owner *sp, 
struct nfs4_state *sta
        ctx = nfs4_state_find_open_context(state);
        if (IS_ERR(ctx))
                return -EAGAIN;
+       clear_bit(NFS_DELEGATED_STATE, &state->flags);
+       nfs_state_clear_open_state_flags(state);
        ret = nfs4_do_open_reclaim(ctx, state);
        put_nfs_open_context(ctx);
        return ret;
@@ -2669,6 +2673,7 @@ static int nfs40_open_expired(struct nfs4_state_owner 
*sp, struct nfs4_state *st
 {
        /* NFSv4.0 doesn't allow for delegation recovery on open expire */
        nfs40_clear_delegation_stateid(state);
+       nfs_state_clear_open_state_flags(state);
        return nfs4_open_expired(sp, state);
 }
 
@@ -2711,13 +2716,13 @@ static int nfs41_test_and_free_expired_stateid(struct 
nfs_server *server,
        return -NFS4ERR_EXPIRED;
 }
 
-static void nfs41_check_delegation_stateid(struct nfs4_state *state)
+static int nfs41_check_delegation_stateid(struct nfs4_state *state)
 {
        struct nfs_server *server = NFS_SERVER(state->inode);
        nfs4_stateid stateid;
        struct nfs_delegation *delegation;
        const struct cred *cred = NULL;
-       int status;
+       int status, ret = NFS_OK;
 
        /* Get the delegation credential for use by test/free_stateid */
        rcu_read_lock();
@@ -2725,20 +2730,15 @@ static void nfs41_check_delegation_stateid(struct 
nfs4_state *state)
        if (delegation == NULL) {
                rcu_read_unlock();
                nfs_state_clear_delegation(state);
-               return;
+               return NFS_OK;
        }
 
        nfs4_stateid_copy(&stateid, &delegation->stateid);
-       if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
-               rcu_read_unlock();
-               nfs_state_clear_delegation(state);
-               return;
-       }
 
        if (!test_and_clear_bit(NFS_DELEGATION_TEST_EXPIRED,
                                &delegation->flags)) {
                rcu_read_unlock();
-               return;
+               return NFS_OK;
        }
 
        if (delegation->cred)
@@ -2748,8 +2748,24 @@ static void nfs41_check_delegation_stateid(struct 
nfs4_state *state)
        trace_nfs4_test_delegation_stateid(state, NULL, status);
        if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
                nfs_finish_clear_delegation_stateid(state, &stateid);
+       else
+               ret = status;
 
        put_cred(cred);
+       return ret;
+}
+
+static void nfs41_delegation_recover_stateid(struct nfs4_state *state)
+{
+       nfs4_stateid tmp;
+
+       if (test_bit(NFS_DELEGATED_STATE, &state->flags) &&
+           nfs4_copy_delegation_stateid(state->inode, state->state,
+                               &tmp, NULL) &&
+           nfs4_stateid_match_other(&state->stateid, &tmp))
+               nfs_state_set_delegation(state, &tmp, state->state);
+       else
+               nfs_state_clear_delegation(state);
 }
 
 /**
@@ -2819,21 +2835,12 @@ static int nfs41_check_open_stateid(struct nfs4_state 
*state)
        const struct cred *cred = state->owner->so_cred;
        int status;
 
-       if (test_bit(NFS_OPEN_STATE, &state->flags) == 0) {
-               if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)  {
-                       if (nfs4_have_delegation(state->inode, state->state))
-                               return NFS_OK;
-                       return -NFS4ERR_OPENMODE;
-               }
+       if (test_bit(NFS_OPEN_STATE, &state->flags) == 0)
                return -NFS4ERR_BAD_STATEID;
-       }
        status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
        trace_nfs4_test_open_stateid(state, NULL, status);
        if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) {
-               clear_bit(NFS_O_RDONLY_STATE, &state->flags);
-               clear_bit(NFS_O_WRONLY_STATE, &state->flags);
-               clear_bit(NFS_O_RDWR_STATE, &state->flags);
-               clear_bit(NFS_OPEN_STATE, &state->flags);
+               nfs_state_clear_open_state_flags(state);
                stateid->type = NFS4_INVALID_STATEID_TYPE;
                return status;
        }
@@ -2846,7 +2853,11 @@ static int nfs41_open_expired(struct nfs4_state_owner 
*sp, struct nfs4_state *st
 {
        int status;
 
-       nfs41_check_delegation_stateid(state);
+       status = nfs41_check_delegation_stateid(state);
+       if (status != NFS_OK)
+               return status;
+       nfs41_delegation_recover_stateid(state);
+
        status = nfs41_check_expired_locks(state);
        if (status != NFS_OK)
                return status;
-- 
2.20.1

Reply via email to