The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v3 00/10] nfsd: copy offload fixes
@ 2026-07-10 14:00 Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, Jeff Layton

This version fixes a couple of problems noticed by Sashiko.

These patches fix bugs in inter-server copy offload code noticed by LLM
inspection. The first 3 were found via kres, and the next 4 fix 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.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v3:
- Write new stateid to cp_res to prevent potential bad CB_OFFLOAD call
- Fix interim regression that could cause IDR leak
- Make comments much more terse
- Link to v2: https://lore.kernel.org/r/20260709-nfsd-testing-v2-0-0a1ba233bf87@kernel.org

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/20260624-nfsd-testing-v1-0-b8853eb22e45@kernel.org

---
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  | 261 +++++++++++++++++++++++++++++++---------------------
 fs/nfsd/nfs4state.c | 187 ++++++++++++++++++++++++++++---------
 fs/nfsd/nfs4xdr.c   |  13 +++
 fs/nfsd/state.h     |   6 +-
 fs/nfsd/xdr4.h      |  30 ++++--
 5 files changed, 337 insertions(+), 160 deletions(-)
---
base-commit: bc7d6a41a6282da7c175c1638bdfef69c10f78d5
change-id: 20260624-nfsd-testing-8439f0163312

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, Jeff Layton

From: Chris Mason <clm@meta.com>

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
Signed-off-by: Chris Mason <clm@meta.com>
---
 fs/nfsd/nfs4state.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a4398dc861a5..3f83c5107e28 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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,34 @@ 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;
+		/*
+		 * Set cs_type and link onto sc_cp_list under the same lock
+		 * that installed the IDR entry, so a concurrent
+		 * manage_cpntf_state() sees either no entry or a fully
+		 * linked cp_list.
+		 */
+		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 +992,13 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	cps = kzalloc_obj(struct nfs4_cpntf_state);
 	if (!cps)
 		return NULL;
+	/* So a stale list_del_init() before linking is a no-op. */
+	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 +7869,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);

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 02/10] nfsd: fix UAF in async copy cancel and shutdown
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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().

  - Because unlinking hides the copy from the reaper, its
    cleanup_async_copy() can no longer remove the copy's s2s_cp_stateids
    entry; the cancel/shutdown/sb-cancel paths now call
    nfs4_free_copy_state() themselves (while cp_clp is still valid) so
    the entry does not dangle at freed memory for the laundromat and
    manage_cpntf_state() to dereference.

  - 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c | 120 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 85 insertions(+), 35 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 669896be08b6..b2f1a9bc9202 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1519,6 +1519,9 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
 {
 	if (!refcount_dec_and_test(&copy->refcount))
 		return;
+	/* Drop the task_struct pinned in nfsd4_copy(); NULL on sync copies. */
+	if (copy->copy_task)
+		put_task_struct(copy->copy_task);
 	kfree(copy->cp_src);
 	kfree(copy);
 }
@@ -1528,20 +1531,18 @@ 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.
+	 * Join the kthread before releasing its resources. The task_struct is
+	 * pinned in nfsd4_copy(), so kthread_stop() is safe even after the
+	 * one-shot kthread has exited. The caller already unlinked the copy,
+	 * so this runs 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 +1556,13 @@ 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;
+		/*
+		 * Unlinking hides the copy from the reaper, so drop its
+		 * s2s_cp_stateids entry here while cp_clp is still valid.
+		 */
+		nfs4_free_copy_state(copy);
+		/* 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);
 	}
@@ -1567,8 +1574,11 @@ 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);
+		/* Reaper can't reach the unhashed copy; drop its membership ref. */
+		nfs4_put_copy(copy);
+	}
 }
 
 static bool nfsd4_copy_on_sb(const struct nfsd4_copy *copy,
@@ -1636,7 +1646,11 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 		struct nfs4_client *clp = copy->cp_clp;
 
 		list_del_init(&copy->copies);
+		/* Reaper can't reach it; drop the s2s entry while cp_clp is valid. */
+		nfs4_free_copy_state(copy);
 		nfsd4_stop_copy(copy);
+		/* Drop the membership ref the reaper would have dropped. */
+		nfs4_put_copy(copy);
 		nfsd4_put_client(clp);
 	}
 }
@@ -1931,6 +1945,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 +2078,29 @@ static void release_copy_files(struct nfsd4_copy *copy)
 	}
 }
 
+/*
+ * Called from the reaper and from nfsd4_copy()'s error path; in both
+ * cases the copy is already 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(&cp_clp) in find_async_copy() and
+	 * nfsd4_unhash_copy(); the set_bit/clear_bit writers are unordered.
+	 * cp_clp is NULL once the copy was canceled; 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 +2112,12 @@ 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 the client and the copy across the in-flight callback; co_cb is
+	 * embedded in the copy, so it must outlive the callback. Both are
+	 * dropped in nfsd4_cb_offload_release().
 	 */
 	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 +2169,20 @@ 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);
-
+	/*
+	 * Don't set NFSD4_COPY_F_STOPPED here: it tells a teardown caller it
+	 * may skip kthread_stop(), which would then release nf_dst and the
+	 * client while still in use. Only nfsd4_stop_copy() sets it, after
+	 * joining.
+	 */
 	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 reference (taken in nfsd4_copy()); copy may be freed after this. */
+	nfs4_put_copy(copy);
 	return 0;
 }
 
@@ -2198,6 +2221,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 +2250,27 @@ 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 kthread_stop() is safe after this
+		 * one-shot kthread exits. Released by nfs4_put_copy().
+		 */
+		get_task_struct(task);
+		async_copy->copy_task = task;
+		/*
+		 * Take the kthread's ref and wake it before publishing, so the
+		 * publisher touches async_copy no further and teardown can
+		 * drain it.
+		 */
+		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 +2324,18 @@ 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);
+		nfs4_free_copy_state(copy);
+		/*
+		 * Mirror nfsd4_unhash_copy(): unlink and clear cp_clp under
+		 * async_lock so the reaper can't reach it. Caller drops the
+		 * membership ref after nfsd4_stop_copy().
+		 */
+		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 +2354,11 @@ 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() unlinked it from the reaper; drop the membership ref. */
+		nfs4_put_copy(copy);
+	}
 
 	return nfs_ok;
 }

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b2f1a9bc9202..b9049a3d7c07 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2237,11 +2237,12 @@ 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);
+
+		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;

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 04/10] nfsd: initialize copy-notify stateid before publishing it
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (2 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  | 16 +++++++++-------
 fs/nfsd/nfs4state.c | 10 +++++++++-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b9049a3d7c07..506cd4910fb7 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2373,7 +2373,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,
@@ -2387,12 +2386,14 @@ 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 published and fully initialized, and carries an
+	 * extra reference for us; drop it once we are done with it.
+	 */
 	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.
@@ -2401,10 +2402,11 @@ 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 our extra reference. The membership reference keeps the entry
+	 * alive for a later inter-server READ, or until the laundromat reaps it.
+	 */
+	nfs4_put_cpntf_state(nn, cps);
 out:
 	nfs4_put_stid(stid);
 	return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3f83c5107e28..9ae0f18121ef 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -995,7 +995,15 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	/* So a stale list_del_init() before linking is a no-op. */
 	INIT_LIST_HEAD(&cps->cp_list);
 	cps->cpntf_time = ktime_get_boottime_seconds();
-	refcount_set(&cps->cp_stateid.cs_count, 1);
+	/*
+	 * Fully initialize the entry before nfs4_init_cp_state() publishes it,
+	 * since a concurrent OFFLOAD_CANCEL could then free it. Take an extra
+	 * reference for the caller (dropped with nfs4_put_cpntf_state()).
+	 */
+	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;

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 05/10] nfsd: check client ownership when cancelling a copy-notify stateid
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (3 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9ae0f18121ef..b3fe163a148d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7905,10 +7905,20 @@ __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 creating client may cancel.
+			 * so_id is guessable, so without this check any client
+			 * could free another's cpntf state.
+			 */
+			state = NULL;
+			goto unlock;
+		} else {
 			_free_cpntf_state_locked(nn, state);
+		}
 	}
 unlock:
 	spin_unlock(&nn->s2s_cp_lock);

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 06/10] nfsd: revoke copy-notify stateids before dropping their reference
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (4 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 78 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b3fe163a148d..99f450f292a0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1026,18 +1026,66 @@ void nfs4_free_copy_state(struct nfsd4_copy *copy)
 	spin_unlock(&nn->s2s_cp_lock);
 }
 
+/*
+ * Drop the parent's reference on an already-unlinked cpntf entry. If a
+ * concurrent holder still owns a reference, its nfs4_put_cpntf_state() does
+ * the final free.
+ *
+ * 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 from the IDR and sc_cp_list. Gated on list_empty() to avoid
+ * evicting a recycled so_id.
+ */
+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 IDR and sc_cp_list first
+ * so no new finder can discover it, then drop the membership reference. Every
+ * revoke path (cancel, laundromat, drain) must use this rather than
+ * _free_cpntf_state_locked(), which unlinks only at refcount zero and so could
+ * let a second revoke free the entry under a concurrent reader.
+ *
+ * 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);
-	}
+	/*
+	 * Revoke unlinks each entry before dropping the parent's reference, so
+	 * the drain terminates in one pass per entry regardless of cs_count; a
+	 * concurrent holder does the final kfree via nfs4_put_cpntf_state().
+	 */
+	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);
 }
 
@@ -7484,7 +7532,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);
@@ -7871,16 +7919,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
@@ -7917,7 +7963,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:

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (5 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index aef48fb0fac2..3873d037a763 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -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,18 @@ 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:
+		/*
+		 * Well-formed XDR, but only NL4_NETADDR is supported. Consume
+		 * the utf8str_cis to keep the stream aligned, then return
+		 * 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;
 	}

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (6 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  | 137 +++++++++++++++++++++++++++-------------------------
 fs/nfsd/nfs4state.c |   6 +--
 fs/nfsd/state.h     |   5 +-
 fs/nfsd/xdr4.h      |  30 ++++++++----
 4 files changed, 98 insertions(+), 80 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 506cd4910fb7..9f897c4ffc16 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -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,52 +1510,53 @@ 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 pinned in nfsd4_copy(); NULL on sync copies. */
+	/* Drop the task_struct pinned 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);
 	/*
 	 * Join the kthread before releasing its resources. The task_struct is
 	 * pinned in nfsd4_copy(), so kthread_stop() is safe even after the
 	 * one-shot kthread has exited. The caller already unlinked the copy,
 	 * so this runs 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);
 		/*
 		 * Unlinking hides the copy from the reaper, so drop its
@@ -1562,7 +1564,7 @@ static struct nfsd4_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
 		 */
 		nfs4_free_copy_state(copy);
 		/* 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);
 	}
@@ -1572,7 +1574,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);
@@ -1605,7 +1607,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);
@@ -1619,7 +1621,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
@@ -1631,9 +1633,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);
 				}
 			}
@@ -1643,7 +1645,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);
 		/* Reaper can't reach it; drop the s2s entry while cp_clp is valid. */
@@ -1940,10 +1942,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);
@@ -2059,7 +2061,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));
@@ -2082,14 +2083,14 @@ static void release_copy_files(struct nfsd4_copy *copy)
  * Called from the reaper and from nfsd4_copy()'s error path; in both
  * cases the copy is already 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;
@@ -2100,15 +2101,15 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 	 * cp_clp is NULL once the copy was canceled; 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;
 
 	/*
@@ -2125,7 +2126,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);
 }
 
@@ -2138,7 +2140,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);
@@ -2180,9 +2183,9 @@ 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 reference (taken in nfsd4_copy()); copy may be freed after this. */
-	nfs4_put_copy(copy);
+	nfs4_put_copy(async);
 	return 0;
 }
 
@@ -2191,7 +2194,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;
@@ -2223,10 +2226,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;
@@ -2234,18 +2237,22 @@ 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);
 
 		if (!nfs4_init_copy_state(nn, async_copy))
 			goto out_dec_async_copy_err;
 		memcpy(&result->cb_stateid, &async_copy->cp_stateid.cs_stid,
 			sizeof(result->cb_stateid));
+		/*
+		 * dup after writing cb_stateid; duplicating first would leave
+		 * the callback stateid zeroed.
+		 */
+		dup_copy_fields(copy, &async_copy->cp_copy);
 		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);
@@ -2268,10 +2275,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,
@@ -2303,10 +2310,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);
 
@@ -2318,10 +2325,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);
@@ -2333,7 +2340,7 @@ find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
 		 * async_lock so the reaper can't reach it. Caller drops the
 		 * membership ref after nfsd4_stop_copy().
 		 */
-		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);
 	}
@@ -2347,7 +2354,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);
@@ -2440,17 +2447,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;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 99f450f292a0..1feda9eaf3a0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -979,7 +979,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);
 }
@@ -1013,13 +1013,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);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index bc0181ef1cb6..4526b67c90d4 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -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);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 805c7122eb93..62311fb9351f 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -759,28 +759,38 @@ 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 async (background) server-side COPY.
+ *
+ * struct nfsd4_copy is transient: it lives in the COMPOUND argument buffer
+ * and is reused once the op returns. An async COPY outlives the COMPOUND
+ * (worker kthread, reaper linkage, CB_OFFLOAD), so its params and result are
+ * snapshotted into the embedded cp_copy and it 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)

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (7 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 14:00 ` [PATCH v3 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
  2026-07-10 19:51 ` [PATCH v3 00/10] nfsd: copy offload fixes Chuck Lever
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  | 35 +++++++++++++-------------------
 fs/nfsd/nfs4state.c | 58 +++++++++++++++++++++++++++++++++++++++--------------
 fs/nfsd/state.h     |  4 ++--
 fs/nfsd/xdr4.h      |  2 +-
 4 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9f897c4ffc16..14e1c56654ec 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1521,11 +1521,11 @@ static void nfs4_put_copy(struct nfsd4_async_copy *copy)
 {
 	if (!refcount_dec_and_test(&copy->refcount))
 		return;
-	/* Drop the task_struct pinned in nfsd4_copy(). */
-	if (copy->copy_task)
-		put_task_struct(copy->copy_task);
-	kfree(copy->cp_copy.cp_src);
-	kfree(copy);
+	/*
+	 * Drop the copy offload stateid's sole reference: removes it from
+	 * cl_stateids and frees the async_copy via nfsd4_free_async_copy_stid().
+	 */
+	nfs4_put_stid(&copy->cp_stid);
 }
 
 static void release_copy_files(struct nfsd4_copy *copy);
@@ -1558,11 +1558,6 @@ static struct nfsd4_async_copy *nfsd4_unhash_copy(struct nfs4_client *clp)
 		copy = list_first_entry(&clp->async_copies,
 					struct nfsd4_async_copy, copies);
 		refcount_inc(&copy->refcount);
-		/*
-		 * Unlinking hides the copy from the reaper, so drop its
-		 * s2s_cp_stateids entry here while cp_clp is still valid.
-		 */
-		nfs4_free_copy_state(copy);
 		/* Pairs with smp_load_acquire() in nfsd4_send_cb_offload(). */
 		smp_store_release(&copy->cp_copy.cp_clp, NULL);
 		if (!list_empty(&copy->copies))
@@ -1648,10 +1643,8 @@ void nfsd4_cancel_copy_by_sb(struct net *net, struct super_block *sb)
 		struct nfs4_client *clp = copy->cp_copy.cp_clp;
 
 		list_del_init(&copy->copies);
-		/* Reaper can't reach it; drop the s2s entry while cp_clp is valid. */
-		nfs4_free_copy_state(copy);
 		nfsd4_stop_copy(copy);
-		/* Drop the membership ref the reaper would have dropped. */
+		/* Reaper can't reach the unlinked copy; drop the membership ref here. */
 		nfs4_put_copy(copy);
 		nfsd4_put_client(clp);
 	}
@@ -2085,7 +2078,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);
 }
@@ -2226,7 +2218,12 @@ 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 offload stateid is a
+		 * first-class nfs4_stid in clp->cl_stateids, returned to the
+		 * client and 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;
@@ -2240,10 +2237,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		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;
-
-		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));
 		/*
 		 * dup after writing cb_stateid; duplicating first would leave
@@ -2318,7 +2312,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;
 	}
@@ -2334,7 +2328,6 @@ find_async_copy(struct nfs4_client *clp, stateid_t *stateid)
 	copy = find_async_copy_locked(clp, stateid);
 	if (copy) {
 		refcount_inc(&copy->refcount);
-		nfs4_free_copy_state(copy);
 		/*
 		 * Mirror nfsd4_unhash_copy(): unlink and clear cp_clp under
 		 * async_lock so the reaper can't reach it. Caller drops the
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1feda9eaf3a0..900aca361413 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -121,6 +121,7 @@ static struct kmem_cache *file_slab;
 static struct kmem_cache *stateid_slab;
 static struct kmem_cache *deleg_slab;
 static struct kmem_cache *odstate_slab;
+static struct kmem_cache *async_copy_slab;
 
 static void free_session(struct nfsd4_session *);
 
@@ -979,9 +980,35 @@ 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_async_copy *copy)
+/* sc_free for a copy offload stateid; runs from nfs4_put_stid(). */
+static void nfsd4_free_async_copy_stid(struct nfs4_stid *stid)
 {
-	return nfs4_init_cp_state(nn, &copy->cp_stateid, NFS4_COPY_STID, NULL);
+	struct nfsd4_async_copy *copy =
+		container_of(stid, struct nfsd4_async_copy, cp_stid);
+
+	if (copy->copy_task)
+		put_task_struct(copy->copy_task);
+	kfree(copy->cp_copy.cp_src);
+	kmem_cache_free(async_copy_slab, copy);
+}
+
+/*
+ * Allocate durable async COPY state. The offload stateid is a first-class
+ * nfs4_stid (SC_TYPE_COPY) in the client's cl_stateids, so it is per-client
+ * and uses the common refcounting/teardown. find_stateid_locked() hides it;
+ * OFFLOAD_CANCEL/OFFLOAD_STATUS find it via clp->async_copies.
+ */
+struct nfsd4_async_copy *nfs4_alloc_copy_stid(struct nfs4_client *clp)
+{
+	struct nfs4_stid *stid;
+
+	stid = nfs4_alloc_stid(clp, async_copy_slab, nfsd4_free_async_copy_stid);
+	if (!stid)
+		return NULL;
+	stid->sc_type = SC_TYPE_COPY;
+	/* RFC 7862 Section 4.8: a copy offload stateid's seqid MUST NOT be 0 */
+	stid->sc_stateid.si_generation = 1;
+	return container_of(stid, struct nfsd4_async_copy, cp_stid);
 }
 
 struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
@@ -1013,19 +1040,6 @@ struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
 	return NULL;
 }
 
-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_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);
-	spin_unlock(&nn->s2s_cp_lock);
-}
-
 /*
  * Drop the parent's reference on an already-unlinked cpntf entry. If a
  * concurrent holder still owns a reference, its nfs4_put_cpntf_state() does
@@ -3047,6 +3061,14 @@ find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
 	ret = idr_find(&cl->cl_stateids, t->si_opaque.so_id);
 	if (!ret || !ret->sc_type)
 		return NULL;
+	/*
+	 * Copy offload stateids live in cl_stateids only for id allocation and
+	 * refcounting; per RFC 7862 they are not valid targets for generic
+	 * stateid ops (FREE_STATEID, TEST_STATEID, I/O). Hide them so those
+	 * paths return NFS4ERR_BAD_STATEID.
+	 */
+	if (ret->sc_type == SC_TYPE_COPY)
+		return NULL;
 	return ret;
 }
 
@@ -5382,6 +5404,7 @@ nfsd4_free_slabs(void)
 	kmem_cache_destroy(stateid_slab);
 	kmem_cache_destroy(deleg_slab);
 	kmem_cache_destroy(odstate_slab);
+	kmem_cache_destroy(async_copy_slab);
 }
 
 int
@@ -5408,8 +5431,13 @@ nfsd4_init_slabs(void)
 	odstate_slab = KMEM_CACHE(nfs4_clnt_odstate, 0);
 	if (odstate_slab == NULL)
 		goto out_free_deleg_slab;
+	async_copy_slab = KMEM_CACHE(nfsd4_async_copy, 0);
+	if (async_copy_slab == NULL)
+		goto out_free_odstate_slab;
 	return 0;
 
+out_free_odstate_slab:
+	kmem_cache_destroy(odstate_slab);
 out_free_deleg_slab:
 	kmem_cache_destroy(deleg_slab);
 out_free_stateid_slab:
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 4526b67c90d4..5dc4a473246e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -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);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 62311fb9351f..67491e2843fb 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -781,13 +781,13 @@ struct nfsd4_copy {
  * 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;
 };
 

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (8 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
@ 2026-07-10 14:00 ` Jeff Layton
  2026-07-10 19:51 ` [PATCH v3 00/10] nfsd: copy offload fixes Chuck Lever
  10 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-10 14:00 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: Chris Mason, linux-nfs, linux-kernel, 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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 30 ++++++++++++------------------
 fs/nfsd/state.h     |  1 -
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 900aca361413..b9dd50f39757 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -942,10 +942,11 @@ 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's sc_cp_list. That IDR holds only COPY_NOTIFY 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 +957,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;
 		/*
@@ -964,14 +968,8 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
 		 * manage_cpntf_state() sees either no entry or a fully
 		 * linked cp_list.
 		 */
-		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();
@@ -1031,8 +1029,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:
@@ -7556,10 +7553,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);
@@ -7971,14 +7968,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,
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 5dc4a473246e..ee5c429edfa2 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -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;

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 00/10] nfsd: copy offload fixes
  2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
                   ` (9 preceding siblings ...)
  2026-07-10 14:00 ` [PATCH v3 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
@ 2026-07-10 19:51 ` Chuck Lever
  10 siblings, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2026-07-10 19:51 UTC (permalink / raw)
  To: NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Jeff Layton
  Cc: Chris Mason, linux-nfs, linux-kernel

On Fri, 10 Jul 2026 10:00:04 -0400, Jeff Layton wrote:
> This version fixes a couple of problems noticed by Sashiko.
> 
> These patches fix bugs in inter-server copy offload code noticed by LLM
> inspection. The first 3 were found via kres, and the next 4 fix problems
> that were flagged in agentic review of those patches (and some via
> testing).
> 
> [...]

Applied to nfsd-testing, thanks!

[01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state
        commit: 2644ead835d12198cceef3c407c25ed94717b378
[02/10] nfsd: fix UAF in async copy cancel and shutdown
        commit: a3cddbf9713ae5ac03915b29975dc200ea6493fe
[03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY
        commit: 527c422ddaa3e0a452658faca10d1634f151ca6b
[04/10] nfsd: initialize copy-notify stateid before publishing it
        commit: 23fc79d2a8869ab25f0de5642a0f10d030d67d05
[05/10] nfsd: check client ownership when cancelling a copy-notify stateid
        commit: ee6c8255190685955762f66d23a887c4f86d24c6
[06/10] nfsd: revoke copy-notify stateids before dropping their reference
        commit: 24dd7409daafc00a0f19b61cb91d8f051cdc6b4b
[07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types
        commit: 673c92979f3ab32c92df687625eee76ce42e55ed
[08/10] nfsd: split nfsd4_copy into transient and durable async copy objects
        commit: bf9b4f03a032cc111a6ec707627964f23c9b1209
[09/10] nfsd: make the copy offload stateid a first-class nfs4_stid
        commit: 74d681696ac194f355a2fc21693a44c816715bd2
[10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR
        commit: 9b33df55dcac6950d1e176c7bb02a127d2d3dc1f

--
Chuck Lever


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-07-10 19:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 14:00 [PATCH v3 00/10] nfsd: copy offload fixes Jeff Layton
2026-07-10 14:00 ` [PATCH v3 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
2026-07-10 14:00 ` [PATCH v3 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
2026-07-10 14:00 ` [PATCH v3 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
2026-07-10 14:00 ` [PATCH v3 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
2026-07-10 14:00 ` [PATCH v3 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
2026-07-10 14:00 ` [PATCH v3 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
2026-07-10 14:00 ` [PATCH v3 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
2026-07-10 14:00 ` [PATCH v3 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
2026-07-10 14:00 ` [PATCH v3 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
2026-07-10 14:00 ` [PATCH v3 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
2026-07-10 19:51 ` [PATCH v3 00/10] nfsd: copy offload fixes Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox