[PATCH v2 3/6] NFSD: Prevent client use-after-free during admin state revocation

[Chuck Lever <cel@kernel.org>]

A stateid stores only a bare pointer to its owning nfs4_client; a
reference on the stateid does not keep the client alive.  The client
outlives its stateids only because __destroy_client() drains them
before free_client() runs.

nfsd4_revoke_states() drops nn->client_lock across revoke_one_stid(),
which revokes the delegation, open, lock, or layout stateid and
dereferences its client -- revoke_delegation() and revoke_ol_stid()
both reacquire clp->cl_lock.  The stateid reference it holds does not
pin the client, so a teardown racing the dropped lock can run
__destroy_client() and free the client first.  The subsequent read of
clp->cl_minorversion is exposed the same way.

A DESTROY_CLIENTID or superseding EXCHANGE_ID reaches teardown through
mark_client_expired_locked(), which refuses while cl_rpc_users is
non-zero, so pinning the client under client_lock holds those paths
off.  force_expire_client() does not consult cl_rpc_users: it zeroes
cl_time under client_lock, waits once for cl_rpc_users to reach zero,
then unhashes and frees.  A pin taken after that wait goes unnoticed,
so the walk must also skip a client that is already expiring.

Under client_lock, skip a client whose cl_time is zero and otherwise
pin it with cl_rpc_users before dropping the lock.  cl_time is set
under the same lock, so the walk either sees the expiry and skips, or
pins early enough that force_expire_client() waits for the revoke and
the cl_minorversion update to finish.

Fixes: 1c13bf9f2e3c ("nfsd: allow lock state ids to be revoked and then freed")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs4state.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index efeb2a2e9c8f..cdb62b3bf718 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1965,9 +1965,19 @@ void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *sb)
 		struct nfs4_client *clp;
 	retry:
 		list_for_each_entry(clp, head, cl_idhash) {
-			struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
-								  sc_types);
+			struct nfs4_stid *stid;
+
+			/*
+			 * force_expire_client() ignores cl_rpc_users once
+			 * its wait_event() has passed, so pinning cannot
+			 * keep an already-expiring client alive; the
+			 * expiry path revokes its states instead.
+			 */
+			if (is_client_expired(clp))
+				continue;
+			stid = find_one_sb_stid(clp, sb, sc_types);
 			if (stid) {
+				atomic_inc(&clp->cl_rpc_users);
 				spin_unlock(&nn->client_lock);
 				revoke_one_stid(nn, clp, stid);
 				nfs4_put_stid(stid);
@@ -1980,6 +1990,9 @@ void nfsd4_revoke_states(struct nfsd_net *nn, struct super_block *sb)
 					 */
 					nn->nfs40_last_revoke =
 						ktime_get_boottime_seconds();
+				if (atomic_dec_and_test(&clp->cl_rpc_users) &&
+				    is_client_expired(clp))
+					wake_up_all(&expiry_wq);
 				goto retry;
 			}
 		}

-- 
2.54.0