6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jeff Layton <jlayton@kernel.org>
commit 129643893b79f8a3c6b72045f933fbab5ee424ca upstream.
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")
Cc: <redacted>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260710-nfsd-testing-v3-4-a0ff7db6aa3e@kernel.org (local)
Signed-off-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/nfsd/nfs4proc.c | 16 +++++++++-------
fs/nfsd/nfs4state.c | 10 +++++++++-
2 files changed, 18 insertions(+), 8 deletions(-)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1884,7 +1884,6 @@ nfsd4_copy_notify(struct svc_rqst *rqstp
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
struct nfs4_stid *stid;
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,
@@ -1896,12 +1895,14 @@ nfsd4_copy_notify(struct svc_rqst *rqstp
cn->cpn_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.
@@ -1910,10 +1911,11 @@ nfsd4_copy_notify(struct svc_rqst *rqstp
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;
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c@@ -1035,7 +1035,15 @@ struct nfs4_cpntf_state *nfs4_alloc_init
/* 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;