* [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, ©->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 +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(©->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, ©->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.
+ * 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, ©->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 +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(©->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(©->cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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(©->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, ©->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(©->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(); 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(©->cp_clp);
if (!clp) {
set_bit(NFSD4_COPY_F_OFFLOAD_DONE, ©->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(©->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, ©->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);
-
+ /*
+ * 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, ©->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 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(©->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(©->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(©->cp_clp, NULL);
+ if (!list_empty(©->copies))
+ list_del_init(©->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, ©->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(<, 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, ©->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,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(©->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 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(©->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, ©->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);
/*
* 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(©->cp_clp, NULL);
+ smp_store_release(©->cp_copy.cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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(©->cp_copy, sb)) {
refcount_inc(©->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,
- ©->cp_flags);
+ ©->cp_copy.cp_flags);
list_move(©->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(©->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, ©->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);
@@ -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(©->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;
@@ -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(©->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;
/*
@@ -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(©->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(©->cp_clp, NULL);
+ smp_store_release(©->cp_copy.cp_clp, NULL);
if (!list_empty(©->copies))
list_del_init(©->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, ©->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 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, ©->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(©->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(©->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(©->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(©->cp_copy.cp_clp, NULL);
if (!list_empty(©->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(©->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(©->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(©->cp_stateid.cs_stid, stateid, NFS4_STATEID_SIZE))
+ if (memcmp(©->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(©->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, ©->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(<, cps->cpntf_time))
+ if (state_expired(<, 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