* [PATCH v2 00/10] nfsd: copy offload fixes
@ 2026-07-09 18:47 Jeff Layton
2026-07-09 18:47 ` [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
` (9 more replies)
0 siblings, 10 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
Cc: Chris Mason, linux-nfs, linux-kernel, Jeff Layton
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.
Signed-off-by: Jeff Layton <jlayton@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 | 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,
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a4398dc861a5..b8946db3ebaa 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,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, ©->cp_stateid, NFS4_COPY_STID);
+ return nfs4_init_cp_state(nn, ©->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);
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 02/10] nfsd: fix UAF in async copy cancel and shutdown
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
2026-07-09 18:47 ` [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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().
- 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 | 163 +++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 128 insertions(+), 35 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 669896be08b6..fad01d67bf3f 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1519,6 +1519,12 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
{
if (!refcount_dec_and_test(©->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, ©->cp_flags)) {
- kthread_stop(copy->copy_task);
- if (!test_bit(NFSD4_COPY_F_CB_ERROR, ©->cp_flags))
- copy->nfserr = nfs_ok;
- set_bit(NFSD4_COPY_F_COMPLETED, ©->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, ©->cp_flags);
+ kthread_stop(copy->copy_task);
+ if (!test_bit(NFSD4_COPY_F_CB_ERROR, ©->cp_flags))
+ copy->nfserr = nfs_ok;
+ set_bit(NFSD4_COPY_F_COMPLETED, ©->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(©->refcount);
- copy->cp_clp = NULL;
+ /*
+ * Pairs with smp_load_acquire in nfsd4_send_cb_offload();
+ * see find_async_copy() for rationale.
+ */
+ smp_store_release(©->cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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(©->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, ©->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(©->cp_clp->async_lock);
- if (!list_empty(©->copies))
- list_del_init(©->copies);
- spin_unlock(©->cp_clp->async_lock);
- }
nfs4_put_copy(copy);
}
static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
{
struct nfsd4_cb_offload *cbo = ©->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(). 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(©->cp_clp);
if (!clp) {
set_bit(NFSD4_COPY_F_OFFLOAD_DONE, ©->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(©->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, ©->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, ©->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, ©->cp_flags);
trace_nfsd_copy_async_done(copy);
atomic_dec(©->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(©->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(©->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(©->cp_clp, NULL);
+ if (!list_empty(©->copies))
+ list_del_init(©->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;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
2026-07-09 18:47 ` [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
2026-07-09 18:47 ` [PATCH v2 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 21:18 ` Chuck Lever
2026-07-09 18:47 ` [PATCH v2 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
` (6 subsequent siblings)
9 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index fad01d67bf3f..1c674479d4ca 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -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, ©->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 v2 04/10] nfsd: initialize copy-notify stateid before publishing it
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (2 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 19 ++++++++++++-------
fs/nfsd/nfs4state.c | 14 +++++++++++++-
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 1c674479d4ca..2dbc99e76837 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -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;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b8946db3ebaa..1ef015cbc055 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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;
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 05/10] nfsd: check client ownership when cancelling a copy-notify stateid
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (3 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1ef015cbc055..76c0e08711df 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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);
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 06/10] nfsd: revoke copy-notify stateids before dropping their reference
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (4 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 93 ++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 77 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 76c0e08711df..3f58c729edbf 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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(<, 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:
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (5 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index aef48fb0fac2..23f50a947bf1 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,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;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (6 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
2026-07-09 18:47 ` [PATCH v2 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 136 ++++++++++++++++++++++++++--------------------------
fs/nfsd/nfs4state.c | 6 +--
fs/nfsd/state.h | 5 +-
fs/nfsd/xdr4.h | 33 +++++++++----
4 files changed, 97 insertions(+), 83 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2dbc99e76837..7d44c85335c7 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, ©->cp_flags) &&
- !test_bit(NFSD4_COPY_F_STOPPED, ©->cp_flags)) {
+ if (!test_bit(NFSD4_COPY_F_COMPLETED, ©->cp_copy.cp_flags) &&
+ !test_bit(NFSD4_COPY_F_STOPPED, ©->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, ©->cp_flags)) {
+ copy = list_entry(pos, struct nfsd4_async_copy, copies);
+ if (test_bit(NFSD4_COPY_F_OFFLOAD_DONE,
+ ©->cp_copy.cp_flags)) {
if (!--copy->cp_ttl) {
list_del_init(©->copies);
list_add(©->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(©->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(©->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(©->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, ©->cp_flags);
+ set_bit(NFSD4_COPY_F_STOPPED, ©->cp_copy.cp_flags);
kthread_stop(copy->copy_task);
- if (!test_bit(NFSD4_COPY_F_CB_ERROR, ©->cp_flags))
- copy->nfserr = nfs_ok;
- set_bit(NFSD4_COPY_F_COMPLETED, ©->cp_flags);
+ if (!test_bit(NFSD4_COPY_F_CB_ERROR, ©->cp_copy.cp_flags))
+ copy->cp_copy.nfserr = nfs_ok;
+ set_bit(NFSD4_COPY_F_COMPLETED, ©->cp_copy.cp_flags);
- release_copy_files(copy);
+ release_copy_files(©->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(©->refcount);
/*
* Pairs with smp_load_acquire in nfsd4_send_cb_offload();
* see find_async_copy() for rationale.
*/
- smp_store_release(©->cp_clp, NULL);
+ smp_store_release(©->cp_copy.cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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(©->cp_copy, sb)) {
refcount_inc(©->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,
- ©->cp_flags);
+ ©->cp_copy.cp_flags);
list_move(©->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(©->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, ©->cp_flags);
+ set_bit(NFSD4_COPY_F_OFFLOAD_DONE, ©->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(©->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 = ©->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(©->cp_clp);
+ clp = smp_load_acquire(©->cp_copy.cp_clp);
if (!clp) {
- set_bit(NFSD4_COPY_F_OFFLOAD_DONE, ©->cp_flags);
+ set_bit(NFSD4_COPY_F_OFFLOAD_DONE, ©->cp_copy.cp_flags);
return;
}
- memcpy(&cbo->co_res, ©->cp_res, sizeof(copy->cp_res));
- memcpy(&cbo->co_fh, ©->fh, sizeof(copy->fh));
- cbo->co_nfserr = copy->nfserr;
+ memcpy(&cbo->co_res, ©->cp_copy.cp_res, sizeof(copy->cp_copy.cp_res));
+ memcpy(&cbo->co_fh, ©->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(©->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(©->cp_clp, NULL);
+ smp_store_release(©->cp_copy.cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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, ©->cp_flags)) {
+ os->count = copy->cp_copy.cp_res.wr_bytes_written;
+ if (test_bit(NFSD4_COPY_F_COMPLETED, ©->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 3f58c729edbf..594cf392f61f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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, ©->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);
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..f1c817e2f93c 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -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)
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (7 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
2026-07-09 18:47 ` [PATCH v2 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 37 +++++++++++++-----------------
fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++------------
fs/nfsd/state.h | 4 ++--
fs/nfsd/xdr4.h | 2 +-
4 files changed, 70 insertions(+), 39 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7d44c85335c7..64cc830830e9 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1521,11 +1521,13 @@ static void nfs4_put_copy(struct nfsd4_async_copy *copy)
{
if (!refcount_dec_and_test(©->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(©->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(©->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(©->cp_stateid.cs_stid, stateid, NFS4_STATEID_SIZE))
+ if (memcmp(©->cp_stid.sc_stateid, stateid, NFS4_STATEID_SIZE))
continue;
return copy;
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 594cf392f61f..8ac76db31fbb 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 *);
@@ -982,9 +983,41 @@ 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 callback for a copy offload stateid. Runs from nfs4_put_stid()
+ * once the stid's last reference is dropped and it has been removed from
+ * cl_stateids.
+ */
+static void nfsd4_free_async_copy_stid(struct nfs4_stid *stid)
+{
+ 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 the durable state for an asynchronous COPY. The copy offload
+ * stateid is a first-class nfs4_stid (SC_TYPE_COPY) living in the issuing
+ * client's cl_stateids IDR, so it is per-client by construction and shares
+ * the common stateid refcounting and teardown. It is never matched by the
+ * generic stateid lookups (find_stateid_locked() skips SC_TYPE_COPY);
+ * OFFLOAD_CANCEL/OFFLOAD_STATUS find it by walking clp->async_copies.
+ */
+struct nfsd4_async_copy *nfs4_alloc_copy_stid(struct nfs4_client *clp)
{
- return nfs4_init_cp_state(nn, ©->cp_stateid, NFS4_COPY_STID, NULL);
+ 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,
@@ -1024,19 +1057,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 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
@@ -3073,6 +3093,16 @@ 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 for id allocation and
+ * refcounting, but they are not valid targets for the generic
+ * stateid operations (FREE_STATEID, TEST_STATEID, I/O). Per RFC 7862
+ * they are managed only via OFFLOAD_CANCEL/OFFLOAD_STATUS/CB_OFFLOAD,
+ * which locate them through clp->async_copies. Hide them here so those
+ * paths continue to return NFS4ERR_BAD_STATEID.
+ */
+ if (ret->sc_type == SC_TYPE_COPY)
+ return NULL;
return ret;
}
@@ -5408,6 +5438,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
@@ -5434,8 +5465,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 f1c817e2f93c..cbd40bf3ee2a 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -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;
};
--
2.55.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
` (8 preceding siblings ...)
2026-07-09 18:47 ` [PATCH v2 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
@ 2026-07-09 18:47 ` Jeff Layton
9 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2026-07-09 18:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
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 | 31 +++++++++++++------------------
fs/nfsd/state.h | 1 -
2 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8ac76db31fbb..380214e90f44 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -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(<, cps->cpntf_time))
+ if (state_expired(<, 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,
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 v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY
2026-07-09 18:47 ` [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
@ 2026-07-09 21:18 ` Chuck Lever
0 siblings, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2026-07-09 21:18 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Andy Adamson
Cc: Chris Mason, linux-nfs, linux-kernel
On Thu, Jul 9, 2026, at 2:47 PM, Jeff Layton wrote:
> 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 | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index fad01d67bf3f..1c674479d4ca 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -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, ©->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
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.
--
Chuck Lever
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-07-09 21:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
2026-07-09 18:47 ` [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state Jeff Layton
2026-07-09 18:47 ` [PATCH v2 02/10] nfsd: fix UAF in async copy cancel and shutdown Jeff Layton
2026-07-09 18:47 ` [PATCH v2 03/10] nfsd: fix stale s2s_cp_stateids IDR entry for async COPY Jeff Layton
2026-07-09 21:18 ` Chuck Lever
2026-07-09 18:47 ` [PATCH v2 04/10] nfsd: initialize copy-notify stateid before publishing it Jeff Layton
2026-07-09 18:47 ` [PATCH v2 05/10] nfsd: check client ownership when cancelling a copy-notify stateid Jeff Layton
2026-07-09 18:47 ` [PATCH v2 06/10] nfsd: revoke copy-notify stateids before dropping their reference Jeff Layton
2026-07-09 18:47 ` [PATCH v2 07/10] nfsd: return NFS4ERR_NOTSUPP for unsupported netloc4 types Jeff Layton
2026-07-09 18:47 ` [PATCH v2 08/10] nfsd: split nfsd4_copy into transient and durable async copy objects Jeff Layton
2026-07-09 18:47 ` [PATCH v2 09/10] nfsd: make the copy offload stateid a first-class nfs4_stid Jeff Layton
2026-07-09 18:47 ` [PATCH v2 10/10] nfsd: drop dead COPY-vs-COPYNOTIFY type handling from s2s stateid IDR Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox