Skip to contentSkip to conversation

nfsd: copy offload fixes

PATCH Superseded Jeff Layton 2 participants 12 messages Jul 9

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jeff Layton

author maintainer
#0 lore raw

v2 cover nfsd: copy offload fixes

These patches fix bugs in inter-server copy offload code noticed by LLM inspection. The first 3 were found via kres, and the next 5 were problems that were flagged in agentic review of those patches and some via testing.

The last 3 patches attempt to rework the code to be less problematic in the future. It breaks up the nfsd4_copy object into two separate objects with well-defined lifetimes, and then integrates the COPY stateids into the normal nfs4_stid model. The idea is to bring clarity to the copy object lifetimes that was the underlying cause of most of these bugs.

As part of this work, I've had Claude cook up a set of pynfs tests for COPY that I'll post separately.

---
Changes in v2:
- Add new patches to fix neighboring bugs found via testing and LLM review
- Fix error return when given NL_NAME or NL_URL netlocs
- Split nfsd4_copy into transient and durable async copy objects
- Rework COPY stateids to use normal stateid infrastructure
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Chris Mason (1):
nfsd: fix cpntf publish race in nfs4_init_cp_state

Jeff Layton (9):
nfsd: fix UAF in async copy cancel and shutdown
nfsd: fix stale s2s_cp_stateids IDR entry for async COPY
nfsd: initialize copy-notify stateid before publishing it
nfsd: check client ownership when cancelling a copy-notify stateid
nfsd: revoke copy-notify stateids before dropping their reference
nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types
nfsd: split nfsd4_copy into transient and durable async copy objects
nfsd: make the copy offload stateid a first-class nfs4_stid
nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR

 fs/nfsd/nfs4proc.c  | 311 ++++++++++++++++++++++++++++++++++------------------
 fs/nfsd/nfs4state.c | 224 +++++++++++++++++++++++++++++--------
 fs/nfsd/nfs4xdr.c   |  15 +++
 fs/nfsd/state.h     |   6 +-
 fs/nfsd/xdr4.h      |  33 ++++--
 5 files changed, 429 insertions(+), 160 deletions(-)

---
base-commit: bc7d6a41a6282da7c175c1638bdfef69c10f78d5
change-id: 20260624-nfsd-testing-8439f0163312

Best regards,

Signature · 1 line
Jeff Layton <[email protected]>
author maintainer
#1 lore raw

PATCH v2 1/10 nfsd: fix cpntf publish race in nfs4_init_cp_state

From: Chris Mason <[email protected]>

nfs4_alloc_init_cpntf_state() published the new cpntf entry into the s2s_cp_stateids IDR (with cs_type set) in one s2s_cp_lock section, then took the lock again to list_add() it onto p_stid->sc_cp_list. In the gap the entry is reachable by so_id but cp_list is still {NULL,NULL} from kzalloc. A racing OFFLOAD_CANCEL (so_id is echoed to the client as cnr_stateid, so any NFSv4.2 client can drive it) reaches manage_cpntf_state() -> _free_cpntf_state_locked() and does list_del() on the zeroed list_head, oopsing the server.

Fold the cs_type assignment and the list_add() into the same critical section as idr_alloc_cyclic(), so a concurrent lookup either misses the entry or sees a fully linked cp_list. INIT_LIST_HEAD() the entry after allocation and switch _free_cpntf_state_locked() to list_del_init() so a stale unlink is a no-op. nfs4_init_copy_state() passes NULL p_stid and skips the list_add, preserving NFS4_COPY_STID semantics.

Fixes 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Assisted-by kres:claude-opus-4-7
---
 fs/nfsd/nfs4state.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)
fs/nfsd/nfs4state.c +32 -10
index a4398dc861a5..b8946db3ebaa 100644
@@ -944,7 +944,7 @@ struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *sla
  * Create a unique stateid_t to represent each COPY.
  */
 static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
-			      unsigned char cs_type)
+			      unsigned char cs_type, struct nfs4_stid *p_stid)
 {
 	int new_id;
 
@@ -954,19 +954,37 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
 	idr_preload(GFP_KERNEL);
 	spin_lock(&nn->s2s_cp_lock);
 	new_id = idr_alloc_cyclic(&nn->s2s_cp_stateids, stid, 0, 0, GFP_NOWAIT);
-	stid->cs_stid.si_opaque.so_id = new_id;
-	stid->cs_stid.si_generation = 1;
+	if (new_id >= 0) {
+		stid->cs_stid.si_opaque.so_id = new_id;
+		stid->cs_stid.si_generation = 1;
+		/*
+		 * Publish cs_type and link onto the parent stid's
+		 * sc_cp_list inside the same critical section that
+		 * installed the entry into nn->s2s_cp_stateids. A
+		 * concurrent manage_cpntf_state() either fails the
+		 * idr_find() (entry not yet visible) or observes a
+		 * fully linked cp_list, so list_del_init() in
+		 * _free_cpntf_state_locked() is always well-defined.
+		 */
+		stid->cs_type = cs_type;
+		if (p_stid) {
+			struct nfs4_cpntf_state *cps =
+				container_of(stid, struct nfs4_cpntf_state,
+					     cp_stateid);
+
+			list_add(&cps->cp_list, &p_stid->sc_cp_list);
+		}
+	}
 	spin_unlock(&nn->s2s_cp_lock);
 	idr_preload_end();
 	if (new_id < 0)
 		return 0;
-	stid->cs_type = cs_type;
 	return 1;
 }
 
 int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy)
 {
-	return nfs4_init_cp_state(nn, &copy->cp_stateid, NFS4_COPY_STID);
+	return nfs4_init_cp_state(nn, &copy->cp_stateid, NFS4_COPY_STID, NULL);
 }
 
 struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
@@ -977,13 +995,17 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	cps = kzalloc_obj(struct nfs4_cpntf_state);
 	if (!cps)
 		return NULL;
+	/*
+	 * Initialize cp_list so any stale unlink (e.g. on an
+	 * entry that never reached its parent's sc_cp_list)
+	 * degrades to a benign self-unlink via list_del_init().
+	 */
+	INIT_LIST_HEAD(&cps->cp_list);
 	cps->cpntf_time = ktime_get_boottime_seconds();
 	refcount_set(&cps->cp_stateid.cs_count, 1);
-	if (!nfs4_init_cp_state(nn, &cps->cp_stateid, NFS4_COPYNOTIFY_STID))
+	if (!nfs4_init_cp_state(nn, &cps->cp_stateid, NFS4_COPYNOTIFY_STID,
+				p_stid))
 		goto out_free;
-	spin_lock(&nn->s2s_cp_lock);
-	list_add(&cps->cp_list, &p_stid->sc_cp_list);
-	spin_unlock(&nn->s2s_cp_lock);
 	return cps;
 out_free:
 	kfree(cps);
@@ -7854,7 +7876,7 @@ _free_cpntf_state_locked(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
 	WARN_ON_ONCE(cps->cp_stateid.cs_type != NFS4_COPYNOTIFY_STID);
 	if (!refcount_dec_and_test(&cps->cp_stateid.cs_count))
 		return;
-	list_del(&cps->cp_list);
+	list_del_init(&cps->cp_list);
 	idr_remove(&nn->s2s_cp_stateids,
 		   cps->cp_stateid.cs_stid.si_opaque.so_id);
 	kfree(cps);
Signature · 1 line
2.55.0
author maintainer
#2 lore raw

PATCH v2 2/10 nfsd: fix UAF in async copy cancel and shutdown

In reply to Jeff Layton

An async copy could be freed or used after free while a teardown caller (OFFLOAD_CANCEL, nfsd4_shutdown_copy, nfsd4_cancel_copy_by_sb) raced the copy kthread:

- find_async_copy() bumped copy->refcount but left the copy on clp->async_copies, so the reaper's cleanup_async_copy() could run release_copy_files() concurrently with a cancel/shutdown caller. Both put and NULL nf_src/nf_dst without a common lock, double-putting the nfsd_file and freeing it early.

- nfsd4_do_async_copy() set NFSD4_COPY_F_STOPPED before its final uses of the copy (nfsd_update_cmtime_attr() on copy->nf_dst, nfsd4_send_cb_offload()). nfsd4_stop_copy() treats a set STOPPED bit as "kthread done, skip kthread_stop()", so a teardown caller ran release_copy_files() -- which puts and NULLs nf_dst -- while the kthread still dereferenced it (NULL/UAF).

- copy->copy_task was never pinned. The one-shot kthread self-reaps on return, so kthread_stop()'s get_task_struct() could touch a freed task_struct.

- co_cb is embedded in the copy, but nfsd4_send_cb_offload() held a reference only on the client, so a concurrent teardown could free the copy while the CB_OFFLOAD callback was in flight.

Fix the teardown lifetime as a whole:

- find_async_copy() unlinks the copy (clear cp_clp, list_del_init) under async_lock; the cancel, shutdown, and sb-cancel paths drop the list-membership reference via nfs4_put_copy() after nfsd4_stop_copy(). Drop the now-redundant list_del fixup from cleanup_async_copy().

- Give the kthread its own reference, taken in nfsd4_copy() before wake_up_process() and dropped at the end of nfsd4_do_async_copy(); call wake_up_process() before list_add().

- Pin the task_struct with get_task_struct() in nfsd4_copy(), released in nfs4_put_copy(), so kthread_stop() is safe whenever the kthread exits. Set NFSD4_COPY_F_STOPPED only in nfsd4_stop_copy(), which now always kthread_stop()s before release_copy_files(); completion is still reported via NFSD4_COPY_F_COMPLETED, so nfsd4_has_active_async_copies() is unaffected. Each teardown caller removes the copy from clp->async_copies first, so kthread_stop() runs exactly once.

- Take a copy reference in nfsd4_send_cb_offload(), dropped in nfsd4_cb_offload_release(). The kthread still holds its own reference there, so the refcount_inc() cannot race the final free.

- Read cp_clp with smp_load_acquire() to pair with the unordered set_bit()/clear_bit() writers (Documentation/atomic_bitops.rst).

Fixes e0639dc5805a ("NFSD introduce async copy feature")
Fixes ac0514f4d198 ("NFSD: Add a laundromat reaper for async copy state")
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c | 163 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 128 insertions(+), 35 deletions(-)
fs/nfsd/nfs4proc.c +128 -35
index 669896be08b6..fad01d67bf3f 100644
@@ -1519,6 +1519,12 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
 {
 	if (!refcount_dec_and_test(&copy->refcount))
 		return;
+	/*
+	 * Drop the task_struct reference taken in nfsd4_copy(). Only async
+	 * copies have a copy_task; it is left NULL on every other path.
+	 */
+	if (copy->copy_task)
+		put_task_struct(copy->copy_task);
 	kfree(copy->cp_src);
 	kfree(copy);
 }
@@ -1528,20 +1534,23 @@ static void release_copy_files(struct nfsd4_copy *copy);
 static void nfsd4_stop_copy(struct nfsd4_copy *copy)
 {
 	trace_nfsd_copy_async_cancel(copy);
-	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags)) {
-		kthread_stop(copy->copy_task);
-		if (!test_bit(NFSD4_COPY_F_CB_ERROR, &copy->cp_flags))
-			copy->nfserr = nfs_ok;
-		set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
-	}
-
 	/*
-	 * The copy was removed from async_copies before this function
-	 * was called, so the reaper cannot clean it up. Release files
-	 * here regardless of who won the STOPPED race. If the thread
-	 * set STOPPED, it has finished using the files. If STOPPED
-	 * was set here, kthread_stop() waited for the thread to exit.
+	 * Always join the copy kthread before touching its resources. The
+	 * task_struct is pinned by get_task_struct() in nfsd4_copy(), so
+	 * kthread_stop() is safe even if this one-shot kthread has already
+	 * returned. Joining guarantees the kthread is no longer using
+	 * copy->nf_dst or the client by the time release_copy_files() runs.
+	 *
+	 * The caller has already removed the copy from clp->async_copies and
+	 * is the sole owner of this teardown, so kthread_stop() runs exactly
+	 * once per copy.
 	 */
+	set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags);
+	kthread_stop(copy->copy_task);
+	if (!test_bit(NFSD4_COPY_F_CB_ERROR, &copy->cp_flags))
+		copy->nfserr = nfs_ok;
+	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
+
 	release_copy_files(copy);
 	nfs4_put_copy(copy);
 }
@@ -1555,7 +1564,11 @@ static struct nfsd4_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
 		copy = list_first_entry(&clp->async_copies, struct nfsd4_copy,
 					copies);
 		refcount_inc(&copy->refcount);
-		copy->cp_clp = NULL;
+		/*
+		 * Pairs with smp_load_acquire in nfsd4_send_cb_offload();
+		 * see find_async_copy() for rationale.
+		 */
+		smp_store_release(&copy->cp_clp, NULL);
 		if (!list_empty(&copy->copies))
 			list_del_init(&copy->copies);
 	}
@@ -1567,8 +1580,16 @@ void nfsd4_shutdown_copy(struct nfs4_client *clp)
 {
 	struct nfsd4_copy *copy;
 
-	while ((copy = nfsd4_unhash_copy(clp)) != NULL)
+	while ((copy = nfsd4_unhash_copy(clp)) != NULL) {
 		nfsd4_stop_copy(copy);
+		/*
+		 * nfsd4_unhash_copy() removed the copy from
+		 * clp->async_copies and cleared cp_clp, so the reaper
+		 * can no longer reach it and drop the list-membership
+		 * reference via cleanup_async_copy(). Drop it here.
+		 */
+		nfs4_put_copy(copy);
+	}
 }
 
 static bool nfsd4_copy_on_sb(const struct nfsd4_copy *copy,
@@ -1637,6 +1658,14 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 
 		list_del_init(&copy->copies);
 		nfsd4_stop_copy(copy);
+		/*
+		 * The copy was moved off clp->async_copies under
+		 * async_lock above, so the reaper can no longer reach
+		 * it and drop the list-membership reference via
+		 * cleanup_async_copy(). Drop it here, mirroring
+		 * nfsd4_offload_cancel() and nfsd4_shutdown_copy().
+		 */
+		nfs4_put_copy(copy);
 		nfsd4_put_client(clp);
 	}
 }
@@ -1931,6 +1960,8 @@ static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
 
 	set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_flags);
 	nfsd4_put_client(cb->cb_clp);
+	/* Drop the copy reference taken in nfsd4_send_cb_offload(). */
+	nfs4_put_copy(copy);
 }
 
 static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
@@ -2062,28 +2093,38 @@ static void release_copy_files(struct nfsd4_copy *copy)
 	}
 }
 
+/*
+ * Called from the async copy reaper (after unlinking from async_copies
+ * under async_lock) and from nfsd4_copy()'s out_err path (where the copy
+ * was never list_add'd). In both cases the copy is unreachable from
+ * clp->async_copies.
+ */
 static void cleanup_async_copy(struct nfsd4_copy *copy)
 {
 	nfs4_free_copy_state(copy);
 	release_copy_files(copy);
-	if (copy->cp_clp) {
-		spin_lock(&copy->cp_clp->async_lock);
-		if (!list_empty(&copy->copies))
-			list_del_init(&copy->copies);
-		spin_unlock(&copy->cp_clp->async_lock);
-	}
 	nfs4_put_copy(copy);
 }
 
 static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 {
 	struct nfsd4_cb_offload *cbo = &copy->cp_cb_offload;
-	struct nfs4_client *clp = copy->cp_clp;
+	struct nfs4_client *clp;
 
 	/*
-	 * cp_clp is NULL when called via nfsd4_shutdown_copy() during
-	 * client destruction. Skip the callback; the client is gone.
+	 * Pairs with smp_store_release(&copy->cp_clp) in find_async_copy()
+	 * and nfsd4_unhash_copy(). set_bit/clear_bit are unordered atomics
+	 * (Documentation/atomic_bitops.rst), so the acquire is needed to
+	 * prevent the cp_clp load being reordered past the clp dereference
+	 * below on weakly-ordered hardware. The kthread holds its own
+	 * reference across this call (taken before wake_up_process in
+	 * nfsd4_copy()); see commit log for per-path client lifetime.
+	 *
+	 * cp_clp is NULL when the copy was canceled (find_async_copy,
+	 * nfsd4_unhash_copy) before the kthread reached this point. Skip
+	 * the callback; the canceling path owns the notification.
 	 */
+	clp = smp_load_acquire(&copy->cp_clp);
 	if (!clp) {
 		set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_flags);
 		return;
@@ -2095,10 +2136,16 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 	cbo->co_retries = 5;
 
 	/*
-	 * Hold a reference on the client while the callback is in flight.
-	 * Released in nfsd4_cb_offload_release().
+	 * Hold a reference on the client and on the copy while the callback
+	 * is in flight. co_cb is embedded in the copy, so the copy must
+	 * outlive the callback; a concurrent OFFLOAD_CANCEL or shutdown can
+	 * otherwise drop the last copy reference and free it while the RPC
+	 * layer still references co_cb. Both are released in
+	 * nfsd4_cb_offload_release(). The kthread still holds its own copy
+	 * reference here, so this refcount_inc() cannot race the final free.
 	 */
 	kref_get(&clp->cl_nfsdfs.cl_ref);
+	refcount_inc(&copy->refcount);
 
 	nfsd4_init_cb(&cbo->co_cb, clp, &nfsd4_cb_offload_ops,
 		      NFSPROC4_CLNT_CB_OFFLOAD);
@@ -2150,16 +2197,27 @@ static int nfsd4_do_async_copy(void *data)
 do_callback:
 	if (!test_bit(NFSD4_COPY_F_CB_ERROR, &copy->cp_flags))
 		copy->nfserr = nfserr;
-	/* The kthread exits forthwith. Ensure that a subsequent
-	 * OFFLOAD_CANCEL won't try to kill it again. */
-	set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags);
-
+	/*
+	 * Do not set NFSD4_COPY_F_STOPPED here. That bit tells a teardown
+	 * caller it may skip kthread_stop(); setting it before the kthread
+	 * is done using copy->nf_dst and the client lets a concurrent
+	 * nfsd4_stop_copy() release those resources out from under us.
+	 * STOPPED is now set only by nfsd4_stop_copy(), which always joins
+	 * via kthread_stop().
+	 */
 	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
 	trace_nfsd_copy_async_done(copy);
 	atomic_dec(&copy->cp_nn->pending_async_copies);
 	if (copy->cp_res.wr_bytes_written > 0 && copy->attr_update)
 		nfsd_update_cmtime_attr(copy->nf_dst->nf_file, 0);
 	nfsd4_send_cb_offload(copy);
+	/*
+	 * Drop the kthread's own reference (taken before
+	 * wake_up_process() in nfsd4_copy()). After this point, copy
+	 * may be freed by a concurrent teardown caller's pending
+	 * nfs4_put_copy().
+	 */
+	nfs4_put_copy(copy);
 	return 0;
 }
 
@@ -2198,6 +2256,8 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
 		sizeof(struct knfsd_fh));
 	if (nfsd4_copy_is_async(copy)) {
+		struct task_struct *task;
+
 		async_copy = kzalloc_obj(struct nfsd4_copy);
 		if (!async_copy)
 			goto out_err;
@@ -2225,15 +2285,29 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		       NFS4_MAX_SESSIONID_LEN);
 		async_copy->cp_cb_offload.co_referring_slotid = cstate->slot->sl_index;
 		async_copy->cp_cb_offload.co_referring_seqno = cstate->slot->sl_seqid;
-		async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
-				async_copy, "%s", "copy thread");
-		if (IS_ERR(async_copy->copy_task))
+		task = kthread_create(nfsd4_do_async_copy, async_copy,
+				      "%s", "copy thread");
+		if (IS_ERR(task))
 			goto out_dec_async_copy_err;
+		/*
+		 * Pin the task_struct so a later nfsd4_stop_copy() can call
+		 * kthread_stop() safely even after this one-shot kthread has
+		 * exited and been reaped. Released by nfs4_put_copy().
+		 */
+		get_task_struct(task);
+		async_copy->copy_task = task;
+		/*
+		 * Take the kthread's reference and wake it before publishing
+		 * on async_copies, so the publisher does not touch async_copy
+		 * after spin_unlock and a concurrent teardown caller can drain
+		 * all remaining references safely. See commit log for details.
+		 */
+		refcount_inc(&async_copy->refcount);
+		wake_up_process(async_copy->copy_task);
 		spin_lock(&async_copy->cp_clp->async_lock);
 		list_add(&async_copy->copies,
 				&async_copy->cp_clp->async_copies);
 		spin_unlock(&async_copy->cp_clp->async_lock);
-		wake_up_process(async_copy->copy_task);
 		status = nfs_ok;
 	} else {
 		status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
@@ -2287,8 +2361,19 @@ find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
 
 	spin_lock(&clp->async_lock);
 	copy = find_async_copy_locked(clp, stateid);
-	if (copy)
+	if (copy) {
 		refcount_inc(&copy->refcount);
+		/*
+		 * Mirror nfsd4_unhash_copy(): unlink and clear cp_clp under
+		 * async_lock so the reaper cannot reach the copy. Caller drops
+		 * the list-membership reference via nfs4_put_copy() after
+		 * nfsd4_stop_copy(). smp_store_release() pairs with
+		 * smp_load_acquire() in nfsd4_send_cb_offload().
+		 */
+		smp_store_release(&copy->cp_clp, NULL);
+		if (!list_empty(&copy->copies))
+			list_del_init(&copy->copies);
+	}
 	spin_unlock(&clp->async_lock);
 	return copy;
 }
@@ -2307,8 +2392,16 @@ nfsd4_offload_cancel(struct svc_rqst *rqstp,
 		struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
 		return manage_cpntf_state(nn, &os->stateid, clp, NULL);
-	} else
+	} else {
 		nfsd4_stop_copy(copy);
+		/*
+		 * find_async_copy() removed the copy from
+		 * clp->async_copies, so the reaper can no longer
+		 * reach it and drop the list-membership reference
+		 * via cleanup_async_copy(). Drop it here.
+		 */
+		nfs4_put_copy(copy);
+	}
 
 	return nfs_ok;
 }
Signature · 1 line
2.55.0
author maintainer
#3 lore raw

PATCH v2 3/10 nfsd: fix stale s2s_cp_stateids IDR entry for async COPY

In reply to Jeff Layton

For an async COPY, nfsd4_copy() called nfs4_init_copy_state() before dup_copy_fields(), so the s2s_cp_stateids IDR was pointed at &u->copy->cp_stateid -- memory in the per-rqstp COMPOUND buffer that is reused by the next request. dup_copy_fields() copies only the value into async_copy, so the IDR slot dangled at the transient buffer for the whole background copy. Any IDR walker then dereferences reused request memory:
the laundromat reads cs_type from it and, if the bytes look like an expired NFS4_COPYNOTIFY_STID, follows into refcount_dec()/idr_remove()/kfree() on garbage; manage_cpntf_state() has the same exposure via idr_find().

Duplicate the fields first, then register the stateid on the stable async_copy. result->cb_stateid is unchanged.

Fixes e0639dc5805a ("NFSD introduce async copy feature")
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
fs/nfsd/nfs4proc.c +13 -3
index fad01d67bf3f..1c674479d4ca 100644
@@ -2272,11 +2272,21 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		async_copy->cp_src = kmalloc_obj(*async_copy->cp_src);
 		if (!async_copy->cp_src)
 			goto out_dec_async_copy_err;
-		if (!nfs4_init_copy_state(nn, copy))
+		dup_copy_fields(copy, async_copy);
+		/*
+		 * Register the copy stateid on the long-lived async_copy
+		 * rather than on the transient COMPOUND argument buffer
+		 * (&u->copy). nfs4_init_copy_state() installs a pointer to
+		 * the copy_stateid_t in nn->s2s_cp_stateids, and that pointer
+		 * outlives this call (it is removed only when the background
+		 * copy finishes). Pointing it at &u->copy would leave a stale
+		 * pointer into reused request memory that the laundromat and
+		 * OFFLOAD_CANCEL later dereference.
+		 */
+		if (!nfs4_init_copy_state(nn, async_copy))
 			goto out_dec_async_copy_err;
-		memcpy(&result->cb_stateid, &copy->cp_stateid.cs_stid,
+		memcpy(&result->cb_stateid, &async_copy->cp_stateid.cs_stid,
 			sizeof(result->cb_stateid));
-		dup_copy_fields(copy, async_copy);
 		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
 			       FMODE_NOCMTIME) != 0)
 			async_copy->attr_update = true;
Signature · 1 line
2.55.0
maintainer
#4 lore raw

Re: [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY

Quoted 50 lines · Jeff Layton

For an async COPY, nfsd4_copy() called nfs4_init_copy_state() before dup_copy_fields(), so the s2s_cp_stateids IDR was pointed at &u->copy->cp_stateid -- memory in the per-rqstp COMPOUND buffer that is reused by the next request. dup_copy_fields() copies only the value into async_copy, so the IDR slot dangled at the transient buffer for the whole background copy. Any IDR walker then dereferences reused request memory:
the laundromat reads cs_type from it and, if the bytes look like an expired NFS4_COPYNOTIFY_STID, follows into refcount_dec()/idr_remove()/kfree() on garbage; manage_cpntf_state() has the same exposure via idr_find().

Duplicate the fields first, then register the stateid on the stable async_copy. result->cb_stateid is unchanged.

Fixes e0639dc5805a ("NFSD introduce async copy feature")
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
fs/nfsd/nfs4proc.c +13 -3
index fad01d67bf3f..1c674479d4ca 100644
@@ -2272,11 +2272,21 @@ nfsd4_copy(struct svc_rqst *rqstp, struct 
 nfsd4_compound_state *cstate,
 		async_copy->cp_src = kmalloc_obj(*async_copy->cp_src);
 		if (!async_copy->cp_src)
 			goto out_dec_async_copy_err;
-		if (!nfs4_init_copy_state(nn, copy))
+		dup_copy_fields(copy, async_copy);
+		/*
+		 * Register the copy stateid on the long-lived async_copy
+		 * rather than on the transient COMPOUND argument buffer
+		 * (&u->copy). nfs4_init_copy_state() installs a pointer to
+		 * the copy_stateid_t in nn->s2s_cp_stateids, and that pointer
+		 * outlives this call (it is removed only when the background
+		 * copy finishes). Pointing it at &u->copy would leave a stale
+		 * pointer into reused request memory that the laundromat and
+		 * OFFLOAD_CANCEL later dereference.
+		 */
+		if (!nfs4_init_copy_state(nn, async_copy))
 			goto out_dec_async_copy_err;
-		memcpy(&result->cb_stateid, &copy->cp_stateid.cs_stid,
+		memcpy(&result->cb_stateid, &async_copy->cp_stateid.cs_stid,
 			sizeof(result->cb_stateid));
-		dup_copy_fields(copy, async_copy);
 		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
 			       FMODE_NOCMTIME) != 0)
Quoted 4 lines

async_copy->attr_update = true;

Signature · 1 line
2.55.0

Sashiko spotted some severe issues:

- [Critical] Async copy cancellation paths remove the copy from clp->async_copies and free the async_copy object, but fail to call nfs4_free_copy_state(). This leaks the IDR entry in nn->s2s_cp_stateids and leaves it pointing to freed memory, causing a remote Use-After-Free (UAF) DoS.
- [High] Reordering dup_copy_fields() before nfs4_init_copy_state() leaves async_copy->cp_res.cb_stateid uninitialized, causing the server to send an invalid (all-zero) stateid in the CB_OFFLOAD callback.

The second one was also spotted by gpt-5.6-sol.

The new code comment here is a design breadcrumb. Probably not useful to carry it as part of the code, but YMMV.

Signature · 1 line
Chuck Lever
author maintainer
#5 lore raw

PATCH v2 4/10 nfsd: initialize copy-notify stateid before publishing it

In reply to Jeff Layton

nfsd4_copy_notify() finished initializing the cpntf state after nfs4_alloc_init_cpntf_state() had already linked it into the s2s_cp_stateids IDR and the parent's sc_cp_list, with cs_count == 1 (the membership reference) and none held for the caller. A racing OFFLOAD_CANCEL (crafted cl_id == nn->s2s_cp_cl_id plus the guessable so_id) could reach manage_cpntf_state() and free the entry, turning the caller's subsequent cpn_cnr_stateid read and cp_p_stateid/cp_p_clid writes into use-after-free. The owning clientid was also only recorded after publication, so it could not gate an ownership check in that window.

Record cp_p_stateid and cp_p_clid inside nfs4_alloc_init_cpntf_state() before nfs4_init_cp_state() publishes the entry, and return it with an extra reference. The caller reads the stateid under that reference and drops it with nfs4_put_cpntf_state(); on a late error the laundromat reaps the entry.

Fixes 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c  | 19 ++++++++++++-------
 fs/nfsd/nfs4state.c | 14 +++++++++++++-
 2 files changed, 25 insertions(+), 8 deletions(-)
fs/nfsd/nfs4proc.c +12 -7
index 1c674479d4ca..2dbc99e76837 100644
@@ -2425,7 +2425,6 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 	struct nfs4_stid *stid = NULL;
 	struct nfs4_cpntf_state *cps;
-	struct nfs4_client *clp = cstate->clp;
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					&cn->cpn_src_stateid, RD_STATE, NULL,
@@ -2439,12 +2438,15 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	cn->cpn_lease_time.tv_nsec = 0;
 
 	status = nfserrno(-ENOMEM);
+	/*
+	 * The returned cps is already published in s2s_cp_stateids and
+	 * fully initialized (including cp_p_stateid/cp_p_clid). It carries
+	 * an extra reference for us; drop it once we are done touching cps.
+	 */
 	cps = nfs4_alloc_init_cpntf_state(nn, stid);
 	if (!cps)
 		goto out;
 	memcpy(&cn->cpn_cnr_stateid, &cps->cp_stateid.cs_stid, sizeof(stateid_t));
-	memcpy(&cps->cp_p_stateid, &stid->sc_stateid, sizeof(stateid_t));
-	memcpy(&cps->cp_p_clid, &clp->cl_clientid, sizeof(clientid_t));
 
 	/* For now, only return one server address in cpn_src, the
 	 * address used by the client to connect to this server.
@@ -2453,10 +2455,13 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	status = nfsd4_set_netaddr((struct sockaddr *)&rqstp->rq_daddr,
 				 &cn->cpn_src->u.nl4_addr);
 	WARN_ON_ONCE(status);
-	if (status) {
-		nfs4_put_cpntf_state(nn, cps);
-		goto out;
-	}
+	/*
+	 * Drop the extra reference taken by nfs4_alloc_init_cpntf_state().
+	 * On success the entry stays on the parent's sc_cp_list for a later
+	 * inter-server READ; if status is set the client discards it and the
+	 * laundromat reaps it once it expires.
+	 */
+	nfs4_put_cpntf_state(nn, cps);
 out:
 	nfs4_put_stid(stid);
 	return status;
fs/nfsd/nfs4state.c +13 -1
index b8946db3ebaa..1ef015cbc055 100644
@@ -1002,7 +1002,19 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	 */
 	INIT_LIST_HEAD(&cps->cp_list);
 	cps->cpntf_time = ktime_get_boottime_seconds();
-	refcount_set(&cps->cp_stateid.cs_count, 1);
+	/*
+	 * Record the parent stateid and the owning clientid before
+	 * nfs4_init_cp_state() publishes the entry in s2s_cp_stateids.
+	 * Once published, a concurrent OFFLOAD_CANCEL can find and free
+	 * the entry, so it must be fully initialized first. Take an extra
+	 * reference for the caller (released with nfs4_put_cpntf_state())
+	 * so the returned object stays alive while the caller reads back
+	 * the stateid.
+	 */
+	memcpy(&cps->cp_p_stateid, &p_stid->sc_stateid, sizeof(stateid_t));
+	memcpy(&cps->cp_p_clid, &p_stid->sc_client->cl_clientid,
+	       sizeof(clientid_t));
+	refcount_set(&cps->cp_stateid.cs_count, 2);
 	if (!nfs4_init_cp_state(nn, &cps->cp_stateid, NFS4_COPYNOTIFY_STID,
 				p_stid))
 		goto out_free;
Signature · 1 line
2.55.0
author maintainer
#6 lore raw

PATCH v2 5/10 nfsd: check client ownership when cancelling a copy-notify stateid

In reply to Jeff Layton

On the OFFLOAD_CANCEL path (clp != NULL), manage_cpntf_state() freed the target cpntf state without checking ownership. The lookup key st->si_opaque.so_id is allocated cyclically (guessable) and the embedded clientid is the fixed per-net nn->s2s_cp_cl_id, so any authenticated NFSv4.2 client could cancel and free another client's copy-notify stateid.

Compare the creating clientid recorded in state->cp_p_clid against the requesting client's cl_clientid and return nfserr_bad_stateid on a mismatch instead of freeing the entry.

Fixes ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4state.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +14 -2
index 1ef015cbc055..76c0e08711df 100644
@@ -7916,10 +7916,22 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 			state = NULL;
 			goto unlock;
 		}
-		if (!clp)
+		if (!clp) {
 			refcount_inc(&state->cp_stateid.cs_count);
-		else
+		} else if (memcmp(&clp->cl_clientid, &state->cp_p_clid,
+				  sizeof(clientid_t))) {
+			/*
+			 * OFFLOAD_CANCEL: only the client that created the
+			 * copy-notify stateid may cancel it. so_id values are
+			 * cyclic and guessable, so without this check any
+			 * authenticated client could free another client's
+			 * cpntf state.
+			 */
+			state = NULL;
+			goto unlock;
+		} else {
 			_free_cpntf_state_locked(nn, state);
+		}
 	}
 unlock:
 	spin_unlock(&nn->s2s_cp_lock);
Signature · 1 line
2.55.0
author maintainer
#7 lore raw

PATCH v2 6/10 nfsd: revoke copy-notify stateids before dropping their reference

In reply to Jeff Layton

Copy-notify stateids live in the s2s_cp_stateids IDR and on their parent stid's sc_cp_list, pinned by a single membership reference. _free_cpntf_state_locked() only unlinks an entry once its refcount reaches zero, so any revoke path that runs while a concurrent find_cpntf_state()/manage_cpntf_state() holder has elevated cs_count drops the reference without unlinking, leaving the entry discoverable with its membership reference already consumed. A second revoke or a laundromat tick then frees it while the reader still holds the pointer -- a KASAN-detectable use-after-free at the reader's nfs4_put_cpntf_state().

This affected all three revoke paths:

- The parent-stid drain (nfs4_free_cpntf_statelist()) repeatedly called _free_cpntf_state_locked() on the first list entry; a holder that had bumped cs_count made it return early, so the next iteration re-decremented and burned the holder's reference.

- OFFLOAD_CANCEL (manage_cpntf_state()) and laundromat expiry likewise

    used _free_cpntf_state_locked() and could drop 2->1 without unlinking.

Add revoke_cpntf_state_locked(), which unhashes the entry from the IDR and sc_cp_list first (deferring the final free to any holder), and use it from all three revoke paths. The drain now walks with list_for_each_entry_safe() and revokes each entry unconditionally, so it terminates in one pass per entry regardless of cs_count. The unhash is gated on !list_empty(&cps->cp_list); the idr_remove() gate matters because idr_alloc_cyclic() may have recycled the so_id by then. Keep _free_cpntf_state_locked() for the reference-holder put path only, where a concurrent revoke may already have unlinked the entry (its list_del_init() then a no-op).

Fixes 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Assisted-by Claude:claude-opus-4-7
Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4state.c | 93 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 77 insertions(+), 16 deletions(-)
fs/nfsd/nfs4state.c +77 -16
index 76c0e08711df..3f58c729edbf 100644
@@ -1037,18 +1037,81 @@ void nfs4_free_copy_state(struct nfsd4_copy *copy)
 	spin_unlock(&nn->s2s_cp_lock);
 }
 
+/*
+ * Drop the parent stid's reference on a cpntf entry that has already been
+ * removed from sc_cp_list and the s2s_cp_stateids IDR. If a concurrent holder
+ * still owns a reference (acquired viamanage_cpntf_state() before the unlink),
+ * that holder's nfs4_put_cpntf_state() will perform the final free.
+ *
+ * The nn->s2s_cp_lock must be held!
+ */
+static void put_cpntf_state_unlinked_locked(struct nfs4_cpntf_state *cps)
+{
+	WARN_ON_ONCE(cps->cp_stateid.cs_type != NFS4_COPYNOTIFY_STID);
+	WARN_ON_ONCE(!list_empty(&cps->cp_list));
+
+	if (refcount_dec_and_test(&cps->cp_stateid.cs_count))
+		kfree(cps);
+}
+
+/*
+ * Unhash the stateid from the s2s stateid hash, and detach it from the sc_cp_list.
+ * Note that this is gated on a list_empty() check, to avoid problems with IDR
+ * hashval reuse.
+ */
+static void nfsd4_unhash_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
+{
+	lockdep_assert_held(&nn->s2s_cp_lock);
+
+	if (!list_empty(&cps->cp_list)) {
+		list_del_init(&cps->cp_list);
+		idr_remove(&nn->s2s_cp_stateids, cps->cp_stateid.cs_stid.si_opaque.so_id);
+	}
+}
+
+/*
+ * Revoke a copy-notify stateid. Unlink it from the s2s_cp_stateids IDR and
+ * its parent's sc_cp_list first, so no new finder (OFFLOAD_CANCEL, laundromat,
+ * or find_cpntf_state()) can discover it, then drop the membership reference.
+ *
+ * This must be used by every revoke path (cancel, laundromat expiry, drain)
+ * instead of _free_cpntf_state_locked(): the latter only unlinks once the
+ * refcount reaches zero, so revoking while a concurrent reader holds a
+ * reference would leave the entry discoverable with its membership reference
+ * already consumed, allowing a second revoke to over-decrement and free it
+ * out from under the reader.
+ *
+ * If a concurrent holder still owns a reference (e.g. acquired via
+ * find_cpntf_state()), its nfs4_put_cpntf_state() performs the final free.
+ *
+ * The nn->s2s_cp_lock must be held!
+ */
+static void revoke_cpntf_state_locked(struct nfsd_net *nn,
+				      struct nfs4_cpntf_state *cps)
+{
+	nfsd4_unhash_cpntf_state(nn, cps);
+	put_cpntf_state_unlinked_locked(cps);
+}
+
 static void nfs4_free_cpntf_statelist(struct net *net, struct nfs4_stid *stid)
 {
-	struct nfs4_cpntf_state *cps;
+	struct nfs4_cpntf_state *cps, *tmp;
 	struct nfsd_net *nn;
 
 	nn = net_generic(net, nfsd_net_id);
 	spin_lock(&nn->s2s_cp_lock);
-	while (!list_empty(&stid->sc_cp_list)) {
-		cps = list_first_entry(&stid->sc_cp_list,
-				       struct nfs4_cpntf_state, cp_list);
-		_free_cpntf_state_locked(nn, cps);
-	}
+	/*
+	 * Unlink every entry from sc_cp_list and the IDR before dropping
+	 * the parent's reference.  This makes the drain terminate in one
+	 * pass per entry regardless of cs_count: a concurrent holder that
+	 * obtained the entry via manage_cpntf_state() retains its own
+	 * reference, and its eventual nfs4_put_cpntf_state() will see the
+	 * entry already unlinked (list_del_init() in
+	 * _free_cpntf_state_locked makes that a no-op) and drive the final
+	 * kfree itself.
+	 */
+	list_for_each_entry_safe(cps, tmp, &stid->sc_cp_list, cp_list)
+		revoke_cpntf_state_locked(nn, cps);
 	spin_unlock(&nn->s2s_cp_lock);
 }
 
@@ -7495,7 +7558,7 @@ nfs4_laundromat(struct nfsd_net *nn)
 		cps = container_of(cps_t, struct nfs4_cpntf_state, cp_stateid);
 		if (cps->cp_stateid.cs_type == NFS4_COPYNOTIFY_STID &&
 				state_expired(&lt, cps->cpntf_time))
-			_free_cpntf_state_locked(nn, cps);
+			revoke_cpntf_state_locked(nn, cps);
 	}
 	spin_unlock(&nn->s2s_cp_lock);
 	nfsd4_async_copy_reaper(nn);
@@ -7882,16 +7945,14 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s,
 out:
 	return status;
 }
-static void
-_free_cpntf_state_locked(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
+
+static void _free_cpntf_state_locked(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
 {
 	WARN_ON_ONCE(cps->cp_stateid.cs_type != NFS4_COPYNOTIFY_STID);
-	if (!refcount_dec_and_test(&cps->cp_stateid.cs_count))
-		return;
-	list_del_init(&cps->cp_list);
-	idr_remove(&nn->s2s_cp_stateids,
-		   cps->cp_stateid.cs_stid.si_opaque.so_id);
-	kfree(cps);
+	if (refcount_dec_and_test(&cps->cp_stateid.cs_count)) {
+		nfsd4_unhash_cpntf_state(nn, cps);
+		kfree(cps);
+	}
 }
 /*
  * A READ from an inter server to server COPY will have a
@@ -7930,7 +7991,7 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 			state = NULL;
 			goto unlock;
 		} else {
-			_free_cpntf_state_locked(nn, state);
+			revoke_cpntf_state_locked(nn, state);
 		}
 	}
 unlock:
Signature · 1 line
2.55.0
author maintainer
#8 lore raw

PATCH v2 7/10 nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types

In reply to Jeff Layton

nfsd4_decode_nl4_server() handled only NL4_NETADDR and returned nfserr_bad_xdr for NL4_NAME and NL4_URL. Those forms are well-formed XDR, so BADXDR is misleading -- the request is unsupported, not malformed.

Decode and discard the utf8str_cis for NL4_NAME and NL4_URL to keep the stream consistent, and return nfserr_notsupp. nfsd4_proc_compound() honors a decode-time op->status, so the op fails without executing.

Fixes 84e1b21d5ec4 ("NFSD add ca_source_server<> to COPY")
---
 fs/nfsd/nfs4xdr.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
fs/nfsd/nfs4xdr.c +15 -0
index aef48fb0fac2..23f50a947bf1 100644
@@ -2123,6 +2123,7 @@ static __be32 nfsd4_decode_nl4_server(struct nfsd4_compoundargs *argp,
 {
 	struct nfs42_netaddr *naddr;
 	__be32 *p;
+	u32 str_len;
 
 	if (xdr_stream_decode_u32(argp->xdr, &ns->nl4_type) < 0)
 		return nfserr_bad_xdr;
@@ -2152,6 +2153,20 @@ static __be32 nfsd4_decode_nl4_server(struct nfsd4_compoundargs *argp,
 			return nfserr_bad_xdr;
 		memcpy(naddr->addr, p, naddr->addr_len);
 		break;
+	case NL4_NAME:
+	case NL4_URL:
+		/*
+		 * A netloc name or URL is well-formed XDR, but this server
+		 * only supports NL4_NETADDR.  Consume the utf8str_cis so the
+		 * stream stays aligned for any following operations, then
+		 * reject the operation with NFS4ERR_NOTSUPP rather than the
+		 * misleading NFS4ERR_BADXDR.
+		 */
+		if (xdr_stream_decode_u32(argp->xdr, &str_len) < 0)
+			return nfserr_bad_xdr;
+		if (!xdr_inline_decode(argp->xdr, str_len))
+			return nfserr_bad_xdr;
+		return nfserr_notsupp;
 	default:
 		return nfserr_bad_xdr;
 	}
Signature · 1 line
2.55.0
author maintainer
#9 lore raw

PATCH v2 8/10 nfsd: split nfsd4_copy into transient and durable async copy objects

In reply to Jeff Layton

struct nfsd4_copy served two roles: as &u->copy it is a transient per-COMPOUND argument in the request buffer; as the heap async_copy it is a durable object (worker kthread, reaper linkage, CB_OFFLOAD callback, IDR stateid) that outlives the COMPOUND, with dup_copy_fields() shuttling state between them. That dual identity was the root of the recent lifetime bugs.

Introduce struct nfsd4_async_copy for the durable object. It embeds a struct nfsd4_copy (cp_copy) for the operation parameters/result and adds the durable-only fields: async_copies linkage, task_struct, refcount, reaper TTL, copy stateid, and CB_OFFLOAD callback. The durable object therefore never points into the request buffer. cp_clp stays in nfsd4_copy -- it is a request property read by the sync-copy tracepoints on the transient object.

Mechanical split, no intended behavioral change; a step toward folding the copy stateids into the common nfs4_stid infrastructure.

Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c  | 136 ++++++++++++++++++++++++++--------------------------
 fs/nfsd/nfs4state.c |   6 +--
 fs/nfsd/state.h     |   5 +-
 fs/nfsd/xdr4.h      |  33 +++++++++----
 4 files changed, 97 insertions(+), 83 deletions(-)
fs/nfsd/nfs4proc.c +68 -68
index 2dbc99e76837..7d44c85335c7 100644
@@ -57,7 +57,7 @@ module_param(inter_copy_offload_enable, bool, 0644);
 MODULE_PARM_DESC(inter_copy_offload_enable,
 		 "Enable inter server to server copy offload. Default: false");
 
-static void cleanup_async_copy(struct nfsd4_copy *copy);
+static void cleanup_async_copy(struct nfsd4_async_copy *copy);
 
 #ifdef CONFIG_NFSD_V4_2_INTER_SSC
 static int nfsd4_ssc_umount_timeout = 900000;		/* default to 15 mins */
@@ -1465,13 +1465,13 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
  */
 bool nfsd4_has_active_async_copies(struct nfs4_client *clp)
 {
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 	bool result = false;
 
 	spin_lock(&clp->async_lock);
 	list_for_each_entry(copy, &clp->async_copies, copies) {
-		if (!test_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags) &&
-		    !test_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags)) {
+		if (!test_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_copy.cp_flags) &&
+		    !test_bit(NFSD4_COPY_F_STOPPED, &copy->cp_copy.cp_flags)) {
 			result = true;
 			break;
 		}
@@ -1487,7 +1487,7 @@ bool nfsd4_has_active_async_copies(struct nfs4_client *clp)
 void nfsd4_async_copy_reaper(struct nfsd_net *nn)
 {
 	struct nfs4_client *clp;
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 	LIST_HEAD(reaplist);
 
 	spin_lock(&nn->client_lock);
@@ -1496,8 +1496,9 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
 
 		spin_lock(&clp->async_lock);
 		list_for_each_safe(pos, next, &clp->async_copies) {
-			copy = list_entry(pos, struct nfsd4_copy, copies);
-			if (test_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_flags)) {
+			copy = list_entry(pos, struct nfsd4_async_copy, copies);
+			if (test_bit(NFSD4_COPY_F_OFFLOAD_DONE,
+				     &copy->cp_copy.cp_flags)) {
 				if (!--copy->cp_ttl) {
 					list_del_init(&copy->copies);
 					list_add(&copy->copies, &reaplist);
@@ -1509,31 +1510,29 @@ void nfsd4_async_copy_reaper(struct nfsd_net *nn)
 	spin_unlock(&nn->client_lock);
 
 	while (!list_empty(&reaplist)) {
-		copy = list_first_entry(&reaplist, struct nfsd4_copy, copies);
+		copy = list_first_entry(&reaplist, struct nfsd4_async_copy,
+					copies);
 		list_del_init(&copy->copies);
 		cleanup_async_copy(copy);
 	}
 }
 
-static void nfs4_put_copy(struct nfsd4_copy *copy)
+static void nfs4_put_copy(struct nfsd4_async_copy *copy)
 {
 	if (!refcount_dec_and_test(&copy->refcount))
 		return;
-	/*
-	 * Drop the task_struct reference taken in nfsd4_copy(). Only async
-	 * copies have a copy_task; it is left NULL on every other path.
-	 */
+	/* Drop the task_struct reference taken in nfsd4_copy(). */
 	if (copy->copy_task)
 		put_task_struct(copy->copy_task);
-	kfree(copy->cp_src);
+	kfree(copy->cp_copy.cp_src);
 	kfree(copy);
 }
 
 static void release_copy_files(struct nfsd4_copy *copy);
 
-static void nfsd4_stop_copy(struct nfsd4_copy *copy)
+static void nfsd4_stop_copy(struct nfsd4_async_copy *copy)
 {
-	trace_nfsd_copy_async_cancel(copy);
+	trace_nfsd_copy_async_cancel(&copy->cp_copy);
 	/*
 	 * Always join the copy kthread before touching its resources. The
 	 * task_struct is pinned by get_task_struct() in nfsd4_copy(), so
@@ -1545,30 +1544,30 @@ static void nfsd4_stop_copy(struct nfsd4_copy *copy)
 	 * is the sole owner of this teardown, so kthread_stop() runs exactly
 	 * once per copy.
 	 */
-	set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags);
+	set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_copy.cp_flags);
 	kthread_stop(copy->copy_task);
-	if (!test_bit(NFSD4_COPY_F_CB_ERROR, &copy->cp_flags))
-		copy->nfserr = nfs_ok;
-	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
+	if (!test_bit(NFSD4_COPY_F_CB_ERROR, &copy->cp_copy.cp_flags))
+		copy->cp_copy.nfserr = nfs_ok;
+	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_copy.cp_flags);
 
-	release_copy_files(copy);
+	release_copy_files(&copy->cp_copy);
 	nfs4_put_copy(copy);
 }
 
-static struct nfsd4_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
+static struct nfsd4_async_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
 {
-	struct nfsd4_copy *copy = NULL;
+	struct nfsd4_async_copy *copy = NULL;
 
 	spin_lock(&clp->async_lock);
 	if (!list_empty(&clp->async_copies)) {
-		copy = list_first_entry(&clp->async_copies, struct nfsd4_copy,
-					copies);
+		copy = list_first_entry(&clp->async_copies,
+					struct nfsd4_async_copy, copies);
 		refcount_inc(&copy->refcount);
 		/*
 		 * Pairs with smp_load_acquire in nfsd4_send_cb_offload();
 		 * see find_async_copy() for rationale.
 		 */
-		smp_store_release(&copy->cp_clp, NULL);
+		smp_store_release(&copy->cp_copy.cp_clp, NULL);
 		if (!list_empty(&copy->copies))
 			list_del_init(&copy->copies);
 	}
@@ -1578,7 +1577,7 @@ static struct nfsd4_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
 
 void nfsd4_shutdown_copy(struct nfs4_client *clp)
 {
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 
 	while ((copy = nfsd4_unhash_copy(clp)) != NULL) {
 		nfsd4_stop_copy(copy);
@@ -1616,7 +1615,7 @@ static bool nfsd4_copy_on_sb(const struct nfsd4_copy *copy,
 void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 {
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
-	struct nfsd4_copy *copy, *tmp;
+	struct nfsd4_async_copy *copy, *tmp;
 	struct nfs4_client *clp;
 	unsigned int idhashval;
 	LIST_HEAD(to_cancel);
@@ -1630,7 +1629,7 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 			spin_lock(&clp->async_lock);
 			list_for_each_entry_safe(copy, tmp,
 						 &clp->async_copies, copies) {
-				if (nfsd4_copy_on_sb(copy, sb)) {
+				if (nfsd4_copy_on_sb(&copy->cp_copy, sb)) {
 					refcount_inc(&copy->refcount);
 					/*
 					 * Hold a reference on the client while
@@ -1642,9 +1641,9 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 					 * survive callback flight.
 					 */
 					kref_get(&clp->cl_nfsdfs.cl_ref);
-					copy->nfserr = nfserr_admin_revoked;
+					copy->cp_copy.nfserr = nfserr_admin_revoked;
 					set_bit(NFSD4_COPY_F_CB_ERROR,
-						&copy->cp_flags);
+						&copy->cp_copy.cp_flags);
 					list_move(&copy->copies, &to_cancel);
 				}
 			}
@@ -1654,7 +1653,7 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 	spin_unlock(&nn->client_lock);
 
 	list_for_each_entry_safe(copy, tmp, &to_cancel, copies) {
-		struct nfs4_client *clp = copy->cp_clp;
+		struct nfs4_client *clp = copy->cp_copy.cp_clp;
 
 		list_del_init(&copy->copies);
 		nfsd4_stop_copy(copy);
@@ -1955,10 +1954,10 @@ static void nfsd4_cb_offload_release(struct nfsd4_callback *cb)
 {
 	struct nfsd4_cb_offload *cbo =
 		container_of(cb, struct nfsd4_cb_offload, co_cb);
-	struct nfsd4_copy *copy =
-		container_of(cbo, struct nfsd4_copy, cp_cb_offload);
+	struct nfsd4_async_copy *copy =
+		container_of(cbo, struct nfsd4_async_copy, cp_cb_offload);
 
-	set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_flags);
+	set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_copy.cp_flags);
 	nfsd4_put_client(cb->cb_clp);
 	/* Drop the copy reference taken in nfsd4_send_cb_offload(). */
 	nfs4_put_copy(copy);
@@ -2074,7 +2073,6 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst)
 	if (!nfsd4_ssc_is_inter(src))
 		dst->nf_src = nfsd_file_get(src->nf_src);
 
-	memcpy(&dst->cp_stateid, &src->cp_stateid, sizeof(src->cp_stateid));
 	memcpy(dst->cp_src, src->cp_src, sizeof(struct nl4_server));
 	memcpy(&dst->stateid, &src->stateid, sizeof(src->stateid));
 	memcpy(&dst->c_fh, &src->c_fh, sizeof(src->c_fh));
@@ -2099,14 +2097,14 @@ static void release_copy_files(struct nfsd4_copy *copy)
  * was never list_add'd). In both cases the copy is unreachable from
  * clp->async_copies.
  */
-static void cleanup_async_copy(struct nfsd4_copy *copy)
+static void cleanup_async_copy(struct nfsd4_async_copy *copy)
 {
 	nfs4_free_copy_state(copy);
-	release_copy_files(copy);
+	release_copy_files(&copy->cp_copy);
 	nfs4_put_copy(copy);
 }
 
-static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
+static void nfsd4_send_cb_offload(struct nfsd4_async_copy *copy)
 {
 	struct nfsd4_cb_offload *cbo = &copy->cp_cb_offload;
 	struct nfs4_client *clp;
@@ -2124,15 +2122,15 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 	 * nfsd4_unhash_copy) before the kthread reached this point. Skip
 	 * the callback; the canceling path owns the notification.
 	 */
-	clp = smp_load_acquire(&copy->cp_clp);
+	clp = smp_load_acquire(&copy->cp_copy.cp_clp);
 	if (!clp) {
-		set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_flags);
+		set_bit(NFSD4_COPY_F_OFFLOAD_DONE, &copy->cp_copy.cp_flags);
 		return;
 	}
 
-	memcpy(&cbo->co_res, &copy->cp_res, sizeof(copy->cp_res));
-	memcpy(&cbo->co_fh, &copy->fh, sizeof(copy->fh));
-	cbo->co_nfserr = copy->nfserr;
+	memcpy(&cbo->co_res, &copy->cp_copy.cp_res, sizeof(copy->cp_copy.cp_res));
+	memcpy(&cbo->co_fh, &copy->cp_copy.fh, sizeof(copy->cp_copy.fh));
+	cbo->co_nfserr = copy->cp_copy.nfserr;
 	cbo->co_retries = 5;
 
 	/*
@@ -2153,7 +2151,8 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 				 cbo->co_referring_slotid,
 				 cbo->co_referring_seqno);
 	trace_nfsd_cb_offload(clp, &cbo->co_res.cb_stateid,
-			      &cbo->co_fh, copy->cp_count, copy->nfserr);
+			      &cbo->co_fh, copy->cp_copy.cp_count,
+			      copy->cp_copy.nfserr);
 	nfsd4_try_run_cb(&cbo->co_cb);
 }
 
@@ -2166,7 +2165,8 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
  */
 static int nfsd4_do_async_copy(void *data)
 {
-	struct nfsd4_copy *copy = (struct nfsd4_copy *)data;
+	struct nfsd4_async_copy *async = data;
+	struct nfsd4_copy *copy = &async->cp_copy;
 	__be32 nfserr = nfs_ok;
 
 	trace_nfsd_copy_async(copy);
@@ -2210,14 +2210,14 @@ static int nfsd4_do_async_copy(void *data)
 	atomic_dec(&copy->cp_nn->pending_async_copies);
 	if (copy->cp_res.wr_bytes_written > 0 && copy->attr_update)
 		nfsd_update_cmtime_attr(copy->nf_dst->nf_file, 0);
-	nfsd4_send_cb_offload(copy);
+	nfsd4_send_cb_offload(async);
 	/*
 	 * Drop the kthread's own reference (taken before
 	 * wake_up_process() in nfsd4_copy()). After this point, copy
 	 * may be freed by a concurrent teardown caller's pending
 	 * nfs4_put_copy().
 	 */
-	nfs4_put_copy(copy);
+	nfs4_put_copy(async);
 	return 0;
 }
 
@@ -2226,7 +2226,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		union nfsd4_op_u *u)
 {
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
-	struct nfsd4_copy *async_copy = NULL;
+	struct nfsd4_async_copy *async_copy = NULL;
 	struct nfsd4_copy *copy = &u->copy;
 	struct nfsd42_write_res *result;
 	__be32 status;
@@ -2258,10 +2258,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (nfsd4_copy_is_async(copy)) {
 		struct task_struct *task;
 
-		async_copy = kzalloc_obj(struct nfsd4_copy);
+		async_copy = kzalloc_obj(struct nfsd4_async_copy);
 		if (!async_copy)
 			goto out_err;
-		async_copy->cp_nn = nn;
+		async_copy->cp_copy.cp_nn = nn;
 		INIT_LIST_HEAD(&async_copy->copies);
 		refcount_set(&async_copy->refcount, 1);
 		async_copy->cp_ttl = NFSD_COPY_INITIAL_TTL;
@@ -2269,10 +2269,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		if (atomic_inc_return(&nn->pending_async_copies) >
 				(int)rqstp->rq_pool->sp_nrthreads)
 			goto out_dec_async_copy_err;
-		async_copy->cp_src = kmalloc_obj(*async_copy->cp_src);
-		if (!async_copy->cp_src)
+		async_copy->cp_copy.cp_src = kmalloc_obj(*async_copy->cp_copy.cp_src);
+		if (!async_copy->cp_copy.cp_src)
 			goto out_dec_async_copy_err;
-		dup_copy_fields(copy, async_copy);
+		dup_copy_fields(copy, &async_copy->cp_copy);
 		/*
 		 * Register the copy stateid on the long-lived async_copy
 		 * rather than on the transient COMPOUND argument buffer
@@ -2289,7 +2289,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			sizeof(result->cb_stateid));
 		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
 			       FMODE_NOCMTIME) != 0)
-			async_copy->attr_update = true;
+			async_copy->cp_copy.attr_update = true;
 		memcpy(async_copy->cp_cb_offload.co_referring_sessionid.data,
 		       cstate->session->se_sessionid.data,
 		       NFS4_MAX_SESSIONID_LEN);
@@ -2314,10 +2314,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 */
 		refcount_inc(&async_copy->refcount);
 		wake_up_process(async_copy->copy_task);
-		spin_lock(&async_copy->cp_clp->async_lock);
+		spin_lock(&async_copy->cp_copy.cp_clp->async_lock);
 		list_add(&async_copy->copies,
-				&async_copy->cp_clp->async_copies);
-		spin_unlock(&async_copy->cp_clp->async_lock);
+				&async_copy->cp_copy.cp_clp->async_copies);
+		spin_unlock(&async_copy->cp_copy.cp_clp->async_lock);
 		status = nfs_ok;
 	} else {
 		status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
@@ -2349,10 +2349,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	goto out;
 }
 
-static struct nfsd4_copy *
+static struct nfsd4_async_copy *
 find_async_copy_locked(struct nfs4_client *clp, stateid_t *stateid)
 {
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 
 	lockdep_assert_held(&clp->async_lock);
 
@@ -2364,10 +2364,10 @@ find_async_copy_locked(struct nfs4_client *clp, stateid_t *stateid)
 	return NULL;
 }
 
-static struct nfsd4_copy *
+static struct nfsd4_async_copy *
 find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
 {
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 
 	spin_lock(&clp->async_lock);
 	copy = find_async_copy_locked(clp, stateid);
@@ -2380,7 +2380,7 @@ find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
 		 * nfsd4_stop_copy(). smp_store_release() pairs with
 		 * smp_load_acquire() in nfsd4_send_cb_offload().
 		 */
-		smp_store_release(&copy->cp_clp, NULL);
+		smp_store_release(&copy->cp_copy.cp_clp, NULL);
 		if (!list_empty(&copy->copies))
 			list_del_init(&copy->copies);
 	}
@@ -2394,7 +2394,7 @@ nfsd4_offload_cancel(struct svc_rqst *rqstp,
 		     union nfsd4_op_u *u)
 {
 	struct nfsd4_offload_status *os = &u->offload_status;
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 	struct nfs4_client *clp = cstate->clp;
 
 	copy = find_async_copy(clp, &os->stateid);
@@ -2495,17 +2495,17 @@ nfsd4_offload_status(struct svc_rqst *rqstp,
 {
 	struct nfsd4_offload_status *os = &u->offload_status;
 	__be32 status = nfs_ok;
-	struct nfsd4_copy *copy;
+	struct nfsd4_async_copy *copy;
 	struct nfs4_client *clp = cstate->clp;
 
 	os->completed = false;
 	spin_lock(&clp->async_lock);
 	copy = find_async_copy_locked(clp, &os->stateid);
 	if (copy) {
-		os->count = copy->cp_res.wr_bytes_written;
-		if (test_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags)) {
+		os->count = copy->cp_copy.cp_res.wr_bytes_written;
+		if (test_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_copy.cp_flags)) {
 			os->completed = true;
-			os->status = copy->nfserr;
+			os->status = copy->cp_copy.nfserr;
 		}
 	} else
 		status = nfserr_bad_stateid;
fs/nfsd/nfs4state.c +3 -3
index 3f58c729edbf..594cf392f61f 100644
@@ -982,7 +982,7 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
 	return 1;
 }
 
-int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy)
+int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_async_copy *copy)
 {
 	return nfs4_init_cp_state(nn, &copy->cp_stateid, NFS4_COPY_STID, NULL);
 }
@@ -1024,13 +1024,13 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	return NULL;
 }
 
-void nfs4_free_copy_state(struct nfsd4_copy *copy)
+void nfs4_free_copy_state(struct nfsd4_async_copy *copy)
 {
 	struct nfsd_net *nn;
 
 	if (copy->cp_stateid.cs_type != NFS4_COPY_STID)
 		return;
-	nn = net_generic(copy->cp_clp->net, nfsd_net_id);
+	nn = net_generic(copy->cp_copy.cp_clp->net, nfsd_net_id);
 	spin_lock(&nn->s2s_cp_lock);
 	idr_remove(&nn->s2s_cp_stateids,
 		   copy->cp_stateid.cs_stid.si_opaque.so_id);
fs/nfsd/state.h +3 -2
index bc0181ef1cb6..4526b67c90d4 100644
@@ -872,6 +872,7 @@ struct nfsd4_blocked_lock {
 struct nfsd4_compound_state;
 struct nfsd_net;
 struct nfsd4_copy;
+struct nfsd4_async_copy;
 
 extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
@@ -883,8 +884,8 @@ __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 			    struct nfs4_stid **s, struct nfsd_net *nn);
 struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
 				  void (*sc_free)(struct nfs4_stid *));
-int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy);
-void nfs4_free_copy_state(struct nfsd4_copy *copy);
+int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_async_copy *copy);
+void nfs4_free_copy_state(struct nfsd4_async_copy *copy);
 struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 			struct nfs4_stid *p_stid);
 void nfs4_put_stid(struct nfs4_stid *s);
fs/nfsd/xdr4.h +23 -10
index 805c7122eb93..f1c817e2f93c 100644
@@ -759,28 +759,41 @@ struct nfsd4_copy {
 	struct nfsd42_write_res	cp_res;
 	struct knfsd_fh		fh;
 
-	/* offload callback */
-	struct nfsd4_cb_offload	cp_cb_offload;
-
 	struct nfs4_client      *cp_clp;
 
 	struct nfsd_file        *nf_src;
 	struct nfsd_file        *nf_dst;
 	bool			attr_update;
 
-	copy_stateid_t		cp_stateid;
-
-	struct list_head	copies;
-	struct task_struct	*copy_task;
-	refcount_t		refcount;
-	unsigned int		cp_ttl;
-
 	struct nfsd4_ssc_umount_item *ss_nsui;
 	struct nfs_fh		c_fh;
 	nfs4_stateid		stateid;
 	struct nfsd_net		*cp_nn;
 };
 
+/*
+ * Durable state for an asynchronous (background) server-side COPY.
+ *
+ * struct nfsd4_copy is a transient object that lives in the COMPOUND
+ * argument buffer (union nfsd4_op_u) and is reused once the operation
+ * returns. An async COPY, however, outlives the COMPOUND: a worker kthread
+ * keeps copying, the reaper tracks it on nfs4_client.async_copies, and a
+ * CB_OFFLOAD callback fires when it finishes. nfsd4_async_copy holds that
+ * long-lived state. The operation parameters and result are kept in the
+ * embedded cp_copy (populated by dup_copy_fields()), so the durable object
+ * never points into the request buffer.
+ */
+struct nfsd4_async_copy {
+	struct nfsd4_copy	cp_copy;	/* operation params + result */
+
+	struct list_head	copies;		/* nfs4_client.async_copies */
+	struct task_struct	*copy_task;
+	refcount_t		refcount;
+	unsigned int		cp_ttl;
+	copy_stateid_t		cp_stateid;	/* s2s_cp_stateids IDR entry */
+	struct nfsd4_cb_offload	cp_cb_offload;
+};
+
 static inline void nfsd4_copy_set_sync(struct nfsd4_copy *copy, bool sync)
 {
 	if (sync)
Signature · 1 line
2.55.0
author maintainer
#10 lore raw

PATCH v2 9/10 nfsd: make the copy offload stateid a first-class nfs4_stid

In reply to Jeff Layton

The async COPY offload stateid was a copy_stateid_t in the per-net nn->s2s_cp_stateids IDR, sharing that table with COPY_NOTIFY stateids even though every reader (laundromat, manage_cpntf_state()) accepts only NFS4_COPYNOTIFY_STID. It was inserted there only to mint a unique so_id; OFFLOAD_CANCEL and OFFLOAD_STATUS find the copy by walking clp->async_copies.

Building on the nfsd4_async_copy split, promote it to a first-class nfs4_stid (SC_TYPE_COPY) embedded at the head of nfsd4_async_copy and allocated from the client's cl_stateids via nfs4_alloc_stid(). This:

- makes the stateid per-client by construction rather than relying on a

    guessable cyclic id in a global table;

- reuses the common id allocation, refcounting, and teardown (nfs4_put_stid() + sc_free), removing the bespoke nfs4_init_copy_state()/nfs4_free_copy_state(); and

- leaves nn->s2s_cp_stateids exclusively for COPY_NOTIFY stateids.

The async-copy lifetime model is unchanged; nf4_put_copy() now drops the stid's single reference, which removes it from cl_stateids and frees the slab.

Per RFC 7862 Section 4.8 a copy offload stateid is valid only for COPY/OFFLOAD_CANCEL/OFFLOAD_STATUS/CB_OFFLOAD, not FREE_STATEID or TEST_STATEID, so find_stateid_locked() hides SC_TYPE_COPY and those paths keep returning bad_stateid as before. Its seqid MUST NOT be zero, so set si_generation to 1 (nfs4_alloc_stid() leaves it zero).

Follow-ups (not done here): NFS4_COPY_STID, the now-always-COPYNOTIFY branch in nfs4_init_cp_state(), and the redundant cs_type checks in the laundromat and manage_cpntf_state() are vestigial.

Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4proc.c  | 37 +++++++++++++-----------------
 fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++------------
 fs/nfsd/state.h     |  4 ++--
 fs/nfsd/xdr4.h      |  2 +-
 4 files changed, 70 insertions(+), 39 deletions(-)
fs/nfsd/nfs4proc.c +16 -21
index 7d44c85335c7..64cc830830e9 100644
@@ -1521,11 +1521,13 @@ static void nfs4_put_copy(struct nfsd4_async_copy *copy)
 {
 	if (!refcount_dec_and_test(&copy->refcount))
 		return;
-	/* Drop the task_struct reference taken in nfsd4_copy(). */
-	if (copy->copy_task)
-		put_task_struct(copy->copy_task);
-	kfree(copy->cp_copy.cp_src);
-	kfree(copy);
+	/*
+	 * Release the copy offload stateid. This drops the stid's sole
+	 * reference, which removes it from cl_stateids and frees the
+	 * nfsd4_async_copy via nfsd4_free_async_copy_stid(). The task_struct
+	 * pin and cp_src are released there.
+	 */
+	nfs4_put_stid(&copy->cp_stid);
 }
 
 static void release_copy_files(struct nfsd4_copy *copy);
@@ -2099,7 +2101,6 @@ static void release_copy_files(struct nfsd4_copy *copy)
  */
 static void cleanup_async_copy(struct nfsd4_async_copy *copy)
 {
-	nfs4_free_copy_state(copy);
 	release_copy_files(&copy->cp_copy);
 	nfs4_put_copy(copy);
 }
@@ -2258,7 +2259,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (nfsd4_copy_is_async(copy)) {
 		struct task_struct *task;
 
-		async_copy = kzalloc_obj(struct nfsd4_async_copy);
+		/*
+		 * Allocate the durable async copy. Its copy offload stateid is
+		 * a first-class nfs4_stid in cstate->clp->cl_stateids, minted
+		 * here and returned to the client; it outlives this COMPOUND and
+		 * is freed only when the background copy is torn down.
+		 */
+		async_copy = nfs4_alloc_copy_stid(cstate->clp);
 		if (!async_copy)
 			goto out_err;
 		async_copy->cp_copy.cp_nn = nn;
@@ -2273,19 +2280,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		if (!async_copy->cp_copy.cp_src)
 			goto out_dec_async_copy_err;
 		dup_copy_fields(copy, &async_copy->cp_copy);
-		/*
-		 * Register the copy stateid on the long-lived async_copy
-		 * rather than on the transient COMPOUND argument buffer
-		 * (&u->copy). nfs4_init_copy_state() installs a pointer to
-		 * the copy_stateid_t in nn->s2s_cp_stateids, and that pointer
-		 * outlives this call (it is removed only when the background
-		 * copy finishes). Pointing it at &u->copy would leave a stale
-		 * pointer into reused request memory that the laundromat and
-		 * OFFLOAD_CANCEL later dereference.
-		 */
-		if (!nfs4_init_copy_state(nn, async_copy))
-			goto out_dec_async_copy_err;
-		memcpy(&result->cb_stateid, &async_copy->cp_stateid.cs_stid,
+		memcpy(&result->cb_stateid, &async_copy->cp_stid.sc_stateid,
 			sizeof(result->cb_stateid));
 		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
 			       FMODE_NOCMTIME) != 0)
@@ -2357,7 +2352,7 @@ find_async_copy_locked(struct nfs4_client *clp, stateid_t *stateid)
 	lockdep_assert_held(&clp->async_lock);
 
 	list_for_each_entry(copy, &clp->async_copies, copies) {
-		if (memcmp(&copy->cp_stateid.cs_stid, stateid, NFS4_STATEID_SIZE))
+		if (memcmp(&copy->cp_stid.sc_stateid, stateid, NFS4_STATEID_SIZE))
 			continue;
 		return copy;
 	}
fs/nfsd/nfs4state.c +51 -15

105 lines not on this page · raw message

fs/nfsd/state.h +2 -2
index 4526b67c90d4..5dc4a473246e 100644
@@ -121,6 +121,7 @@ struct nfs4_stid {
 #define SC_TYPE_LOCK		BIT(1)
 #define SC_TYPE_DELEG		BIT(2)
 #define SC_TYPE_LAYOUT		BIT(3)
+#define SC_TYPE_COPY		BIT(4)
 	unsigned short		sc_type;
 
 /* nn->deleg_lock protects sc_status for delegation stateids.
@@ -884,8 +885,7 @@ __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 			    struct nfs4_stid **s, struct nfsd_net *nn);
 struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
 				  void (*sc_free)(struct nfs4_stid *));
-int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_async_copy *copy);
-void nfs4_free_copy_state(struct nfsd4_async_copy *copy);
+struct nfsd4_async_copy *nfs4_alloc_copy_stid(struct nfs4_client *clp);
 struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 			struct nfs4_stid *p_stid);
 void nfs4_put_stid(struct nfs4_stid *s);
fs/nfsd/xdr4.h +1 -1
index f1c817e2f93c..cbd40bf3ee2a 100644
@@ -784,13 +784,13 @@ struct nfsd4_copy {
  * never points into the request buffer.
  */
 struct nfsd4_async_copy {
+	struct nfs4_stid	cp_stid;	/* SC_TYPE_COPY, in cl_stateids */
 	struct nfsd4_copy	cp_copy;	/* operation params + result */
 
 	struct list_head	copies;		/* nfs4_client.async_copies */
 	struct task_struct	*copy_task;
 	refcount_t		refcount;
 	unsigned int		cp_ttl;
-	copy_stateid_t		cp_stateid;	/* s2s_cp_stateids IDR entry */
 	struct nfsd4_cb_offload	cp_cb_offload;
 };
 
Signature · 1 line
2.55.0
author maintainer
#11 lore raw

PATCH v2 10/10 nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR

In reply to Jeff Layton

Now that the COPY offload stateid is a first-class nfs4_stid, nn->s2s_cp_stateids holds COPY_NOTIFY stateids exclusively (its only inserter, nfs4_init_cp_state(), runs only from nfs4_alloc_init_cpntf_state()). The type-distinguishing machinery is dead:

- remove the unreferenced NFS4_COPY_STID definition;

- drop nfs4_init_cp_state()'s cs_type argument (hardcode

    NFS4_COPYNOTIFY_STID) and its now-always-true "if (p_stid)" guard;

- remove the cs_type == NFS4_COPYNOTIFY_STID gates in manage_cpntf_state() and the laundromat, which can no longer be false.

copy_stateid_t.cs_type is retained for the WARN_ON_ONCE() sanity checks on the free paths. No functional change.

Assisted-by Claude:claude-opus-4-8
---
 fs/nfsd/nfs4state.c | 31 +++++++++++++------------------
 fs/nfsd/state.h     |  1 -
 2 files changed, 13 insertions(+), 19 deletions(-)
fs/nfsd/nfs4state.c +13 -18
index 8ac76db31fbb..380214e90f44 100644
@@ -942,10 +942,12 @@ struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *sla
 }
 
 /*
- * Create a unique stateid_t to represent each COPY.
+ * Publish a COPY_NOTIFY stateid in nn->s2s_cp_stateids and link it onto the
+ * parent stid's sc_cp_list. s2s_cp_stateids holds only COPY_NOTIFY stateids;
+ * the COPY offload stateid is a first-class nfs4_stid in cl_stateids.
  */
 static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
-			      unsigned char cs_type, struct nfs4_stid *p_stid)
+			      struct nfs4_stid *p_stid)
 {
 	int new_id;
 
@@ -956,6 +958,9 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
 	spin_lock(&nn->s2s_cp_lock);
 	new_id = idr_alloc_cyclic(&nn->s2s_cp_stateids, stid, 0, 0, GFP_NOWAIT);
 	if (new_id >= 0) {
+		struct nfs4_cpntf_state *cps =
+			container_of(stid, struct nfs4_cpntf_state, cp_stateid);
+
 		stid->cs_stid.si_opaque.so_id = new_id;
 		stid->cs_stid.si_generation = 1;
 		/*
@@ -967,14 +972,8 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
 		 * fully linked cp_list, so list_del_init() in
 		 * _free_cpntf_state_locked() is always well-defined.
 		 */
-		stid->cs_type = cs_type;
-		if (p_stid) {
-			struct nfs4_cpntf_state *cps =
-				container_of(stid, struct nfs4_cpntf_state,
-					     cp_stateid);
-
-			list_add(&cps->cp_list, &p_stid->sc_cp_list);
-		}
+		stid->cs_type = NFS4_COPYNOTIFY_STID;
+		list_add(&cps->cp_list, &p_stid->sc_cp_list);
 	}
 	spin_unlock(&nn->s2s_cp_lock);
 	idr_preload_end();
@@ -1048,8 +1047,7 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	memcpy(&cps->cp_p_clid, &p_stid->sc_client->cl_clientid,
 	       sizeof(clientid_t));
 	refcount_set(&cps->cp_stateid.cs_count, 2);
-	if (!nfs4_init_cp_state(nn, &cps->cp_stateid, NFS4_COPYNOTIFY_STID,
-				p_stid))
+	if (!nfs4_init_cp_state(nn, &cps->cp_stateid, p_stid))
 		goto out_free;
 	return cps;
 out_free:
@@ -7590,10 +7588,10 @@ nfs4_laundromat(struct nfsd_net *nn)
 	nfsd4_end_grace(nn);
 
 	spin_lock(&nn->s2s_cp_lock);
+	/* s2s_cp_stateids holds only COPY_NOTIFY stateids */
 	idr_for_each_entry(&nn->s2s_cp_stateids, cps_t, i) {
 		cps = container_of(cps_t, struct nfs4_cpntf_state, cp_stateid);
-		if (cps->cp_stateid.cs_type == NFS4_COPYNOTIFY_STID &&
-				state_expired(&lt, cps->cpntf_time))
+		if (state_expired(&lt, cps->cpntf_time))
 			revoke_cpntf_state_locked(nn, cps);
 	}
 	spin_unlock(&nn->s2s_cp_lock);
@@ -8005,14 +8003,11 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 	if (st->si_opaque.so_clid.cl_id != nn->s2s_cp_cl_id)
 		return nfserr_bad_stateid;
 	spin_lock(&nn->s2s_cp_lock);
+	/* s2s_cp_stateids holds only COPY_NOTIFY stateids */
 	cps_t = idr_find(&nn->s2s_cp_stateids, st->si_opaque.so_id);
 	if (cps_t) {
 		state = container_of(cps_t, struct nfs4_cpntf_state,
 				     cp_stateid);
-		if (state->cp_stateid.cs_type != NFS4_COPYNOTIFY_STID) {
-			state = NULL;
-			goto unlock;
-		}
 		if (!clp) {
 			refcount_inc(&state->cp_stateid.cs_count);
 		} else if (memcmp(&clp->cl_clientid, &state->cp_p_clid,
fs/nfsd/state.h +0 -1
index 5dc4a473246e..ee5c429edfa2 100644
@@ -59,7 +59,6 @@ typedef struct {
 
 typedef struct {
 	stateid_t		cs_stid;
-#define NFS4_COPY_STID 1
 #define NFS4_COPYNOTIFY_STID 2
 	unsigned char		cs_type;
 	refcount_t		cs_count;
Signature · 1 line
2.55.0