[PATCH v5 05/11] nfsd: drop explicit tests for special stateids which would be invalid.

[NeilBrown <neilb@ownmail.net>]

From: NeilBrown <neil@brown.name>

In two places nfsd has code to test for special stateids and to report
nfserr_bad_stateid if they are found.
One is for handling TEST_STATEID ops which always forbid these stateids,
and one is for all other places that a stateid is used, and the code is
*after* any checks for special stateids which might be permitted.

These tests add no value.  In each case there is a subsequent lookup for
the stateid which will return the same error code if the stateid is not
found, and special stateids never will be found.

Special stateids have a si.opaque.so_id which is either 0 or UINT_MAX.
Stateids stored in the idr only have so_id ranging from 1 or INT_MAX.
So there is no possibility of a special stateid being found.

Having the explicit test optimises the unexpected case where a special
stateid is incorrectly given, and adds unnecessary comparisons to the
common case of a non-special stateid being given.

In nfsd4_lookup_stateid(), simply removing the test would mean that
a special stateid could result in the incorrect nfserr_stale_stateid
error, as the validity of so_clid is checked before so_id.  So we
add extra checks to only return nfserr_stale_stateid if the stateid
looks like it might have been locally generated - so_id not
all zeroes or all ones.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/nfsd/nfs4state.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 35004568d43e..ea931e606f40 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -74,6 +74,23 @@ static const stateid_t close_stateid = {
 	.si_generation = 0xffffffffU,
 };
 
+/*
+ * In NFSv4.0 there is a case were we should return NFS4ERR_STALE_STATEID
+ * if the stateid looks like one we might have created previously, and
+ * NFS4ERR_BAD_STATEID if it looks like it was never valid.
+ * There is not a lot of redundancy in the stateid that can be used to make
+ * this distinction, but it would be useful to differentiate special
+ * stateids from locally generated stateid.
+ * Special stateids have si.opaque.so_id being either all zeros or all 1s,
+ * so 0 or (u32)-1. Locally generated stateids have si.opaque.so_id as
+ * a number from 1 to INT_MAX (as generated by idr_alloc_cyclic()).
+ * We can test for the later range with some simple arithmetic.
+ */
+static inline bool stateid_well_formed(stateid_t *stid)
+{
+	return (stid->si_opaque.so_id - 1) < INT_MAX;
+}
+
 static u64 current_sessionid = 1;
 
 #define ZERO_STATEID(stateid) (!memcmp((stateid), &zero_stateid, sizeof(stateid_t)))
@@ -7129,9 +7146,6 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 	struct nfs4_stid *s;
 	__be32 status = nfserr_bad_stateid;
 
-	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
-		CLOSE_STATEID(stateid))
-		return status;
 	spin_lock(&cl->cl_lock);
 	s = find_stateid_locked(cl, stateid);
 	if (!s)
@@ -7186,14 +7200,15 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 
 	statusmask |= SC_STATUS_ADMIN_REVOKED | SC_STATUS_FREEABLE;
 
-	if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
-		CLOSE_STATEID(stateid))
-		return nfserr_bad_stateid;
 	status = set_client(&stateid->si_opaque.so_clid, cstate, nn);
 	if (status == nfserr_stale_clientid) {
-		if (cstate->session)
-			return nfserr_bad_stateid;
-		return nfserr_stale_stateid;
+		if (!cstate->session && stateid_well_formed(stateid))
+			/*
+			 * Might be from earlier instance - v4.0 likes
+			 * to know
+			 */
+			return nfserr_stale_stateid;
+		return nfserr_bad_stateid;
 	}
 	if (status)
 		return status;
-- 
2.50.0.107.gf914562f5916.dirty