NFSD: Consolidate the revocation-path client unpin

The client use-after-free fixes in the state-revocation paths left
four open-coded copies of one idiom: drop a cl_rpc_users pin without
renewing the client's lease, waking force_expire_client() when the
last pin drops on a client it is tearing down.  The accompanying "do
not renew" rationale was documented at only one of the four sites.

put_client_renew_locked() and put_client_renew() already carry the
same pin-drop logic, but they renew a non-expired client's lease and
so would resurrect the client whose state is being revoked.  Factor
the common pin-drop into __put_client_locked(), parameterized by
whether to renew.  The renew helpers pass true; the new
put_client_no_renew_locked() and put_client_no_renew() pass false and
carry the revocation paths, which must not revive the client they are
tearing down.  No change in behavior.

Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260709-cel-v4-6-1d519d9be0cb@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
This commit is contained in:
Chuck Lever
2026-07-09 13:40:29 -04:00
parent 7b4f8a1586
commit 3308cf3f11

View File

@@ -206,18 +206,28 @@ renew_client_locked(struct nfs4_client *clp)
clp->cl_state = NFSD4_ACTIVE;
}
/*
* Finish a cl_rpc_users unpin with the client_lock held. A
* revocation walk clears @renew so the client whose state it is
* revoking is not revived; every other caller renews the lease of
* a still-active client.
*/
static void __put_client_locked(struct nfs4_client *clp, bool renew)
{
if (is_client_expired(clp))
wake_up_all(&expiry_wq);
else if (renew)
renew_client_locked(clp);
}
static void put_client_renew_locked(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
lockdep_assert_held(&nn->client_lock);
if (!atomic_dec_and_test(&clp->cl_rpc_users))
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
else
wake_up_all(&expiry_wq);
if (atomic_dec_and_test(&clp->cl_rpc_users))
__put_client_locked(clp, true);
}
static void put_client_renew(struct nfs4_client *clp)
@@ -226,10 +236,27 @@ static void put_client_renew(struct nfs4_client *clp)
if (!atomic_dec_and_lock(&clp->cl_rpc_users, &nn->client_lock))
return;
if (!is_client_expired(clp))
renew_client_locked(clp);
else
wake_up_all(&expiry_wq);
__put_client_locked(clp, true);
spin_unlock(&nn->client_lock);
}
static void put_client_no_renew_locked(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
lockdep_assert_held(&nn->client_lock);
if (atomic_dec_and_test(&clp->cl_rpc_users))
__put_client_locked(clp, false);
}
static void put_client_no_renew(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
if (!atomic_dec_and_lock(&clp->cl_rpc_users, &nn->client_lock))
return;
__put_client_locked(clp, false);
spin_unlock(&nn->client_lock);
}
@@ -1990,9 +2017,7 @@ 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);
put_client_no_renew_locked(clp);
goto retry;
}
}
@@ -2070,9 +2095,7 @@ void nfsd4_revoke_export_states(struct nfsd_net *nn, const struct path *path)
if (clp->cl_minorversion == 0)
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);
put_client_no_renew_locked(clp);
goto retry;
}
}
@@ -7465,9 +7488,7 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
nfsd4_drop_revoked_stid(stid);
nfs4_put_stid(stid);
spin_lock(&nn->client_lock);
if (atomic_dec_and_test(&clp->cl_rpc_users) &&
is_client_expired(clp))
wake_up_all(&expiry_wq);
put_client_no_renew_locked(clp);
goto retry;
}
spin_unlock(&clp->cl_lock);
@@ -7540,15 +7561,7 @@ nfs4_laundromat(struct nfsd_net *nn)
clp = dp->dl_stid.sc_client;
list_del_init(&dp->dl_recall_lru);
revoke_delegation(dp);
/*
* Unpin without renewing: put_client_renew() would
* renew the reaped client's lease.
*/
if (atomic_dec_and_lock(&clp->cl_rpc_users, &nn->client_lock)) {
if (is_client_expired(clp))
wake_up_all(&expiry_wq);
spin_unlock(&nn->client_lock);
}
put_client_no_renew(clp);
}
spin_lock(&nn->client_lock);