Linux NFS development
 help / color / mirror / Atom feed
From: Jinpyo Lee <bint4b13@gmail.com>
To: linux-nfs@vger.kernel.org
Cc: Chuck Lever <cel@kernel.org>, Jeff Layton <jlayton@kernel.org>,
	NeilBrown <neil@brown.name>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	bobtobabz@gmail.com, Jinpyo Lee <bint4b13@gmail.com>
Subject: [PATCH v2] nfsd: drain pNFS fence work during state teardown
Date: Mon, 28 Sep 2026 17:37:39 +0900	[thread overview]
Message-ID: <20260928083739.643010-1-bint4b13@gmail.com> (raw)

The pNFS layout-recall timeout path holds a layout stateid reference
while a delayed fence worker runs and retries. A stateid reference does
not retain sc_client, so client expiry can free the nfs4_client while
the worker still uses client-owned state and eventually releases the
stateid through the stale client pointer.

Keeping only the client alive is not sufficient. The worker also uses
clp->net, and neither the stateid nor a client reference prevents NFSD
per-net state or module code from being torn down while a retry is
pending.

Prevent new fence work from being scheduled once layout teardown starts,
and synchronously cancel or drain an existing worker before releasing
the layout's client association. Check the stop state under ls_lock at
both the initial scheduling point and the retry point so cancellation
cannot miss a newly queued retry. Serialize teardown callers so only one
of them disposes of the worker-owned stateid reference.

The worker clears ls_fence_inflight before its final nfs4_put_stid(), so
that flag alone does not prove the worker has finished. Always synchronize
with the delayed work during teardown, even when the flag is already
clear. Release the worker-owned stateid reference in the teardown path
only when pending work was actually canceled.

nfsd4_return_all_client_layouts() is reached from __destroy_client(), so
the drain covers administrator expiry, laundromat expiry, client
replacement during CREATE_SESSION, and nfs4_state_shutdown_net().
Stopping revoked layout stateids separately also prevents a worker from
escaping the client list before shutdown. Module unload reaches the same
per-net state shutdown path before the pNFS caches and module text are
released.

The reproducer uses a pNFS SCSI export and a failed storage fence to keep
the retry pending. A source reproducer and the complete KASAN log are
available privately on request.

The original KASAN use-after-free was reproduced in nfs4_put_stid() on
the unpatched nfsd-testing tree. The patched x86_64 kernel was built
and boot-tested with Generic KASAN, lockdep, modular NFSD, and pNFS block
and SCSI layout support. Targeted runs covered administrator expiry,
natural lease expiry followed by courtesy-client shrinker reclamation,
CREATE_SESSION replacement, per-net shutdown, module unload, retry, and
successful storage fencing. No KASAN, lockdep, or refcount report occurred.

Named-netns removal followed by explicit NFSD shutdown and final
namespace exit also produced no KASAN, lockdep, or refcount report.
A test-only run confirmed teardown waited for the worker's final
stateid release.

Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed. The full
kernel and modules were built with GCC 13.3 without new compiler warnings.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: f52792f484ba ("NFSD: Enforce timeout on layout recall and integrate lease manager fencing")
Assisted-by: LLM
Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
---
Changes in v2:
- Replace client pinning with synchronized fence-work shutdown that blocks
  initial scheduling and retries and waits for the final stateid release.
- Handle revoked layout stateids and client, per-net, and module teardown.
- Describe targeted runtime validation and its limits.

Validation details for reviewers:

- NFSD holds a network namespace reference until nfs4_state_destroy_net(),
  after client teardown has drained fence work. Removing a namespace name
  during a pending retry did not destroy it; explicit rpc.nfsd 0 drained
  the worker before final namespace exit. This does not test namespace
  destruction while the worker runs.
- Test-only instrumentation widened the interval between clearing
  ls_fence_inflight and the final nfs4_put_stid(). During expiry,
  cancel_delayed_work_sync() waited for worker completion (2.027 seconds).
  The test-only changes are not in this patch.
- Matching unpatched runs also logged GETDEVICEINFO nfserrno() warnings
  (values 2 and 917504). This patch does not modify that path.

 fs/nfsd/nfs4layouts.c | 106 +++++++++++++++++++++++++++++++++---------
 fs/nfsd/nfs4state.c   |   2 +
 fs/nfsd/pnfs.h        |   5 ++
 fs/nfsd/state.h       |   2 +
 4 files changed, 94 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 12acb68cb..7ac379fd5 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -246,6 +246,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
 	spin_lock_init(&ls->ls_lock);
 	INIT_LIST_HEAD(&ls->ls_layouts);
 	mutex_init(&ls->ls_mutex);
+	mutex_init(&ls->ls_fence_mutex);
 	ls->ls_layout_type = layout_type;
 	nfsd4_init_cb(&ls->ls_recall, clp, &nfsd4_cb_layout_ops,
 			NFSPROC4_CLNT_CB_LAYOUT);
@@ -265,6 +266,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
 
 	ls->ls_fenced = false;
 	ls->ls_fence_inflight = false;
+	ls->ls_fence_stopped = false;
 	ls->ls_fence_delay = 0;
 	INIT_DELAYED_WORK(&ls->ls_fence_work, nfsd4_layout_fence_worker);
 
@@ -602,13 +604,35 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
 void
 nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
-	struct nfs4_layout_stateid *ls, *n;
+	struct nfs4_layout_stateid *ls;
 	LIST_HEAD(reaplist);
 
-	spin_lock(&clp->cl_lock);
-	list_for_each_entry_safe(ls, n, &clp->cl_lo_states, ls_perclnt)
+	/*
+	 * A fence worker dereferences sc_client and clp->net.  Drain every
+	 * worker before client or per-net state can be released.  Take a
+	 * temporary stateid reference because stopping a worker can sleep.
+	 */
+	for (;;) {
+		spin_lock(&clp->cl_lock);
+		ls = list_first_entry_or_null(&clp->cl_lo_states,
+					      struct nfs4_layout_stateid,
+					      ls_perclnt);
+		if (!ls) {
+			spin_unlock(&clp->cl_lock);
+			break;
+		}
+		if (!refcount_inc_not_zero(&ls->ls_stid.sc_count)) {
+			spin_unlock(&clp->cl_lock);
+			cond_resched();
+			continue;
+		}
+		list_del_init(&ls->ls_perclnt);
+		spin_unlock(&clp->cl_lock);
+
+		nfsd4_stop_layout_fence(ls);
 		nfsd4_return_all_layouts(ls, &reaplist);
-	spin_unlock(&clp->cl_lock);
+		nfs4_put_stid(&ls->ls_stid);
+	}
 
 	nfsd4_free_layouts(&reaplist);
 }
@@ -792,8 +816,47 @@ nfsd4_layout_lm_open_conflict(struct file *filp, int arg)
 	return 0;
 }
 
-static void
-nfsd4_layout_fence_worker(struct work_struct *work)
+static void nfsd4_layout_fence_done(struct nfs4_layout_stateid *ls)
+{
+	/* Unlock the lease so that tasks waiting on it can proceed. */
+	nfsd4_close_layout(ls);
+
+	spin_lock(&ls->ls_lock);
+	ls->ls_fenced = true;
+	ls->ls_fence_inflight = false;
+	spin_unlock(&ls->ls_lock);
+	nfs4_put_stid(&ls->ls_stid);
+}
+
+void nfsd4_stop_layout_fence(struct nfs4_layout_stateid *ls)
+{
+	/* Serialize teardown callers which can arrive through different paths. */
+	mutex_lock(&ls->ls_fence_mutex);
+	spin_lock(&ls->ls_lock);
+	if (ls->ls_fence_stopped) {
+		spin_unlock(&ls->ls_lock);
+		mutex_unlock(&ls->ls_fence_mutex);
+		return;
+	}
+	ls->ls_fence_stopped = true;
+	spin_unlock(&ls->ls_lock);
+
+	/*
+	 * The worker clears ls_fence_inflight before its final nfs4_put_stid().
+	 * Always wait for it, even if that flag is already clear.  New work
+	 * cannot be queued after ls_fence_stopped is set under ls_lock.
+	 * If pending work was canceled, release its stateid reference here.
+	 */
+	if (cancel_delayed_work_sync(&ls->ls_fence_work))
+		nfsd4_layout_fence_done(ls);
+
+	spin_lock(&ls->ls_lock);
+	WARN_ON_ONCE(ls->ls_fence_inflight);
+	spin_unlock(&ls->ls_lock);
+	mutex_unlock(&ls->ls_fence_mutex);
+}
+
+static void nfsd4_layout_fence_worker(struct work_struct *work)
 {
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct nfs4_layout_stateid *ls = container_of(dwork,
@@ -804,18 +867,10 @@ nfsd4_layout_fence_worker(struct work_struct *work)
 	struct nfsd_net *nn;
 
 	spin_lock(&ls->ls_lock);
-	if (list_empty(&ls->ls_layouts)) {
+	if (ls->ls_fence_stopped || list_empty(&ls->ls_layouts)) {
 		spin_unlock(&ls->ls_lock);
 dispose:
-		cancel_delayed_work(&ls->ls_fence_work);
-		/* unlock the lease so that tasks waiting on it can proceed */
-		nfsd4_close_layout(ls);
-
-		ls->ls_fenced = true;
-		spin_lock(&ls->ls_lock);
-		ls->ls_fence_inflight = false;
-		spin_unlock(&ls->ls_lock);
-		nfs4_put_stid(&ls->ls_stid);
+		nfsd4_layout_fence_done(ls);
 		return;
 	}
 	spin_unlock(&ls->ls_lock);
@@ -862,12 +917,19 @@ dispose:
 	 *    clid: is the unique client identifier displayed in
 	 *          the warning message above.
 	 */
+	spin_lock(&ls->ls_lock);
+	if (ls->ls_fence_stopped || list_empty(&ls->ls_layouts)) {
+		spin_unlock(&ls->ls_lock);
+		goto dispose;
+	}
 	if (!ls->ls_fence_delay)
 		ls->ls_fence_delay = HZ;
 	else
 		ls->ls_fence_delay = min(ls->ls_fence_delay << 1,
 					 MAX_FENCE_DELAY);
-	mod_delayed_work(system_dfl_wq, &ls->ls_fence_work, ls->ls_fence_delay);
+	mod_delayed_work(system_dfl_wq, &ls->ls_fence_work,
+			 ls->ls_fence_delay);
+	spin_unlock(&ls->ls_lock);
 }
 
 /**
@@ -897,8 +959,7 @@ nfsd4_layout_lm_breaker_timedout(struct file_lease *fl)
 {
 	struct nfs4_layout_stateid *ls = fl->c.flc_owner;
 
-	if ((!nfsd4_layout_ops[ls->ls_layout_type]->fence_client) ||
-			ls->ls_fenced)
+	if (!nfsd4_layout_ops[ls->ls_layout_type]->fence_client)
 		return true;
 	/*
 	 * Make sure layout has not been returned yet before
@@ -910,6 +971,10 @@ nfsd4_layout_lm_breaker_timedout(struct file_lease *fl)
 	 * fresh schedule that takes an extra unmatched reference.
 	 */
 	spin_lock(&ls->ls_lock);
+	if (ls->ls_fenced || ls->ls_fence_stopped) {
+		spin_unlock(&ls->ls_lock);
+		return true;
+	}
 	if (ls->ls_fence_inflight) {
 		spin_unlock(&ls->ls_lock);
 		return false;
@@ -920,9 +985,8 @@ nfsd4_layout_lm_breaker_timedout(struct file_lease *fl)
 		return true;
 	}
 	ls->ls_fence_inflight = true;
-	spin_unlock(&ls->ls_lock);
-
 	mod_delayed_work(system_dfl_wq, &ls->ls_fence_work, 0);
+	spin_unlock(&ls->ls_lock);
 	return false;
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0f9340eb2..4075db57e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2082,6 +2082,7 @@ static void revoke_one_stid(struct nfsd_net *nn, struct nfs4_client *clp,
 			atomic_inc(&clp->cl_admin_revoked);
 		}
 		spin_unlock(&clp->cl_lock);
+		nfsd4_stop_layout_fence(layoutstateid(stid));
 		nfsd4_close_layout(layoutstateid(stid));
 		drop_stid_export(clp, stid);
 		break;
@@ -5861,6 +5862,7 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 		ls = layoutstateid(s);
 		list_del_init(&ls->ls_perclnt);
 		spin_unlock(&cl->cl_lock);
+		nfsd4_stop_layout_fence(ls);
 		nfs4_put_stid(s);
 		break;
 	default:
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index f7bee4dc5..559df0e5b 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -78,6 +78,7 @@ void nfsd4_return_all_client_layouts(struct nfs4_client *);
 void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp);
 void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
+void nfsd4_stop_layout_fence(struct nfs4_layout_stateid *ls);
 int nfsd4_init_pnfs(void);
 void nfsd4_exit_pnfs(void);
 #else
@@ -99,6 +100,10 @@ static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 static inline void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
 {
 }
+
+static inline void nfsd4_stop_layout_fence(struct nfs4_layout_stateid *ls)
+{
+}
 static inline void nfsd4_exit_pnfs(void)
 {
 }
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index cd9294f02..209ea43a2 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -861,9 +861,11 @@ struct nfs4_layout_stateid {
 	struct mutex			ls_mutex;
 
 	struct delayed_work		ls_fence_work;
+	struct mutex			ls_fence_mutex; /* serializes fence shutdown */
 	unsigned int			ls_fence_delay;
 	bool				ls_fenced;
 	bool				ls_fence_inflight;
+	bool				ls_fence_stopped;
 };
 
 static inline struct nfs4_layout_stateid *layoutstateid(struct nfs4_stid *s)

base-commit: cab95e6be3ba82bcf4c8be27c2eb20e55238aa41
-- 
2.50.1 (Apple Git-155)

             reply	other threads:[~2026-09-28  8:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28  8:37 Jinpyo Lee [this message]
2026-09-30  0:44 ` [PATCH v2] nfsd: drain pNFS fence work during state teardown Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260928083739.643010-1-bint4b13@gmail.com \
    --to=bint4b13@gmail.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=bobtobabz@gmail.com \
    --cc=cel@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox