[PATCH v5 01/11] nfsd: rename ALLOWED_WITHOUT_FH to ALLOWED_WITHOUT_LOCAL_FH and revise use

[NeilBrown <neilb@ownmail.net>]

From: NeilBrown <neil@brown.name>

nfsdv4 ops which do not have ALLOWED_WITHOUT_FH can assume that a PUTFH
has been called and may assume that current_fh->fh_dentry is non-NULL.
nfsd4_setattr(), for example, assumes fh_dentry != NULL.

However the possibility of foreign filehandles (needed for v4.2 COPY)
means that there maybe a filehandle present while fh_dentry is NULL.

Sending a COMPOUND containing:
   SEQUENCE
   PUTFH - foreign filehandle
   SETATTR - new mode
   SAVEFH
   COPY - with non-empty server list

to an NFS server with inter-server copy enabled will cause a NULL
pointer dereference when nfsd4_setattr() calls fh_want_write().

Most NFSv4 ops actually want a "local" filehandle, not just any
filehandle.  So this patch renames ALLOWED_WITHOUT_FH to
ALLOWED_WITHOUT_LOCAL_FH and sets it on those which don't require a local
filehandle.  That is all that don't require any filehandle together with
SAVEFH, which is the only OP which needs to handle a foreign current_fh.
(COPY must handle a foreign save_fh, but all ops which access save_fh
already do any required validity tests themselves).

nfsd4_savefh() is changed to validate the filehandle itself as the
caller no longer validates it.

nfsd4_proc_compound no longer allows ops without
ALLOWED_WITHOUT_LOCAL_FH to be called with a foreign fh - current_fh
must be local and ->fh_dentry must be non-NULL.  This protects
nfsd4_setattr() and any others that might use ->fh_dentry without
checking.

The
       current_fh->fh_export &&
test is removed from an "else if" because that condition is now only
tested when current_fh->fh_dentry is not NULL, and in that case
current_fh->fh_export is also guaranteed to not be NULL.

Fixes: b9e8638e3d9e ("NFSD: allow inter server COPY to have a STALE source server fh")
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/nfsd/nfs4proc.c | 58 ++++++++++++++++++++++++++--------------------
 fs/nfsd/xdr4.h     |  2 +-
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index dcad50846a97..e5871e861dce 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -729,6 +729,15 @@ static __be32
 nfsd4_savefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	     union nfsd4_op_u *u)
 {
+	/*
+	 * SAVEFH is "ALLOWED_WITHOUT_LOCAL_FH" in that current_fh.fh_dentry
+	 * is not required, but fh_handle *is*.  Thus a foreign fh
+	 * can be saved as needed for inter-server COPY.
+	 */
+	if (!current_fh->fh_dentry &&
+	    !HAS_FH_FLAG(current_fh, NFSD4_FH_FOREIGN))
+		return nfserr_nofilehandle;
+
 	fh_dup2(&cstate->save_fh, &cstate->current_fh);
 	if (HAS_CSTATE_FLAG(cstate, CURRENT_STATE_ID_FLAG)) {
 		memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
@@ -2919,14 +2928,12 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 				op->status = nfsd4_open_omfg(rqstp, cstate, op);
 			goto encode_op;
 		}
-		if (!current_fh->fh_dentry &&
-				!HAS_FH_FLAG(current_fh, NFSD4_FH_FOREIGN)) {
-			if (!(op->opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
+		if (!current_fh->fh_dentry) {
+			if (!(op->opdesc->op_flags & ALLOWED_WITHOUT_LOCAL_FH)) {
 				op->status = nfserr_nofilehandle;
 				goto encode_op;
 			}
-		} else if (current_fh->fh_export &&
-			   current_fh->fh_export->ex_fslocs.migrated &&
+		} else if (current_fh->fh_export->ex_fslocs.migrated &&
 			  !(op->opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
 			op->status = nfserr_moved;
 			goto encode_op;
@@ -3507,21 +3514,21 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_PUTFH] = {
 		.op_func = nfsd4_putfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
 		.op_name = "OP_PUTFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_PUTPUBFH] = {
 		.op_func = nfsd4_putrootfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
 		.op_name = "OP_PUTPUBFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_PUTROOTFH] = {
 		.op_func = nfsd4_putrootfh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
 		.op_name = "OP_PUTROOTFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
@@ -3557,7 +3564,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_RENEW] = {
 		.op_func = nfsd4_renew,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_RENEW",
 		.op_rsize_bop = nfsd4_only_status_rsize,
@@ -3565,14 +3572,15 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_RESTOREFH] = {
 		.op_func = nfsd4_restorefh,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_RESTOREFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_SAVEFH] = {
 		.op_func = nfsd4_savefh,
-		.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH
+				| OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_SAVEFH",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
@@ -3593,7 +3601,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SETCLIENTID] = {
 		.op_func = nfsd4_setclientid,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_MODIFIES_SOMETHING | OP_CACHEME
 				| OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_name = "OP_SETCLIENTID",
@@ -3601,7 +3609,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SETCLIENTID_CONFIRM] = {
 		.op_func = nfsd4_setclientid_confirm,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_MODIFIES_SOMETHING | OP_CACHEME,
 		.op_name = "OP_SETCLIENTID_CONFIRM",
 		.op_rsize_bop = nfsd4_only_status_rsize,
@@ -3620,7 +3628,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_RELEASE_LOCKOWNER] = {
 		.op_func = nfsd4_release_lockowner,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_ON_ABSENT_FS
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_RELEASE_LOCKOWNER",
 		.op_rsize_bop = nfsd4_only_status_rsize,
@@ -3630,54 +3638,54 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	[OP_EXCHANGE_ID] = {
 		.op_func = nfsd4_exchange_id,
 		.op_release = nfsd4_exchange_id_release,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_EXCHANGE_ID",
 		.op_rsize_bop = nfsd4_exchange_id_rsize,
 	},
 	[OP_BACKCHANNEL_CTL] = {
 		.op_func = nfsd4_backchannel_ctl,
-		.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_BACKCHANNEL_CTL",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_BIND_CONN_TO_SESSION] = {
 		.op_func = nfsd4_bind_conn_to_session,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_BIND_CONN_TO_SESSION",
 		.op_rsize_bop = nfsd4_bind_conn_to_session_rsize,
 	},
 	[OP_CREATE_SESSION] = {
 		.op_func = nfsd4_create_session,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_CREATE_SESSION",
 		.op_rsize_bop = nfsd4_create_session_rsize,
 	},
 	[OP_DESTROY_SESSION] = {
 		.op_func = nfsd4_destroy_session,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_DESTROY_SESSION",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_SEQUENCE] = {
 		.op_func = nfsd4_sequence,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP,
 		.op_name = "OP_SEQUENCE",
 		.op_rsize_bop = nfsd4_sequence_rsize,
 	},
 	[OP_DESTROY_CLIENTID] = {
 		.op_func = nfsd4_destroy_clientid,
-		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | ALLOWED_AS_FIRST_OP
 				| OP_MODIFIES_SOMETHING,
 		.op_name = "OP_DESTROY_CLIENTID",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
 	[OP_RECLAIM_COMPLETE] = {
 		.op_func = nfsd4_reclaim_complete,
-		.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_RECLAIM_COMPLETE",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
@@ -3690,13 +3698,13 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_TEST_STATEID] = {
 		.op_func = nfsd4_test_stateid,
-		.op_flags = ALLOWED_WITHOUT_FH,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH,
 		.op_name = "OP_TEST_STATEID",
 		.op_rsize_bop = nfsd4_test_stateid_rsize,
 	},
 	[OP_FREE_STATEID] = {
 		.op_func = nfsd4_free_stateid,
-		.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH | OP_MODIFIES_SOMETHING,
 		.op_name = "OP_FREE_STATEID",
 		.op_get_currentstateid = nfsd4_get_freestateid,
 		.op_rsize_bop = nfsd4_only_status_rsize,
@@ -3711,7 +3719,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	[OP_GETDEVICEINFO] = {
 		.op_func = nfsd4_getdeviceinfo,
 		.op_release = nfsd4_getdeviceinfo_release,
-		.op_flags = ALLOWED_WITHOUT_FH,
+		.op_flags = ALLOWED_WITHOUT_LOCAL_FH,
 		.op_name = "OP_GETDEVICEINFO",
 		.op_rsize_bop = nfsd4_getdeviceinfo_rsize,
 	},
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index ae75846b3cd7..1f0967236cc2 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -1006,7 +1006,7 @@ extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
 extern void nfsd4_bump_seqid(struct nfsd4_compound_state *, __be32 nfserr);
 
 enum nfsd4_op_flags {
-	ALLOWED_WITHOUT_FH = 1 << 0,    /* No current filehandle required */
+	ALLOWED_WITHOUT_LOCAL_FH = 1 << 0,    /* No current filehandle fh_dentry required */
 	ALLOWED_ON_ABSENT_FS = 1 << 1,  /* ops processed on absent fs */
 	ALLOWED_AS_FIRST_OP = 1 << 2,   /* ops reqired first in compound */
 	/* For rfc 5661 section 2.6.3.1.1: */
-- 
2.50.0.107.gf914562f5916.dirty