diff mbox series

[2/8] NFSv4: Fix delegation state recovery

Message ID 20190803145826.15504-2-trond.myklebust@hammerspace.com (mailing list archive)
State New, archived
Headers show
Series [1/8] NFSv4: Fix a credential refcount leak in nfs41_check_delegation_stateid | expand

Commit Message

Trond Myklebust Aug. 3, 2019, 2:58 p.m. UTC 
Once we clear the NFS_DELEGATED_STATE flag, we're telling
nfs_delegation_claim_opens() that we're done recovering all open state
for that stateid, so we really need to ensure that we test for all
open modes that are currently cached and recover them before exiting
nfs4_open_delegation_recall().

Fixes: 24311f884189d ("NFSv4: Recovery of recalled read delegations...")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: stable@vger.kernel.org # v4.3+
---
 fs/nfs/delegation.c |  2 +-
 fs/nfs/delegation.h |  2 +-
 fs/nfs/nfs4proc.c   | 25 ++++++++++++-------------
 3 files changed, 14 insertions(+), 15 deletions(-)

Comments

Sasha Levin Aug. 3, 2019, 5:09 p.m. UTC | #1 
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 24311f884189 NFSv4: Recovery of recalled read delegations is broken.

The bot has tested the following trees: v5.2.5, v4.19.63, v4.14.135, v4.9.186, v4.4.186.

v5.2.5: Build OK!
v4.19.63: Failed to apply! Possible dependencies:
    07d02a67b7fa ("SUNRPC: Simplify lookup code")
    79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
    8276c902bbe9 ("SUNRPC: remove uid and gid from struct auth_cred")
    95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
    97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.14.135: Failed to apply! Possible dependencies:
    07d02a67b7fa ("SUNRPC: Simplify lookup code")
    12f275cdd163 ("NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.")
    1eb5d98f16f6 ("nfs: convert to new i_version API")
    35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
    79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
    95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
    97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.9.186: Failed to apply! Possible dependencies:
    1eb5d98f16f6 ("nfs: convert to new i_version API")
    35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
    39bc88e5e38e ("arm64: Disable TTBR0_EL1 during normal kernel execution")
    7c0f6ba682b9 ("Replace <asm/uaccess.h> with <linux/uaccess.h> globally")
    9cf09d68b89a ("arm64: xen: Enable user access before a privcmd hvc call")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    bd38967d406f ("arm64: Factor out PAN enabling/disabling into separate uaccess_* macros")

v4.4.186: Failed to apply! Possible dependencies:
    0654cc726fc6 ("NFSv4.1/pNFS: Add a helper to mark the layout as returned")
    10335556c9e6 ("NFSv4.1/pNFS: pnfs_error_mark_layout_for_return() must always return layout")
    13c13a6ad71f ("pNFS: Fix missing layoutreturn calls")
    2454dfea0aef ("NFSv4.x/pnfs: Fix a race between layoutget and pnfs_destroy_layout")
    3982a6a2d0e6 ("pnfs: keep track of the return sequence number in pnfs_layout_hdr")
    4b0934baf931 ("NFSv4.1/pNFS: Fix a race in initiate_file_draining()")
    506c0d68269e ("NFSv4.1/pNFS: Cleanup constify struct pnfs_layout_range arguments")
    50f563ef5d41 ("NFSv4.1/pNFS: Use nfs4_stateid_copy for copying stateids")
    5c97f5de2c7c ("NFSv4.1/pNFS: pnfs_mark_matching_lsegs_return() should set the iomode")
    68d264cf02b0 ("NFS42: handle layoutstats stateid error")
    6d597e175012 ("pnfs: only tear down lsegs that precede seqid in LAYOUTRETURN args")
    71b39854a500 ("NFSv4.1/pNFS: Cleanup pnfs_mark_matching_lsegs_invalid()")
    9fd4b9fc7695 ("NFSv4.x/pnfs: Fix a race between layoutget and bulk recalls")
    a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
    b20135d0b243 ("NFSv4.1/pNFS: Don't queue up a new commit if the layout segment is invalid")
    b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
    e036f46453f2 ("NFS: pnfs_mark_matching_lsegs_return() should match the layout sequence id")
    e0d9243048fd ("NFSv4.1/pNFS: Don't return NFS4ERR_DELAY unnecessarily in CB_LAYOUTRECALL")
    ed429d6b934d ("NFSv4.1/pNFS: Don't pass stateids by value to pnfs_send_layoutreturn()")
    fc7ff36747b9 ("pNFS: If we have to delay the layout callback, mark the layout for return")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks,
Sasha
diff mbox series

Patch

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 0ff3facf81da..0af854cce8ff 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -153,7 +153,7 @@  static int nfs_delegation_claim_opens(struct inode *inode,
 		/* Block nfs4_proc_unlck */
 		mutex_lock(&sp->so_delegreturn_mutex);
 		seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
-		err = nfs4_open_delegation_recall(ctx, state, stateid, type);
+		err = nfs4_open_delegation_recall(ctx, state, stateid);
 		if (!err)
 			err = nfs_delegation_claim_locks(state, stateid);
 		if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 5799777df5ec..9eb87ae4c982 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -63,7 +63,7 @@  void nfs_reap_expired_delegations(struct nfs_client *clp);
 
 /* NFSv4 delegation-related procedures */
 int nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred, const nfs4_stateid *stateid, int issync);
-int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
+int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid);
 int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
 bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_stateid *dst, const struct cred **cred);
 bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a6d73609b163..21e3c159bc69 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2177,12 +2177,10 @@  static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 		case -NFS4ERR_BAD_HIGH_SLOT:
 		case -NFS4ERR_CONN_NOT_BOUND_TO_SESSION:
 		case -NFS4ERR_DEADSESSION:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			nfs4_schedule_session_recovery(server->nfs_client->cl_session, err);
 			return -EAGAIN;
 		case -NFS4ERR_STALE_CLIENTID:
 		case -NFS4ERR_STALE_STATEID:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			/* Don't recall a delegation if it was lost */
 			nfs4_schedule_lease_recovery(server->nfs_client);
 			return -EAGAIN;
@@ -2203,7 +2201,6 @@  static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 			return -EAGAIN;
 		case -NFS4ERR_DELAY:
 		case -NFS4ERR_GRACE:
-			set_bit(NFS_DELEGATED_STATE, &state->flags);
 			ssleep(1);
 			return -EAGAIN;
 		case -ENOMEM:
@@ -2219,8 +2216,7 @@  static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 }
 
 int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
-		struct nfs4_state *state, const nfs4_stateid *stateid,
-		fmode_t type)
+		struct nfs4_state *state, const nfs4_stateid *stateid)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
 	struct nfs4_opendata *opendata;
@@ -2231,20 +2227,23 @@  int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
 	if (IS_ERR(opendata))
 		return PTR_ERR(opendata);
 	nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
-	nfs_state_clear_delegation(state);
-	switch (type & (FMODE_READ|FMODE_WRITE)) {
-	case FMODE_READ|FMODE_WRITE:
-	case FMODE_WRITE:
+	if (!test_bit(NFS_O_RDWR_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
 		if (err)
-			break;
+			goto out;
+	}
+	if (!test_bit(NFS_O_WRONLY_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_WRITE);
 		if (err)
-			break;
-		/* Fall through */
-	case FMODE_READ:
+			goto out;
+	}
+	if (!test_bit(NFS_O_RDONLY_STATE, &state->flags)) {
 		err = nfs4_open_recover_helper(opendata, FMODE_READ);
+		if (err)
+			goto out;
 	}
+	nfs_state_clear_delegation(state);
+out:
 	nfs4_opendata_put(opendata);
 	return nfs4_handle_delegation_recall_error(server, state, stateid, NULL, err);
 }