[PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid

[PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid

[Hyunsol Mun]

A COPY_NOTIFY token can resolve to a stateid owned by the source client.
find_cpntf_state() drops its source-client reference before returning
that stateid, so concurrent source-client expiry can free sc_client
while stateid validation or the caller still uses the stateid.

Add a local nfsd4_get_client() helper as the counterpart to
nfsd4_put_client(). Return a paired client reference from
nfs4_preprocess_stateid_op() when lookup crosses through COPY_NOTIFY
state. Keep it until the caller releases the returned stateid; internal
and error paths release it locally.

An unprivileged NFSv4.2 client with access to the exported file can execute
the OPEN, COPY_NOTIFY, source-client replacement, and token READ sequence.
Four hundred natural trials reached the protocol path without reproducing
the report, so a reliable timing-assistance-free trigger was not
demonstrated.

For deterministic validation, a test-only kprobe delayed nfsd_permission()
after COPY_NOTIFY state lookup. The module only widened the race window; it
did not allocate, free, or modify an NFSD object. Validation used the
nfsd-testing base recorded below. On the unmodified KASAN kernel, five
trials registered four delayed calls and reproduced the source-client
use-after-free in nfs4_put_stid(). With this patch, five trials registered
five delayed calls and produced no KASAN report or kernel failure. Three
token READs returned NFS4_OK with 23 bytes; two returned NFS4ERR_EXPIRED as
client replacement won the race.

The full KASAN kernel built with CONFIG_WERROR without a compiler
diagnostic. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.
A source reproducer, timing-module source, complete logs, and the kernel
configuration are available privately on request.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Assisted-by: LLM
Signed-off-by: Hyunsol Mun <muumthf@gmail.com>
---
Changes in v2:
- Describe current timing-assisted baseline and patched results.
- Add full-build and NFSv4.2/NFSv3 smoke-test results.
- Keep nfsd4_get_client() local to nfs4state.c.

 fs/nfsd/nfs4proc.c  | 31 +++++++++++++++++++++----------
 fs/nfsd/nfs4state.c | 34 +++++++++++++++++++++++++++-------
 fs/nfsd/state.h     |  2 +-
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7df60abfb..d5b5f59f1 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1144,7 +1144,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	/* check stateid */
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					&read->rd_stateid, RD_STATE,
-					&read->rd_nf, NULL);
+					&read->rd_nf, NULL, NULL);
 
 	read->rd_rqstp = rqstp;
 	read->rd_fhp = &cstate->current_fh;
@@ -1342,6 +1342,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		.na_dpacl	= posix_acl_dup(setattr->sa_dpacl),
 	};
 	bool save_no_wcc, deleg_attrs;
+	struct nfs4_client *stid_clp = NULL;
 	struct nfs4_stid *st = NULL;
 	struct inode *inode;
 	__be32 status = nfs_ok;
@@ -1358,7 +1359,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 		status = nfs4_preprocess_stateid_op(rqstp, cstate,
 				&cstate->current_fh, &setattr->sa_stateid,
-				flags, NULL, &st);
+				flags, NULL, &st, &stid_clp);
 		if (status)
 			goto out_err;
 	}
@@ -1375,8 +1376,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			}
 		}
 	}
-	if (st)
+	if (st) {
 		nfs4_put_stid(st);
+		if (stid_clp)
+			nfsd4_put_client(stid_clp);
+	}
 	if (status)
 		goto out_err;
 
@@ -1439,6 +1443,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd4_write *write = &u->write;
 	stateid_t *stateid = &write->wr_stateid;
 	struct nfs4_stid *stid = NULL;
+	struct nfs4_client *stid_clp = NULL;
 	struct nfsd_file *nf = NULL;
 	__be32 status = nfs_ok;
 	unsigned long cnt;
@@ -1451,13 +1456,16 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	trace_nfsd_write_start(rqstp, &cstate->current_fh,
 			       write->wr_offset, cnt);
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
-						stateid, WR_STATE, &nf, &stid);
+						stateid, WR_STATE, &nf, &stid,
+						&stid_clp);
 	if (status)
 		return status;
 
 	if (stid) {
 		nfsd4_file_mark_deleg_written(stid->sc_file);
 		nfs4_put_stid(stid);
+		if (stid_clp)
+			nfsd4_put_client(stid_clp);
 	}
 
 	write->wr_how_written = write->wr_stable_how;
@@ -1484,12 +1492,12 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return nfserr_nofilehandle;
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
-					    src_stateid, RD_STATE, src, NULL);
+					    src_stateid, RD_STATE, src, NULL, NULL);
 	if (status)
 		goto out;
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
-					    dst_stateid, WR_STATE, dst, NULL);
+					    dst_stateid, WR_STATE, dst, NULL, NULL);
 	if (status)
 		goto out_put_src;
 
@@ -1954,7 +1962,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp,
 	/* Verify the destination stateid and set dst struct file*/
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					    &copy->cp_dst_stateid,
-					    WR_STATE, &copy->nf_dst, NULL);
+					    WR_STATE, &copy->nf_dst, NULL, NULL);
 	if (status)
 		goto out;
 
@@ -2498,12 +2506,13 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfsd4_copy_notify *cn = &u->copy_notify;
 	__be32 status;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct nfs4_client *stid_clp = NULL;
 	struct nfs4_stid *stid = NULL;
 	struct nfs4_cpntf_state *cps;
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					&cn->cpn_src_stateid, RD_STATE, NULL,
-					&stid);
+					&stid, &stid_clp);
 	if (status)
 		return status;
 	if (!stid)
@@ -2536,6 +2545,8 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	nfs4_put_cpntf_state(nn, cps);
 out:
 	nfs4_put_stid(stid);
+	if (stid_clp)
+		nfsd4_put_client(stid_clp);
 	return status;
 }
 
@@ -2548,7 +2559,7 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					    &fallocate->falloc_stateid,
-					    WR_STATE, &nf, NULL);
+					    WR_STATE, &nf, NULL, NULL);
 	if (status != nfs_ok)
 		return status;
 
@@ -2612,7 +2623,7 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
 					    &seek->seek_stateid,
-					    RD_STATE, &nf, NULL);
+					    RD_STATE, &nf, NULL, NULL);
 	if (status)
 		return status;
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bbc16dd22..43906f87e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
 	kmem_cache_free(client_slab, clp);
 }
 
+/**
+ * nfsd4_get_client - acquire a reference on an nfs4_client
+ * @clp: the client to be acquired
+ */
+static void nfsd4_get_client(struct nfs4_client *clp)
+{
+	kref_get(&clp->cl_nfsdfs.cl_ref);
+}
+
 /**
  * nfsd4_put_client - release a reference on an nfs4_client
  * @clp: the client to be released
@@ -8504,7 +8513,8 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 }
 
 static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
-			       struct nfs4_stid **stid)
+			       struct nfs4_stid **stid,
+			       struct nfs4_client **stid_clp)
 {
 	__be32 status;
 	struct nfs4_cpntf_state *cps = NULL;
@@ -8524,10 +8534,13 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
 	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
 				     SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
 				     0);
-	if (*stid)
+	if (*stid) {
+		nfsd4_get_client(found);
+		*stid_clp = found;
 		status = nfs_ok;
-	else
+	} else {
 		status = nfserr_bad_stateid;
+	}
 
 	put_client_renew(found);
 out:
@@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
  * @flags: flags describing type of operation to be done
  * @nfp: optional nfsd_file return pointer (may be NULL)
  * @cstid: optional returned nfs4_stid pointer (may be NULL)
+ * @cstid_clp: client reference paired with @cstid (required with @cstid)
  *
  * Given info from the client, look up a nfs4_stid for the operation. On
  * success, it returns a reference to the nfs4_stid and/or the nfsd_file
@@ -8560,10 +8574,11 @@ __be32
 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
 		stateid_t *stateid, int flags, struct nfsd_file **nfp,
-		struct nfs4_stid **cstid)
+		struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)
 {
 	struct net *net = SVC_NET(rqstp);
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct nfs4_client *stid_clp = NULL;
 	struct nfs4_stid *s = NULL;
 	__be32 status;
 
@@ -8579,7 +8594,7 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 				SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
 				0, &s, nn);
 	if (status == nfserr_bad_stateid)
-		status = find_cpntf_state(nn, stateid, &s);
+		status = find_cpntf_state(nn, stateid, &s, &stid_clp);
 	if (status)
 		return status;
 	status = nfsd4_stid_check_stateid_generation(stateid, s,
@@ -8605,11 +8620,16 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		status = nfs4_check_file(rqstp, fhp, s, nfp, flags);
 out:
 	if (s) {
-		if (!status && cstid)
+		if (!status && cstid) {
 			*cstid = s;
-		else
+			*cstid_clp = stid_clp;
+			stid_clp = NULL;
+		} else {
 			nfs4_put_stid(s);
+		}
 	}
+	if (stid_clp)
+		nfsd4_put_client(stid_clp);
 	return status;
 }
 
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 1a0da82cb..7910dabb4 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -911,7 +911,7 @@ struct nfsd4_async_copy;
 extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
 		stateid_t *stateid, int flags, struct nfsd_file **filp,
-		struct nfs4_stid **cstid);
+		struct nfs4_stid **cstid, struct nfs4_client **cstid_clp);
 __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 			    stateid_t *stateid, unsigned short typemask,
 			    unsigned short statusmask,

base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid

[Jeff Layton]

On Wed, 2026-09-30 at 14:39 +0900, Hyunsol Mun wrote:
> A COPY_NOTIFY token can resolve to a stateid owned by the source client.
> find_cpntf_state() drops its source-client reference before returning
> that stateid, so concurrent source-client expiry can free sc_client
> while stateid validation or the caller still uses the stateid.
> 
> Add a local nfsd4_get_client() helper as the counterpart to
> nfsd4_put_client(). Return a paired client reference from
> nfs4_preprocess_stateid_op() when lookup crosses through COPY_NOTIFY
> state. Keep it until the caller releases the returned stateid; internal
> and error paths release it locally.
> 
> An unprivileged NFSv4.2 client with access to the exported file can execute
> the OPEN, COPY_NOTIFY, source-client replacement, and token READ sequence.
> Four hundred natural trials reached the protocol path without reproducing
> the report, so a reliable timing-assistance-free trigger was not
> demonstrated.
> 
> For deterministic validation, a test-only kprobe delayed nfsd_permission()
> after COPY_NOTIFY state lookup. The module only widened the race window; it
> did not allocate, free, or modify an NFSD object. Validation used the
> nfsd-testing base recorded below. On the unmodified KASAN kernel, five
> trials registered four delayed calls and reproduced the source-client
> use-after-free in nfs4_put_stid(). With this patch, five trials registered
> five delayed calls and produced no KASAN report or kernel failure. Three
> token READs returned NFS4_OK with 23 bytes; two returned NFS4ERR_EXPIRED as
> client replacement won the race.
> 
> The full KASAN kernel built with CONFIG_WERROR without a compiler
> diagnostic. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.
> A source reproducer, timing-module source, complete logs, and the kernel
> configuration are available privately on request.
> 
> The vulnerability research and validation were conducted by members of
> the Tobabz team as part of the Best of the Best 15th program.
> 
> Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
> Assisted-by: LLM
> Signed-off-by: Hyunsol Mun <muumthf@gmail.com>
> ---
> Changes in v2:
> - Describe current timing-assisted baseline and patched results.
> - Add full-build and NFSv4.2/NFSv3 smoke-test results.
> - Keep nfsd4_get_client() local to nfs4state.c.
> 
>  fs/nfsd/nfs4proc.c  | 31 +++++++++++++++++++++----------
>  fs/nfsd/nfs4state.c | 34 +++++++++++++++++++++++++++-------
>  fs/nfsd/state.h     |  2 +-
>  3 files changed, 49 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 7df60abfb..d5b5f59f1 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1144,7 +1144,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	/* check stateid */
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
>  					&read->rd_stateid, RD_STATE,
> -					&read->rd_nf, NULL);
> +					&read->rd_nf, NULL, NULL);
>  
>  	read->rd_rqstp = rqstp;
>  	read->rd_fhp = &cstate->current_fh;
> @@ -1342,6 +1342,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		.na_dpacl	= posix_acl_dup(setattr->sa_dpacl),
>  	};
>  	bool save_no_wcc, deleg_attrs;
> +	struct nfs4_client *stid_clp = NULL;
>  	struct nfs4_stid *st = NULL;
>  	struct inode *inode;
>  	__be32 status = nfs_ok;
> @@ -1358,7 +1359,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  		status = nfs4_preprocess_stateid_op(rqstp, cstate,
>  				&cstate->current_fh, &setattr->sa_stateid,
> -				flags, NULL, &st);
> +				flags, NULL, &st, &stid_clp);
>  		if (status)
>  			goto out_err;
>  	}
> @@ -1375,8 +1376,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  			}
>  		}
>  	}
> -	if (st)
> +	if (st) {
>  		nfs4_put_stid(st);
> +		if (stid_clp)
> +			nfsd4_put_client(stid_clp);
> +	}
>  	if (status)
>  		goto out_err;
>  
> @@ -1439,6 +1443,7 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfsd4_write *write = &u->write;
>  	stateid_t *stateid = &write->wr_stateid;
>  	struct nfs4_stid *stid = NULL;
> +	struct nfs4_client *stid_clp = NULL;
>  	struct nfsd_file *nf = NULL;
>  	__be32 status = nfs_ok;
>  	unsigned long cnt;
> @@ -1451,13 +1456,16 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	trace_nfsd_write_start(rqstp, &cstate->current_fh,
>  			       write->wr_offset, cnt);
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> -						stateid, WR_STATE, &nf, &stid);
> +						stateid, WR_STATE, &nf, &stid,
> +						&stid_clp);
>  	if (status)
>  		return status;
>  
>  	if (stid) {
>  		nfsd4_file_mark_deleg_written(stid->sc_file);
>  		nfs4_put_stid(stid);
> +		if (stid_clp)
> +			nfsd4_put_client(stid_clp);
>  	}
>  
>  	write->wr_how_written = write->wr_stable_how;
> @@ -1484,12 +1492,12 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		return nfserr_nofilehandle;
>  
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
> -					    src_stateid, RD_STATE, src, NULL);
> +					    src_stateid, RD_STATE, src, NULL, NULL);
>  	if (status)
>  		goto out;
>  
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
> -					    dst_stateid, WR_STATE, dst, NULL);
> +					    dst_stateid, WR_STATE, dst, NULL, NULL);
>  	if (status)
>  		goto out_put_src;
>  
> @@ -1954,7 +1962,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp,
>  	/* Verify the destination stateid and set dst struct file*/
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
>  					    &copy->cp_dst_stateid,
> -					    WR_STATE, &copy->nf_dst, NULL);
> +					    WR_STATE, &copy->nf_dst, NULL, NULL);
>  	if (status)
>  		goto out;
>  
> @@ -2498,12 +2506,13 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfsd4_copy_notify *cn = &u->copy_notify;
>  	__be32 status;
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> +	struct nfs4_client *stid_clp = NULL;
>  	struct nfs4_stid *stid = NULL;
>  	struct nfs4_cpntf_state *cps;
>  
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
>  					&cn->cpn_src_stateid, RD_STATE, NULL,
> -					&stid);
> +					&stid, &stid_clp);
>  	if (status)
>  		return status;
>  	if (!stid)
> @@ -2536,6 +2545,8 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	nfs4_put_cpntf_state(nn, cps);
>  out:
>  	nfs4_put_stid(stid);
> +	if (stid_clp)
> +		nfsd4_put_client(stid_clp);
>  	return status;
>  }
>  
> @@ -2548,7 +2559,7 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
>  					    &fallocate->falloc_stateid,
> -					    WR_STATE, &nf, NULL);
> +					    WR_STATE, &nf, NULL, NULL);
>  	if (status != nfs_ok)
>  		return status;
>  
> @@ -2612,7 +2623,7 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
>  					    &seek->seek_stateid,
> -					    RD_STATE, &nf, NULL);
> +					    RD_STATE, &nf, NULL, NULL);
>  	if (status)
>  		return status;
>  
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index bbc16dd22..43906f87e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
>  	kmem_cache_free(client_slab, clp);
>  }
>  
> +/**
> + * nfsd4_get_client - acquire a reference on an nfs4_client
> + * @clp: the client to be acquired
> + */
> +static void nfsd4_get_client(struct nfs4_client *clp)
> +{
> +	kref_get(&clp->cl_nfsdfs.cl_ref);
> +}
> +

If you're going to add this helper, then you should convert all of the
other places that call kref_get() on this object to use it, ideally in
a separate patch.

>  /**
>   * nfsd4_put_client - release a reference on an nfs4_client
>   * @clp: the client to be released
> @@ -8504,7 +8513,8 @@ __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
>  }
>  
>  static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> -			       struct nfs4_stid **stid)
> +			       struct nfs4_stid **stid,
> +			       struct nfs4_client **stid_clp)
>  {
>  	__be32 status;
>  	struct nfs4_cpntf_state *cps = NULL;
> @@ -8524,10 +8534,13 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
>  	*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
>  				     SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
>  				     0);
> -	if (*stid)
> +	if (*stid) {
> +		nfsd4_get_client(found);
> +		*stid_clp = found;
>  		status = nfs_ok;
> -	else
> +	} else {
>  		status = nfserr_bad_stateid;
> +	}
>  
>  	put_client_renew(found);
>  out:
> @@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
>   * @flags: flags describing type of operation to be done
>   * @nfp: optional nfsd_file return pointer (may be NULL)
>   * @cstid: optional returned nfs4_stid pointer (may be NULL)
> + * @cstid_clp: client reference paired with @cstid (required with @cstid)
>   *
>   * Given info from the client, look up a nfs4_stid for the operation. On
>   * success, it returns a reference to the nfs4_stid and/or the nfsd_file
> @@ -8560,10 +8574,11 @@ __be32
>  nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
>  		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
>  		stateid_t *stateid, int flags, struct nfsd_file **nfp,
> -		struct nfs4_stid **cstid)
> +		struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)

The bug seems possible (esp given the fault-injected reproducer), but
this function is really devolving into a big mess. It would be very
nice to move the COPY-specific code into a dedicated helper that is
only called from the COPY-related codepaths. Consider reorganizing this
code along those lines.

>  {
>  	struct net *net = SVC_NET(rqstp);
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct nfs4_client *stid_clp = NULL;
>  	struct nfs4_stid *s = NULL;
>  	__be32 status;
>  
> @@ -8579,7 +8594,7 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
>  				SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
>  				0, &s, nn);
>  	if (status == nfserr_bad_stateid)
> -		status = find_cpntf_state(nn, stateid, &s);
> +		status = find_cpntf_state(nn, stateid, &s, &stid_clp);
>  	if (status)
>  		return status;
>  	status = nfsd4_stid_check_stateid_generation(stateid, s,
> @@ -8605,11 +8620,16 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
>  		status = nfs4_check_file(rqstp, fhp, s, nfp, flags);
>  out:
>  	if (s) {
> -		if (!status && cstid)
> +		if (!status && cstid) {
>  			*cstid = s;
> -		else
> +			*cstid_clp = stid_clp;
> +			stid_clp = NULL;
> +		} else {
>  			nfs4_put_stid(s);
> +		}
>  	}
> +	if (stid_clp)
> +		nfsd4_put_client(stid_clp);
>  	return status;
>  }
>  
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1a0da82cb..7910dabb4 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -911,7 +911,7 @@ struct nfsd4_async_copy;
>  extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
>  		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
>  		stateid_t *stateid, int flags, struct nfsd_file **filp,
> -		struct nfs4_stid **cstid);
> +		struct nfs4_stid **cstid, struct nfs4_client **cstid_clp);
>  __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
>  			    stateid_t *stateid, unsigned short typemask,
>  			    unsigned short statusmask,
> 
> base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2] nfsd: pin source client while using COPY_NOTIFY stateid

[Chuck Lever]

On Fri, Oct 2, 2026, at 12:20 AM, Jeff Layton wrote:
> On Wed, 2026-09-30 at 14:39 +0900, Hyunsol Mun wrote:
>> A COPY_NOTIFY token can resolve to a stateid owned by the source client.
>> find_cpntf_state() drops its source-client reference before returning
>> that stateid, so concurrent source-client expiry can free sc_client
>> while stateid validation or the caller still uses the stateid.

>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index bbc16dd22..43906f87e 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -2805,6 +2805,15 @@ static void __free_client(struct kref *k)
>>  	kmem_cache_free(client_slab, clp);
>>  }
>>  
>> +/**
>> + * nfsd4_get_client - acquire a reference on an nfs4_client
>> + * @clp: the client to be acquired
>> + */
>> +static void nfsd4_get_client(struct nfs4_client *clp)
>> +{
>> +	kref_get(&clp->cl_nfsdfs.cl_ref);
>> +}
>> +
>
> If you're going to add this helper, then you should convert all of the
> other places that call kref_get() on this object to use it, ideally in
> a separate patch.

I applied this one to my private tree, but it conflicted with an earlier
fix, so it is modified in my tree. Perhaps I should drop it instead, and
muumthf can rebase this fix and repost, once I have refreshed the public
version of nfsd-testing in a couple of days.

Jeff, note that one of the other one-off fixes posted this week also adds
the same helper...


>> @@ -8551,6 +8564,7 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps)
>>   * @flags: flags describing type of operation to be done
>>   * @nfp: optional nfsd_file return pointer (may be NULL)
>>   * @cstid: optional returned nfs4_stid pointer (may be NULL)
>> + * @cstid_clp: client reference paired with @cstid (required with @cstid)
>>   *
>>   * Given info from the client, look up a nfs4_stid for the operation. On
>>   * success, it returns a reference to the nfs4_stid and/or the nfsd_file
>> @@ -8560,10 +8574,11 @@ __be32
>>  nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
>>  		struct nfsd4_compound_state *cstate, struct svc_fh *fhp,
>>  		stateid_t *stateid, int flags, struct nfsd_file **nfp,
>> -		struct nfs4_stid **cstid)
>> +		struct nfs4_stid **cstid, struct nfs4_client **cstid_clp)
>
> The bug seems possible (esp given the fault-injected reproducer), but
> this function is really devolving into a big mess. It would be very
> nice to move the COPY-specific code into a dedicated helper that is
> only called from the COPY-related codepaths. Consider reorganizing this
> code along those lines.

That's another reason I should drop this version of the fix and let
muumthf rework it.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)