* [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation
@ 2026-07-09 17:40 Chuck Lever
2026-07-09 17:40 ` [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown Chuck Lever
` (8 more replies)
0 siblings, 9 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Wolfgang Walter, Chuck Lever, sashiko-bot
An NFSv4 stateid, and a bare lock owner reachable through the client's
owner hash, hold only a raw pointer to the owning nfs4_client. That
client outlives its state only because __destroy_client() drains every
stateid and owner before free_client() runs. A walk that dereferences
the client, or the owner, through one of those raw pointers after
dropping the lock that kept it reachable races a concurrent teardown
that can free it first. The NULL-pointer dereference reported during
client teardown was one such race; auditing the pattern found more
across the revocation and laundromat-reaping paths.
Where the racing object is the client, the fix is uniform: pin it with
cl_rpc_users across the window in which nn->client_lock is dropped.
Skipping a client that is already expiring is part of the correctness
argument, not cleanup. force_expire_client() stops consulting
cl_rpc_users once its wait has passed, so a pin taken then would not
hold the client; the walk must instead leave that client's state for
its own teardown to drain. Testing the expiry and taking the pin under
nn->client_lock is what makes the choice atomic against the expiry.
Pinning the client from these paths introduces a lock-order edge that
did not exist before: nn->client_lock now nests outside the inner
region of nn->deleg_lock and outside nn->blocked_locks_lock, both leaf
acquisitions everywhere else. The nesting is one-directional, and
netns.h records the deleg_lock relationship; a reaping path added later
must take nn->client_lock first.
Two changes sit apart from the lifetime races. The revocation and
reaping fixes left four open-coded copies of the pin-drop idiom,
consolidated here into put_client_no_renew() helpers (patch 6). The
same audit turned up an unrelated svc_export leak:
free_ol_stateid_reaplist() reaps open and lock stateids through
->sc_free() directly, bypassing the reference drop nfs4_put_stid()
performs, so the export stays pinned and blocks unmount (patch 9).
---
Changes in v4:
- Fix svc_export double put in nfs4_put_stid() (sashiko)
- Link to v3: https://patch.msgid.link/20260707-cel-v3-0-7c0cc16fd54f@kernel.org
Changes in v3:
- Fix client UAF in laundromat blocked-lock reaping (Neil)
- Fix client UAF in laundromat close_lru reaping (sashiko)
- Fix svc_export leak when reaping open stateids (sashiko)
- Link to v2: https://patch.msgid.link/20260705-cel-v2-0-d88c3b68e8bc@kernel.org
Changes in v2:
- Add matching UAF fixes in several other paths
---
Chuck Lever (9):
NFSD: Prevent lock owner use-after-free during client teardown
NFSD: Prevent client use-after-free during delegation revoke
NFSD: Prevent client use-after-free during admin state revocation
NFSD: Prevent client use-after-free during export state revocation
NFSD: Prevent client use-after-free during NFSv4.0 revoked-state cleanup
NFSD: Consolidate the revocation-path client unpin
NFSD: Prevent client use-after-free during blocked-lock reaping
NFSD: Prevent client use-after-free during close_lru reaping
NFSD: Release the export reference when reaping open stateids
fs/nfsd/netns.h | 6 ++-
fs/nfsd/nfs4state.c | 149 ++++++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 131 insertions(+), 24 deletions(-)
---
base-commit: ee6ae4a6bf3565b880dfb420017337475dfbc9ea
change-id: 20260705-cel-61c1c70caa03
Best regards,
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 2/9] NFSD: Prevent client use-after-free during delegation revoke Chuck Lever
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Wolfgang Walter, Chuck Lever
__destroy_client() releases a client's open owners, but a lock owner
whose only reference is a blocked lock (nbl) stays on
cl_ownerstr_hashtbl. client_has_state() does not count a bare owner,
so DESTROY_CLIENTID can reach __destroy_client() with such owners
present.
__destroy_client() then walks the table, calling remove_blocked_locks()
on each owner without a reference. Freeing a blocked lock drops the
owner reference held via flc_owner. The per-net laundromat reaps
blocked locks from nn->blocked_locks_lru independently of client state.
The two paths share blocked_locks_lock only for the list splice, not
the owner's lifetime. The laundromat therefore frees the owner as
__destroy_client() dereferences it, a NULL dereference in
remove_blocked_locks().
nfsd4_release_lockowner() holds a reference across the same call;
__destroy_client() does not. Hold cl_lock across the walk, taking a
reference and unhashing each owner, then drop it before
remove_blocked_locks() and nfs4_put_stateowner(), which take
blocked_locks_lock and cl_lock.
Reported-by: Wolfgang Walter <linux@stwm.de>
Closes: https://lore.kernel.org/linux-nfs/6eccafaaaa60651ef091257c3439c46b@stwm.de/
Fixes: 68ef3bc31664 ("nfsd: remove blocked locks on client teardown")
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a4398dc861a5..e000ed3e96e9 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2758,14 +2758,24 @@ __destroy_client(struct nfs4_client *clp)
release_openowner(oo);
}
for (i = 0; i < OWNER_HASH_SIZE; i++) {
- struct nfs4_stateowner *so, *tmp;
+ struct nfs4_stateowner *so;
- list_for_each_entry_safe(so, tmp, &clp->cl_ownerstr_hashtbl[i],
- so_strhash) {
+ spin_lock(&clp->cl_lock);
+ while (!list_empty(&clp->cl_ownerstr_hashtbl[i])) {
+ so = list_first_entry(&clp->cl_ownerstr_hashtbl[i],
+ struct nfs4_stateowner, so_strhash);
/* Should be no openowners at this point */
WARN_ON_ONCE(so->so_is_open_owner);
+ nfs4_get_stateowner(so);
+ unhash_lockowner_locked(lockowner(so));
+ spin_unlock(&clp->cl_lock);
+
remove_blocked_locks(lockowner(so));
+ nfs4_put_stateowner(so);
+
+ spin_lock(&clp->cl_lock);
}
+ spin_unlock(&clp->cl_lock);
}
nfsd4_return_all_client_layouts(clp);
nfsd4_shutdown_copy(clp);
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/9] NFSD: Prevent client use-after-free during delegation revoke
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
2026-07-09 17:40 ` [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 3/9] NFSD: Prevent client use-after-free during admin state revocation Chuck Lever
` (6 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
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@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 3/9] NFSD: Prevent client use-after-free during admin state revocation
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
2026-07-09 17:40 ` [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown Chuck Lever
2026-07-09 17:40 ` [PATCH v4 2/9] NFSD: Prevent client use-after-free during delegation revoke Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 4/9] NFSD: Prevent client use-after-free during export " Chuck Lever
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
A stateid holds only a bare pointer to its nfs4_client; a stateid
reference does not pin it. The client survives only because
__destroy_client() drains its stateids before free_client() runs.
nfsd4_revoke_states() drops nn->client_lock across revoke_one_stid(),
which dereferences the client to revoke a stateid and read
clp->cl_minorversion. A teardown racing the dropped lock can free
the client first.
Pinning cl_rpc_users under client_lock blocks the DESTROY_CLIENTID and
EXCHANGE_ID teardown, which refuses while cl_rpc_users is non-zero.
force_expire_client() ignores it: once its wait for cl_rpc_users to
reach zero has passed, a later pin goes unnoticed.
Under client_lock, skip a client whose cl_time is already zero --
force_expire_client() clears it there before waiting -- otherwise pin
cl_rpc_users before dropping the lock. The walk then either sees the
expiry and skips, or pins in time for that wait to cover the revoke.
Fixes: 1c13bf9f2e3c ("nfsd: allow lock state ids to be revoked and then freed")
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index efeb2a2e9c8f..565fa2ff5ba5 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;
}
}
@@ -3394,6 +3407,11 @@ static void force_expire_client(struct nfs4_client *clp)
trace_nfsd_clid_admin_expired(&clp->cl_clientid);
+ /*
+ * cl_time is cleared under client_lock before the wait so a
+ * revocation walk pinning cl_rpc_users under it either skips
+ * this client or is seen by this wait_event().
+ */
spin_lock(&nn->client_lock);
clp->cl_time = 0;
spin_unlock(&nn->client_lock);
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 4/9] NFSD: Prevent client use-after-free during export state revocation
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (2 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 3/9] NFSD: Prevent client use-after-free during admin state revocation Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 5/9] NFSD: Prevent client use-after-free during NFSv4.0 revoked-state cleanup Chuck Lever
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
nfsd4_revoke_export_states() has the same use-after-free as
nfsd4_revoke_states(): it drops nn->client_lock across
revoke_one_stid() and the following read of clp->cl_minorversion, but
the stateid reference it holds does not pin the client. A teardown
racing the dropped lock can free the client while revoke_one_stid()
still dereferences it.
exportfs -u drives this path through NFSD_CMD_UNLOCK_EXPORT, so an
administrator removing an export can race a client expiry.
Skip a client that is already expiring and otherwise pin it with
cl_rpc_users under client_lock before dropping the lock, matching
nfsd4_revoke_states().
Fixes: 2eac189bb059 ("NFSD: Add NFSD_CMD_UNLOCK_EXPORT netlink command")
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 565fa2ff5ba5..9de535a2a4bb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2055,10 +2055,14 @@ void nfsd4_revoke_export_states(struct nfsd_net *nn, const struct path *path)
struct nfs4_client *clp;
retry:
list_for_each_entry(clp, head, cl_idhash) {
- struct nfs4_stid *stid = find_one_export_stid(
- clp, path,
- sc_types);
+ struct nfs4_stid *stid;
+
+ /* Skip or pin clp as in nfsd4_revoke_states(). */
+ if (is_client_expired(clp))
+ continue;
+ stid = find_one_export_stid(clp, path, 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);
@@ -2066,6 +2070,9 @@ 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);
goto retry;
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 5/9] NFSD: Prevent client use-after-free during NFSv4.0 revoked-state cleanup
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (3 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 4/9] NFSD: Prevent client use-after-free during export " Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 6/9] NFSD: Consolidate the revocation-path client unpin Chuck Lever
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
nfs40_clean_admin_revoked() takes a stateid reference under
clp->cl_lock, drops nn->client_lock, and calls
nfsd4_drop_revoked_stid(), which dereferences the stateid's client
through s->sc_client->cl_lock. The stateid reference does not pin the
client, so a teardown racing the dropped lock can free the client
while nfsd4_drop_revoked_stid() is still using it.
This cleanup runs from the laundromat, so a periodic sweep can race
force_expire_client() driven by a write to the clients/<id>/ctl file.
Skip a client that is already expiring and otherwise pin it with
cl_rpc_users under client_lock before dropping the lock, matching
nfsd4_revoke_states().
Fixes: d688d8585e6b ("nfsd: allow admin-revoked NFSv4.0 state to be freed.")
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9de535a2a4bb..445a0f40d5ff 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7452,16 +7452,22 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
if (atomic_read(&clp->cl_admin_revoked) == 0)
continue;
+ if (is_client_expired(clp))
+ continue;
spin_lock(&clp->cl_lock);
idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
refcount_inc(&stid->sc_count);
+ atomic_inc(&clp->cl_rpc_users);
spin_unlock(&nn->client_lock);
/* this function drops ->cl_lock */
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);
goto retry;
}
spin_unlock(&clp->cl_lock);
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 6/9] NFSD: Consolidate the revocation-path client unpin
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (4 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 5/9] NFSD: Prevent client use-after-free during NFSv4.0 revoked-state cleanup Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 17:40 ` [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping Chuck Lever
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
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>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 69 +++++++++++++++++++++++++++++++----------------------
1 file changed, 41 insertions(+), 28 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 445a0f40d5ff..142ba7d80539 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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);
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (5 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 6/9] NFSD: Consolidate the revocation-path client unpin Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 18:36 ` Jeff Layton
2026-07-09 17:40 ` [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping Chuck Lever
2026-07-09 17:40 ` [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids Chuck Lever
8 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
A bare lock owner -- its only remaining reference a blocked lock on
nn->blocked_locks_lru -- holds a raw pointer to its nfs4_client but
no reference keeping the client alive. When the per-net laundromat
reaps such a lock, freeing the nbl drops the owner reference
held through flc_owner, and the final nfs4_put_stateowner()
takes the client's cl_lock. Because the laundromat detaches the
nbl first, __destroy_client() no longer finds it, so a concurrent
force_expire_client() can free the client before nfs4_put_stateowner()
runs, dereferencing cl_lock in freed memory.
Pin the client with cl_rpc_users before dropping
nn->blocked_locks_lock, and skip clients already expiring, whose
blocked locks __destroy_client() frees while holding an owner
reference. Take nn->client_lock outside nn->blocked_locks_lock.
Every other site holds nn->blocked_locks_lock as a leaf, acquiring
no further lock, so placing nn->client_lock outside it cannot form
a lock-order cycle.
Fixes: 7919d0a27f1e ("nfsd: add a LRU list for blocked locks")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 142ba7d80539..4acd02f1642c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -357,6 +357,16 @@ free_blocked_lock(struct nfsd4_blocked_lock *nbl)
kref_put(&nbl->nbl_kref, free_nbl);
}
+/* A blocked lock's flc_owner is its nfs4_lockowner. */
+static struct nfs4_client *
+nbl_client(struct nfsd4_blocked_lock *nbl)
+{
+ struct nfs4_lockowner *lo;
+
+ lo = (struct nfs4_lockowner *)nbl->nbl_lock.c.flc_owner;
+ return lo->lo_owner.so_client;
+}
+
static void
remove_blocked_locks(struct nfs4_lockowner *lo)
{
@@ -7591,22 +7601,29 @@ nfs4_laundromat(struct nfsd_net *nn)
* indefinitely once the lock does become free.
*/
BUG_ON(!list_empty(&reaplist));
+ spin_lock(&nn->client_lock);
spin_lock(&nn->blocked_locks_lock);
- while (!list_empty(&nn->blocked_locks_lru)) {
- nbl = list_first_entry(&nn->blocked_locks_lru,
- struct nfsd4_blocked_lock, nbl_lru);
+ list_for_each_safe(pos, next, &nn->blocked_locks_lru) {
+ nbl = list_entry(pos, struct nfsd4_blocked_lock, nbl_lru);
if (!state_expired(<, nbl->nbl_time))
break;
+ clp = nbl_client(nbl);
+ if (is_client_expired(clp))
+ continue;
+ atomic_inc(&clp->cl_rpc_users);
list_move(&nbl->nbl_lru, &reaplist);
list_del_init(&nbl->nbl_list);
}
spin_unlock(&nn->blocked_locks_lock);
+ spin_unlock(&nn->client_lock);
while (!list_empty(&reaplist)) {
nbl = list_first_entry(&reaplist,
struct nfsd4_blocked_lock, nbl_lru);
+ clp = nbl_client(nbl);
list_del_init(&nbl->nbl_lru);
free_blocked_lock(nbl);
+ put_client_no_renew(clp);
}
#ifdef CONFIG_NFSD_V4_2_INTER_SSC
/* service the server-to-server copy delayed unmount list */
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (6 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 18:37 ` Jeff Layton
2026-07-09 17:40 ` [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids Chuck Lever
8 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
An nfs4_openowner left on nn->close_lru after its final CLOSE keeps
its last closed stateid in oo_last_closed_stid, holding only a raw
pointer to its nfs4_client. The laundromat reaps timed-out entries,
drops nn->client_lock, and calls nfs4_put_stid(), which dereferences
the client through cl_lock. Nothing pins the client across that
window, so a concurrent force_expire_client() can free it and
nfs4_put_stid() reads freed memory. __destroy_client() hits the same
race, walking clp->cl_openowners without cl_lock.
Pin the client with cl_rpc_users before dropping client_lock, and
skip clients already expiring. __destroy_client() then cleans up its
own close_lru entries through release_last_closed_stateid(), so
teardown no longer races the laundromat.
Fixes: 217526e7ecc9 ("nfsd: protect the close_lru list and oo_last_closed_stid with client_lock")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4acd02f1642c..20556b8f186a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7581,11 +7581,16 @@ nfs4_laundromat(struct nfsd_net *nn)
if (!state_expired(<, oo->oo_time))
break;
list_del_init(&oo->oo_close_lru);
+ clp = oo->oo_owner.so_client;
+ if (is_client_expired(clp))
+ continue;
stp = oo->oo_last_closed_stid;
oo->oo_last_closed_stid = NULL;
+ atomic_inc(&clp->cl_rpc_users);
spin_unlock(&nn->client_lock);
nfs4_put_stid(&stp->st_stid);
spin_lock(&nn->client_lock);
+ put_client_no_renew_locked(clp);
}
spin_unlock(&nn->client_lock);
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
` (7 preceding siblings ...)
2026-07-09 17:40 ` [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping Chuck Lever
@ 2026-07-09 17:40 ` Chuck Lever
2026-07-09 18:40 ` Jeff Layton
8 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 17:40 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, sashiko-bot, Chuck Lever
nfs4_put_stid() releases the svc_export tracked in
nfs4_stid.sc_export, but free_ol_stateid_reaplist() frees open and
lock stateids by calling ->sc_free() directly, bypassing that path.
An open stateid takes an sc_export reference in nfs4_open() and a
lock stateid takes its own in init_lock_stateid(); both reach
free_ol_stateid_reaplist() through their normal teardown, the open
stateid via release_open_stateid() and the lock stateid via
nfsd4_release_lockowner(), each through put_ol_stateid_locked().
The reference is therefore never dropped, pinning the export and
blocking unmount for the lifetime of the stateid.
Release sc_export in free_ol_stateid_reaplist() the way
nfs4_put_stid() does. ->sc_free() runs once per stateid, and a
stateid reaches free_ol_stateid_reaplist() or nfs4_put_stid() but
never both, so the reference is dropped exactly once. Revoked
stateids reach this path with sc_export already cleared by
drop_stid_export(), so they are skipped rather than double-freed.
nfs4_put_stid() itself read sc_export before acquiring cl_lock.
drop_stid_export() clears that field and releases the reference
under cl_lock, so a concurrent revocation could drop the export in
the window between the read and the final put, releasing the same
reference twice. Read sc_export while cl_lock is held so the two
paths serialize and the reference is released exactly once.
Fixes: ba0cde5dc81d ("NFSD: Track svc_export in nfs4_stid")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260707-cel-v3-0-7c0cc16fd54f@kernel.org?part=9
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 20556b8f186a..e988dfebf75e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1272,9 +1272,9 @@ alloc_init_dir_deleg(struct nfs4_client *clp, struct nfs4_file *fp)
void
nfs4_put_stid(struct nfs4_stid *s)
{
- struct svc_export *exp = s->sc_export;
struct nfs4_file *fp = s->sc_file;
struct nfs4_client *clp = s->sc_client;
+ struct svc_export *exp;
might_lock(&clp->cl_lock);
@@ -1285,6 +1285,8 @@ nfs4_put_stid(struct nfs4_stid *s)
idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
if (s->sc_status & SC_STATUS_ADMIN_REVOKED)
atomic_dec(&s->sc_client->cl_admin_revoked);
+ /* Read under cl_lock to serialize with drop_stid_export(). */
+ exp = s->sc_export;
nfs4_free_cpntf_statelist(clp->net, s);
spin_unlock(&clp->cl_lock);
s->sc_free(s);
@@ -1744,6 +1746,7 @@ static void
free_ol_stateid_reaplist(struct list_head *reaplist)
{
struct nfs4_ol_stateid *stp;
+ struct svc_export *exp;
struct nfs4_file *fp;
might_sleep();
@@ -1753,9 +1756,12 @@ free_ol_stateid_reaplist(struct list_head *reaplist)
st_locks);
list_del(&stp->st_locks);
fp = stp->st_stid.sc_file;
+ exp = stp->st_stid.sc_export;
stp->st_stid.sc_free(&stp->st_stid);
if (fp)
put_nfs4_file(fp);
+ if (exp)
+ exp_put(exp);
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping
2026-07-09 17:40 ` [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping Chuck Lever
@ 2026-07-09 18:36 ` Jeff Layton
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Layton @ 2026-07-09 18:36 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey; +Cc: linux-nfs
On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
> A bare lock owner -- its only remaining reference a blocked lock on
> nn->blocked_locks_lru -- holds a raw pointer to its nfs4_client but
> no reference keeping the client alive. When the per-net laundromat
> reaps such a lock, freeing the nbl drops the owner reference
> held through flc_owner, and the final nfs4_put_stateowner()
> takes the client's cl_lock. Because the laundromat detaches the
> nbl first, __destroy_client() no longer finds it, so a concurrent
> force_expire_client() can free the client before nfs4_put_stateowner()
> runs, dereferencing cl_lock in freed memory.
>
> Pin the client with cl_rpc_users before dropping
> nn->blocked_locks_lock, and skip clients already expiring, whose
> blocked locks __destroy_client() frees while holding an owner
> reference. Take nn->client_lock outside nn->blocked_locks_lock.
> Every other site holds nn->blocked_locks_lock as a leaf, acquiring
> no further lock, so placing nn->client_lock outside it cannot form
> a lock-order cycle.
>
> Fixes: 7919d0a27f1e ("nfsd: add a LRU list for blocked locks")
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> fs/nfsd/nfs4state.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 142ba7d80539..4acd02f1642c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -357,6 +357,16 @@ free_blocked_lock(struct nfsd4_blocked_lock *nbl)
> kref_put(&nbl->nbl_kref, free_nbl);
> }
>
> +/* A blocked lock's flc_owner is its nfs4_lockowner. */
> +static struct nfs4_client *
> +nbl_client(struct nfsd4_blocked_lock *nbl)
> +{
> + struct nfs4_lockowner *lo;
> +
> + lo = (struct nfs4_lockowner *)nbl->nbl_lock.c.flc_owner;
> + return lo->lo_owner.so_client;
> +}
> +
> static void
> remove_blocked_locks(struct nfs4_lockowner *lo)
> {
> @@ -7591,22 +7601,29 @@ nfs4_laundromat(struct nfsd_net *nn)
> * indefinitely once the lock does become free.
> */
> BUG_ON(!list_empty(&reaplist));
> + spin_lock(&nn->client_lock);
> spin_lock(&nn->blocked_locks_lock);
> - while (!list_empty(&nn->blocked_locks_lru)) {
> - nbl = list_first_entry(&nn->blocked_locks_lru,
> - struct nfsd4_blocked_lock, nbl_lru);
> + list_for_each_safe(pos, next, &nn->blocked_locks_lru) {
> + nbl = list_entry(pos, struct nfsd4_blocked_lock, nbl_lru);
> if (!state_expired(<, nbl->nbl_time))
> break;
> + clp = nbl_client(nbl);
> + if (is_client_expired(clp))
> + continue;
> + atomic_inc(&clp->cl_rpc_users);
> list_move(&nbl->nbl_lru, &reaplist);
> list_del_init(&nbl->nbl_list);
> }
> spin_unlock(&nn->blocked_locks_lock);
> + spin_unlock(&nn->client_lock);
>
> while (!list_empty(&reaplist)) {
> nbl = list_first_entry(&reaplist,
> struct nfsd4_blocked_lock, nbl_lru);
> + clp = nbl_client(nbl);
> list_del_init(&nbl->nbl_lru);
> free_blocked_lock(nbl);
> + put_client_no_renew(clp);
> }
> #ifdef CONFIG_NFSD_V4_2_INTER_SSC
> /* service the server-to-server copy delayed unmount list */
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping
2026-07-09 17:40 ` [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping Chuck Lever
@ 2026-07-09 18:37 ` Jeff Layton
0 siblings, 0 replies; 14+ messages in thread
From: Jeff Layton @ 2026-07-09 18:37 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey; +Cc: linux-nfs
On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
> An nfs4_openowner left on nn->close_lru after its final CLOSE keeps
> its last closed stateid in oo_last_closed_stid, holding only a raw
> pointer to its nfs4_client. The laundromat reaps timed-out entries,
> drops nn->client_lock, and calls nfs4_put_stid(), which dereferences
> the client through cl_lock. Nothing pins the client across that
> window, so a concurrent force_expire_client() can free it and
> nfs4_put_stid() reads freed memory. __destroy_client() hits the same
> race, walking clp->cl_openowners without cl_lock.
>
> Pin the client with cl_rpc_users before dropping client_lock, and
> skip clients already expiring. __destroy_client() then cleans up its
> own close_lru entries through release_last_closed_stateid(), so
> teardown no longer races the laundromat.
>
> Fixes: 217526e7ecc9 ("nfsd: protect the close_lru list and oo_last_closed_stid with client_lock")
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> fs/nfsd/nfs4state.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 4acd02f1642c..20556b8f186a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7581,11 +7581,16 @@ nfs4_laundromat(struct nfsd_net *nn)
> if (!state_expired(<, oo->oo_time))
> break;
> list_del_init(&oo->oo_close_lru);
> + clp = oo->oo_owner.so_client;
> + if (is_client_expired(clp))
> + continue;
> stp = oo->oo_last_closed_stid;
> oo->oo_last_closed_stid = NULL;
> + atomic_inc(&clp->cl_rpc_users);
> spin_unlock(&nn->client_lock);
> nfs4_put_stid(&stp->st_stid);
> spin_lock(&nn->client_lock);
> + put_client_no_renew_locked(clp);
> }
> spin_unlock(&nn->client_lock);
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids
2026-07-09 17:40 ` [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids Chuck Lever
@ 2026-07-09 18:40 ` Jeff Layton
2026-07-09 19:55 ` Chuck Lever
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Layton @ 2026-07-09 18:40 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, sashiko-bot
On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
> nfs4_put_stid() releases the svc_export tracked in
> nfs4_stid.sc_export, but free_ol_stateid_reaplist() frees open and
> lock stateids by calling ->sc_free() directly, bypassing that path.
> An open stateid takes an sc_export reference in nfs4_open() and a
> lock stateid takes its own in init_lock_stateid(); both reach
> free_ol_stateid_reaplist() through their normal teardown, the open
> stateid via release_open_stateid() and the lock stateid via
> nfsd4_release_lockowner(), each through put_ol_stateid_locked().
> The reference is therefore never dropped, pinning the export and
> blocking unmount for the lifetime of the stateid.
>
> Release sc_export in free_ol_stateid_reaplist() the way
> nfs4_put_stid() does. ->sc_free() runs once per stateid, and a
> stateid reaches free_ol_stateid_reaplist() or nfs4_put_stid() but
> never both, so the reference is dropped exactly once. Revoked
> stateids reach this path with sc_export already cleared by
> drop_stid_export(), so they are skipped rather than double-freed.
>
> nfs4_put_stid() itself read sc_export before acquiring cl_lock.
> drop_stid_export() clears that field and releases the reference
> under cl_lock, so a concurrent revocation could drop the export in
> the window between the read and the final put, releasing the same
> reference twice. Read sc_export while cl_lock is held so the two
> paths serialize and the reference is released exactly once.
>
> Fixes: ba0cde5dc81d ("NFSD: Track svc_export in nfs4_stid")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260707-cel-v3-0-7c0cc16fd54f@kernel.org?part=9
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> fs/nfsd/nfs4state.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 20556b8f186a..e988dfebf75e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1272,9 +1272,9 @@ alloc_init_dir_deleg(struct nfs4_client *clp, struct nfs4_file *fp)
> void
> nfs4_put_stid(struct nfs4_stid *s)
> {
> - struct svc_export *exp = s->sc_export;
> struct nfs4_file *fp = s->sc_file;
> struct nfs4_client *clp = s->sc_client;
> + struct svc_export *exp;
>
> might_lock(&clp->cl_lock);
>
> @@ -1285,6 +1285,8 @@ nfs4_put_stid(struct nfs4_stid *s)
> idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
> if (s->sc_status & SC_STATUS_ADMIN_REVOKED)
> atomic_dec(&s->sc_client->cl_admin_revoked);
> + /* Read under cl_lock to serialize with drop_stid_export(). */
> + exp = s->sc_export;
> nfs4_free_cpntf_statelist(clp->net, s);
> spin_unlock(&clp->cl_lock);
> s->sc_free(s);
> @@ -1744,6 +1746,7 @@ static void
> free_ol_stateid_reaplist(struct list_head *reaplist)
> {
> struct nfs4_ol_stateid *stp;
> + struct svc_export *exp;
> struct nfs4_file *fp;
>
> might_sleep();
> @@ -1753,9 +1756,12 @@ free_ol_stateid_reaplist(struct list_head *reaplist)
> st_locks);
> list_del(&stp->st_locks);
> fp = stp->st_stid.sc_file;
> + exp = stp->st_stid.sc_export;
> stp->st_stid.sc_free(&stp->st_stid);
> if (fp)
> put_nfs4_file(fp);
> + if (exp)
> + exp_put(exp);
nit: nfs4_put_stid() does this in the reverse order. It doesn't
actually matter, but it looks a bit weird if you want to fix it up
before merging.
> }
> }
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids
2026-07-09 18:40 ` Jeff Layton
@ 2026-07-09 19:55 ` Chuck Lever
0 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2026-07-09 19:55 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, sashiko-bot
On Thu, Jul 9, 2026, at 2:40 PM, Jeff Layton wrote:
> On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
>> nfs4_put_stid() releases the svc_export tracked in
>> nfs4_stid.sc_export, but free_ol_stateid_reaplist() frees open and
>> lock stateids by calling ->sc_free() directly, bypassing that path.
>> An open stateid takes an sc_export reference in nfs4_open() and a
>> lock stateid takes its own in init_lock_stateid(); both reach
>> free_ol_stateid_reaplist() through their normal teardown, the open
>> stateid via release_open_stateid() and the lock stateid via
>> nfsd4_release_lockowner(), each through put_ol_stateid_locked().
>> The reference is therefore never dropped, pinning the export and
>> blocking unmount for the lifetime of the stateid.
>>
>> Release sc_export in free_ol_stateid_reaplist() the way
>> nfs4_put_stid() does. ->sc_free() runs once per stateid, and a
>> stateid reaches free_ol_stateid_reaplist() or nfs4_put_stid() but
>> never both, so the reference is dropped exactly once. Revoked
>> stateids reach this path with sc_export already cleared by
>> drop_stid_export(), so they are skipped rather than double-freed.
>>
>> nfs4_put_stid() itself read sc_export before acquiring cl_lock.
>> drop_stid_export() clears that field and releases the reference
>> under cl_lock, so a concurrent revocation could drop the export in
>> the window between the read and the final put, releasing the same
>> reference twice. Read sc_export while cl_lock is held so the two
>> paths serialize and the reference is released exactly once.
>>
>> Fixes: ba0cde5dc81d ("NFSD: Track svc_export in nfs4_stid")
>> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260707-cel-v3-0-7c0cc16fd54f@kernel.org?part=9
>> Signed-off-by: Chuck Lever <cel@kernel.org>
>> ---
>> fs/nfsd/nfs4state.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 20556b8f186a..e988dfebf75e 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -1753,9 +1756,12 @@ free_ol_stateid_reaplist(struct list_head *reaplist)
>> st_locks);
>> list_del(&stp->st_locks);
>> fp = stp->st_stid.sc_file;
>> + exp = stp->st_stid.sc_export;
>> stp->st_stid.sc_free(&stp->st_stid);
>> if (fp)
>> put_nfs4_file(fp);
>> + if (exp)
>> + exp_put(exp);
>
> nit: nfs4_put_stid() does this in the reverse order. It doesn't
> actually matter, but it looks a bit weird if you want to fix it up
> before merging.
>
>> }
>> }
>>
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
Swap applied, series pushed to nfsd-testing. Thanks for your review!
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-07-09 19:55 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 17:40 [PATCH v4 0/9] NFSD: Fix UAFs in client teardown and state revocation Chuck Lever
2026-07-09 17:40 ` [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown Chuck Lever
2026-07-09 17:40 ` [PATCH v4 2/9] NFSD: Prevent client use-after-free during delegation revoke Chuck Lever
2026-07-09 17:40 ` [PATCH v4 3/9] NFSD: Prevent client use-after-free during admin state revocation Chuck Lever
2026-07-09 17:40 ` [PATCH v4 4/9] NFSD: Prevent client use-after-free during export " Chuck Lever
2026-07-09 17:40 ` [PATCH v4 5/9] NFSD: Prevent client use-after-free during NFSv4.0 revoked-state cleanup Chuck Lever
2026-07-09 17:40 ` [PATCH v4 6/9] NFSD: Consolidate the revocation-path client unpin Chuck Lever
2026-07-09 17:40 ` [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping Chuck Lever
2026-07-09 18:36 ` Jeff Layton
2026-07-09 17:40 ` [PATCH v4 8/9] NFSD: Prevent client use-after-free during close_lru reaping Chuck Lever
2026-07-09 18:37 ` Jeff Layton
2026-07-09 17:40 ` [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids Chuck Lever
2026-07-09 18:40 ` Jeff Layton
2026-07-09 19:55 ` Chuck Lever
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.