From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <cel@kernel.org>, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
Andy Adamson <andros@netapp.com>
Cc: Chris Mason <clm@meta.com>,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
Jeff Layton <jlayton@kernel.org>
Subject: [PATCH v2 01/10] nfsd: fix cpntf publish race in nfs4_init_cp_state
Date: Thu, 09 Jul 2026 14:47:38 -0400 [thread overview]
Message-ID: <20260709-nfsd-testing-v2-1-0a1ba233bf87@kernel.org> (raw)
In-Reply-To: <20260709-nfsd-testing-v2-0-0a1ba233bf87@kernel.org>
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
next prev parent reply other threads:[~2026-07-09 18:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-09 18:47 [PATCH v2 00/10] nfsd: copy offload fixes Jeff Layton
2026-07-09 18:47 ` Jeff Layton [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260709-nfsd-testing-v2-1-0a1ba233bf87@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=andros@netapp.com \
--cc=cel@kernel.org \
--cc=clm@meta.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.