[PATCH v3 2/9] NFSD: Prevent client use-after-free during delegation revoke
Chuck Lever <[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
A delegation stateid holds only a bare pointer to its owning
nfs4_client and does not keep it alive. The client survives its
stateids only because __destroy_client() drains cl_delegations and
cl_revoked before free_client() runs.
nfs4_laundromat() breaks that invariant: it unhashes an
expired delegation from cl_delegations, drops deleg_lock, then
revoke_delegation() relinks it onto cl_revoked under cl_lock. In that
window the delegation is on neither list, so client_has_state() can
report no remaining state.
Every teardown path first requires cl_rpc_users to be zero, but
the laundromat holds no such reference. A client whose recalled
delegation has just timed out can therefore reach free_client()
while revoke_delegation() is still about to dereference cl_lock,
a use-after-free.
Pin the client with cl_rpc_users across the revoke so teardown blocks
until it completes, then reap the delegation from cl_revoked. A client
already expiring reaps its own, so skip it and leave the delegation on
del_recall_lru.
Fixes: 3bd64a5ba171 ("nfsd4: implement SEQ4_STATUS_RECALLABLE_STATE_REVOKED")
Reviewed-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/netns.h | 6 ++++--
fs/nfsd/nfs4state.c | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 03724bef10a7..a7bd7b67fa4f 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -115,7 +115,8 @@ struct nfsd_net {
struct list_head client_lru;
struct list_head close_lru;
- /* protects del_recall_lru and delegation hash/unhash */
+ /* protects del_recall_lru and delegation hash/unhash;
+ * nests outside client_lock */
spinlock_t deleg_lock ____cacheline_aligned;
struct list_head del_recall_lru;
@@ -124,7 +125,8 @@ struct nfsd_net {
struct delayed_work laundromat_work;
- /* client_lock protects the client lru list and session hash table */
+ /* client_lock protects the client lru list and session hash
+ * table; nests inside deleg_lock */
spinlock_t client_lock;
/* protects blocked_locks_lru */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e000ed3e96e9..efeb2a2e9c8f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7457,6 +7457,7 @@ nfs4_laundromat(struct nfsd_net *nn)
.new_timeo = nn->nfsd4_lease
};
struct nfs4_cpntf_state *cps;
+ struct nfs4_client *clp;
copy_stateid_t *cps_t;
int i;
@@ -7485,6 +7486,18 @@ nfs4_laundromat(struct nfsd_net *nn)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
if (!state_expired(<, dp->dl_time))
break;
+ clp = dp->dl_stid.sc_client;
+ spin_lock(&nn->client_lock);
+ if (is_client_expired(clp)) {
+ spin_unlock(&nn->client_lock);
+ continue;
+ }
+ /*
+ * Pin without reviving: get_client_locked() would
+ * flip a courtesy client back to NFSD4_ACTIVE.
+ */
+ atomic_inc(&clp->cl_rpc_users);
+ spin_unlock(&nn->client_lock);
refcount_inc(&dp->dl_stid.sc_count);
unhash_delegation_locked(dp, SC_STATUS_REVOKED);
list_add(&dp->dl_recall_lru, &reaplist);
@@ -7493,8 +7506,18 @@ nfs4_laundromat(struct nfsd_net *nn)
while (!list_empty(&reaplist)) {
dp = list_first_entry(&reaplist, struct nfs4_delegation,
dl_recall_lru);
+ 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);
+ }
}
spin_lock(&nn->client_lock);
--
2.54.0