[PATCH 1/3] nfsd: fix CB_NOTIFY workqueue loop when queue overflows

Jeff Layton <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.nfs
Message-ID <[email protected]>
When NOTIFY4_CHANGE_DIR_ATTRS is requested, nfsd4_cb_notify_prepare()
lowers its limit to NOTIFY4_EVENT_QUEUE_SIZE - 1 to reserve a slot for
the dir attribute update. The producer in nfsd_handle_dir_event() caps
ncn_evt_cnt at NOTIFY4_EVENT_QUEUE_SIZE, so the queue can fill to one
more than that limit.

In that case prepare() took the "we can't keep up!" branch and jumped to
out_recall without draining the queue, leaving ncn_evt_cnt nonzero.
Returning false from prepare causes nfsd41_destroy_cb() to run the
release op, and nfsd4_cb_notify_release() requeues the callback whenever
ncn_evt_cnt > 0. The requeued callback hits the same overflow check and
recalls again, spinning a workqueue thread forever. Because each cycle
holds an stid reference across the requeue, sc_count never reaches zero,
so the delegation is never freed even after the client returns it -
exhausting CPU and leaking the delegation.

Drain the queued events (dropping their references) and reset
ncn_evt_cnt before recalling, so the release op won't requeue. This also
fixes an event reference leak that previously existed on the recall path.

Also guard the release-side requeue against a revoked delegation: there
is no point notifying a client that no longer holds the delegation, and
skipping the requeue avoids pinning the stid and spinning the workqueue.

Fixes: fd0d6dde2a57 ("nfsd: add support to CB_NOTIFY for dir attribute changes")
Reported-by: Sashiko AI <https://sashiko.dev>
Signed-off-by: Jeff Layton <[email protected]>
---
 fs/nfsd/nfs4state.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2f7210accdf1..b830aed7ae39 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3546,16 +3546,21 @@ nfsd4_cb_notify_prepare(struct nfsd4_callback *cb)
 		return false;
 	}
 
-	/* we can't keep up! */
-	if (count > limit) {
-		spin_unlock(&ncn->ncn_lock);
-		goto out_recall;
-	}
-
 	memcpy(events, ncn->ncn_evt, sizeof(*events) * count);
 	ncn->ncn_evt_cnt = 0;
 	spin_unlock(&ncn->ncn_lock);
 
+	/*
+	 * We can't keep up! Drop the queued events and recall. The queue must
+	 * be drained here: out_recall leaves ncn_evt_cnt at 0, so the release
+	 * op won't see leftover events and requeue this callback forever.
+	 */
+	if (count > limit) {
+		for (i = 0; i < count; ++i)
+			nfsd_notify_event_put(events[i]);
+		goto out_recall;
+	}
+
 	rcu_read_lock();
 	nf = nfsd_file_get(rcu_dereference(dp->dl_stid.sc_file->fi_deleg_file));
 	rcu_read_unlock();
@@ -3663,8 +3668,13 @@ nfsd4_cb_notify_release(struct nfsd4_callback *cb)
 	struct nfs4_delegation *dp =
 			container_of(ncn, struct nfs4_delegation, dl_cb_notify);
 
-	/* Drain events that arrived while this callback was in flight */
-	if (READ_ONCE(ncn->ncn_evt_cnt) > 0)
+	/*
+	 * Drain events that arrived while this callback was in flight, but
+	 * don't requeue against a revoked delegation: there's no point in
+	 * notifying a client that no longer holds it, and doing so can pin the
+	 * stid and spin the workqueue.
+	 */
+	if (!dp->dl_stid.sc_status && READ_ONCE(ncn->ncn_evt_cnt) > 0)
 		nfsd4_run_cb_notify(ncn);
 	nfs4_put_stid(&dp->dl_stid);
 }

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.